diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index 0b7382f905..12333c56f9 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -274,9 +274,10 @@ std::vector< DocumentObject* > GeoFeatureGroupExtension::getCSRelevantLinks(Docu result.insert(result.end(), out.begin(), out.end()); } - //there will be many douplicates + //there will be many douplicates and also the passed obj in std::sort(result.begin(), result.end()); result.erase(std::unique(result.begin(), result.end()), result.end()); + result.erase(std::remove(result.begin(), result.end(), obj), result.end()); return result; } diff --git a/src/App/PropertyContainerPyImp.cpp b/src/App/PropertyContainerPyImp.cpp index 6ff2a4e6c8..f50347672f 100644 --- a/src/App/PropertyContainerPyImp.cpp +++ b/src/App/PropertyContainerPyImp.cpp @@ -1,3 +1,4 @@ + /*************************************************************************** * Copyright (c) Jürgen Riegel (juergen.riegel@web.de) 2007 * * * diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index cb92d0621c..62838f2d0e 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -39,11 +39,79 @@ #include "Document.h" #include "PropertyLinks.h" +#include "GeoFeatureGroupExtension.h" using namespace App; using namespace Base; using namespace std; + +void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { + + if(!container || !object) + return; + + if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) + throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + + auto cont = static_cast(container); + + //on restore we don't need to do anything + if(cont->isRestoring() || object->isRestoring()) + return; + + //recursively traverse the object links and see if we reach the container, which would be a + //cyclic graph --> not allowed + for(auto obj : object->getOutList()) { + if(obj == cont) + throw Base::Exception("This link would create a cyclic dependency, hence it is rejected"); + + ensureDAG(container, obj); + } +}; + +//helper functions to ensure correct geofeaturegroups. Each object is only allowed to be in a +//single group, and links are not allowed to cross GeoFeatureGroup borders +void ensureCorrectGroups(PropertyContainer* container, App::DocumentObject* object) { + + //on object creation the container may be null, and linked objects may be null anyhow + if(!container || !object) + return; + + if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) + throw Base::Exception("Only DocumentObjects are allowed to use PropertyLinks"); + + auto cont = static_cast(container); + + //on restore we don't need to do anything + if(cont->isRestoring() || object->isRestoring()) + return; + + auto objs = GeoFeatureGroupExtension::getCSRelevantLinks(object); + objs.push_back(object); + + for(auto obj : objs) { + App::DocumentObject* grp = GeoFeatureGroupExtension::getGroupOfObject(obj); + + //if the container is a GeoFeatureGroup there are two allowed possibilites: + //1. the objet is in no group, hence in a single one after adding + //2. the object is already in the container group + if(cont->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) { + + if(!grp || (grp == cont)) + return; + } + //if the container is a normal object there is only a single allowed possibility: + //1. The object has the exact same GeoFeatureGrou as the container (null included) + else if(grp == GeoFeatureGroupExtension::getGroupOfObject(cont)) { + return; + } + + //if we arrive here the container and the object don't have compatible GeoFeatureGroups + throw Base::Exception("Links are only allowed within a GeoFeatureGroup"); + } +}; + //************************************************************************** //************************************************************************** // PropertyLink @@ -72,6 +140,8 @@ PropertyLink::~PropertyLink() void PropertyLink::setValue(App::DocumentObject * lValue) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); aboutToSetValue(); #ifndef USE_OLD_DAG // maintain the back link in the DocumentObject class @@ -205,6 +275,9 @@ int PropertyLinkList::getSize(void) const void PropertyLinkList::setValue(DocumentObject* lValue) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class for(auto *obj : _lValueList) @@ -228,6 +301,11 @@ void PropertyLinkList::setValue(DocumentObject* lValue) void PropertyLinkList::setValues(const std::vector& lValue) { + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + aboutToSetValue(); #ifndef USE_OLD_DAG //maintain the back link in the DocumentObject class @@ -380,6 +458,9 @@ PropertyLinkSub::~PropertyLinkSub() void PropertyLinkSub::setValue(App::DocumentObject * lValue, const std::vector &SubList) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + aboutToSetValue(); #ifndef USE_OLD_DAG if (_pcLinkSub) @@ -576,6 +657,9 @@ int PropertyLinkSubList::getSize(void) const void PropertyLinkSubList::setValue(DocumentObject* lValue,const char* SubName) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain backlinks for(auto *obj : _lValueList) @@ -605,6 +689,11 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -631,6 +720,11 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c if (lValue.size() != lSubNames.size()) throw Base::ValueError("PropertyLinkSubList::setValues: size of subelements list != size of objects list"); + for(auto obj : lValue) { + ensureDAG(getContainer(), obj); + ensureCorrectGroups(getContainer(), obj); + } + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works @@ -651,6 +745,9 @@ void PropertyLinkSubList::setValues(const std::vector& lValue,c void PropertyLinkSubList::setValue(DocumentObject* lValue, const std::vector &SubList) { + ensureDAG(getContainer(), lValue); + ensureCorrectGroups(getContainer(), lValue); + #ifndef USE_OLD_DAG //maintain backlinks. _lValueList can contain items multiple times, but we trust the document //object to ensure that this works diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py index 5f07cebb5d..c1a9d051e9 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -328,10 +328,6 @@ class DocumentBasicCases(unittest.TestCase): L1.touch() self.failUnless(self.Doc.recompute()==1) self.failUnless((7, 5, 4, 1, 2, 1)==(L1.ExecCount,L2.ExecCount,L3.ExecCount,L4.ExecCount,L5.ExecCount,L6.ExecCount)) - L6.Link = L1 # create a circular dependency - self.failUnless(self.Doc.recompute()==-1) - L6.Link = None # resolve the circular dependency - self.failUnless(self.Doc.recompute()==3) self.Doc.removeObject(L1.Name) self.Doc.removeObject(L2.Name) @@ -427,6 +423,7 @@ class DocumentSaveRestoreCases(unittest.TestCase): self.Doc = FreeCAD.newDocument("SaveRestoreTests") L1 = self.Doc.addObject("App::FeatureTest","Label_1") L2 = self.Doc.addObject("App::FeatureTest","Label_2") + L3 = self.Doc.addObject("App::FeatureTest","Label_3") self.TempPath = tempfile.gettempdir() FreeCAD.Console.PrintLog( ' Using temp path: ' + self.TempPath + '\n') @@ -437,9 +434,9 @@ class DocumentSaveRestoreCases(unittest.TestCase): self.Doc.Label_1.TypeTransient = 4712 # setup Linking self.Doc.Label_1.Link = self.Doc.Label_2 - self.Doc.Label_2.Link = self.Doc.Label_1 + self.Doc.Label_2.Link = self.Doc.Label_3 self.Doc.Label_1.LinkSub = (self.Doc.Label_2,["Sub1","Sub2"]) - self.Doc.Label_2.LinkSub = (self.Doc.Label_1,["Sub3","Sub4"]) + self.Doc.Label_2.LinkSub = (self.Doc.Label_3,["Sub3","Sub4"]) # save the document self.Doc.saveAs(SaveName) FreeCAD.closeDocument("SaveRestoreTests") @@ -448,9 +445,9 @@ class DocumentSaveRestoreCases(unittest.TestCase): self.failUnless(self.Doc.Label_2.Integer == 4711) # test Linkage self.failUnless(self.Doc.Label_1.Link == self.Doc.Label_2) - self.failUnless(self.Doc.Label_2.Link == self.Doc.Label_1) + self.failUnless(self.Doc.Label_2.Link == self.Doc.Label_3) self.failUnless(self.Doc.Label_1.LinkSub == (self.Doc.Label_2,["Sub1","Sub2"])) - self.failUnless(self.Doc.Label_2.LinkSub == (self.Doc.Label_1,["Sub3","Sub4"])) + self.failUnless(self.Doc.Label_2.LinkSub == (self.Doc.Label_3,["Sub3","Sub4"])) # do NOT save transient properties self.failUnless(self.Doc.Label_1.TypeTransient == 4711) self.failUnless(self.Doc == FreeCAD.getDocument(self.Doc.Name)) @@ -871,6 +868,7 @@ class UndoRedoCases(unittest.TestCase): #to test: try add obj to second group, once by addObject, once by .Group = [] + #test set links over geofeaturegroup borders def tearDown(self):