diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 54b6d79f064b..22768f46feba 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -64,6 +64,7 @@ recompute path. Also, it enables more complicated dependencies beyond trees. #endif #include +#include #include #ifdef USE_OLD_DAG @@ -102,6 +103,7 @@ recompute path. Also, it enables more complicated dependencies beyond trees. #include "License.h" #include "Link.h" #include "MergeDocuments.h" +#include "StringHasher.h" #include "Transactions.h" #ifdef _MSC_VER @@ -136,6 +138,7 @@ static bool globalIsRelabeling; DocumentP::DocumentP() { + Hasher = new StringHasher; static std::random_device _RD; static std::mt19937 _RGEN(_RD()); static std::uniform_int_distribution<> _RDIST(0, 5000); @@ -921,11 +924,27 @@ std::string Document::getTransientDirectoryName(const std::string& uuid, const s void Document::Save (Base::Writer &writer) const { + d->hashers.clear(); + addStringHasher(d->Hasher); + writer.Stream() << R"(" << endl; + << "\" FileVersion=\"" << writer.getFileVersion() + << "\" StringHasher=\"1\">\n"; + + writer.incInd(); + + d->Hasher->setPersistenceFileName("StringHasher.Table"); + for (auto o : d->objectArray) { + o->beforeSave(); + } + beforeSave(); + + d->Hasher->Save(writer); + + writer.decInd(); PropertyContainer::Save(writer); @@ -937,7 +956,9 @@ void Document::Save (Base::Writer &writer) const void Document::Restore(Base::XMLReader &reader) { int i,Cnt; + d->hashers.clear(); d->touchedObjs.clear(); + addStringHasher(d->Hasher); setStatus(Document::PartialDoc,false); reader.readElement("Document"); @@ -954,6 +975,12 @@ void Document::Restore(Base::XMLReader &reader) reader.FileVersion = 0; } + if (reader.hasAttribute("StringHasher")) { + d->Hasher->Restore(reader); + } else { + d->Hasher->clear(); + } + // When this document was created the FileName and Label properties // were set to the absolute path or file name, respectively. To save // the document to the file it was loaded from or to show the file name @@ -1018,6 +1045,30 @@ void Document::Restore(Base::XMLReader &reader) reader.readEndElement("Document"); } +std::pair Document::addStringHasher(const StringHasherRef & hasher) const { + if (!hasher) + return std::make_pair(false, 0); + auto ret = d->hashers.left.insert(HasherMap::left_map::value_type(hasher,(int)d->hashers.size())); + if (ret.second) + hasher->clearMarks(); + return std::make_pair(ret.second,ret.first->second); +} + +StringHasherRef Document::getStringHasher(int idx) const { + if(idx<0) { + return d->Hasher; + return d->Hasher; + } + StringHasherRef hasher; + auto it = d->hashers.right.find(idx); + if(it == d->hashers.right.end()) { + hasher = new StringHasher; + d->hashers.right.insert(HasherMap::right_map::value_type(idx,hasher)); + }else + hasher = it->second; + return hasher; +} + struct DocExportStatus { Document::ExportStatus status; std::set objs; @@ -1054,6 +1105,7 @@ Document::ExportStatus Document::isExporting(const App::DocumentObject *obj) con void Document::exportObjects(const std::vector& obj, std::ostream& out) { DocumentExporting exporting(obj); + d->hashers.clear(); if(FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { for(auto o : obj) { @@ -1090,6 +1142,7 @@ void Document::exportObjects(const std::vector& obj, std:: // write additional files writer.writeFiles(); + d->hashers.clear(); } #define FC_ATTR_DEPENDENCIES "Dependencies" @@ -1401,6 +1454,7 @@ void Document::addRecomputeObject(DocumentObject *obj) { std::vector Document::importObjects(Base::XMLReader& reader) { + d->hashers.clear(); Base::FlagToggler<> flag(globalIsRestoring, false); Base::ObjectStatusLocker restoreBit(Status::Restoring, this); Base::ObjectStatusLocker restoreBit2(Status::Importing, this); @@ -1452,6 +1506,7 @@ Document::importObjects(Base::XMLReader& reader) o->setStatus(App::ObjImporting,false); } + d->hashers.clear(); return objs; } @@ -1464,6 +1519,8 @@ unsigned int Document::getMemSize () const for (it = d->objectArray.begin(); it != d->objectArray.end(); ++it) size += (*it)->getMemSize(); + size += d->Hasher->getMemSize(); + // size of the document properties... size += PropertyContainer::getMemSize(); diff --git a/src/App/Document.h b/src/App/Document.h index ad202a31d3e0..1f89b74ec538 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -23,6 +23,12 @@ #ifndef APP_DOCUMENT_H #define APP_DOCUMENT_H +#include +#include +#include +#include +#include + #include "PropertyContainer.h" #include "PropertyLinks.h" #include "PropertyStandard.h" @@ -44,6 +50,8 @@ namespace App class DocumentPy; // the python document class class Application; class Transaction; + class StringHasher; + using StringHasherRef = Base::Reference; } namespace App @@ -465,6 +473,33 @@ class AppExport Document : public App::PropertyContainer (const App::DocumentObject* from, const App::DocumentObject* to) const; //@} + /** Called by a property during save to store its StringHasher + * + * @param hasher: the input hasher + * @return Returns a pair. The boolean indicates if the + * StringHasher has been added before. The integer is the hasher index. + * + * The StringHasher object is designed to be shared among multiple objects. + * We must not save duplicate copies of the same hasher, and must be + * able to restore with the same sharing relationship. This function returns + * whether the hasher has been added before by other objects, and the index + * of the hasher. If the hasher has not been added before, the object must + * save the hasher by calling StringHasher::Save + */ + std::pair addStringHasher(const StringHasherRef & hasher) const; + + /** Called by property to restore its StringHasher + * + * @param index: the index previously returned by calling addStringHasher() + * during save. Or if is negative, then return document's own string hasher. + * + * @return Return the resulting string hasher. + * + * The caller is responsible for restoring the hasher if the caller is the first + * owner of the hasher, i.e. if addStringHasher() returns true during save. + */ + StringHasherRef getStringHasher(int index=-1) const; + /** Return the links to a given object * * @param links: holds the links found diff --git a/src/App/Property.h b/src/App/Property.h index 895795612499..59c6ff505bd2 100644 --- a/src/App/Property.h +++ b/src/App/Property.h @@ -261,6 +261,8 @@ class AppExport Property : public Base::Persistence */ int64_t getID() const {return _id;} + virtual void beforeSave() const {} + friend class PropertyContainer; friend struct PropertyData; friend class DynamicProperty; diff --git a/src/App/PropertyContainer.cpp b/src/App/PropertyContainer.cpp index 719c12469958..e4f76e92aac8 100644 --- a/src/App/PropertyContainer.cpp +++ b/src/App/PropertyContainer.cpp @@ -219,6 +219,23 @@ void PropertyContainer::handleChangedPropertyType(XMLReader &reader, const char PropertyData PropertyContainer::propertyData; +void PropertyContainer::beforeSave() const +{ + std::map Map; + getPropertyMap(Map); + for (auto& entry : Map) { + auto prop = entry.second; + if (!prop->testStatus(Property::PropDynamic) + && (prop->testStatus(Property::Transient) + || ((getPropertyType(prop) & Prop_Transient) != 0))) { + // Nothing + } + else { + prop->beforeSave(); + } + } +} + void PropertyContainer::Save (Base::Writer &writer) const { std::map Map; @@ -237,8 +254,9 @@ void PropertyContainer::Save (Base::Writer &writer) const { transients.push_back(prop); it = Map.erase(it); - }else + } else { ++it; + } } writer.incInd(); // indentation for 'Properties Count' diff --git a/src/App/PropertyContainer.h b/src/App/PropertyContainer.h index 03d9fbe562a1..17a2102efcea 100644 --- a/src/App/PropertyContainer.h +++ b/src/App/PropertyContainer.h @@ -220,6 +220,7 @@ class AppExport PropertyContainer: public Base::Persistence void Save (Base::Writer &writer) const override; void Restore(Base::XMLReader &reader) override; + virtual void beforeSave() const; virtual void editProperty(const char * /*propName*/) {} diff --git a/src/App/private/DocumentP.h b/src/App/private/DocumentP.h index a065ce7141f9..a85d90af501c 100644 --- a/src/App/private/DocumentP.h +++ b/src/App/private/DocumentP.h @@ -25,11 +25,14 @@ #include #include +#include #include +#include #include #include #include + // using VertexProperty = boost::property; using DependencyList = boost::adjacency_list < boost::vecS, // class OutEdgeListS : a Sequence or an AssociativeContainer @@ -47,6 +50,7 @@ using Node = std::vector ; using Path = std::vector ; namespace App { +using HasherMap = boost::bimap; class Transaction; // Pimpl class @@ -74,6 +78,7 @@ struct DocumentP unsigned int UndoMemSize; unsigned int UndoMaxStackSize; std::string programVersion; + mutable HasherMap hashers; #ifdef USE_OLD_DAG DependencyList DepList; std::map VertexObjectList; @@ -82,6 +87,8 @@ struct DocumentP std::multimap > _RecomputeLog; + StringHasherRef Hasher; + DocumentP(); void addRecomputeLog(const char *why, App::DocumentObject *obj) { diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index 2e3019782a5e..14f763c1a8a6 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -4,6 +4,7 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Application.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Branding.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ComplexGeoData.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Document.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ElementMap.cpp ${CMAKE_CURRENT_SOURCE_DIR}/IndexedName.cpp diff --git a/tests/src/App/Document.cpp b/tests/src/App/Document.cpp new file mode 100644 index 000000000000..027c60c583da --- /dev/null +++ b/tests/src/App/Document.cpp @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" +#include + +#include "App/Application.h" +#include "App/Document.h" +#include "App/StringHasher.h" +#include "Base/Writer.h" + +using ::testing::Eq; +using ::testing::Ne; + +// NOLINTBEGIN(readability-magic-numbers) + +class FakeWriter: public Base::Writer +{ + void writeFiles() override + {} + std::ostream& Stream() override + { + return std::cout; + } +}; + +class DocumentTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + if (App::Application::GetARGC() == 0) { + constexpr int argc = 1; + std::array argv {"FreeCAD"}; + App::Application::Config()["ExeName"] = "FreeCAD"; + App::Application::init(argc, const_cast(argv.data())); // NOLINT + } + } + + void SetUp() override + { + _docName = App::GetApplication().getUniqueDocumentName("test"); + _doc = App::GetApplication().newDocument(_docName.c_str(), "testUser"); + } + + void TearDown() override + { + App::GetApplication().closeDocument(_docName.c_str()); + } + + App::Document* doc() + { + return _doc; + } + +private: + std::string _docName; + App::Document* _doc {}; +}; + + +TEST_F(DocumentTest, addStringHasherIndicatesUnwrittenWhenNew) +{ + // Arrange + App::StringHasherRef hasher(new App::StringHasher); + + // Act + auto addResult = doc()->addStringHasher(hasher); + + // Assert + EXPECT_TRUE(addResult.first); + EXPECT_THAT(addResult.second, Ne(-1)); +} + +TEST_F(DocumentTest, addStringHasherIndicatesAlreadyWritten) +{ + // Arrange + App::StringHasherRef hasher(new App::StringHasher); + doc()->addStringHasher(hasher); + + // Act + auto addResult = doc()->addStringHasher(hasher); + + // Assert + EXPECT_FALSE(addResult.first); +} + +TEST_F(DocumentTest, getStringHasherGivesExpectedHasher) +{ + // Arrange + App::StringHasherRef hasher(new App::StringHasher); + auto pair = doc()->addStringHasher(hasher); + int index = pair.second; + + // Act + auto foundHasher = doc()->getStringHasher(index); + + // Assert + EXPECT_EQ(hasher, foundHasher); +} + +// NOLINTEND(readability-magic-numbers)