sc: improve SparklineList to track added SparklineGroups

SparklineList used to only track added Sparklines for a sheet, but
usually (in an export) we want to start with SparklineGroups and
then search for all sparklines belonging to a group. This changes
to use that. Now there is a method getSparklineGroups() and then
another method getSparklineFor(), which returns all sparklines for
the input group.

Also added SparklineListTest, and refactored the export code for
OOXML and ODF.

Change-Id: I975e30f649788d41aab92a9a3220e38998e39670
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132543
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>

10 files changed, 218 insertions, 114 deletions

diff --git a/sc/inc/Sparkline.hxx b/sc/inc/Sparkline.hxx
index e0fbbe125bc7..77f249428288 100644
--- a/sc/inc/Sparkline.hxx
+++ b/sc/inc/Sparkline.hxx
@@ -54,42 +54,6 @@ public:
     SCROW getRow() const { return m_nRow; }
 };
 
-/** Contains a list of all created sparklines */
-class SC_DLLPUBLIC SparklineList
-{
-private:
-    std::vector<std::weak_ptr<Sparkline>> m_pSparklines;
-
-public:
-    SparklineList() {}
-
-    void addSparkline(std::shared_ptr<Sparkline> const& pSparkline)
-    {
-        m_pSparklines.push_back(pSparkline);
-    }
-
-    std::vector<std::shared_ptr<Sparkline>> getSparklines()
-    {
-        std::vector<std::shared_ptr<Sparkline>> toReturn;
-
-        std::vector<std::weak_ptr<Sparkline>>::iterator aIter;
-        for (aIter = m_pSparklines.begin(); aIter != m_pSparklines.end();)
-        {
-            if (auto aSparkline = aIter->lock())
-            {
-                toReturn.push_back(aSparkline);
-                aIter++;
-            }
-            else
-            {
-                aIter = m_pSparklines.erase(aIter);
-            }
-        }
-
-        return toReturn;
-    }
-};
-
 } // end sc
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/inc/SparklineList.hxx b/sc/inc/SparklineList.hxx
new file mode 100644
index 000000000000..1abfbd6df019
--- /dev/null
+++ b/sc/inc/SparklineList.hxx
@@ -0,0 +1,101 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+
+#pragma once
+
+#include "scdllapi.h"
+#include <memory>
+#include <map>
+
+#include "rangelst.hxx"
+#include "Sparkline.hxx"
+#include "SparklineGroup.hxx"
+
+namespace sc
+{
+/** Tracks and gathers all created sparklines and sparkline groups.
+ *
+ * All the collections of sparkline groups and sparklines don't take
+ * the ownership of the pointers.
+ */
+class SC_DLLPUBLIC SparklineList
+{
+private:
+    std::vector<std::weak_ptr<SparklineGroup>> m_aSparklineGroups;
+    std::map<std::weak_ptr<SparklineGroup>, std::vector<std::weak_ptr<Sparkline>>,
+             std::owner_less<>>
+        m_aSparklineGroupMap;
+
+public:
+    SparklineList() {}
+
+    void addSparkline(std::shared_ptr<Sparkline> const& pSparkline)
+    {
+        auto pWeakGroup = std::weak_ptr<SparklineGroup>(pSparkline->getSparklineGroup());
+
+        auto[iterator, bInserted]
+            = m_aSparklineGroupMap.try_emplace(pWeakGroup, std::vector<std::weak_ptr<Sparkline>>());
+        iterator->second.push_back(std::weak_ptr<Sparkline>(pSparkline));
+        if (bInserted)
+            m_aSparklineGroups.push_back(pWeakGroup);
+    }
+
+    std::vector<std::shared_ptr<SparklineGroup>> getSparklineGroups()
+    {
+        std::vector<std::shared_ptr<SparklineGroup>> toReturn;
+
+        for (auto iterator = m_aSparklineGroups.begin(); iterator != m_aSparklineGroups.end();)
+        {
+            if (auto pSparklineGroup = iterator->lock())
+            {
+                toReturn.push_back(pSparklineGroup);
+                iterator++;
+            }
+            else
+            {
+                iterator = m_aSparklineGroups.erase(iterator);
+            }
+        }
+        return toReturn;
+    }
+
+    std::vector<std::shared_ptr<Sparkline>>
+    getSparklinesFor(std::shared_ptr<SparklineGroup> const& pSparklineGroup)
+    {
+        std::vector<std::shared_ptr<Sparkline>> toReturn;
+
+        std::weak_ptr<SparklineGroup> pWeakGroup(pSparklineGroup);
+        auto iteratorGroup = m_aSparklineGroupMap.find(pWeakGroup);
+
+        if (iteratorGroup == m_aSparklineGroupMap.end())
+            return toReturn;
+
+        auto& rWeakSparklines = iteratorGroup->second;
+
+        for (auto iterator = rWeakSparklines.begin(); iterator != rWeakSparklines.end();)
+        {
+            if (auto aSparkline = iterator->lock())
+            {
+                toReturn.push_back(aSparkline);
+                iterator++;
+            }
+            else
+            {
+                iterator = rWeakSparklines.erase(iterator);
+            }
+        }
+
+        return toReturn;
+    }
+};
+
+} // end sc
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 7a3ce91a43a0..ca42780c71b1 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -32,6 +32,7 @@
 #include <formula/errorcodes.hxx>
 #include "document.hxx"
 #include "drwlayer.hxx"
+#include "SparklineList.hxx"
 
 #include <algorithm>
 #include <atomic>
diff --git a/sc/qa/unit/SparklineTest.cxx b/sc/qa/unit/SparklineTest.cxx
index 42ac88467b6d..46aed9510ba5 100644
--- a/sc/qa/unit/SparklineTest.cxx
+++ b/sc/qa/unit/SparklineTest.cxx
@@ -15,6 +15,7 @@
 
 #include <Sparkline.hxx>
 #include <SparklineGroup.hxx>
+#include <SparklineList.hxx>
 
 using namespace css;
 
@@ -54,6 +55,7 @@ public:
     void testUndoRedoDeleteSparkline();
     void testUndoRedoClearContentForSparkline();
     void testUndoRedoEditSparklineGroup();
+    void testSparklineList();
 
     CPPUNIT_TEST_SUITE(SparklineTest);
     CPPUNIT_TEST(testAddSparkline);
@@ -64,6 +66,7 @@ public:
     CPPUNIT_TEST(testUndoRedoDeleteSparkline);
     CPPUNIT_TEST(testUndoRedoClearContentForSparkline);
     CPPUNIT_TEST(testUndoRedoEditSparklineGroup);
+    CPPUNIT_TEST(testSparklineList);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -113,10 +116,12 @@ void SparklineTest::testAddSparkline()
 
     CPPUNIT_ASSERT_EQUAL(pGetSparkline.get(), pSparkline);
 
-    sc::SparklineList* pList = rDocument.GetSparklineList(0);
+    sc::SparklineList* pList = rDocument.GetSparklineList(SCTAB(0));
     CPPUNIT_ASSERT(pList);
 
-    std::vector<std::shared_ptr<sc::Sparkline>> aSparklineVector = pList->getSparklines();
+    CPPUNIT_ASSERT_EQUAL(size_t(1), pList->getSparklineGroups().size());
+
+    auto const& aSparklineVector = pList->getSparklinesFor(pGetSparkline->getSparklineGroup());
     CPPUNIT_ASSERT_EQUAL(size_t(1), aSparklineVector.size());
     CPPUNIT_ASSERT_EQUAL(aSparklineVector[0].get(), pSparkline);
 
@@ -496,6 +501,65 @@ void SparklineTest::testUndoRedoEditSparklineGroup()
     xDocSh->DoClose();
 }
 
+void SparklineTest::testSparklineList()
+{
+    ScDocShellRef xDocSh = loadEmptyDocument();
+    CPPUNIT_ASSERT(xDocSh);
+
+    ScDocument& rDocument = xDocSh->GetDocument();
+
+    auto pSparklineGroup = std::make_shared<sc::SparklineGroup>();
+
+    rDocument.CreateSparkline(ScAddress(0, 6, 0), pSparklineGroup);
+
+    {
+        auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0));
+        auto pSparklineGroups = pSparklineList->getSparklineGroups();
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size());
+
+        auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]);
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklines.size());
+    }
+    rDocument.CreateSparkline(ScAddress(1, 6, 0), pSparklineGroup);
+    rDocument.CreateSparkline(ScAddress(2, 6, 0), pSparklineGroup);
+
+    {
+        auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0));
+        auto pSparklineGroups = pSparklineList->getSparklineGroups();
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size());
+
+        auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]);
+        CPPUNIT_ASSERT_EQUAL(size_t(3), pSparklines.size());
+    }
+
+    {
+        auto pSparklineGroup2 = std::make_shared<sc::SparklineGroup>();
+        rDocument.CreateSparkline(ScAddress(3, 6, 0), pSparklineGroup2);
+
+        auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0));
+
+        auto pSparklineGroups = pSparklineList->getSparklineGroups();
+        CPPUNIT_ASSERT_EQUAL(size_t(2), pSparklineGroups.size());
+
+        auto pSparklines2 = pSparklineList->getSparklinesFor(pSparklineGroup2);
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklines2.size());
+    }
+
+    rDocument.DeleteSparkline(ScAddress(3, 6, 0));
+
+    {
+        auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0));
+
+        auto pSparklineGroups = pSparklineList->getSparklineGroups();
+        CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size());
+
+        auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]);
+        CPPUNIT_ASSERT_EQUAL(size_t(3), pSparklines.size());
+    }
+
+    xDocSh->DoClose();
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SparklineTest);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index cf72b9e55685..e428ccb6f568 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -87,6 +87,7 @@
 #include <compressedarray.hxx>
 #include <recursionhelper.hxx>
 #include <SparklineGroup.hxx>
+#include <SparklineList.hxx>
 
 #include <formula/vectortoken.hxx>
 
@@ -6648,11 +6649,12 @@ std::shared_ptr<sc::SparklineGroup> ScDocument::SearchSparklineGroup(tools::Guid
 {
     for (auto const& rTable : maTabs)
     {
-        for (auto const& pSparkline : rTable->GetSparklineList().getSparklines())
+        auto& rSparklineList = rTable->GetSparklineList();
+
+        for (auto const& pSparklineGroup : rSparklineList.getSparklineGroups())
         {
-            auto const& pGroup = pSparkline->getSparklineGroup();
-            if (pGroup->getID() == rGuid)
-                return pGroup;
+            if (pSparklineGroup->getID() == rGuid)
+                return pSparklineGroup;
         }
     }
 
diff --git a/sc/source/filter/excel/export/SparklineExt.cxx b/sc/source/filter/excel/export/SparklineExt.cxx
index b4614dd59ea3..5202c3e92449 100644
--- a/sc/source/filter/excel/export/SparklineExt.cxx
+++ b/sc/source/filter/excel/export/SparklineExt.cxx
@@ -13,37 +13,28 @@
 #include <oox/token/namespaces.hxx>
 #include <oox/token/tokens.hxx>
 #include <SparklineGroup.hxx>
+#include <SparklineList.hxx>
 
 using namespace oox;
 
 namespace xcl::exp
 {
-SparklineExt::SparklineExt(const XclExpRoot& rRoot,
-                           std::vector<std::shared_ptr<sc::Sparkline>> const& pSparklines)
+SparklineExt::SparklineExt(const XclExpRoot& rRoot)
     : XclExpExt(rRoot)
 {
     maURI = "{05C60535-1F16-4fd2-B633-F4F36F0B64E0}";
-
-    for (auto const& pSparkline : pSparklines)
-    {
-        auto* pGroupPointer = pSparkline->getSparklineGroup().get();
-
-        auto aIterator = m_aSparklineGroupMap.find(pGroupPointer);
-        if (aIterator == m_aSparklineGroupMap.end())
-        {
-            std::vector<std::shared_ptr<sc::Sparkline>> aSparklineVector;
-            aSparklineVector.push_back(pSparkline);
-            m_aSparklineGroupMap.emplace(pGroupPointer, aSparklineVector);
-        }
-        else
-        {
-            aIterator->second.push_back(pSparkline);
-        }
-    }
 }
 
 void SparklineExt::SaveXml(XclExpXmlStream& rStream)
 {
+    auto& rDocument = GetDoc();
+
+    auto* pSparklineList = rDocument.GetSparklineList(GetCurrScTab());
+    if (!pSparklineList)
+        return;
+
+    auto const& rSparklineGroups = pSparklineList->getSparklineGroups();
+
     sax_fastparser::FSHelperPtr& rWorksheet = rStream.GetCurrentStream();
     rWorksheet->startElement(XML_ext, FSNS(XML_xmlns, XML_x14),
                              rStream.getNamespaceURL(OOX_NS(xls14Lst)), XML_uri, maURI);
@@ -51,8 +42,9 @@ void SparklineExt::SaveXml(XclExpXmlStream& rStream)
     rWorksheet->startElementNS(XML_x14, XML_sparklineGroups, FSNS(XML_xmlns, XML_xm),
                                rStream.getNamespaceURL(OOX_NS(xm)));
 
-    for (auto const & [ pSparklineGroup, rSparklineVector ] : m_aSparklineGroupMap)
+    for (auto const& pSparklineGroup : rSparklineGroups)
     {
+        auto const& rSparklineVector = pSparklineList->getSparklinesFor(pSparklineGroup);
         addSparklineGroup(rStream, *pSparklineGroup, rSparklineVector);
     }
 
@@ -238,14 +230,7 @@ void SparklineExt::addSparklineGroup(XclExpXmlStream& rStream, sc::SparklineGrou
 SparklineBuffer::SparklineBuffer(const XclExpRoot& rRoot, XclExtLstRef const& xExtLst)
     : XclExpRoot(rRoot)
 {
-    if (sc::SparklineList* pSparklineList = GetDoc().GetSparklineList(GetCurrScTab()))
-    {
-        auto pSparklines = pSparklineList->getSparklines();
-        if (!pSparklines.empty())
-        {
-            xExtLst->AddRecord(new xcl::exp::SparklineExt(GetRoot(), pSparklines));
-        }
-    }
+    xExtLst->AddRecord(new xcl::exp::SparklineExt(GetRoot()));
 }
 
 } // end namespace xcl::exp
diff --git a/sc/source/filter/inc/export/SparklineExt.hxx b/sc/source/filter/inc/export/SparklineExt.hxx
index 9843e910e252..554fe9c7ec34 100644
--- a/sc/source/filter/inc/export/SparklineExt.hxx
+++ b/sc/source/filter/inc/export/SparklineExt.hxx
@@ -26,11 +26,8 @@ namespace xcl::exp
 {
 class SparklineExt : public XclExpExt
 {
-    std::map<sc::SparklineGroup*, std::vector<std::shared_ptr<sc::Sparkline>>> m_aSparklineGroupMap;
-
 public:
-    SparklineExt(const XclExpRoot& rRoot,
-                 std::vector<std::shared_ptr<sc::Sparkline>> const& pSparklines);
+    SparklineExt(const XclExpRoot& rRoot);
 
     void SaveXml(XclExpXmlStream& rStream) override;
     void addSparklineGroup(XclExpXmlStream& rStream, sc::SparklineGroup& rSparklineGroup,
diff --git a/sc/source/filter/xml/SparklineGroupsExport.cxx b/sc/source/filter/xml/SparklineGroupsExport.cxx
index 4de56fcfaf58..077d43beab60 100644
--- a/sc/source/filter/xml/SparklineGroupsExport.cxx
+++ b/sc/source/filter/xml/SparklineGroupsExport.cxx
@@ -11,6 +11,8 @@
 #include "SparklineGroupsExport.hxx"
 #include "xmlexprt.hxx"
 #include <rangeutl.hxx>
+#include <SparklineList.hxx>
+#include <document.hxx>
 
 #include <xmloff/xmluconv.hxx>
 #include <xmloff/xmltoken.hxx>
@@ -25,27 +27,10 @@ using namespace xmloff::token;
 
 namespace sc
 {
-SparklineGroupsExport::SparklineGroupsExport(
-    ScXMLExport& rExport, SCTAB nTable, std::vector<std::shared_ptr<Sparkline>> const& rSparklines)
+SparklineGroupsExport::SparklineGroupsExport(ScXMLExport& rExport, SCTAB nTable)
     : m_rExport(rExport)
     , m_nTable(nTable)
 {
-    for (auto const& pSparkline : rSparklines)
-    {
-        auto* pGroupPointer = pSparkline->getSparklineGroup().get();
-        auto aIterator = m_aSparklineGroupMap.find(pGroupPointer);
-        if (aIterator == m_aSparklineGroupMap.end())
-        {
-            m_aSparklineGroups.push_back(pGroupPointer);
-            std::vector<std::shared_ptr<sc::Sparkline>> aSparklineVector;
-            aSparklineVector.push_back(pSparkline);
-            m_aSparklineGroupMap.emplace(pGroupPointer, aSparklineVector);
-        }
-        else
-        {
-            aIterator->second.push_back(pSparkline);
-        }
-    }
 }
 
 void SparklineGroupsExport::insertColor(Color aColor, XMLTokenEnum eToken)
@@ -183,7 +168,9 @@ void SparklineGroupsExport::addSparklineGroupAttributes(SparklineAttributes cons
     insertColor(rAttributes.getColorLow(), XML_COLOR_LOW);
 }
 
-void SparklineGroupsExport::addSparklineGroup(SparklineGroup* pSparklineGroup)
+void SparklineGroupsExport::addSparklineGroup(
+    std::shared_ptr<SparklineGroup> const& pSparklineGroup,
+    std::vector<std::shared_ptr<Sparkline>> const& rSparklines)
 {
     auto const& rAttributes = pSparklineGroup->getAttributes();
 
@@ -197,7 +184,8 @@ void SparklineGroupsExport::addSparklineGroup(SparklineGroup* pSparklineGroup)
 
     SvXMLElementExport aElementSparklines(m_rExport, XML_NAMESPACE_CALC_EXT, XML_SPARKLINES, true,
                                           true);
-    for (auto const& rSparkline : m_aSparklineGroupMap[pSparklineGroup])
+
+    for (auto const& rSparkline : rSparklines)
     {
         addSparklineAttributes(*rSparkline);
         SvXMLElementExport aElementSparkline(m_rExport, XML_NAMESPACE_CALC_EXT, XML_SPARKLINE, true,
@@ -207,13 +195,24 @@ void SparklineGroupsExport::addSparklineGroup(SparklineGroup* pSparklineGroup)
 
 void SparklineGroupsExport::write()
 {
-    SvXMLElementExport aElement(m_rExport, XML_NAMESPACE_CALC_EXT, XML_SPARKLINE_GROUPS, true,
-                                true);
-    for (auto* pSparklineGroup : m_aSparklineGroups)
+    auto* pDocument = m_rExport.GetDocument();
+    if (sc::SparklineList* pSparklineList = pDocument->GetSparklineList(m_nTable))
     {
-        addSparklineGroup(pSparklineGroup);
+        auto const& aSparklineGroups = pSparklineList->getSparklineGroups();
+        if (!aSparklineGroups.empty())
+        {
+            SvXMLElementExport aElement(m_rExport, XML_NAMESPACE_CALC_EXT, XML_SPARKLINE_GROUPS,
+                                        true, true);
+
+            for (auto const& pSparklineGroup : aSparklineGroups)
+            {
+                auto const& aSparklines = pSparklineList->getSparklinesFor(pSparklineGroup);
+                addSparklineGroup(pSparklineGroup, aSparklines);
+            }
+        }
     }
 }
-}
+
+} // end sc
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/filter/xml/SparklineGroupsExport.hxx b/sc/source/filter/xml/SparklineGroupsExport.hxx
index 4e49f585dbc7..b20fd8529574 100644
--- a/sc/source/filter/xml/SparklineGroupsExport.hxx
+++ b/sc/source/filter/xml/SparklineGroupsExport.hxx
@@ -25,21 +25,18 @@ namespace sc
 class SparklineGroupsExport
 {
     ScXMLExport& m_rExport;
-    std::vector<SparklineGroup*> m_aSparklineGroups;
-    std::unordered_map<SparklineGroup*, std::vector<std::shared_ptr<Sparkline>>>
-        m_aSparklineGroupMap;
     SCTAB m_nTable;
 
     void addSparklineGroupAttributes(sc::SparklineAttributes const& rAttributes);
-    void addSparklineGroup(SparklineGroup* pSparklineGroup);
+    void addSparklineGroup(std::shared_ptr<SparklineGroup> const& pSparklineGroup,
+                           std::vector<std::shared_ptr<Sparkline>> const& rSparklines);
     void addSparklineAttributes(Sparkline const& rSparkline);
 
     void insertColor(Color aColor, xmloff::token::XMLTokenEnum eToken);
     void insertBool(bool bValue, xmloff::token::XMLTokenEnum eToken);
 
 public:
-    SparklineGroupsExport(ScXMLExport& rExport, SCTAB nTable,
-                          std::vector<std::shared_ptr<Sparkline>> const& rSparklines);
+    SparklineGroupsExport(ScXMLExport& rExport, SCTAB nTable);
 
     void write();
 };
diff --git a/sc/source/filter/xml/xmlexprt.cxx b/sc/source/filter/xml/xmlexprt.cxx
index 20da166a0bab..95efb4bb255f 100644
--- a/sc/source/filter/xml/xmlexprt.cxx
+++ b/sc/source/filter/xml/xmlexprt.cxx
@@ -68,6 +68,7 @@
 #include <datamapper.hxx>
 #include <datatransformation.hxx>
 #include "SparklineGroupsExport.hxx"
+#include <SparklineList.hxx>
 
 #include <xmloff/xmltoken.hxx>
 #include <xmloff/xmlnamespace.hxx>
@@ -4516,15 +4517,8 @@ void ScXMLExport::WriteNamedRange(ScRangeName* pRangeName)
 
 void ScXMLExport::exportSparklineGroups(SCTAB nTable)
 {
-    if (sc::SparklineList* pSparklineList = pDoc->GetSparklineList(nTable))
-    {
-        auto pSparklines = pSparklineList->getSparklines();
-        if (!pSparklines.empty())
-        {
-            sc::SparklineGroupsExport aSparklineGroupExport(*this, nTable, pSparklines);
-            aSparklineGroupExport.write();
-        }
-    }
+    sc::SparklineGroupsExport aSparklineGroupExport(*this, nTable);
+    aSparklineGroupExport.write();
 }
 
 namespace {