Parentnodes with same Browsename not allowed, but should be

[swamper123]

Describe the bug
From the specs:

" Since BrowseNames shall be unique in the context of type definitions, a Client may create a browse path that is valid for a type definition and use this path on instances of the type. [...] If a Node has multiple targets with the same BrowseName, the Server shall return a list of NodeIds. "

This is not provided in our server, because I missinterpreted it and made everynode Browsename unique. If a list of Nodes is returned, like metioned in the spec, if multiple Browsenames of a parent are unique, is not tested.

To Reproduce
Import two nodes with the same Browsename to the same parent.

Expected behavior
In case of browsing, this should return a list, instead of a Node. The Client has to figure out, which Node is wanted.

Version
Python-Version: all opcua-asyncio Version: all

[swamper123]

Answer from Randy Armstrong about special cases of uniqueness

OPC UA is a full mesh network with overlapping hierarchies which means the BrowseName cannot be assumed to be unique.

The exceptions are:

1. Properties. All targets of HasProperty reference from any Node must have unique BrowseNames. ✔️
2. TypeDefinitions: All targets of hierarchical references from an ObjectType or VariableType must have unique names.
3. InstanceDeclarations: All targets of hierarchical references from an Object, Variable or Method InstanceDeclaration must have unique names.

ToDo: Revert uniqueness checks, except if a special case occurs.

[jcbastosportela]

Answer from Randy Armstrong about special cases of uniqueness

OPC UA is a full mesh network with overlapping hierarchies which means the BrowseName cannot be assumed to be unique. The exceptions are:

1. Properties. All targets of HasProperty reference from any Node must have unique BrowseNames.
2. TypeDefinitions: All targets of hierarchical references from an ObjectType or VariableType must have unique names.
3. InstanceDeclarations: All targets of hierarchical references from an Object, Variable or Method InstanceDeclaration must have unique names.

ToDo: Revert uniqueness checks, except if a special case occurs.

I think that coincidently I faced this issue when trying to create a set of information model files to reproduce other issue I faced. I have created 4 information model files, all to be imported into the server and I got the following error while importing:

asyncua.ua.uaerrors._auto.BadBrowseNameDuplicated: "The browse name is not unique among nodes that share the same relationship with the parent."(BadBrowseNameDuplicated)

After debugging I believe found the root of the issue: it is in server\address_space.py function def _add_node(self, item, user, check=True); here, on a certain parent it is looking if a certain BrowseName already exists, but only compares the Name and doesn't compare the NamespaceIndex of the BrowseName.

So the server\address_space.py function _add_node could simply be fixed by changing:

if item.BrowseName.Name == ref.BrowseName.Name:
    self.logger.warning(
        f"AddNodesItem: Requested Browsename {item.BrowseName.Name}"
        f" already exists in Parent Node. ParentID:{item.ParentNodeId} --- "
        f"ItemId:{item.RequestedNewNodeId}"
    )
    result.StatusCode = ua.StatusCode(ua.StatusCodes.BadBrowseNameDuplicated)
    return result

to

if item.BrowseName.Name == ref.BrowseName.Name and item.BrowseName.NamespaceIndex == ref.BrowseName.NamespaceIndex:
    self.logger.warning(
        f"AddNodesItem: Requested Browsename {item.BrowseName.Name}"
        f" already exists in Parent Node. ParentID:{item.ParentNodeId} --- "
        f"ItemId:{item.RequestedNewNodeId}"
    )
    result.StatusCode = ua.StatusCode(ua.StatusCodes.BadBrowseNameDuplicated)
    return result

[oroulet]

@swamper123 did you revert that or not? it would be nice to do that before I release master

[swamper123]

@jcbastosportela Thanks for the hint, I will keep that in mind while trying to make a fix. :)

[oroulet]

@swamper123 can't we just disable that check in address space? that is the only place you added code, isn'it?

[swamper123]

@oroulet I have to change that specific part, because like mentioned here uniqueness is still a thing, just not everywhere anymore. Also I added two tests which has to be fixed/deleted as well. Also also, as far as I know, if two Nodes with the same Browsename in the same hierarchy level exist, only one will be returned (which is undefined behaviour then). A list of Nodes should be returned in such a case.

[oroulet]

@swamper123 I fixed some of that : https://github.com/FreeOpcUa/opcua-asyncio/pull/618 Also for some reasons I added a test where I was allowed to create to nodes with different browse names.

[swamper123]

@oroulet

Also for some reasons I added a test where I was allowed to create to nodes with different browse names.

I don't know what you mean... I mean ... that is common practice everywhere in our tests and code. More interessting would be a test case of the upper exceptions (Property, Type or Instance Decleration).

[oroulet]

@swamper123 there was a typo in message over. What I was suprised of is that I was allowed to create two nodes with same browse name, even if you commited some code that should prevent it. But anyway creating two nodes with same browsename was nice to check my new code to return several path when necessary

[swamper123]

@oroulet I was a bit supprised to be honest, that my test failed to fail...anyway now it is gone. ^^

I am now working on point 2 of this list - TypeDefinitions - and need help in that case:

If I understood Randy right:

TypeDefinitions: All targets of hierarchical references from an ObjectType or VariableType must have unique names.

I have to check if the parent Node has an inverse "HasTypeDefinition Reference". https://reference.opcfoundation.org/v104/Core/docs/Part3/7.13/

If this is the case, I know that my parent is an Object- or Variabletype. Then I check if a hierarchical reference exist in my parent and check for uniqueness of the browsename.

Is that correct?

[oroulet]

IF you do not want me to merge, flag you merge request by naming it with Draft: or WIP:

[oroulet]

also I tried to understand when the uniqueness should be applied and did not understand anything. So I hoped you did ;-)

[swamper123]

It was totaly okay for the last one. I am pretty confident that this was right. :D I am just struggeling a bit with the other two, since this TypeDefinition and InstanceDeclaration stuff is a bit more complex than just "properties". Also thinking about a solution which we can code in this specific place is not so easy....and since Randy is THE OPC UA Architecture expert, I strongly believe he knows what shall be unique and what not. ^^

[oroulet]

but yes. you can check the HasTypeDefinition reference, all nodes should have one I think

[swamper123]

Arrrghh... okay, I can't put stuff where I wanted it to be... HasTypeDefinition is a NonHierarchicalReference and InstanceDeclaration has a condition (HasModellingRule) which is NonHierarchical as well. So we can't check them where we check if propertie browsenames are duplicated...

These checks have to be made anywhere, when all references of a Node shall be added/have been added...and in case a duplicated Name happens, things have to be reverted. 😕 This needs a bit more brainpower....