summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKatarina Behrens <Katarina.Behrens@cib.de>2018-12-18 11:12:49 +0100
committerAndras Timar <andras.timar@collabora.com>2019-02-18 11:51:53 +0100
commitb177d3498a9bc450278b023c3b6c84233670dabb (patch)
treedc5fd4b1c44e4d3d5f7b19835e2cdfe05220a325
parentfb0b9f831782ecd9a3d43e5a9ad86b2b79ee66ec (diff)
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) Reviewed-on: https://gerrit.libreoffice.org/65329 Tested-by: Jenkins Reviewed-by: Eike Rathke <erack@redhat.com> Reviewed-by: Kohei Yoshida <libreoffice@kohei.us> Reviewed-by: Katarina Behrens <Katarina.Behrens@cib.de> (cherry picked from commit f70d29fd91d232f0b030f0f76bd23bd2919eb868) Change-Id: I96e8d234a17da0f3cc65c0625aa47b12284b98b8 Reviewed-on: https://gerrit.libreoffice.org/66622 Reviewed-by: Eike Rathke <erack@redhat.com> Tested-by: Jenkins (cherry picked from commit 59d4b488e0d25266f02ca142d18deee7ecc5dc55)
-rw-r--r--sc/inc/dpcache.hxx1
-rw-r--r--sc/qa/unit/data/ods/numgroup_example.odsbin0 -> 17689 bytes
-rw-r--r--sc/qa/unit/helper/qahelper.cxx10
-rw-r--r--sc/qa/unit/helper/qahelper.hxx3
-rw-r--r--sc/qa/unit/subsequent_export-test.cxx33
-rw-r--r--sc/source/core/data/dpcache.cxx5
-rw-r--r--sc/source/core/data/dpobject.cxx4
7 files changed, 52 insertions, 4 deletions
diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx
index 8601d7177aae..807e5417b2f1 100644
--- a/sc/inc/dpcache.hxx
+++ b/sc/inc/dpcache.hxx
@@ -145,6 +145,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
--- /dev/null
+++ b/sc/qa/unit/data/ods/numgroup_example.ods
Binary files differ
diff --git a/sc/qa/unit/helper/qahelper.cxx b/sc/qa/unit/helper/qahelper.cxx
index b53c8377e1a0..f30edb6b4180 100644
--- a/sc/qa/unit/helper/qahelper.cxx
+++ b/sc/qa/unit/helper/qahelper.cxx
@@ -694,7 +694,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);
@@ -713,8 +713,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 1dc8e50a6b2e..2a4eab35b5f4 100644
--- a/sc/qa/unit/helper/qahelper.hxx
+++ b/sc/qa/unit/helper/qahelper.hxx
@@ -240,7 +240,8 @@ public:
ScDocShellRef saveAndReload( ScDocShell* pShell, sal_Int32 nFormat );
- static std::shared_ptr<utl::TempFile> exportTo( ScDocShell* pShell, sal_Int32 nFormat );
+ static std::shared_ptr<utl::TempFile> saveAs(ScDocShell* pShell, sal_Int32 nFormat);
+ static 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 6492d7bdf890..f3532e7278b7 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -44,6 +44,8 @@
#include <attrib.hxx>
#include <globstr.hrc>
#include <global.hxx>
+#include <scmod.hxx>
+#include <dpcache.hxx>
#include <dpobject.hxx>
#include <svx/svdoole2.hxx>
@@ -213,6 +215,7 @@ public:
void testKeepSettingsOfBlankRows();
void testTdf121612();
+ void testPivotCacheAfterExportXLSX();
CPPUNIT_TEST_SUITE(ScExportTest);
CPPUNIT_TEST(test);
@@ -324,6 +327,7 @@ public:
CPPUNIT_TEST(testTdf118990);
CPPUNIT_TEST(testKeepSettingsOfBlankRows);
CPPUNIT_TEST(testTdf121612);
+ CPPUNIT_TEST(testPivotCacheAfterExportXLSX);
CPPUNIT_TEST_SUITE_END();
@@ -4122,6 +4126,35 @@ void ScExportTest::testTdf121612()
CPPUNIT_ASSERT_EQUAL(size_t(1), pDPColl->GetCount());
}
+void ScExportTest::testPivotCacheAfterExportXLSX()
+{
+ ScDocShellRef xDocSh = loadDoc("numgroup_example.", FORMAT_ODS);
+ CPPUNIT_ASSERT(xDocSh.is());
+
+ // export only
+ std::shared_ptr<utl::TempFile> pTemp = ScBootstrapFixture::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 aec55f8c2ef8..88b22a8a7d7f 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 8486df51d431..0e1da2f2ae08 100644
--- a/sc/source/core/data/dpobject.cxx
+++ b/sc/source/core/data/dpobject.cxx
@@ -3560,9 +3560,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);