Add Test for dataprovider and modify the way we fetch and apply stream

Make FetchStreamFromURL::aBuffer a pointer to prevent the crash that
happens because of it.

Add ExternalDataMapper::StopImport() method.

Add a few member functions in CSVFetchThread that help in sync the
line fetch and writing.

Introduce a WriteToDoc() method that iterates over the the range to
apply whatever value or string needs to be applied.

Change-Id: I9baa053fa6d91bdf758a9b035888af50a40674eb
Reviewed-on: https://gerrit.libreoffice.org/32964Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Markus Mohrhard <markus.mohrhard@googlemail.com>

sc/CppunitTest_sc_dataproviders_test.mk

+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+
+$(eval $(call gb_CppunitTest_CppunitTest,sc_dataproviders_test))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,sc_dataproviders_test, \
+    sc/qa/unit/dataproviders_test \
+))
+
+$(eval $(call gb_CppunitTest_use_externals,sc_dataproviders_test, \
+	boost_headers \
+	mdds_headers \
+	libxml2 \
+))
+
+$(eval $(call gb_CppunitTest_use_libraries,sc_dataproviders_test, \
+    basegfx \
+    comphelper \
+    cppu \
+    cppuhelper \
+    drawinglayer \
+    editeng \
+    for \
+    forui \
+    i18nlangtag \
+    msfilter \
+    oox \
+    sal \
+    salhelper \
+    sax \
+    sb \
+    sc \
+    scqahelper \
+    sfx \
+    sot \
+    subsequenttest \
+    svl \
+    svt \
+    svx \
+    svxcore \
+	test \
+    tk \
+    tl \
+    ucbhelper \
+	unotest \
+    utl \
+    vbahelper \
+    vcl \
+    xo \
+	$(gb_UWINAPI) \
+))
+
+$(eval $(call gb_CppunitTest_set_include,sc_dataproviders_test,\
+    -I$(SRCDIR)/sc/source/ui/inc \
+    -I$(SRCDIR)/sc/inc \
+	-I$(SRCDIR)/sc/source/filter/inc \
+    $$(INCLUDE) \
+))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,sc_dataproviders_test,\
+	officecfg/registry \
+))
+
+$(eval $(call gb_CppunitTest_use_sdk_api,sc_dataproviders_test))
+
+$(eval $(call gb_CppunitTest_use_ure,sc_dataproviders_test))
+$(eval $(call gb_CppunitTest_use_vcl,sc_dataproviders_test))
+
+$(eval $(call gb_CppunitTest_use_components,sc_dataproviders_test,\
+	basic/util/sb \
+    chart2/source/chartcore \
+    chart2/source/controller/chartcontroller \
+    comphelper/util/comphelp \
+    configmgr/source/configmgr \
+    dbaccess/util/dba \
+    embeddedobj/util/embobj \
+    eventattacher/source/evtatt \
+    filter/source/config/cache/filterconfig1 \
+    forms/util/frm \
+    framework/util/fwk \
+    i18npool/source/search/i18nsearch \
+    i18npool/util/i18npool \
+    linguistic/source/lng \
+    oox/util/oox \
+    package/source/xstor/xstor \
+    package/util/package2 \
+    sax/source/expatwrap/expwrap \
+    scaddins/source/analysis/analysis \
+    scaddins/source/datefunc/date \
+    sc/util/sc \
+    sc/util/scfilt \
+    sfx2/util/sfx \
+    sot/util/sot \
+    svl/util/svl \
+    svl/source/fsstor/fsstorage \
+    svtools/util/svt \
+    toolkit/util/tk \
+    ucb/source/core/ucb1 \
+    ucb/source/ucp/file/ucpfile1 \
+    ucb/source/ucp/tdoc/ucptdoc1 \
+    unotools/util/utl \
+    unoxml/source/rdf/unordf \
+    unoxml/source/service/unoxml \
+    uui/util/uui \
+    xmloff/util/xo \
+    xmlsecurity/util/xmlsecurity \
+))
+
+$(eval $(call gb_CppunitTest_use_externals,sc_dataproviders_test,\
+	orcus \
+	orcus-parser \
+	boost_filesystem \
+	boost_system \
+	boost_iostreams \
+	zlib \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,sc_dataproviders_test))
+
+# vim: set noet sw=4 ts=4:

sc/Module_sc.mk

@@ -59,6 +59,7 @@ $(eval $(call gb_Module_add_slowcheck_targets,sc, \
 	CppunitTest_sc_subsequent_export_test \
 	CppunitTest_sc_html_export_test \
 	CppunitTest_sc_copypaste \
+	CppunitTest_sc_dataproviders_test \
 ))
 
 # Various function tests fail in 32-bit linux_x86 build due to dreaded floating

sc/qa/unit/data/contentCSV/dataprovider.csv

+-2012,-1,0,1,2012
+-3.14,-0.99,0.01,3.14
+H,"Hello, Calc!"

sc/qa/unit/dataproviders_test.cxx

+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <rtl/ustring.hxx>
+
+#include "helper/qahelper.hxx"
+#include "document.hxx"
+#include <stringutil.hxx>
+#include "address.hxx"
+#include "dataprovider.hxx"
+
+#include <memory>
+
+struct TestImpl
+{
+    ScDocShellRef m_xDocShell;
+};
+
+class ScDataProvidersTest : public ScBootstrapFixture
+{
+public:
+
+    ScDataProvidersTest();
+
+    ScDocShell& getDocShell();
+
+    virtual void setUp() override;
+    virtual void tearDown() override;
+
+    void testCSVImport();
+
+    CPPUNIT_TEST_SUITE(ScDataProvidersTest);
+    CPPUNIT_TEST(testCSVImport);
+    CPPUNIT_TEST_SUITE_END();
+
+private:
+    std::unique_ptr<TestImpl> m_pImpl;
+    ScDocument *m_pDoc;
+};
+
+ScDataProvidersTest::ScDataProvidersTest() :
+    ScBootstrapFixture( "/sc/qa/unit/data" ),
+    m_pImpl(new TestImpl),
+    m_pDoc(nullptr)
+{
+}
+
+ScDocShell& ScDataProvidersTest::getDocShell()
+{
+    return *m_pImpl->m_xDocShell;
+}
+
+void ScDataProvidersTest::testCSVImport()
+{
+    m_pDoc->InsertTab(0, "foo");
+    bool success;
+    OUString aCSVFile("dataprovider.");
+    OUString aCSVPath;
+    createCSVPath( aCSVFile, aCSVPath );
+    OUString aDBName = "TEST";
+    sc::ExternalDataMapper aExternalDataMapper (&getDocShell(), aCSVPath, aDBName, 0, 0, 0, 5, 5, success);
+    aExternalDataMapper.StartImport();
+    Scheduler::ProcessEventsToIdle();
+    CPPUNIT_ASSERT_EQUAL (-2012.0, m_pDoc->GetValue(0, 0, 0));
+    CPPUNIT_ASSERT_EQUAL (-1.0, m_pDoc->GetValue(1, 0, 0));
+    CPPUNIT_ASSERT_EQUAL (0.0, m_pDoc->GetValue(2, 0, 0));
+    CPPUNIT_ASSERT_EQUAL (1.0, m_pDoc->GetValue(3, 0, 0));
+    CPPUNIT_ASSERT_EQUAL (2012.0, m_pDoc->GetValue(4, 0, 0));
+    CPPUNIT_ASSERT_EQUAL (-3.14, m_pDoc->GetValue(0, 1, 0));
+    CPPUNIT_ASSERT_EQUAL (-0.99, m_pDoc->GetValue(1, 1, 0));
+    CPPUNIT_ASSERT_EQUAL (0.01, m_pDoc->GetValue(2, 1, 0));
+    CPPUNIT_ASSERT_EQUAL (3.14, m_pDoc->GetValue(3, 1, 0));
+    CPPUNIT_ASSERT_EQUAL (OUString("H"), m_pDoc->GetString(0, 2, 0));
+    CPPUNIT_ASSERT_EQUAL (OUString("Hello, Calc!"), m_pDoc->GetString(1, 2, 0));
+    CPPUNIT_ASSERT_EQUAL (0.0, m_pDoc->GetValue(2, 2, 0));
+    CPPUNIT_ASSERT_EQUAL (0.0, m_pDoc->GetValue(0, 3, 0));
+}
+
+void ScDataProvidersTest::setUp()
+{
+    ScBootstrapFixture::setUp();
+
+    ScDLL::Init();
+    m_pImpl->m_xDocShell = new ScDocShell(
+        SfxModelFlags::EMBEDDED_OBJECT |
+        SfxModelFlags::DISABLE_EMBEDDED_SCRIPTS |
+        SfxModelFlags::DISABLE_DOCUMENT_RECOVERY);
+
+    m_pImpl->m_xDocShell->SetIsInUcalc();
+    m_pImpl->m_xDocShell->DoInitUnitTest();
+    m_pDoc = &m_pImpl->m_xDocShell->GetDocument();
+}
+
+void ScDataProvidersTest::tearDown()
+{
+    m_pImpl->m_xDocShell->DoClose();
+    m_pImpl->m_xDocShell.clear();
+    ScBootstrapFixture::tearDown();
+}
+
+CPPUNIT_TEST_SUITE_REGISTRATION(ScDataProvidersTest);
+
+CPPUNIT_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sc/source/ui/docshell/dataprovider.cxx

@@ -25,7 +25,9 @@ using namespace com::sun::star;
 
 namespace sc {
 
-SvStream* FetchStreamFromURL (OUString& rURL)
+namespace {
+
+std::unique_ptr<SvStream> FetchStreamFromURL(const OUString& rURL)
 {
     uno::Reference< ucb::XSimpleFileAccess3 > xFileAccess( ucb::SimpleFileAccess::create( comphelper::getProcessComponentContext() ), uno::UNO_QUERY );
 
@@ -34,32 +36,32 @@ SvStream* FetchStreamFromURL (OUString& rURL)
 
     const sal_Int32 BUF_LEN = 8000;
     uno::Sequence< sal_Int8 > buffer( BUF_LEN );
-    OStringBuffer aBuffer( 64000 );
+    OStringBuffer* aBuffer = new OStringBuffer( 64000 );
 
     sal_Int32 nRead = 0;
     while ( ( nRead = xStream->readBytes( buffer, BUF_LEN ) ) == BUF_LEN )
     {
-        aBuffer.append( reinterpret_cast< const char* >( buffer.getConstArray() ), nRead );
+        aBuffer->append( reinterpret_cast< const char* >( buffer.getConstArray() ), nRead );
     }
 
     if ( nRead > 0 )
     {
-        aBuffer.append( reinterpret_cast< const char* >( buffer.getConstArray() ), nRead );
+        aBuffer->append( reinterpret_cast< const char* >( buffer.getConstArray() ), nRead );
     }
 
     xStream->closeInput();
 
-    SvStream* pStream = new SvStream;
-    pStream->WriteCharPtr(aBuffer.getStr());
+    SvStream* pStream = new SvMemoryStream(const_cast<char*>(aBuffer->getStr()), aBuffer->getLength(), StreamMode::READ);
+    return std::unique_ptr<SvStream>(pStream);
+}
 
-    return pStream;
 }
 
 ExternalDataMapper::ExternalDataMapper(ScDocShell* pDocShell, const OUString& rURL, const OUString& rName, SCTAB nTab,
     SCCOL nCol1,SCROW nRow1, SCCOL nCol2, SCROW nRow2, bool& bSuccess):
     maRange (ScRange(nCol1, nRow1, nTab, nCol2, nRow2, nTab)),
     mpDocShell(pDocShell),
-    mpDataProvider (new CSVDataProvider(mpDocShell, maURL, maRange)),
+    mpDataProvider (new CSVDataProvider(mpDocShell, rURL, maRange)),
     mpDBCollection (pDocShell->GetDocument().GetDBCollection()),
     maURL(rURL)
 {
@@ -133,14 +135,18 @@ public:
     }
 };
 
-CSVFetchThread::CSVFetchThread(SvStream *pData, size_t nColCount):
+CSVFetchThread::CSVFetchThread(ScDocument** pDoc, const OUString& mrURL, size_t nColCount):
         Thread("ReaderThread"),
-        mpStream(pData),
+        mpStream(nullptr),
+        mpDocument(new ScDocument),
+        maURL (mrURL),
         mnColCount(nColCount),
         mbTerminate(false)
 {
     maConfig.delimiters.push_back(',');
     maConfig.text_qualifier = '"';
+    mpDocument->InsertTab(0, "blah");
+    *pDoc = mpDocument;
 }
 
 CSVFetchThread::~CSVFetchThread()
@@ -166,40 +172,84 @@ void CSVFetchThread::EndThread()
 
 void CSVFetchThread::execute()
 {
-    LinesType aLines(10);
-
-    // Read & store new lines from stream.
-    for (Line & rLine : aLines)
+    mpStream = FetchStreamFromURL(maURL);
+    if (mpStream->good())
     {
-        rLine.maCells.clear();
-        mpStream->ReadLine(rLine.maLine);
-        CSVHandler aHdl(rLine, mnColCount);
-        orcus::csv_parser<CSVHandler> parser(rLine.maLine.getStr(), rLine.maLine.getLength(), aHdl, maConfig);
-        parser.parse();
+        LinesType* pLines = new LinesType(10);
+        SCROW nCurRow = 0;
+        for (Line & rLine : *pLines)
+        {
+            rLine.maCells.clear();
+            mpStream->ReadLine(rLine.maLine);
+            CSVHandler aHdl(rLine, mnColCount);
+            orcus::csv_parser<CSVHandler> parser(rLine.maLine.getStr(), rLine.maLine.getLength(), aHdl, maConfig);
+            parser.parse();
+
+            if (rLine.maCells.empty())
+            {
+                return;
+            }
+
+            SCCOL nCol = 0;
+            const char* pLineHead = rLine.maLine.getStr();
+            for (auto& rCell : rLine.maCells)
+            {
+                if (rCell.mbValue)
+                {
+                    mpDocument->SetValue(ScAddress(nCol, nCurRow, 0 /* Tab */), rCell.mfValue);
+                }
+                else
+                {
+                    mpDocument->SetString(nCol, nCurRow, 0 /* Tab */, OUString(pLineHead+rCell.maStr.Pos, rCell.maStr.Size, RTL_TEXTENCODING_UTF8));
+                }
+                ++nCol;
+            }
+            nCurRow++;
+        }
     }
+}
+
+osl::Mutex& CSVFetchThread::GetLinesMutex()
+{
+    return maMtxLines;
+}
+
+bool CSVFetchThread::HasNewLines()
+{
+    return !maPendingLines.empty();
+}
 
-    if (!mpStream->good())
-        RequestTerminate();
+void CSVFetchThread::WaitForNewLines()
+{
+    maCondConsume.wait();
+    maCondConsume.reset();
+}
+
+LinesType* CSVFetchThread::GetNewLines()
+{
+    LinesType* pLines = maPendingLines.front();
+    maPendingLines.pop();
+    return pLines;
+}
+
+void CSVFetchThread::ResumeFetchStream()
+{
+    maCondReadStream.set();
 }
 
 CSVDataProvider::CSVDataProvider(ScDocShell* pDocShell, const OUString& rURL, const ScRange& rRange):
     maURL(rURL),
     mrRange(rRange),
     mpDocShell(pDocShell),
+    mpDocument(&pDocShell->GetDocument()),
+    mpLines(nullptr),
+    mnLineCount(0),
     mbImportUnderway(false)
 {
 }
 
 CSVDataProvider::~CSVDataProvider()
 {
-    if(mbImportUnderway)
-        StopImport();
-
-    if (mxCSVFetchThread.is())
-    {
-        mxCSVFetchThread->EndThread();
-        mxCSVFetchThread->join();
-    }
 }
 
 void CSVDataProvider::StartImport()
@@ -209,23 +259,20 @@ void CSVDataProvider::StartImport()
 
     if (!mxCSVFetchThread.is())
     {
-        SvStream* pStream = FetchStreamFromURL(maURL);
-        mxCSVFetchThread = new CSVFetchThread(pStream, mrRange.aEnd.Col() - mrRange.aStart.Col() + 1);
+        ScDocument* pDoc = nullptr;
+        mxCSVFetchThread = new CSVFetchThread(&pDoc, maURL, mrRange.aEnd.Col() - mrRange.aStart.Col() + 1);
         mxCSVFetchThread->launch();
-    }
-    mbImportUnderway = true;
-
-    maImportTimer.Start();
-}
+        if (mxCSVFetchThread.is())
+        {
+            mxCSVFetchThread->EndThread();
+            mxCSVFetchThread->join();
+        }
 
-void CSVDataProvider::StopImport()
-{
-    if (!mbImportUnderway)
-        return;
+        WriteToDoc(pDoc);
+        delete pDoc;
+    }
 
-    mbImportUnderway = false;
     Refresh();
-    maImportTimer.Stop();
 }
 
 void CSVDataProvider::Refresh()
@@ -234,6 +281,51 @@ void CSVDataProvider::Refresh()
     mpDocShell->SetDocumentModified();
 }
 
+Line CSVDataProvider::GetLine()
+{
+    if (!mpLines || mnLineCount >= mpLines->size())
+    {
+        if (mxCSVFetchThread->IsRequestedTerminate())
+            return Line();
+
+        osl::ResettableMutexGuard aGuard(mxCSVFetchThread->GetLinesMutex());
+        while (!mxCSVFetchThread->HasNewLines() && !mxCSVFetchThread->IsRequestedTerminate())
+        {
+            aGuard.clear();
+            mxCSVFetchThread->WaitForNewLines();
+            aGuard.reset();
+        }
+
+        mpLines = mxCSVFetchThread->GetNewLines();
+        mxCSVFetchThread->ResumeFetchStream();
+    }
+
+    return mpLines->at(mnLineCount++);
+}
+
+void CSVDataProvider::WriteToDoc(ScDocument* pDoc)
+{
+    double* pfValue;
+    for (int nRow = mrRange.aStart.Row(); nRow < mrRange.aEnd.Row(); ++nRow)
+    {
+        for (int nCol = mrRange.aStart.Col(); nCol < mrRange.aEnd.Col(); ++nCol)
+        {
+            ScAddress aAddr = ScAddress(nCol, nRow, mrRange.aStart.Tab());
+            pfValue = pDoc->GetValueCell(aAddr);
+
+            if (pfValue == nullptr)
+            {
+                OUString aString = pDoc->GetString(nCol, nRow, mrRange.aStart.Tab());
+                mpDocument->SetString(nCol, nRow, mrRange.aStart.Tab(), aString);
+            }
+            else
+            {
+                mpDocument->SetValue(nCol, nRow, mrRange.aStart.Tab(), *pfValue);
+            }
+        }
+    }
+}
+
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sc/source/ui/inc/dataprovider.hxx

@@ -18,7 +18,6 @@
 #include <address.hxx>
 #include <osl/mutex.hxx>
 #include <osl/conditn.hxx>
-#include <vcl/timer.hxx>
 #include <dbdata.hxx>
 #include <document.hxx>
 
@@ -36,16 +35,15 @@
 
 namespace sc {
 
-/* Fetch Data Stream from local or remote locations */
-SvStream* FetchStreamFromURL(OUString& rUrl);
-
 class DataProvider;
+class CSVDataProvider;
 
 class SC_DLLPUBLIC ExternalDataMapper
 {
     ScRange maRange;
     ScDocShell* mpDocShell;
     std::unique_ptr<DataProvider> mpDataProvider;
+    ScDocument maDocument;
     ScDBCollection* mpDBCollection;
 
     OUString maURL;
@@ -90,17 +88,25 @@ typedef std::vector<Line> LinesType;
 class CSVFetchThread : public salhelper::Thread
 {
     std::unique_ptr<SvStream> mpStream;
+    ScDocument* mpDocument;
+    OUString maURL;
     size_t mnColCount;
 
     bool mbTerminate;
     osl::Mutex maMtxTerminate;
 
+    std::queue<LinesType*> maPendingLines;
+    osl::Mutex maMtxLines;
+
+    osl::Condition maCondReadStream;
+    osl::Condition maCondConsume;
+
     orcus::csv::parser_config maConfig;
 
     virtual void execute() override;
 
 public:
-    CSVFetchThread(SvStream*, size_t);
+    CSVFetchThread(ScDocument** pDoc, const OUString&, size_t);
     virtual ~CSVFetchThread() override;
 
     void RequestTerminate();
@@ -108,6 +114,11 @@ public:
     void Terminate();
     void EndThread();
     void EmptyLineQueue(std::queue<LinesType*>& );
+    osl::Mutex& GetLinesMutex();
+    bool HasNewLines();
+    void WaitForNewLines();
+    LinesType* GetNewLines();
+    void ResumeFetchStream();
 };
 
 class DataProvider
@@ -116,8 +127,8 @@ public:
     virtual ~DataProvider() = 0;
 
     virtual void StartImport() = 0;
-    virtual void StopImport() = 0;
     virtual void Refresh() = 0;
+    virtual void WriteToDoc(ScDocument*) = 0;
 
     virtual ScRange GetRange() const = 0;
     virtual const OUString& GetURL() const = 0;
@@ -127,19 +138,23 @@ class CSVDataProvider : public DataProvider
 {
     OUString maURL;
     ScRange mrRange;
-    Timer maImportTimer;
     rtl::Reference<CSVFetchThread> mxCSVFetchThread;
     ScDocShell* mpDocShell;
+    ScDocument* mpDocument;
+    LinesType* mpLines;
+    size_t mnLineCount;
 
     bool mbImportUnderway;
 
+
 public:
     CSVDataProvider (ScDocShell* pDocShell, const OUString& rUrl, const ScRange& rRange);
     virtual ~CSVDataProvider() override;
 
     virtual void StartImport() override;
-    virtual void StopImport() override;
     virtual void Refresh() override;
+    virtual void WriteToDoc(ScDocument*) override;
+    Line GetLine();
 
     ScRange GetRange() const override
     {