tdf#124676 : use case-insensitive normalization of...

ScDPCache 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.kaganski@collabora.com>
Tested-by: Jenkins
Reviewed-by: Dennis Francis <dennis.francis@collabora.com>
(cherry picked from commit 238cadd315901cbacfd9304bb1205e9f53f13eae)
Reviewed-on: https://gerrit.libreoffice.org/70703

4 files changed, 60 insertions, 62 deletions

diff --git a/sc/inc/dpcache.hxx b/sc/inc/dpcache.hxx
index 1c88c14fff7e..1d1559755369 100644
--- a/sc/inc/dpcache.hxx
+++ b/sc/inc/dpcache.hxx
@@ -209,7 +209,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
--- /dev/null
+++ b/sc/qa/unit/data/ods/caseinsensitive-duplicate-fields.ods
diff --git a/sc/qa/unit/pivottable_filters_test.cxx b/sc/qa/unit/pivottable_filters_test.cxx
index 7550746c3e9b..c2546bc29cb7 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();
 
@@ -126,6 +127,7 @@ public:
     CPPUNIT_TEST(testPivotTableOutlineModeXLSX);
     CPPUNIT_TEST(testPivotTableDuplicatedMemberFilterXLSX);
     CPPUNIT_TEST(testPivotTableTabularModeXLSX);
+    CPPUNIT_TEST(testPivotTableDuplicateFields);
     CPPUNIT_TEST(testTdf112106);
     CPPUNIT_TEST(testTdf123923);
 
@@ -2344,6 +2346,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 7820e9090571..fce310bd1ee6 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 <unotools/collatorwrapper.hxx>
@@ -337,43 +338,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, ScResId(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(ScResId(STR_PIVOT_DATA));
+
+    LabelSet aExistingNames;
+
+    for (sal_Int32 nCol = 0; nCol < nLabelCount; ++nCol)
+    {
+        OUString aColTitle = rDB.getColumnLabel(nCol);
+        normalizeAddLabel(aColTitle, aLabels, aExistingNames);
     }
 
     return aLabels;
@@ -632,14 +641,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(ScResId(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 ");