[Libreoffice-commits] core.git: sc/inc sc/qa sc/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Jan 7 09:13:52 UTC 2019


 sc/inc/dpcache.hxx                       |    1 
 sc/qa/unit/data/ods/numgroup_example.ods |binary
 sc/qa/unit/helper/qahelper.cxx           |   10 ++++++++-
 sc/qa/unit/helper/qahelper.hxx           |    1 
 sc/qa/unit/subsequent_export-test.cxx    |   32 +++++++++++++++++++++++++++++++
 sc/source/core/data/dpcache.cxx          |    5 ++++
 sc/source/core/data/dpobject.cxx         |    4 +--
 7 files changed, 50 insertions(+), 3 deletions(-)

New commits:
commit f70d29fd91d232f0b030f0f76bd23bd2919eb868
Author:     Katarina Behrens <Katarina.Behrens at cib.de>
AuthorDate: Tue Dec 18 11:12:49 2018 +0100
Commit:     Katarina Behrens <Katarina.Behrens at cib.de>
CommitDate: Mon Jan 7 10:13:31 2019 +0100

    Resetting all fields for all dims corrupts pivot cache
    
    test case is exporting ooo55266-3 (contains data grouped in numerical
    intervals) to xlsx and without closing the document, opening filter on
    1st pivot table (kaboom!)
    
    ClearGroupFields corrupts the cache bc it resets Field.mpGroup items
    for all dimensions, not just the one present in ScDPDimensionSaveData
    (all this happening in ScDPCollection::SheetCaches::getCache).
    Consequently, accessing or rebuilding pivot cache may crash bc mpGroup
    now points nowhere.
    
    I split and renamed ScDPCache::ClearGroupFields into 2 parts, one of
    them clears maGroupFields, the other resets mpGroup ptrs in maFields.
    When adding data to cache, the former is used (bc group ptrs get reset
    almost immediately afterwards)
    
    Change-Id: I96e8d234a17da0f3cc65c0625aa47b12284b98b8
    Reviewed-on: https://gerrit.libreoffice.org/65329
    Tested-by: Jenkins
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Reviewed-by: Kohei Yoshida <libreoffice at kohei.us>
    Reviewed-by: Katarina Behrens <Katarina.Behrens at cib.de>

diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx
index b016513421c0..1c88c14fff7e 100644
--- a/sc/inc/dpcache.hxx
+++ b/sc/inc/dpcache.hxx
@@ -149,6 +149,7 @@ public:
     SCROW SetGroupItem(long nDim, const ScDPItemData& rData);
     void GetGroupDimMemberIds(long nDim, std::vector<SCROW>& rIds) const;
     void ClearGroupFields();
+    void ClearAllFields();
     const ScDPNumGroupInfo* GetNumGroupInfo(long nDim) const;
 
     /**
diff --git a/sc/qa/unit/data/ods/numgroup_example.ods b/sc/qa/unit/data/ods/numgroup_example.ods
new file mode 100644
index 000000000000..008e1bfd4970
Binary files /dev/null and b/sc/qa/unit/data/ods/numgroup_example.ods differ
diff --git a/sc/qa/unit/helper/qahelper.cxx b/sc/qa/unit/helper/qahelper.cxx
index 81c4431f7512..c8d9bfdfc655 100644
--- a/sc/qa/unit/helper/qahelper.cxx
+++ b/sc/qa/unit/helper/qahelper.cxx
@@ -695,7 +695,7 @@ ScDocShellRef ScBootstrapFixture::saveAndReload( ScDocShell* pShell, sal_Int32 n
     return xDocSh;
 }
 
-std::shared_ptr<utl::TempFile> ScBootstrapFixture::exportTo( ScDocShell* pShell, sal_Int32 nFormat )
+std::shared_ptr<utl::TempFile> ScBootstrapFixture::saveAs( ScDocShell* pShell, sal_Int32 nFormat )
 {
     OUString aFilterName(aFileFormats[nFormat].pFilterName, strlen(aFileFormats[nFormat].pFilterName), RTL_TEXTENCODING_UTF8) ;
     OUString aFilterType(aFileFormats[nFormat].pTypeName, strlen(aFileFormats[nFormat].pTypeName), RTL_TEXTENCODING_UTF8);
@@ -714,8 +714,16 @@ std::shared_ptr<utl::TempFile> ScBootstrapFixture::exportTo( ScDocShell* pShell,
     pExportFilter.get()->SetVersion(SOFFICE_FILEFORMAT_CURRENT);
     aStoreMedium.SetFilter(pExportFilter);
     pShell->DoSaveAs( aStoreMedium );
+
+    return pTempFile;
+}
+
+std::shared_ptr<utl::TempFile> ScBootstrapFixture::exportTo( ScDocShell* pShell, sal_Int32 nFormat )
+{
+    std::shared_ptr<utl::TempFile> pTempFile = saveAs(pShell, nFormat);
     pShell->DoClose();
 
+    SfxFilterFlags nFormatType = aFileFormats[nFormat].nFormatType;
     if(nFormatType == XLSX_FORMAT_TYPE)
         validate(pTempFile->GetFileName(), test::OOXML);
     else if (nFormatType == ODS_FORMAT_TYPE)
diff --git a/sc/qa/unit/helper/qahelper.hxx b/sc/qa/unit/helper/qahelper.hxx
index 96268dbb9272..11a19d618d3e 100644
--- a/sc/qa/unit/helper/qahelper.hxx
+++ b/sc/qa/unit/helper/qahelper.hxx
@@ -199,6 +199,7 @@ public:
 
     ScDocShellRef saveAndReload( ScDocShell* pShell, sal_Int32 nFormat );
 
+    std::shared_ptr<utl::TempFile> saveAs(ScDocShell* pShell, sal_Int32 nFormat);
     std::shared_ptr<utl::TempFile> exportTo(ScDocShell* pShell, sal_Int32 nFormat);
 
     void miscRowHeightsTest( TestParam const * aTestValues, unsigned int numElems );
diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index 5b1ff5fa66a0..f99029dfcaef 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -39,6 +39,7 @@
 #include <validat.hxx>
 #include <global.hxx>
 #include <scmod.hxx>
+#include <dpcache.hxx>
 #include <dpobject.hxx>
 
 #include <svx/svdpage.hxx>
@@ -208,6 +209,7 @@ public:
 
     void testTdf118990();
     void testTdf121612();
+    void testPivotCacheAfterExportXLSX();
 
     void testXltxExport();
 
@@ -324,6 +326,7 @@ public:
 
     CPPUNIT_TEST(testTdf118990);
     CPPUNIT_TEST(testTdf121612);
+    CPPUNIT_TEST(testPivotCacheAfterExportXLSX);
 
     CPPUNIT_TEST(testXltxExport);
 
@@ -4210,6 +4213,35 @@ void ScExportTest::testXltxExport()
         "ContentType", "application/vnd.openxmlformats-officedocument.spreadsheetml.template.main+xml");
 }
 
+void ScExportTest::testPivotCacheAfterExportXLSX()
+{
+    ScDocShellRef xDocSh = loadDoc("numgroup_example.", FORMAT_ODS);
+    CPPUNIT_ASSERT(xDocSh.is());
+
+    // export only
+    std::shared_ptr<utl::TempFile> pTemp = saveAs(xDocSh.get(), FORMAT_XLSX);
+
+    ScDocument& rDoc = xDocSh->GetDocument();
+    CPPUNIT_ASSERT(rDoc.HasPivotTable());
+
+    // Two pivot tables
+    ScDPCollection* pDPColl = rDoc.GetDPCollection();
+    CPPUNIT_ASSERT(pDPColl);
+    CPPUNIT_ASSERT_EQUAL(size_t(2), pDPColl->GetCount());
+
+    // One cache
+    ScDPCollection::SheetCaches& rSheetCaches = pDPColl->GetSheetCaches();
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rSheetCaches.size());
+    const ScDPCache* pCache = rSheetCaches.getExistingCache(ScRange(0, 0, 0, 3, 30, 0));
+    CPPUNIT_ASSERT_MESSAGE("Pivot cache is expected for A1:D31 on the first sheet.", pCache);
+
+    // See if XLSX export didn't damage group info of the 1st pivot table
+    const ScDPNumGroupInfo* pInfo = pCache->GetNumGroupInfo(1);
+    CPPUNIT_ASSERT_MESSAGE("No number group info :(", pInfo);
+
+    xDocSh->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScExportTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/core/data/dpcache.cxx b/sc/source/core/data/dpcache.cxx
index 333a38d3070d..7820e9090571 100644
--- a/sc/source/core/data/dpcache.cxx
+++ b/sc/source/core/data/dpcache.cxx
@@ -1391,6 +1391,11 @@ struct ClearGroupItems
 void ScDPCache::ClearGroupFields()
 {
     maGroupFields.clear();
+}
+
+void ScDPCache::ClearAllFields()
+{
+    ClearGroupFields();
     std::for_each(maFields.begin(), maFields.end(), ClearGroupItems());
 }
 
diff --git a/sc/source/core/data/dpobject.cxx b/sc/source/core/data/dpobject.cxx
index 59e131c03a0e..a362be2bf457 100644
--- a/sc/source/core/data/dpobject.cxx
+++ b/sc/source/core/data/dpobject.cxx
@@ -3542,9 +3542,9 @@ bool ScDPCollection::ReloadGroupsInCache(const ScDPObject* pDPObj, std::set<ScDP
     if (!pCache)
         return false;
 
-    // Clear the existing group data from the cache, and rebuild it from the
+    // Clear the existing group/field data from the cache, and rebuild it from the
     // dimension data.
-    pCache->ClearGroupFields();
+    pCache->ClearAllFields();
     const ScDPDimensionSaveData* pDimData = pSaveData->GetExistingDimensionData();
     if (pDimData)
         pDimData->WriteToCache(*pCache);


More information about the Libreoffice-commits mailing list