[Libreoffice-commits] core.git: sc/inc sc/qa sc/source
Dennis Francis (via logerrit)
logerrit at kemper.freedesktop.org
Thu Apr 11 22:49:18 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 238cadd315901cbacfd9304bb1205e9f53f13eae
Author: Dennis Francis <dennis.francis at collabora.com>
AuthorDate: Wed Apr 10 04:30:25 2019 +0530
Commit: Dennis Francis <dennis.francis at collabora.com>
CommitDate: Fri Apr 12 00:48:38 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 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
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 d5b1afd64701..719b9bc95e70 100644
--- a/sc/qa/unit/pivottable_filters_test.cxx
+++ b/sc/qa/unit/pivottable_filters_test.cxx
@@ -82,6 +82,7 @@ public:
void testPivotTableOutlineModeXLSX();
void testPivotTableDuplicatedMemberFilterXLSX();
void testPivotTableTabularModeXLSX();
+ void testPivotTableDuplicateFields();
void testTdf112106();
void testTdf123923();
void testTdf123939();
@@ -125,6 +126,7 @@ public:
CPPUNIT_TEST(testPivotTableOutlineModeXLSX);
CPPUNIT_TEST(testPivotTableDuplicatedMemberFilterXLSX);
CPPUNIT_TEST(testPivotTableTabularModeXLSX);
+ CPPUNIT_TEST(testPivotTableDuplicateFields);
CPPUNIT_TEST(testTdf112106);
CPPUNIT_TEST(testTdf123923);
CPPUNIT_TEST(testTdf123939);
@@ -2359,6 +2361,28 @@ void ScPivotTableFiltersTest::testPivotTableTabularModeXLSX()
xShell->DoClose();
}
+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 60011acd6317..de62e25f5d91 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>
@@ -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, 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;
@@ -631,14 +640,7 @@ bool ScDPCache::InitFromDataBase(DBConnector& rDB)
maFields.push_back(std::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;
@@ -958,33 +960,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 ");
More information about the Libreoffice-commits
mailing list