OPCFoundation / UA-.NETStandard

OPC Unified Architecture .NET Standard
Other
1.97k stars 950 forks source link

Incompatible change in PR #2768 causes a change in the behavior of ExpandedNodeId.ToString() #2793

Closed ThomasNehring closed 1 month ago

ThomasNehring commented 1 month ago

Type of issue

Current Behavior

The following code snippet behaves differently after the merge of PR #2768:

            string namespaceUri = "KEPServerEX";
            string nodeName = "Data Type Examples.16 Bit Device.K Registers.Double3";
            String expectedNodeIdString = $"nsu={namespaceUri};s={nodeName}";
            ExpandedNodeId expandedNodeId = new ExpandedNodeId(expectedNodeIdString);

            string stringifiedExpandedNodId = expandedNodeId.ToString();

Before the merge, the value of expandedNodeId.IdentifierText would read "nsu=KEPServerEX;Data Type Examples.16 Bit Device.K Registers.Double3". After the merge it reads "nsu=http://kepserverex/;s=Data Type Examples.16 Bit Device.K Registers.Double3". The same effect applies to the result of the ExpandedNodeId.ToString() method. This change is caused by a change to the method

        public static void Format(
            IFormatProvider formatProvider,
            StringBuilder buffer,
            object identifier,
            IdType identifierType,
            ushort namespaceIndex,
            string namespaceUri,
            uint serverIndex)

which now checks whether the namespaceUri is not null or empty and (if this is true), applies Utils.EscapeUri(...) to the namespaceUri.

Expected Behavior

I believe that the new behavior may actually be the expected behavior, but it breaks existing applications. Not every server uses strings as namespaceUris which are actually compliant (the example from the code snippet is taken from an existing server).

Since the ExpandedNodeId.ToString() method is very likely used to create application configuration on client side (if the client wants to persist node ids of the server, it must use the ExpandedNodeId), this change (if it is not undone) may invalidate client configurations and force migrations of their configurations

Steps To Reproduce

You can just paste the code snipped as a unit test into the BuildInTests of the Opc.Ua.Core.Tests project and run the test (on a branch first without and then with the PR #2768 merged into it.

Environment

- OS:
- Environment:
- Runtime:
- Nuget Version:
- Component:
- Server:
- Client:

Anything else?

No response