tdf#122590: follow-up : import x14:cfRule priorities

If there are x:cfRule's and x14:cfRule's with matching
range-list, then insert the conditional-fmt entries into
the document in the order of the priorities. That is
don't just append the x14:cfRule entries to the document
after the x:cfRule entries are inserted.

There was also a off-by-one bug in the priority export
of x14:cfRule entries. This caused the priority numbers
to be duplicated. This is also fixed.

Reviewed-on: https://gerrit.libreoffice.org/71311
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar@collabora.com>
(cherry picked from commit c2f1c68ffb6dfa1ce7de09dcc428d6c53549e88d)

Change-Id: I5b0d11c4456b2966b808f6ee589075a870f43768
Reviewed-on: https://gerrit.libreoffice.org/72003
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Tested-by: Andras Timar <andras.timar@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/79221

7 files changed, 163 insertions, 12 deletions

diff --git a/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx b/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx
new file mode 100644
index 000000000000..e9af11d00615
--- /dev/null
+++ b/sc/qa/unit/data/xlsx/conditional_fmt_checkpriority.xlsx
diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index f3f0bd291f4a..29849f8531e7 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -112,6 +112,7 @@ public:
     void testDataBarExportXLSX();
     void testConditionalFormatRangeListXLSX();
     void testConditionalFormatContainsTextXLSX();
+    void testConditionalFormatPriorityCheckXLSX();
     void testMiscRowHeightExport();
     void testNamedRangeBugfdo62729();
     void testBuiltinRangesXLSX();
@@ -242,6 +243,7 @@ public:
     CPPUNIT_TEST(testDataBarExportXLSX);
     CPPUNIT_TEST(testConditionalFormatRangeListXLSX);
     CPPUNIT_TEST(testConditionalFormatContainsTextXLSX);
+    CPPUNIT_TEST(testConditionalFormatPriorityCheckXLSX);
     CPPUNIT_TEST(testMiscRowHeightExport);
     CPPUNIT_TEST(testNamedRangeBugfdo62729);
     CPPUNIT_TEST(testBuiltinRangesXLSX);
@@ -373,6 +375,8 @@ void ScExportTest::registerNamespaces(xmlXPathContextPtr& pXmlXPathCtx)
         { BAD_CAST("r"), BAD_CAST("http://schemas.openxmlformats.org/package/2006/relationships") },
         { BAD_CAST("number"), BAD_CAST("urn:oasis:names:tc:opendocument:xmlns:datastyle:1.0") },
         { BAD_CAST("loext"), BAD_CAST("urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0") },
+        { BAD_CAST("x14"), BAD_CAST("http://schemas.microsoft.com/office/spreadsheetml/2009/9/main") },
+        { BAD_CAST("xm"), BAD_CAST("http://schemas.microsoft.com/office/excel/2006/main") },
     };
     for(size_t i = 0; i < SAL_N_ELEMENTS(aNamespaces); ++i)
     {
@@ -3998,6 +4002,50 @@ void ScExportTest::testConditionalFormatContainsTextXLSX()
     assertXPathContent(pDoc, "//x:conditionalFormatting/x:cfRule/x:formula", "NOT(ISERROR(SEARCH(\"test\",A1)))");
 }
 
+void ScExportTest::testConditionalFormatPriorityCheckXLSX()
+{
+    ScDocShellRef xDocSh = loadDoc("conditional_fmt_checkpriority.", FORMAT_XLSX);
+    CPPUNIT_ASSERT(xDocSh.is());
+
+    xmlDocPtr pDoc = XPathHelper::parseExport2(*this, *xDocSh, m_xSFactory, "xl/worksheets/sheet1.xml", FORMAT_XLSX);
+    CPPUNIT_ASSERT(pDoc);
+
+    constexpr bool bHighPriorityExtensionA1 = true;  // Should A1's extension cfRule has higher priority than normal cfRule ?
+    constexpr bool bHighPriorityExtensionA3 = false; // Should A3's extension cfRule has higher priority than normal cfRule ?
+
+    size_t nA1NormalPriority = 0;
+    size_t nA1ExtPriority = 0;
+    size_t nA3NormalPriority = 0;
+    size_t nA3ExtPriority = 0;
+
+    for (size_t nIdx = 1; nIdx <= 2; ++nIdx)
+    {
+        OString aIdx = OString::number(nIdx);
+        OUString aCellAddr = getXPath(pDoc, "//x:conditionalFormatting[" + aIdx + "]", "sqref");
+        OUString aPriority = getXPath(pDoc, "//x:conditionalFormatting[" + aIdx + "]/x:cfRule", "priority");;
+
+        CPPUNIT_ASSERT_MESSAGE("conditionalFormatting sqref must be either A1 or A3", aCellAddr == "A1" || aCellAddr == "A3");
+
+        if (aCellAddr == "A1")
+            nA1NormalPriority = aPriority.toUInt32();
+        else
+            nA3NormalPriority = aPriority.toUInt32();
+
+        aCellAddr = getXPathContent(pDoc, "//x:extLst/x:ext[1]/x14:conditionalFormattings/x14:conditionalFormatting[" + aIdx + "]/xm:sqref");
+        aPriority = getXPath(pDoc, "//x:extLst/x:ext[1]/x14:conditionalFormattings/x14:conditionalFormatting[" + aIdx + "]/x14:cfRule", "priority");
+
+        CPPUNIT_ASSERT_MESSAGE("x14:conditionalFormatting sqref must be either A1 or A3", aCellAddr == "A1" || aCellAddr == "A3");
+
+        if (aCellAddr == "A1")
+            nA1ExtPriority = aPriority.toUInt32();
+        else
+            nA3ExtPriority = aPriority.toUInt32();
+    }
+
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong priorities for A1", bHighPriorityExtensionA1, nA1ExtPriority < nA1NormalPriority);
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong priorities for A3", bHighPriorityExtensionA3, nA3ExtPriority < nA3NormalPriority);
+}
+
 void ScExportTest::testEscapeCharInNumberFormatXLSX()
 {
     ScDocShellRef xDocSh = loadDoc("tdf81939.", FORMAT_XLSX);
diff --git a/sc/source/filter/excel/xeextlst.cxx b/sc/source/filter/excel/xeextlst.cxx
index dc7902065df1..78103231e096 100644
--- a/sc/source/filter/excel/xeextlst.cxx
+++ b/sc/source/filter/excel/xeextlst.cxx
@@ -408,7 +408,7 @@ void XclExpExtCfRule::SaveXml( XclExpXmlStream& rStrm )
     sax_fastparser::FSHelperPtr& rWorksheet = rStrm.GetCurrentStream();
     rWorksheet->startElementNS( XML_x14, XML_cfRule,
                                 XML_type, pType,
-                                XML_priority, mnPriority == -1 ? nullptr : OString::number(mnPriority).getStr(),
+                                XML_priority, mnPriority == -1 ? nullptr : OString::number(mnPriority + 1).getStr(),
                                 XML_operator, mOperator,
                                 XML_id, maId.getStr(),
                                 FSEND );
diff --git a/sc/source/filter/inc/condformatbuffer.hxx b/sc/source/filter/inc/condformatbuffer.hxx
index d67f648e8086..b59db9654be6 100644
--- a/sc/source/filter/inc/condformatbuffer.hxx
+++ b/sc/source/filter/inc/condformatbuffer.hxx
@@ -160,6 +160,9 @@ public:
     /** Imports rule settings from a CFRULE record. */
     void                importCfRule( SequenceInputStream& rStrm );
 
+    /** Directly set a ScFormatEntry with a priority ready for finalizeImport(). */
+    void                setFormatEntry(sal_Int32 nPriority, ScFormatEntry* pEntry);
+
     /** Creates a conditional formatting rule in the Calc document. */
     void                finalizeImport();
 
@@ -174,6 +177,7 @@ private:
     const CondFormat&   mrCondFormat;
     CondFormatRuleModel maModel;
     ScConditionalFormat* mpFormat;
+    ScFormatEntry*       mpFormatEntry;
     std::unique_ptr<ColorScaleRule> mpColor;
     std::unique_ptr<DataBarRule> mpDataBar;
     std::unique_ptr<IconSetRule> mpIconSet;
@@ -190,9 +194,12 @@ struct CondFormatModel
     explicit            CondFormatModel();
 };
 
+class CondFormatBuffer;
+
 /** Represents a conditional formatting object with a list of affected cell ranges. */
 class CondFormat : public WorksheetHelper
 {
+friend class CondFormatBuffer;
 public:
     explicit            CondFormat( const WorksheetHelper& rHelper );
 
@@ -268,15 +275,18 @@ public:
 class ExtCfCondFormat
 {
 public:
-    ExtCfCondFormat(const ScRangeList& aRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries);
+    ExtCfCondFormat(const ScRangeList& aRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries,
+                    std::vector<sal_Int32>* pPriorities = nullptr);
     ~ExtCfCondFormat();
 
     const ScRangeList& getRange();
     const std::vector< std::unique_ptr<ScFormatEntry> >& getEntries();
+    const std::vector<sal_Int32>& getPriorities() const { return maPriorities; }
 
 private:
     std::vector< std::unique_ptr<ScFormatEntry> > maEntries;
     ScRangeList const maRange;
+    std::vector<sal_Int32> maPriorities;
 };
 
 typedef std::shared_ptr< CondFormat > CondFormatRef;
@@ -307,6 +317,7 @@ private:
     CondFormatVec       maCondFormats;      /// All conditional formatting in a sheet.
     ExtCfDataBarRuleVec        maCfRules;          /// All external conditional formatting rules in a sheet.
     std::vector< std::unique_ptr<ExtCfCondFormat> > maExtCondFormats;
+    sal_Int32 mnNonPrioritizedRuleNextPriority = 1048576;
 };
 
 } // namespace xls
diff --git a/sc/source/filter/inc/extlstcontext.hxx b/sc/source/filter/inc/extlstcontext.hxx
index 85aafd1604d1..31754d7bda98 100644
--- a/sc/source/filter/inc/extlstcontext.hxx
+++ b/sc/source/filter/inc/extlstcontext.hxx
@@ -57,11 +57,13 @@ public:
 private:
     OUString aChars; // Characters of between xml elements.
     OUString rStyle; // Style of the corresponding condition
+    sal_Int32 nPriority; // Priority of last cfRule element.
     ScConditionMode eOperator; // Used only when cfRule type is "cellIs"
     bool isPreviousElementF;   // Used to distinguish alone <sqref> from <f> and <sqref>
     std::vector<std::unique_ptr<ScFormatEntry> > maEntries;
-    std::unique_ptr<IconSetRule> mpCurrentRule;
     std::vector< OUString > rFormulas; // It holds formulas for a range, there can be more formula for same range.
+    std::unique_ptr<IconSetRule> mpCurrentRule;
+    std::vector<sal_Int32> maPriorities;
 };
 
 /**
diff --git a/sc/source/filter/oox/condformatbuffer.cxx b/sc/source/filter/oox/condformatbuffer.cxx
index 088e33a7c061..0962cc51a1f0 100644
--- a/sc/source/filter/oox/condformatbuffer.cxx
+++ b/sc/source/filter/oox/condformatbuffer.cxx
@@ -18,6 +18,8 @@
  */
 
 #include <memory>
+#include <unordered_set>
+#include <unordered_map>
 #include <condformatbuffer.hxx>
 #include <formulaparser.hxx>
 
@@ -432,7 +434,8 @@ void CondFormatRuleModel::setBiff12TextType( sal_Int32 nOperator )
 CondFormatRule::CondFormatRule( const CondFormat& rCondFormat, ScConditionalFormat* pFormat ) :
     WorksheetHelper( rCondFormat ),
     mrCondFormat( rCondFormat ),
-    mpFormat(pFormat)
+    mpFormat(pFormat),
+    mpFormatEntry(nullptr)
 {
 }
 
@@ -706,8 +709,20 @@ void CondFormatRule::importCfRule( SequenceInputStream& rStrm )
     }
 }
 
+void CondFormatRule::setFormatEntry(sal_Int32 nPriority, ScFormatEntry* pEntry)
+{
+    maModel.mnPriority = nPriority;
+    mpFormatEntry = pEntry;
+}
+
 void CondFormatRule::finalizeImport()
 {
+    if (mpFormatEntry)
+    {
+        mpFormat->AddEntry(mpFormatEntry);
+        return;
+    }
+
     ScConditionMode eOperator = ScConditionMode::NONE;
 
     /*  Replacement formula for unsupported rule types (text comparison rules,
@@ -1099,10 +1114,63 @@ ScConditionalFormat* findFormatByRange(const ScRangeList& rRange, const ScDocume
     return nullptr;
 }
 
+class ScRangeListHasher
+{
+public:
+  size_t operator() (ScRangeList const& rRanges) const
+  {
+      size_t nHash = 0;
+      for (size_t nIdx = 0; nIdx < rRanges.size(); ++nIdx)
+          nHash += rRanges[nIdx].hashArea();
+      return nHash;
+  }
+};
+
 }
 
 void CondFormatBuffer::finalizeImport()
 {
+    std::unordered_set<size_t> aDoneExtCFs;
+    typedef std::unordered_map<ScRangeList, CondFormat*, ScRangeListHasher> RangeMap;
+    typedef std::vector<std::unique_ptr<ScFormatEntry>> FormatEntries;
+    RangeMap aRangeMap;
+    for (auto& rxCondFormat : maCondFormats)
+    {
+        if (aRangeMap.find(rxCondFormat->getRanges()) != aRangeMap.end())
+            continue;
+        aRangeMap[rxCondFormat->getRanges()] = rxCondFormat.get();
+    }
+
+    size_t nExtCFIndex = 0;
+    for (const auto& rxExtCondFormat : maExtCondFormats)
+    {
+        ScDocument* pDoc = &getScDocument();
+        const ScRangeList& rRange = rxExtCondFormat->getRange();
+        RangeMap::iterator it = aRangeMap.find(rRange);
+        if (it != aRangeMap.end())
+        {
+            CondFormat& rCondFormat = *it->second;
+            const FormatEntries& rEntries = rxExtCondFormat->getEntries();
+            const std::vector<sal_Int32>& rPriorities = rxExtCondFormat->getPriorities();
+            size_t nEntryIdx = 0;
+            for (const auto& rxEntry : rEntries)
+            {
+                CondFormatRuleRef xRule = rCondFormat.createRule();
+                ScFormatEntry* pNewEntry = rxEntry->Clone(pDoc);
+                sal_Int32 nPriority = rPriorities[nEntryIdx];
+                if (nPriority == -1)
+                    nPriority = mnNonPrioritizedRuleNextPriority++;
+                xRule->setFormatEntry(nPriority, pNewEntry);
+                rCondFormat.insertRule(xRule);
+                ++nEntryIdx;
+            }
+
+            aDoneExtCFs.insert(nExtCFIndex);
+        }
+
+        ++nExtCFIndex;
+    }
+
     CondFormatVec::iterator it = maCondFormats.begin();
     CondFormatVec::iterator it_end = maCondFormats.end();
     for( ; it != it_end; ++it )
@@ -1118,11 +1186,17 @@ void CondFormatBuffer::finalizeImport()
             (*ext_it).get()->finalizeImport();
     }
 
-    for (auto itr = maExtCondFormats.begin(); itr != maExtCondFormats.end(); ++itr)
+    nExtCFIndex = 0;
+    for (const auto& rxExtCondFormat : maExtCondFormats)
     {
-        ScDocument* pDoc = &getScDocument();
+        if (aDoneExtCFs.count(nExtCFIndex))
+        {
+            ++nExtCFIndex;
+            continue;
+        }
 
-        const ScRangeList& rRange = (*itr)->getRange();
+        ScDocument* pDoc = &getScDocument();
+        const ScRangeList& rRange = rxExtCondFormat->getRange();
         SCTAB nTab = rRange.front().aStart.Tab();
         ScConditionalFormat* pFormat = findFormatByRange(rRange, pDoc, nTab);
         if (!pFormat)
@@ -1134,11 +1208,13 @@ void CondFormatBuffer::finalizeImport()
             pDoc->AddCondFormatData(rRange, nTab, nKey);
         }
 
-        const std::vector< std::unique_ptr<ScFormatEntry> >& rEntries = (*itr)->getEntries();
+        const std::vector< std::unique_ptr<ScFormatEntry> >& rEntries = rxExtCondFormat->getEntries();
         for (auto i = rEntries.begin(); i != rEntries.end(); ++i)
         {
             pFormat->AddEntry((*i)->Clone(pDoc));
         }
+
+        ++nExtCFIndex;
     }
 
     rStyleIdx = 0; // Resets <extlst> <cfRule> style index.
@@ -1305,10 +1381,15 @@ void ExtCfDataBarRule::importCfvo( const AttributeList& rAttribs )
     maModel.maColorScaleType = rAttribs.getString( XML_type, OUString() );
 }
 
-ExtCfCondFormat::ExtCfCondFormat(const ScRangeList& rRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries):
+ExtCfCondFormat::ExtCfCondFormat(const ScRangeList& rRange, std::vector< std::unique_ptr<ScFormatEntry> >& rEntries,
+                                 std::vector<sal_Int32>* pPriorities):
     maRange(rRange)
 {
     maEntries.swap(rEntries);
+    if (pPriorities)
+        maPriorities = *pPriorities;
+    else
+        maPriorities.resize(maEntries.size(), -1);
 }
 
 ExtCfCondFormat::~ExtCfCondFormat()
diff --git a/sc/source/filter/oox/extlstcontext.cxx b/sc/source/filter/oox/extlstcontext.cxx
index f30e01a0ad08..533f672ac95c 100644
--- a/sc/source/filter/oox/extlstcontext.cxx
+++ b/sc/source/filter/oox/extlstcontext.cxx
@@ -83,6 +83,7 @@ void ExtCfRuleContext::onStartElement( const AttributeList& rAttribs )
 ExtConditionalFormattingContext::ExtConditionalFormattingContext(WorksheetContextBase& rFragment):
     WorksheetContextBase(rFragment)
 {
+    nPriority = -1;
     isPreviousElementF = false;
 }
 
@@ -103,6 +104,7 @@ ContextHandlerRef ExtConditionalFormattingContext::onCreateContext(sal_Int32 nEl
     {
         OUString aType = rAttribs.getString(XML_type, OUString());
         OUString aId = rAttribs.getString(XML_id, OUString());
+        nPriority = rAttribs.getInteger( XML_priority, -1 );
 
         if (aType == "dataBar")
         {
@@ -178,6 +180,7 @@ void ExtConditionalFormattingContext::onEndElement()
         case XM_TOKEN(f):
         {
             rFormulas.push_back(aChars);
+            maPriorities.push_back(nPriority);
         }
         break;
         case XLS14_TOKEN( cfRule ):
@@ -200,9 +203,9 @@ void ExtConditionalFormattingContext::onEndElement()
                 aRange[i].aEnd.SetTab(nTab);
             }
 
-            if(isPreviousElementF) // sqref can be alone in some cases.
+            if (isPreviousElementF) // sqref can be alone in some cases.
             {
-                for(const OUString& rFormula : rFormulas)
+                for (const OUString& rFormula : rFormulas)
                 {
                     ScAddress rPos = aRange.GetTopLeftCorner();
                     rStyle = getStyles().createExtDxfStyle(rStyleIdx);
@@ -214,11 +217,17 @@ void ExtConditionalFormattingContext::onEndElement()
                     maEntries.push_back(std::unique_ptr<ScFormatEntry>(pEntry));
                     rStyleIdx++;
                 }
+
+                assert(rFormulas.size() == maPriorities.size());
                 rFormulas.clear();
             }
 
             std::vector< std::unique_ptr<ExtCfCondFormat> >& rExtFormats =  getCondFormats().importExtCondFormat();
-            rExtFormats.push_back(o3tl::make_unique<ExtCfCondFormat>(aRange, maEntries));
+            rExtFormats.push_back(o3tl::make_unique<ExtCfCondFormat>(aRange, maEntries, &maPriorities));
+
+            if (isPreviousElementF)
+                maPriorities.clear();
+
             isPreviousElementF = false;
         }
         break;