diff options
author | Bartosz Kosiorek <gang65@poczta.onet.pl> | 2017-08-01 05:40:34 +0200 |
---|---|---|
committer | Bartosz Kosiorek <gang65@poczta.onet.pl> | 2017-08-12 23:16:20 +0200 |
commit | 98d7a294180915a0b090000e808fe65c64695b5d (patch) | |
tree | 88401c1e3925f0816629966c3ce33ed7f00c2812 | |
parent | 67c801243fb82833be3b0f0cf0719427d1867088 (diff) |
tdf#89139 Fix PivotCache fields according to OOXML specification
Apply changes to fields:
- XML_containsInteger
- XML_containsBlank
- XML_containsMixedTypes
- XML_containsSemiMixedTypes
According to OOXML specification
https://technet.microsoft.com/en-us/library/documentformat.openxml.spreadsheet.shareditems.aspx
Generally OOXML specification allows listing items for mixed items (example: STRING + NUMBERS).
This patch is fixing that.
Example of mixed types:
<cacheField name="pwdLastSet" numFmtId="0">
<sharedItems containsDate="1" containsBlank="1" containsMixedTypes="1" minDate="2014-07-07T09:30:31" maxDate="2017-03-24T08:38:46" count="4">
<m/>
<d v="2016-06-15T12:18:34"/>
<s v=""/>
<d v="2017-03-21T10:27:39"/>
</sharedItems>
</cacheField>
Change-Id: I02b07c79bea60890e3c995dd70cb5c72901a3d4a
Reviewed-on: https://gerrit.libreoffice.org/40610
Reviewed-by: Bartosz Kosiorek <gang65@poczta.onet.pl>
Tested-by: Bartosz Kosiorek <gang65@poczta.onet.pl>
-rw-r--r-- | sc/qa/unit/data/xlsx/pivot.xlsx | bin | 0 -> 12769 bytes | |||
-rw-r--r-- | sc/qa/unit/subsequent_export-test.cxx | 69 | ||||
-rw-r--r-- | sc/source/filter/excel/xepivotxml.cxx | 79 |
3 files changed, 131 insertions, 17 deletions
diff --git a/sc/qa/unit/data/xlsx/pivot.xlsx b/sc/qa/unit/data/xlsx/pivot.xlsx Binary files differnew file mode 100644 index 000000000000..e6297a91777b --- /dev/null +++ b/sc/qa/unit/data/xlsx/pivot.xlsx diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx index 8823ece151c1..467e3bb2ff23 100644 --- a/sc/qa/unit/subsequent_export-test.cxx +++ b/sc/qa/unit/subsequent_export-test.cxx @@ -112,7 +112,7 @@ public: void testCellNoteExportXLS(); void testFormatExportODS(); - + void testPivotExportXLSX(); void testCommentExportXLSX(); #if HAVE_MORE_FONTS void testCustomColumnWidthExportXLSX(); @@ -223,6 +223,7 @@ public: CPPUNIT_TEST(testCellNoteExportXLS); CPPUNIT_TEST(testFormatExportODS); + CPPUNIT_TEST(testPivotExportXLSX); CPPUNIT_TEST(testCommentExportXLSX); #if HAVE_MORE_FONTS CPPUNIT_TEST(testCustomColumnWidthExportXLSX); @@ -538,6 +539,72 @@ void ScExportTest::testFormatExportODS() xDocSh->DoClose(); } +void ScExportTest::testPivotExportXLSX() +{ + // tdf#89139 FILESAVE xlsx pivot table corrupted after save with LO and re-open with MS Office + // MS Excel is very sensitive for proper values of fields: + // containsMixedTypes, containsSemiMixedTypes, containsInteger, containsBlank + // If it is not properly set, then Excel is not opening spreadsheet properly. + // This test case ensures, that such values are properly set according to documentaion: + // https://technet.microsoft.com/en-us/library/documentformat.openxml.spreadsheet.shareditems.aspx + + ScDocShellRef xShell = loadDoc("pivot.", FORMAT_XLSX); + CPPUNIT_ASSERT(xShell.is()); + + std::shared_ptr<utl::TempFile> pXPathFile = ScBootstrapFixture::exportTo(&(*xShell), FORMAT_XLSX); + xmlDocPtr pSheet = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/pivotCache/pivotCacheDefinition1.xml"); + CPPUNIT_ASSERT(pSheet); + + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField", 5); + + // Four strings and one empty field + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]", "name", "imieinazwisko"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsBlank", "1"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsMixedTypes"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsSemiMixedTypes"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsString"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsNumber"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "containsInteger"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "minValue"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "maxValue"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[1]/x:sharedItems", "count", "5"); + + // Two integers and one empty field + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]", "name", "wartosc"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsBlank", "1"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsMixedTypes"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsSemiMixedTypes"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsString", "0"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsNumber", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "containsInteger", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "minValue", "111"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[2]/x:sharedItems", "maxValue", "222"); + // We list items on round-trip and Excel accepts that; so no check for "count" attribute + + // Five integers + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]", "name", "druga wartosc"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsBlank"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsMixedTypes"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsSemiMixedTypes", "0"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsString", "0"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsNumber", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "containsInteger", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "minValue", "1111"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "maxValue", "5555"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[3]/x:sharedItems", "count"); + + // Three integers and one string + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]", "name", "trzecia wartosc"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsBlank"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsMixedTypes", "1"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsSemiMixedTypes"); + assertXPathNoAttribute(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsString"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsNumber", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "containsInteger", "1"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "minValue", "1234"); + assertXPath(pSheet, "/x:pivotCacheDefinition/x:cacheFields/x:cacheField[4]/x:sharedItems", "maxValue", "5678"); + // We list items on round-trip and Excel accepts that; so no check for "count" attribute +} void ScExportTest::testCommentExportXLSX() { diff --git a/sc/source/filter/excel/xepivotxml.cxx b/sc/source/filter/excel/xepivotxml.cxx index 9f1da86dcb9b..c5872e24da4a 100644 --- a/sc/source/filter/excel/xepivotxml.cxx +++ b/sc/source/filter/excel/xepivotxml.cxx @@ -234,6 +234,8 @@ void XclExpXmlPivotCaches::SavePivotCacheXml( XclExpXmlStream& rStrm, const Entr std::set<ScDPItemData::Type> aDPTypes; double fMin = std::numeric_limits<double>::infinity(), fMax = -std::numeric_limits<double>::infinity(); + bool isValueInteger = true; + double intpart; for (; it != itEnd; ++it) { ScDPItemData::Type eType = it->GetType(); @@ -243,31 +245,76 @@ void XclExpXmlPivotCaches::SavePivotCacheXml( XclExpXmlStream& rStrm, const Entr double fVal = it->GetValue(); fMin = std::min(fMin, fVal); fMax = std::max(fMax, fVal); + // Check if all values are integers + if (isValueInteger && (modf(fVal, &intpart) != 0.0)) + { + isValueInteger = false; + } } } auto aDPTypeEnd = aDPTypes.cend(); auto pAttList = sax_fastparser::FastSerializerHelper::createAttrList(); - // tdf#89139: Only create item list for string-only fields. - // Using containsXXX attributes in this case makes Excel think the file is corrupted. - // OTOH listing items for e.g. number fields also triggers "corrupted" warning in Excel. - bool bListItems = aDPTypes.size() == 1 && aDPTypes.find(ScDPItemData::String) != aDPTypeEnd; - if (bListItems) + + bool bListItems = true; + + std::set<ScDPItemData::Type> aDPTypesWithoutBlank = aDPTypes; + aDPTypesWithoutBlank.erase(ScDPItemData::Empty); + + bool isContainsMoreThanOneType = aDPTypesWithoutBlank.size() > 1; + // XML_containsMixedType possible values: + // 1 - field contains more than one data type + // 0 - (Default) only one data type. The field can still contain blank values (that's why we are using aDPTypesWithoutBlank) + if (isContainsMoreThanOneType) + pAttList->add(XML_containsMixedTypes, ToPsz10(true)); + + bool isContainsString = aDPTypesWithoutBlank.find(ScDPItemData::String) != aDPTypesWithoutBlank.end(); + // XML_containsSemiMixedTypes possible values: + // 1 - (Default) at least one text value, or can also contain a mix of other data types and blank values + // 0 - the field does not have a mix of text and other values + if (!(isContainsString || (aDPTypes.size() > 1))) + pAttList->add(XML_containsSemiMixedTypes, ToPsz10(false)); + + // default for containsString field is true, so we are writing only when is false + if (!isContainsString) + pAttList->add(XML_containsString, ToPsz10(false)); + + bool isContainsBlank = aDPTypes.find(ScDPItemData::Empty) != aDPTypeEnd; + if (isContainsBlank) + pAttList->add(XML_containsBlank, ToPsz10(true)); + + bool isContainsNumber = aDPTypesWithoutBlank.find(ScDPItemData::Value) != aDPTypesWithoutBlank.end(); + if (isContainsNumber) + pAttList->add(XML_containsNumber, ToPsz10(true)); + + if (isValueInteger && isContainsNumber) + pAttList->add(XML_containsInteger, ToPsz10(true)); + + + // Number type fields could be mixed with blank types, and it shouldn't be treated as listed items. + // Example: + // <cacheField name="employeeID" numFmtId="0"> + // <sharedItems containsString="0" containsBlank="1" containsNumber="1" containsInteger="1" minValue="35" maxValue="89"/> + // </cacheField> + if (isContainsNumber) { - pAttList->add(XML_count, OString::number(static_cast<long>(rFieldItems.size()))); + pAttList->add(XML_minValue, OString::number(fMin)); + pAttList->add(XML_maxValue, OString::number(fMax)); + // If there is only numeric types, then we shouldn't list all items + if (aDPTypesWithoutBlank.size() == 1) + bListItems = false; } - else + + if (isContainsBlank && (aDPTypes.size() == 1)) { - pAttList->add(XML_containsMixedTypes, ToPsz10(aDPTypes.size() > 1)); - pAttList->add(XML_containsSemiMixedTypes, ToPsz10(aDPTypes.size() > 1)); - pAttList->add(XML_containsString, ToPsz10(aDPTypes.find(ScDPItemData::String) != aDPTypeEnd)); - if (aDPTypes.find(ScDPItemData::Value) != aDPTypeEnd) - { - pAttList->add(XML_containsNumber, ToPsz10(true)); - pAttList->add(XML_minValue, OString::number(fMin)); - pAttList->add(XML_maxValue, OString::number(fMax)); - } + // If all items are blank, then we shouldn't list all items + bListItems = false; + } + + if (bListItems) + { + pAttList->add(XML_count, OString::number(static_cast<long>(rFieldItems.size()))); } sax_fastparser::XFastAttributeListRef xAttributeList(pAttList); |