From fa4d67735f4df4372fe47e791903a1be38cb5fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Tr=C3=B6ger?= Date: Sat, 3 Jun 2017 15:27:24 +0200 Subject: [PATCH] Revert link integrity checks in properties The DAG test is not needed anymore as the relevant functions are non-DAG save now, and the other check will be moved to the recompute as it is not efficient or save to do it in the links itself. --- src/App/PropertyLinks.cpp | 119 -------------------------------------- src/Mod/Test/Document.py | 14 +++-- 2 files changed, 8 insertions(+), 125 deletions(-) diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index d437f67fb7..cb92d0621c 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -39,101 +39,11 @@ #include "Document.h" #include "PropertyLinks.h" -#include "GeoFeatureGroupExtension.h" -#include "OriginFeature.h" using namespace App; using namespace Base; using namespace std; - -void ensureDAG(PropertyContainer* container, App::DocumentObject* object) { - - if(!container || !object) - return; - - //document containers and other non-object things don't need to be handled - if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - return; - - //undo and redo do not need to be handled as they can only go to already checked stated (the link - //state during those actions can get messed up, we really don't want to check for that) - if(object->getDocument()->performsTransactionOperation()) - return; - - 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 geofeatueregroup, 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; - - //document containers and other non-object things don't need to be handled - if(!container->isDerivedFrom(App::DocumentObject::getClassTypeId())) - return; - - //links to origin feature can go over CS borders, as they are the same everywere anyway. This is - //a workaround to allow moving of objects between GeoFeatureGroups that link to origin features. - //During movement there is always a link in to the wron CS and the error would occure. If we - //surpress the error at least the origin links can be fixed afterwards - //TODO: Find a more elegant solution - if(object->isDerivedFrom(App::OriginFeature::getClassTypeId())) - return; - - //undo and redo do not need to be handled as they can only go to already checked stated (the link - //state during those actions can get messed up, we really don't want to check for that) - if(object->getDocument()->performsTransactionOperation()) - return; - - auto cont = static_cast(container); - - //on restore we don't need to do anything - if(cont->isRestoring() || object->isRestoring()) - return; - - std::vector< DocumentObject* > objs; - GeoFeatureGroupExtension::getCSRelevantLinks(object, objs); - 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 @@ -162,8 +72,6 @@ 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 @@ -297,9 +205,6 @@ 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) @@ -323,11 +228,6 @@ 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 @@ -480,9 +380,6 @@ 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) @@ -679,9 +576,6 @@ 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) @@ -711,11 +605,6 @@ 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 @@ -742,11 +631,6 @@ 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 @@ -767,9 +651,6 @@ 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 0413e9db7f..44b56b86ca 100644 --- a/src/Mod/Test/Document.py +++ b/src/Mod/Test/Document.py @@ -870,12 +870,14 @@ class UndoRedoCases(unittest.TestCase): #to test: try add obj to second group by .Group = [] grp = prt1.Group grp.append(grp2) - try: - prt1.Group=grp - except: - pass - else: - self.fail("No exception at cross geofeaturegroup links") + + #to test: check if cross CS link works + #try: + # prt1.Group=grp + #except: + # pass + #else: + # self.fail("No exception at cross geofeaturegroup links") prt2.addObject(grp1) grp = grp1.Group