Make unique name identifier behaviour consistent for both reference and imported library usage.

[kwokcb]

Issue

It is possible to have different behaviour for unique identifier checking based on whether you use a referenced library (via setDataLibrary) vs an embedded library (via importLibrary).

importLibrary

1. Disallowed for creation of elements with the same id at the same scope. In this case "root" or document level
2. Allowed for search of elements at the same scope.

setDataLibrary

1. Allows for elements with the same name at the root scope.

• Such elements cannot be found due to a small logic error in named child retrieval

2. Allows for search of elements between the root and referenced document. This "preserves" the current logic by hiding that the referenced document isn't really at the same scope.

One simple workflow which will cause issues is creating a node instance with name of a nodeef when using setDataLibrary and then trying to load that into a document which uses importLibrary. It is valid in the first context but invalid in the second resulting in load / validation failure.

Proposal

Make the behaviour for unique name generation and conflict for referenced libraries to behave the same as imported libraries. This entails:

1. Fixing how createValidChildName() works to check for reference document names. This is for all library types including definitions, implementations, typedef etc.
2. Make sure that child addition checks for duplicate elements from referenced documents.
3. As noted fix the search for child elements. This code in particaly does not check the type for the referenced elements so will not look for child elements. Both the name and type should be checked for both cases.

template <class T> shared_ptr<T> Element::getChildOfType(const string& name) const
{
    ElementPtr child;
    ConstDocumentPtr doc = asA<Document>();
    if (doc && doc->hasDataLibrary())
    {
        child = doc->getDataLibrary()->getChild(name);
    }
    if (!child)
    {
        child = getChild(name);
    }
    return child ? child->asA<T>() : shared_ptr<T>();

Discussion

See the Slack thread for more context.

[kwokcb]

Sample script which will behave different if importLibrary is used instead of setDataLibrary.

import MaterialX as mx
doc = mx.createDocument()
#graph = doc.addNodeGraph("graph1")

stdlib = mx.createDocument()
mx.loadLibraries(mx.getDefaultDataLibraryFolders(), mx.getDefaultDataSearchPath(), stdlib)            
doc.setDataLibrary(stdlib)

nodeDef = stdlib.getNodeDef("ND_image_float")
name = doc.createValidChildName(nodeDef.getName());
node = doc.addNodeInstance(nodeDef, name);
if not node:
    print("Failed to added node", node.getName())
else:
    print("Added node", node.getName())

node2 = doc.getNode(node.getName())
print("Created node:", node)
print("Get node:", node2)

content = mx.writeToXmlString(doc)
print(content)

importLibrary will create and retrieve the new node as expected. setDataLibrary will add a node with an existing nodedef name -- which cannot be retrieved due to the retrieval logic error as noded. Loading this content into a document using importLibrary will result in the duplicate named element being found result in load failure.