ObjectIdentifier: Reworked resolve() function to solve issue #2389 and #2418.

* Refactored code; moved mutable fields into a separate inner class.
* Added resolvedProperty to ResolveResults class.
* Set resolved document name, even if it does not resolve correctly, so we can give a better error message later.
* If the document name is explicitly set, and it does not resolve, don't try any further.
* If document name is set, use that instead of the ObjectIdentifier's owner when looking up the document object.
This commit is contained in:
Eivind Kvedalen 2016-01-26 22:22:45 +01:00 committed by wmayer
parent 183f8dfebe
commit e5f1e298a6
2 changed files with 173 additions and 106 deletions

View File

@ -115,18 +115,18 @@ ObjectIdentifier::ObjectIdentifier(const App::PropertyContainer * _owner, const
: owner(_owner)
, documentNameSet(false)
, documentObjectNameSet(false)
, propertyIndex(-1)
{
if (owner) {
const DocumentObject * docObj = freecad_dynamic_cast<const DocumentObject>(owner);
if (!docObj)
throw Base::Exception("Property must be owned by a document object.");
const Document * doc = docObj->getDocument();
documentName = String(doc->getName(), false, true);
documentObjectName = String(docObj->getNameInDocument(), false, true);
if (property.size() > 0) {
const Document * doc = docObj->getDocument();
documentName = String(doc->getName(), false, true);
documentObjectName = String(docObj->getNameInDocument(), false, true);
}
}
if (property.size() > 0)
addComponent(Component::SimpleComponent(property));
@ -141,7 +141,6 @@ ObjectIdentifier::ObjectIdentifier(const Property &prop)
: owner(prop.getContainer())
, documentNameSet(false)
, documentObjectNameSet(false)
, propertyIndex(-1)
{
DocumentObject * docObj = freecad_dynamic_cast<DocumentObject>(prop.getContainer());
@ -163,11 +162,11 @@ ObjectIdentifier::ObjectIdentifier(const Property &prop)
const std::string App::ObjectIdentifier::getPropertyName() const
{
resolve();
ResolveResults result(*this);
assert(propertyIndex >=0 && static_cast<std::size_t>(propertyIndex) < components.size());
assert(result.propertyIndex >=0 && static_cast<std::size_t>(result.propertyIndex) < components.size());
return components[propertyIndex].toString();
return components[result.propertyIndex].toString();
}
/**
@ -178,11 +177,11 @@ const std::string App::ObjectIdentifier::getPropertyName() const
const App::ObjectIdentifier::Component &App::ObjectIdentifier::getPropertyComponent(int i) const
{
resolve();
ResolveResults result(*this);
assert(propertyIndex + i >=0 && static_cast<std::size_t>(propertyIndex) + i < components.size());
assert(result.propertyIndex + i >=0 && static_cast<std::size_t>(result.propertyIndex) + i < components.size());
return components[propertyIndex + i];
return components[result.propertyIndex + i];
}
/**
@ -193,14 +192,14 @@ const App::ObjectIdentifier::Component &App::ObjectIdentifier::getPropertyCompon
bool ObjectIdentifier::operator ==(const ObjectIdentifier &other) const
{
resolve();
other.resolve();
ResolveResults result1(*this);
ResolveResults result2(other);
if (owner != other.owner)
return false;
if (documentName != other.documentName)
if (result1.resolvedDocumentName != result2.resolvedDocumentName)
return false;
if (documentObjectName != other.documentObjectName)
if (result1.resolvedDocumentObjectName != result2.resolvedDocumentObjectName)
return false;
if (components != other.components)
return false;
@ -226,19 +225,19 @@ bool ObjectIdentifier::operator !=(const ObjectIdentifier &other) const
bool ObjectIdentifier::operator <(const ObjectIdentifier &other) const
{
resolve();
other.resolve();
ResolveResults result1(*this);
ResolveResults result2(other);
if (documentName < other.documentName)
if (result1.resolvedDocumentName < result2.resolvedDocumentName)
return true;
if (documentName > other.documentName)
if (result1.resolvedDocumentName > result2.resolvedDocumentName)
return false;
if (documentObjectName < other.documentObjectName)
if (result1.resolvedDocumentObjectName < result2.resolvedDocumentObjectName)
return true;
if (documentObjectName > other.documentObjectName)
if (result1.resolvedDocumentObjectName > result2.resolvedDocumentObjectName)
return false;
if (components.size() < other.components.size())
@ -289,7 +288,9 @@ int ObjectIdentifier::numComponents() const
int ObjectIdentifier::numSubComponents() const
{
return components.size() - propertyIndex;
ResolveResults result(*this);
return components.size() - result.propertyIndex;
}
/**
@ -305,15 +306,14 @@ int ObjectIdentifier::numSubComponents() const
std::string ObjectIdentifier::toString() const
{
std::stringstream s;
resolve();
ResolveResults result(*this);
if (documentNameSet)
s << getDocumentName().toString() << "#";
s << documentName.toString() << "#";
if (documentObjectNameSet)
s << getDocumentObjectName().toString() << ".";
else if (propertyIndex > 0)
s << documentObjectName.toString() << ".";
else if (result.propertyIndex > 0)
s << components[0].toString() << ".";
s << getPropertyName() << getSubPathStr();
@ -348,17 +348,19 @@ bool ObjectIdentifier::renameDocumentObject(const std::string &oldName, const st
else
documentObjectName = ObjectIdentifier::String(newName, true);
resolve();
return true;
}
else if (propertyIndex == 1 && documentObjectName == oldName) {
if (ExpressionParser::isTokenAnIndentifier(newName))
components[0].name = newName;
else
components[0].name = ObjectIdentifier::String(newName, true);
else {
ResolveResults result(*this);
resolve();
return true;
if (result.propertyIndex == 1 && result.resolvedDocumentObjectName == oldName) {
if (ExpressionParser::isTokenAnIndentifier(newName))
components[0].name = newName;
else
components[0].name = ObjectIdentifier::String(newName, true);
return true;
}
}
return false;
}
@ -374,11 +376,18 @@ bool ObjectIdentifier::renameDocument(const std::string &oldName, const std::str
if (oldName == newName)
return false;
if (documentName == oldName) {
if (documentNameSet && documentName == oldName) {
documentName = newName;
resolve();
return true;
}
else {
ResolveResults result(*this);
if (result.resolvedDocumentName == oldName) {
documentName = newName;
return true;
}
}
return false;
}
@ -390,10 +399,10 @@ bool ObjectIdentifier::renameDocument(const std::string &oldName, const std::str
std::string ObjectIdentifier::getSubPathStr() const
{
resolve();
ResolveResults result(*this);
std::stringstream s;
std::vector<Component>::const_iterator i = components.begin() + propertyIndex + 1;
std::vector<Component>::const_iterator i = components.begin() + result.propertyIndex + 1;
while (i != components.end()) {
s << "." << i->toString();
++i;
@ -538,8 +547,10 @@ App::DocumentObject * ObjectIdentifier::getDocumentObject(const App::Document *
// No object found with matching label, try using name directly
objectById = doc->getObject(static_cast<const char*>(name));
if (name.isForceIdentifier())
if (name.isForceIdentifier()) {
byIdentifier = true;
return objectById;
}
for (std::vector<DocumentObject*>::iterator j = docObjects.begin(); j != docObjects.end(); ++j) {
if (strcmp((*j)->Label.getValue(), static_cast<const char*>(name)) == 0) {
@ -571,79 +582,109 @@ App::DocumentObject * ObjectIdentifier::getDocumentObject(const App::Document *
/**
* @brief Resolve the object identifier to a concrete document, documentobject, and property.
*
* This method is a helper methos that updates mutable data in the object, to be used by
* other public methods of this class.
* This method is a helper method that fills out data in the given ResolveResults object.
*
*/
void ObjectIdentifier::resolve() const
void ObjectIdentifier::resolve(ResolveResults &results) const
{
const App::Document * doc;
const App::DocumentObject * docObject;
if (freecad_dynamic_cast<DocumentObject>(owner) == 0)
return;
/* Document name specified? */
if (documentName.getString().size() > 0) {
doc = getDocument(documentName);
results.resolvedDocument = getDocument(documentName);
results.resolvedDocumentName = documentName;
}
else {
results.resolvedDocument = freecad_dynamic_cast<DocumentObject>(owner)->getDocument();
results.resolvedDocumentName = String(results.resolvedDocument->getName(), false, true);
}
else
doc = freecad_dynamic_cast<DocumentObject>(owner)->getDocument();
propertyName = "";
propertyIndex = 0;
results.propertyName = "";
results.propertyIndex = 0;
// Assume document name and object name from owner if not found
if (doc == 0) {
doc = freecad_dynamic_cast<DocumentObject>(owner)->getDocument();
if (doc == 0) {
documentName = String();
documentObjectName = String();
if (results.resolvedDocument == 0) {
if (documentName.getString().size() > 0)
return;
results.resolvedDocument = freecad_dynamic_cast<DocumentObject>(owner)->getDocument();
if (results.resolvedDocument == 0)
return;
}
}
documentName = String(doc->getName(), false, documentName.isForceIdentifier());
results.resolvedDocumentName = String(results.resolvedDocument->getName(), false, true);
/* Document object name specified? */
if (documentObjectNameSet) {
if (documentObjectName.getString().size() > 0) {
bool dummy;
docObject = getDocumentObject(doc, documentObjectName, dummy);
if (!docObject)
results.resolvedDocumentObjectName = documentObjectName;
results.resolvedDocumentObject = getDocumentObject(results.resolvedDocument, documentObjectName, dummy);
if (!results.resolvedDocumentObject)
return;
if (components.size() > 0) {
propertyName = components[0].name.getString();
propertyIndex = 0;
results.propertyName = components[0].name.getString();
results.propertyIndex = 0;
results.resolvedProperty = results.resolvedDocumentObject->getPropertyByName(results.propertyName.c_str());
}
else
return;
}
else {
/* Document object name not specified, resolve from path */
/* One component? */
if (components.size() == 1) {
documentObjectName = String(static_cast<const DocumentObject*>(owner)->getNameInDocument(), false, documentObjectName.isForceIdentifier());
propertyName = components[0].name.getString();
propertyIndex = 0;
/* Yes -- then this must be a property, so we get the document object's name from the owner */
bool byIdentifier;
results.resolvedDocumentObjectName = String(static_cast<const DocumentObject*>(owner)->getNameInDocument(), false, true);
results.resolvedDocumentObject = getDocumentObject(results.resolvedDocument, results.resolvedDocumentObjectName, byIdentifier);
results.propertyName = components[0].name.getString();
if (results.resolvedDocumentObject)
results.resolvedProperty = results.resolvedDocumentObject->getPropertyByName(results.propertyName.c_str());
results.propertyIndex = 0;
}
else if (components.size() >= 2) {
/* No -- */
bool byIdentifier;
if (!components[0].isSimple())
return;
docObject = getDocumentObject(doc, components[0].name, byIdentifier);
results.resolvedDocumentObject = getDocumentObject(results.resolvedDocument, components[0].name, byIdentifier);
if (docObject) {
documentObjectName = String(components[0].name, false, byIdentifier);
propertyName = components[1].name.getString();
propertyIndex = 1;
/* Possible to resolve component to a document object? */
if (results.resolvedDocumentObject) {
/* Yes */
results.resolvedDocumentObjectName = String(components[0].name, false, byIdentifier);
results.propertyName = components[1].name.getString();
results.resolvedProperty = results.resolvedDocumentObject->getPropertyByName(results.propertyName.c_str());
results.propertyIndex = 1;
}
else {
documentObjectName = String(static_cast<const DocumentObject*>(owner)->getNameInDocument(), false, true);
propertyName = components[0].name.getString();
propertyIndex = 0;
/* Document name set explicitely? */
if (documentName.getString().size() > 0) {
/* Yes; then document object must follow */
results.resolvedDocumentObjectName = String(components[0].name, false, false);
results.resolvedDocumentObject = results.resolvedDocument->getObject(static_cast<const DocumentObject*>(owner)->getNameInDocument());
results.propertyIndex = 1;
}
else {
/* No, assume component is a property, and get document object's name from owner */
const DocumentObject * docObj = static_cast<const DocumentObject*>(owner);
results.resolvedDocument = docObj->getDocument();
results.resolvedDocumentName = String(results.resolvedDocument->getName(), false, true);
results.resolvedDocumentObjectName = String(docObj->getNameInDocument(), false, true);
results.resolvedDocumentObject = docObj->getDocument()->getObject(docObj->getNameInDocument());
results.propertyIndex = 0;
}
results.propertyName = components[results.propertyIndex].name.getString();
if (results.resolvedDocumentObject)
results.resolvedProperty = results.resolvedDocumentObject->getPropertyByName(results.propertyName.c_str());
}
}
else
@ -705,7 +746,9 @@ DocumentObject *ObjectIdentifier::getDocumentObject() const
if (!doc)
return 0;
return getDocumentObject(doc, documentObjectName, dummy);
ResolveResults result(*this);
return getDocumentObject(doc, result.resolvedDocumentObjectName, dummy);
}
/**
@ -716,11 +759,12 @@ DocumentObject *ObjectIdentifier::getDocumentObject() const
std::vector<std::string> ObjectIdentifier::getStringList() const
{
std::vector<std::string> l;
ResolveResults result(*this);
if (documentNameSet)
l.push_back(documentName.toString());
l.push_back(result.resolvedDocumentName.toString());
if (documentObjectNameSet)
l.push_back(documentObjectName.toString());
l.push_back(result.resolvedDocumentObjectName.toString());
std::vector<Component>::const_iterator i = components.begin();
while (i != components.end()) {
@ -740,13 +784,15 @@ std::vector<std::string> ObjectIdentifier::getStringList() const
ObjectIdentifier ObjectIdentifier::relativeTo(const ObjectIdentifier &other) const
{
ObjectIdentifier result(owner);
ResolveResults thisresult(*this);
ResolveResults otherresult(other);
if (other.getDocument() != getDocument())
result.setDocumentName(getDocumentName(), true);
if (other.getDocumentObject() != getDocumentObject())
result.setDocumentObjectName(getDocumentObjectName(), true);
if (otherresult.resolvedDocument != thisresult.resolvedDocument)
result.setDocumentName(thisresult.resolvedDocumentName, true);
if (otherresult.resolvedDocumentObject != thisresult.resolvedDocumentObject)
result.setDocumentObjectName(thisresult.resolvedDocumentObjectName, true);
for (std::size_t i = propertyIndex; i < components.size(); ++i)
for (std::size_t i = thisresult.propertyIndex; i < components.size(); ++i)
result << components[i];
return result;
@ -792,18 +838,9 @@ ObjectIdentifier &ObjectIdentifier::operator <<(const ObjectIdentifier::Componen
Property *ObjectIdentifier::getProperty() const
{
const App::Document * doc = getDocument();
bool dummy;
ResolveResults result(*this);
if (!doc)
return 0;
App::DocumentObject * docObj = getDocumentObject(doc, documentObjectName, dummy);
if (!docObj)
return 0;
return docObj->getPropertyByName(getPropertyComponent(0).getName().c_str());
return result.resolvedProperty;
}
/**
@ -820,7 +857,9 @@ ObjectIdentifier ObjectIdentifier::canonicalPath() const
// Simplify input path by ensuring that components array only has property + optional sub-properties first.
ObjectIdentifier simplified(getDocumentObject());
for (std::size_t i = propertyIndex; i < components.size(); ++i)
ResolveResults result(*this);
for (std::size_t i = result.propertyIndex; i < components.size(); ++i)
simplified << components[i];
Property * prop = getProperty();
@ -853,8 +892,9 @@ void ObjectIdentifier::setDocumentName(const ObjectIdentifier::String &name, boo
const ObjectIdentifier::String ObjectIdentifier::getDocumentName() const
{
resolve();
return documentName;
ResolveResults result(*this);
return result.resolvedDocumentName;
}
/**
@ -880,8 +920,9 @@ void ObjectIdentifier::setDocumentObjectName(const ObjectIdentifier::String &nam
const ObjectIdentifier::String ObjectIdentifier::getDocumentObjectName() const
{
resolve();
return documentObjectName;
ResolveResults result(*this);
return result.resolvedDocumentObjectName;
}
/**
@ -1007,3 +1048,20 @@ void ObjectIdentifier::setValue(const boost::any &value) const
Base::Interpreter().runString(ss.str().c_str());
}
/** Construct and initialize a ResolveResults object, given an ObjectIdentifier instance.
*
* The constructor will invoke the ObjectIdentifier's resolve() method to initialize the object's data.
*/
ObjectIdentifier::ResolveResults::ResolveResults(const ObjectIdentifier &oi)
: propertyIndex(-1)
, resolvedDocument(0)
, resolvedDocumentName()
, resolvedDocumentObject(0)
, resolvedDocumentObjectName()
, resolvedProperty(0)
, propertyName()
{
oi.resolve(*this);
}

View File

@ -227,23 +227,32 @@ public:
protected:
struct ResolveResults {
ResolveResults(const ObjectIdentifier & oi);
int propertyIndex;
App::Document * resolvedDocument;
String resolvedDocumentName;
App::DocumentObject * resolvedDocumentObject;
String resolvedDocumentObjectName;
App::Property * resolvedProperty;
std::string propertyName;
};
std::string getPythonAccessor() const;
void resolve() const;
void resolve(ResolveResults & results) const;
App::DocumentObject *getDocumentObject(const App::Document *doc, const String &name, bool &byIdentifier) const;
const App::PropertyContainer * owner;
String documentName;
bool documentNameSet;
String documentObjectName;
bool documentObjectNameSet;
std::vector<Component> components;
/// Mutable elements, updated by resolve()
mutable int propertyIndex;
mutable String documentName;
mutable String documentObjectName;
mutable std::string propertyName;
};
std::size_t hash_value(const App::ObjectIdentifier & path);