sc/inc/dpcache.hxx | 1 sc/qa/unit/data/ods/caseinsensitive-duplicate-fields.ods |binary sc/qa/unit/pivottable_filters_test.cxx | 24 +++ sc/source/core/data/dpcache.cxx | 97 +++++---------- 4 files changed, 60 insertions(+), 62 deletions(-)
New commits: commit cba44da5458824dfddfa785b3fd41347cd975756 Author: Dennis Francis <dennis.fran...@collabora.com> AuthorDate: Wed Apr 10 04:30:25 2019 +0530 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Fri Apr 12 15:08:45 2019 +0200 dpcache : use case-insensitive normalization of... field labels, else on export to xlsx, Excel will fail to load the pivot table due to case-insensitive duplicate field labels in the pivotCacheDefinition1.xml. This could be done just for xlsx export filter, but we do normalization in dpcache.cxx anyway and it would not hurt if we do a case-insensitive normalization here. The private member ScDPCache::AddLabel had code duplication and more importantly it is called in loop for every label in the database so results in O(n^2) time complexity where n is the number of labels, so removed it to reuse normalizeLabels() at the only call-site. Also added a unit test that checks case-insensitive normalization. Change-Id: Id563dee232a98a2aea9f4fc29254f6942e1c5ba7 Reviewed-on: https://gerrit.libreoffice.org/70498 Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Tested-by: Jenkins Reviewed-by: Dennis Francis <dennis.fran...@collabora.com> diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx index 807e5417b2f1..d6c0be289eb7 100644 --- a/sc/inc/dpcache.hxx +++ b/sc/inc/dpcache.hxx @@ -205,7 +205,6 @@ public: private: void PostInit(); void Clear(); - void AddLabel(const OUString& rLabel); const GroupItems* GetGroupItems(long nDim) const; }; diff --git a/sc/qa/unit/data/ods/caseinsensitive-duplicate-fields.ods b/sc/qa/unit/data/ods/caseinsensitive-duplicate-fields.ods new file mode 100644 index 000000000000..795b74ca3c00 Binary files /dev/null and b/sc/qa/unit/data/ods/caseinsensitive-duplicate-fields.ods differ diff --git a/sc/qa/unit/pivottable_filters_test.cxx b/sc/qa/unit/pivottable_filters_test.cxx index d54cb9dc18c6..9caf4d6a61b1 100644 --- a/sc/qa/unit/pivottable_filters_test.cxx +++ b/sc/qa/unit/pivottable_filters_test.cxx @@ -85,6 +85,7 @@ public: void testPivotTableOutlineModeXLSX(); void testPivotTableDuplicatedMemberFilterXLSX(); void testPivotTableTabularModeXLSX(); + void testPivotTableDuplicateFields(); void testTdf112106(); void testTdf123923(); void testTdf123939(); @@ -127,6 +128,7 @@ public: CPPUNIT_TEST(testPivotTableOutlineModeXLSX); CPPUNIT_TEST(testPivotTableDuplicatedMemberFilterXLSX); CPPUNIT_TEST(testPivotTableTabularModeXLSX); + CPPUNIT_TEST(testPivotTableDuplicateFields); CPPUNIT_TEST(testTdf112106); CPPUNIT_TEST(testTdf123923); CPPUNIT_TEST(testTdf123939); @@ -2346,6 +2348,28 @@ void ScPivotTableFiltersTest::testPivotTableTabularModeXLSX() assertXPath(pTable, "/x:pivotTableDefinition/x:pivotFields/x:pivotField[1]", "outline", "0"); } +void ScPivotTableFiltersTest::testPivotTableDuplicateFields() +{ + ScDocShellRef xShell = loadDoc("caseinsensitive-duplicate-fields.", FORMAT_ODS); + CPPUNIT_ASSERT(xShell.is()); + + std::shared_ptr<utl::TempFile> pXPathFile + = ScBootstrapFixture::exportTo(&(*xShell), FORMAT_XLSX); + xmlDocPtr pCacheDef + = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/pivotCache/pivotCacheDefinition1.xml"); + CPPUNIT_ASSERT(pCacheDef); + + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields", "count", "6"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]", "name", "ID"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]", "name", "Name"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]", "name", "Score"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]", "name", "Method"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[5]", "name", "method2"); + assertXPath(pCacheDef, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[6]", "name", "Method3"); + + xShell->DoClose(); +} + void ScPivotTableFiltersTest::testTdf112106() { ScDocShellRef xDocSh = loadDoc("tdf112106.", FORMAT_XLSX); diff --git a/sc/source/core/data/dpcache.cxx b/sc/source/core/data/dpcache.cxx index 88b22a8a7d7f..306ba153ff15 100644 --- a/sc/source/core/data/dpcache.cxx +++ b/sc/source/core/data/dpcache.cxx @@ -33,6 +33,7 @@ #include <cellvalue.hxx> #include <rtl/math.hxx> +#include <unotools/charclass.hxx> #include <unotools/textsearch.hxx> #include <unotools/localedatawrapper.hxx> #include <formula/errorcodes.hxx> @@ -336,43 +337,51 @@ struct InitDocData typedef std::unordered_set<OUString> LabelSet; -class InsertLabel +void normalizeAddLabel(const OUString& rLabel, std::vector<OUString>& rLabels, LabelSet& rExistingNames) { - LabelSet& mrNames; -public: - explicit InsertLabel(LabelSet& rNames) : mrNames(rNames) {} - void operator() (const OUString& r) + const OUString aLabelLower = ScGlobal::pCharClass->lowercase(rLabel); + sal_Int32 nSuffix = 1; + OUString aNewLabel = rLabel; + OUString aNewLabelLower = aLabelLower; + while (true) { - mrNames.insert(r); + if (!rExistingNames.count(aNewLabelLower)) + { + // this is a unique label. + rLabels.push_back(aNewLabel); + rExistingNames.insert(aNewLabelLower); + break; + } + + // This name already exists. + aNewLabel = rLabel + OUString::number(++nSuffix); + aNewLabelLower = aLabelLower + OUString::number(nSuffix); } -}; +} -std::vector<OUString> normalizeLabels( const std::vector<InitColumnData>& rColData ) +std::vector<OUString> normalizeLabels(const std::vector<InitColumnData>& rColData) { std::vector<OUString> aLabels(1u, ScGlobal::GetRscString(STR_PIVOT_DATA)); LabelSet aExistingNames; for (const InitColumnData& rCol : rColData) - { - const OUString& rLabel = rCol.maLabel; - sal_Int32 nSuffix = 1; - OUString aNewLabel = rLabel; - while (true) - { - if (!aExistingNames.count(aNewLabel)) - { - // this is a unique label. - aLabels.push_back(aNewLabel); - aExistingNames.insert(aNewLabel); - break; - } + normalizeAddLabel(rCol.maLabel, aLabels, aExistingNames); - // This name already exists. - OUStringBuffer aBuf(rLabel); - aBuf.append(++nSuffix); - aNewLabel = aBuf.makeStringAndClear(); - } + return aLabels; +} + +std::vector<OUString> normalizeLabels(const ScDPCache::DBConnector& rDB, const sal_Int32 nLabelCount) +{ + std::vector<OUString> aLabels(nLabelCount+1); + aLabels.push_back(ScGlobal::GetRscString(STR_PIVOT_DATA)); + + LabelSet aExistingNames; + + for (sal_Int32 nCol = 0; nCol < nLabelCount; ++nCol) + { + OUString aColTitle = rDB.getColumnLabel(nCol); + normalizeAddLabel(aColTitle, aLabels, aExistingNames); } return aLabels; @@ -631,14 +640,7 @@ bool ScDPCache::InitFromDataBase(DBConnector& rDB) maFields.push_back(o3tl::make_unique<Field>()); // Get column titles and types. - maLabelNames.clear(); - maLabelNames.reserve(mnColumnCount+1); - - for (sal_Int32 nCol = 0; nCol < mnColumnCount; ++nCol) - { - OUString aColTitle = rDB.getColumnLabel(nCol); - AddLabel(aColTitle); - } + maLabelNames = normalizeLabels(rDB, mnColumnCount); std::vector<Bucket> aBuckets; ScDPItemData aData; @@ -959,33 +961,6 @@ void ScDPCache::Clear() maStringPools.clear(); } -void ScDPCache::AddLabel(const OUString& rLabel) -{ - - if ( maLabelNames.empty() ) - maLabelNames.push_back(ScGlobal::GetRscString(STR_PIVOT_DATA)); - - //reset name if needed - LabelSet aExistingNames; - std::for_each(maLabelNames.begin(), maLabelNames.end(), InsertLabel(aExistingNames)); - sal_Int32 nSuffix = 1; - OUString aNewName = rLabel; - while (true) - { - if (!aExistingNames.count(aNewName)) - { - // unique name found! - maLabelNames.push_back(aNewName); - return; - } - - // Name already exists. - OUStringBuffer aBuf(rLabel); - aBuf.append(++nSuffix); - aNewName = aBuf.makeStringAndClear(); - } -} - SCROW ScDPCache::GetItemDataId(sal_uInt16 nDim, SCROW nRow, bool bRepeatIfEmpty) const { OSL_ENSURE(nDim < mnColumnCount, "ScDPTableDataCache::GetItemDataId "); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits