[Libreoffice-commits] core.git: sc/inc sc/qa sc/source

Attila Szűcs (via logerrit) logerrit at kemper.freedesktop.org
Thu Dec 3 13:01:09 UTC 2020


 sc/inc/externalrefmgr.hxx                                |   10 +++
 sc/qa/unit/data/ods/tdf87973_externalLinkSkipUnuseds.ods |binary
 sc/qa/unit/data/ods/tdf87973_externalSource.ods          |binary
 sc/qa/unit/subsequent_export-test.cxx                    |   42 +++++++++++++++
 sc/source/core/tool/compiler.cxx                         |    3 -
 sc/source/filter/excel/xelink.cxx                        |   19 +++++-
 sc/source/filter/excel/xestream.cxx                      |    3 +
 sc/source/ui/docshell/externalrefmgr.cxx                 |   25 ++++++++
 8 files changed, 98 insertions(+), 4 deletions(-)

New commits:
commit f85d860ccbebd99bc128218148e2992c9415f221
Author:     Attila Szűcs <szucs.attila3 at nisz.hu>
AuthorDate: Mon Nov 30 15:18:09 2020 +0100
Commit:     László Németh <nemeth at numbertext.org>
CommitDate: Thu Dec 3 14:00:27 2020 +0100

    tdf#87973 XLSX export: fix lost file names in modified links
    
    Calculate new indexes for external reference files to export it to xlsx.
    These indexes are calculated only temporary, only for exporting.
    
    Much better solution would be to change the indexes permanently,
    but the original indexes are stored so many places in the code
    (for example it is stored in cells formula tokens converted to string).
    So it would be a much bigger refactoring to be able to delete an
    external reference permanently... even just to reorder the indexes,
    require to modify a lot of code.
    
    Co-authored-by: Tibor Nagy (NISZ)
    
    Change-Id: If9254f6e62ec739e2b159a066ada7efdbceb3ad8
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106895
    Tested-by: László Németh <nemeth at numbertext.org>
    Reviewed-by: László Németh <nemeth at numbertext.org>

diff --git a/sc/inc/externalrefmgr.hxx b/sc/inc/externalrefmgr.hxx
index fd07fa0e657a..2669781e9caa 100644
--- a/sc/inc/externalrefmgr.hxx
+++ b/sc/inc/externalrefmgr.hxx
@@ -597,6 +597,13 @@ public:
      */
     const OUString* getExternalFileName(sal_uInt16 nFileId, bool bForceOriginal = false);
 
+    /**
+     * Reindex external file references to skip unused files, if skipping is enabled.
+     */
+    sal_uInt16 convertFileIdToUsedFileId(sal_uInt16 nFileId);
+    void setSkipUnusedFileIds(std::vector<sal_uInt16>& pExternFileIds);
+    void disableSkipUnusedFileIds();
+
     /**
      * Get all cached external file names as an array. Array indices of the
      * returned name array correspond with external file ID's.
@@ -845,6 +852,9 @@ private:
      */
     bool mbUserInteractionEnabled:1;
 
+    bool mbSkipUnusedFileIds = false;
+    std::vector<sal_uInt16> maConvertFileIdToUsedFileId;
+
     bool mbDocTimerEnabled:1;
 
     AutoTimer maSrcDocTimer;
diff --git a/sc/qa/unit/data/ods/tdf87973_externalLinkSkipUnuseds.ods b/sc/qa/unit/data/ods/tdf87973_externalLinkSkipUnuseds.ods
new file mode 100644
index 000000000000..cdaf9d4e7007
Binary files /dev/null and b/sc/qa/unit/data/ods/tdf87973_externalLinkSkipUnuseds.ods differ
diff --git a/sc/qa/unit/data/ods/tdf87973_externalSource.ods b/sc/qa/unit/data/ods/tdf87973_externalSource.ods
new file mode 100644
index 000000000000..59228e390e4d
Binary files /dev/null and b/sc/qa/unit/data/ods/tdf87973_externalSource.ods differ
diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index 54918a017797..e46d356d677b 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -267,6 +267,7 @@ public:
     void testTdf137000_handle_upright();
     void testTdf126305_DataValidatyErrorAlert();
     void testTdf76047_externalLink();
+    void testTdf87973_externalLinkSkipUnuseds();
     void testTdf129969();
     void testTdf84874();
     void testTdf136721_paper_size();
@@ -438,6 +439,7 @@ public:
     CPPUNIT_TEST(testTdf137000_handle_upright);
     CPPUNIT_TEST(testTdf126305_DataValidatyErrorAlert);
     CPPUNIT_TEST(testTdf76047_externalLink);
+    CPPUNIT_TEST(testTdf87973_externalLinkSkipUnuseds);
     CPPUNIT_TEST(testTdf129969);
     CPPUNIT_TEST(testTdf84874);
     CPPUNIT_TEST(testTdf136721_paper_size);
@@ -5536,6 +5538,46 @@ void ScExportTest::testTdf76047_externalLink()
     }
 }
 
+void ScExportTest::testTdf87973_externalLinkSkipUnuseds()
+{
+    ScDocShellRef pShell = loadDoc("tdf87973_externalLinkSkipUnuseds.", FORMAT_ODS);
+    CPPUNIT_ASSERT(pShell.is());
+
+    // try to load data from external link: tdf132105_external.ods
+    // that file has to be in the same directory as tdf87973_externalLinkSkipUnuseds.ods
+    pShell->ReloadAllLinks();
+    ScDocument& rDoc = pShell->GetDocument();
+
+    // change external link to: 87973_externalSource.ods
+    OUString aFormula, bFormula;
+    rDoc.GetFormula(3, 1, 0, aFormula);
+    auto nIdxOfFilename = aFormula.indexOf("tdf132105_external.ods");
+    aFormula = aFormula.replaceAt(nIdxOfFilename, 22, "87973_externalSource.ods");
+    auto nIdxOfFile = aFormula.indexOf("file");
+
+    // saveAndReload save the file to a temporary directory
+    // the link must be changed to point to that directory
+    utl::TempFile aTempFile;
+    auto aTempFilename = aTempFile.GetURL();
+    auto nIdxOfTmpFile = aTempFilename.lastIndexOf('/');
+    aTempFilename = aTempFilename.copy(0, nIdxOfTmpFile + 1);
+
+    aFormula = aFormula.replaceAt(nIdxOfFile, nIdxOfFilename - nIdxOfFile, aTempFilename);
+    rDoc.SetFormula(ScAddress(3, 1, 0), aFormula, formula::FormulaGrammar::GRAM_NATIVE_UI);
+
+    // save and load back
+    ScDocShellRef pDocSh = saveAndReload(&(*pShell), FORMAT_XLSX);
+    CPPUNIT_ASSERT(pDocSh.is());
+
+    // check if the the new filename is present in the link (and not replaced by '[2]')
+    ScDocument& rDoc2 = pDocSh->GetDocument();
+    rDoc2.GetFormula(3, 1, 0, bFormula);
+    CPPUNIT_ASSERT(bFormula.indexOf("tdf132105_external.ods") < 0);
+    CPPUNIT_ASSERT(bFormula.indexOf("87973_externalSource.ods") > 0);
+
+    pDocSh->DoClose();
+}
+
 void ScExportTest::testTdf129969()
 {
     ScDocShellRef xShell = loadDoc("external_hyperlink.", FORMAT_ODS);
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index c35f809197c2..6fcee51fa4ae 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -5076,6 +5076,7 @@ void ScCompiler::CreateStringFromExternal( OUStringBuffer& rBuffer, const Formul
     const FormulaToken* t = pTokenP;
     sal_uInt16 nFileId = t->GetIndex();
     ScExternalRefManager* pRefMgr = rDoc.GetExternalRefManager();
+    sal_uInt16 nUsedFileId = pRefMgr->convertFileIdToUsedFileId(nFileId);
     const OUString* pFileName = pRefMgr->getExternalFileName(nFileId);
     if (!pFileName)
         return;
@@ -5101,7 +5102,7 @@ void ScCompiler::CreateStringFromExternal( OUStringBuffer& rBuffer, const Formul
                     *pFileName << "' '" << t->GetString().getString() << "'");
 
             pConv->makeExternalRefStr(
-                rDoc.GetSheetLimits(), rBuffer, GetPos(), nFileId, *pFileName, aTabNames, t->GetString().getString(),
+                rDoc.GetSheetLimits(), rBuffer, GetPos(), nUsedFileId, *pFileName, aTabNames, t->GetString().getString(),
                 *t->GetDoubleRef());
         }
         break;
diff --git a/sc/source/filter/excel/xelink.cxx b/sc/source/filter/excel/xelink.cxx
index d8b3a954f278..0522b042c67b 100644
--- a/sc/source/filter/excel/xelink.cxx
+++ b/sc/source/filter/excel/xelink.cxx
@@ -2077,6 +2077,19 @@ void XclExpSupbookBuffer::Save( XclExpStream& rStrm )
 
 void XclExpSupbookBuffer::SaveXml( XclExpXmlStream& rStrm )
 {
+    // Unused external references are not saved, only kept in memory
+    // saveds must be indexed from 1, so indexes must be reordered
+    ScExternalRefManager* pRefMgr = GetDoc().GetExternalRefManager();
+    vector<sal_uInt16> aExternFileIds;
+    for (size_t nPos = 0, nSize = maSupbookList.GetSize(); nPos < nSize; ++nPos)
+    {
+        XclExpSupbookRef xRef(maSupbookList.GetRecord(nPos));
+        if (xRef->GetType() == XclSupbookType::Extern)
+            aExternFileIds.push_back(xRef->GetFileId() - 1);
+    }
+    if (aExternFileIds.size() > 0)
+        pRefMgr->setSkipUnusedFileIds(aExternFileIds);
+
     ::std::map< sal_uInt16, OUString > aMap;
     for (size_t nPos = 0, nSize = maSupbookList.GetSize(); nPos < nSize; ++nPos)
     {
@@ -2085,6 +2098,7 @@ void XclExpSupbookBuffer::SaveXml( XclExpXmlStream& rStrm )
             continue;   // handle only external reference (for now?)
 
         sal_uInt16 nId = xRef->GetFileId();
+        sal_uInt16 nUsedId = pRefMgr->convertFileIdToUsedFileId(nId - 1) + 1;
         const OUString& rUrl = xRef->GetUrl();
         ::std::pair< ::std::map< sal_uInt16, OUString >::iterator, bool > aInsert(
                 aMap.insert( ::std::make_pair( nId, rUrl)));
@@ -2095,11 +2109,10 @@ void XclExpSupbookBuffer::SaveXml( XclExpXmlStream& rStrm )
                     (rUrl == (*aInsert.first).second ? " multiple Supbook not supported" : ""));
             continue;
         }
-
         OUString sId;
         sax_fastparser::FSHelperPtr pExternalLink = rStrm.CreateOutputStream(
-                XclXmlUtils::GetStreamName( "xl/", "externalLinks/externalLink", nId),
-                XclXmlUtils::GetStreamName( nullptr, "externalLinks/externalLink", nId),
+                XclXmlUtils::GetStreamName( "xl/", "externalLinks/externalLink", nUsedId),
+                XclXmlUtils::GetStreamName( nullptr, "externalLinks/externalLink", nUsedId),
                 rStrm.GetCurrentStream()->getOutputStream(),
                 "application/vnd.openxmlformats-officedocument.spreadsheetml.externalLink+xml",
                 CREATE_OFFICEDOC_RELATION_TYPE("externalLink"),
diff --git a/sc/source/filter/excel/xestream.cxx b/sc/source/filter/excel/xestream.cxx
index b0ea61b1fc36..c0a2646461bf 100644
--- a/sc/source/filter/excel/xestream.cxx
+++ b/sc/source/filter/excel/xestream.cxx
@@ -66,6 +66,8 @@
 #include <memory>
 #include <comphelper/storagehelper.hxx>
 
+#include <externalrefmgr.hxx>
+
 #define DEBUG_XL_ENCRYPTION 0
 
 using ::com::sun::star::uno::XInterface;
@@ -1103,6 +1105,7 @@ bool XclExpXmlStream::exportDocument()
         if (xStatusIndicator.is())
             xStatusIndicator->setValue(40);
         aDocRoot.WriteXml( *this );
+        rDoc.GetExternalRefManager()->disableSkipUnusedFileIds();
     }
 
     PopStream();
diff --git a/sc/source/ui/docshell/externalrefmgr.cxx b/sc/source/ui/docshell/externalrefmgr.cxx
index 3243bfb6f6ed..6f032cafe090 100644
--- a/sc/source/ui/docshell/externalrefmgr.cxx
+++ b/sc/source/ui/docshell/externalrefmgr.cxx
@@ -2740,6 +2740,31 @@ const OUString* ScExternalRefManager::getExternalFileName(sal_uInt16 nFileId, bo
     return &maSrcFiles[nFileId].maFileName;
 }
 
+sal_uInt16 ScExternalRefManager::convertFileIdToUsedFileId(sal_uInt16 nFileId)
+{
+    if (!mbSkipUnusedFileIds)
+        return nFileId;
+    else
+        return maConvertFileIdToUsedFileId[nFileId];
+}
+
+void ScExternalRefManager::setSkipUnusedFileIds(std::vector<sal_uInt16>& rExternFileIds)
+{
+    mbSkipUnusedFileIds = true;
+    maConvertFileIdToUsedFileId.resize(maRefCells.size());
+    std::fill(maConvertFileIdToUsedFileId.begin(), maConvertFileIdToUsedFileId.end(), 0);
+    int nUsedCount = 0;
+    for (auto nEntry : rExternFileIds)
+    {
+        maConvertFileIdToUsedFileId[nEntry] = nUsedCount++;
+    }
+}
+
+void ScExternalRefManager::disableSkipUnusedFileIds()
+{
+    mbSkipUnusedFileIds = false;
+}
+
 std::vector<OUString> ScExternalRefManager::getAllCachedExternalFileNames() const
 {
     std::vector<OUString> aNames;


More information about the Libreoffice-commits mailing list