[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.0' - sc/inc sc/qa sc/source
Dennis Francis (via logerrit)
logerrit at kemper.freedesktop.org
Fri Apr 12 13:16:20 UTC 2019
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.francis at collabora.com>
AuthorDate: Wed Apr 10 04:30:25 2019 +0530
Commit: Andras Timar <andras.timar at 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.kaganski at collabora.com>
Tested-by: Jenkins
Reviewed-by: Dennis Francis <dennis.francis at 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 ");
More information about the Libreoffice-commits
mailing list