Repository: gopcua/opcua
Pull request #683 (2023-08-24 15:11:37)

fix: ReferenceNodes usage with mask set
resolves https://github.com/gopcua/opcua/issues/682
resolves https://github.com/gopcua/opcua/issues/550

sruehl: Another way would be to add a factory function to `node.go` accepting a `ExpandedNode`. Might be a bit more efficient.
kung-foo: I think your idea for a static factory function sounds cleanest. It would make it clear what is happening, and who should be using it.
sruehl: ok, I rebased the branch on main (not sure if that was necessary) and added a commit migrating this to the factory function (method). Instead adding it as a static I added it to client analogous to the `c.Node` call. This could also be moved to `node.go` and made be private if the exposed `NodeFromExpandedNodeID` doesn't make sense outside the `ReferencedNodes` call
kung-foo: Can you add a few tests to cover the conversion?
sruehl: sure will do
sruehl: 731d545 contains the most relevant test where the arguments ensure the flag is set and the conversion ensure those are unset

Patch:
diff --git a/client.go b/client.go
index d0520c7..b8954cd 100644
--- a/client.go
+++ b/client.go
@@ -962,6 +962,13 @@ func (c *Client) Node(id *ua.NodeID) *Node {
 	return &Node{ID: id, c: c}
 }
 
+// NodeFromExpandedNodeID returns a node object which accesses its attributes
+// through this client connection. This is usually needed when working with node ids returned
+// from browse responses by the server.
+func (c *Client) NodeFromExpandedNodeID(id *ua.ExpandedNodeID) *Node {
+	return &Node{ID: ua.NewNodeIDFromExpandedNodeID(id), c: c}
+}
+
 // FindServers finds the servers available at an endpoint
 func (c *Client) FindServers(ctx context.Context) (*ua.FindServersResponse, error) {
 	stats.Client().Add("FindServers", 1)
diff --git a/node.go b/node.go
index 9719b03..2d08329 100644
--- a/node.go
+++ b/node.go
@@ -160,7 +160,7 @@ func (n *Node) ReferencedNodes(ctx context.Context, refs uint32, dir ua.BrowseDi
 		return nil, err
 	}
 	for _, r := range res {
-		nodes = append(nodes, n.c.Node(r.NodeID.NodeID))
+		nodes = append(nodes, n.c.NodeFromExpandedNodeID(r.NodeID))
 	}
 	return nodes, nil
 }
diff --git a/ua/node_id.go b/ua/node_id.go
index 594829d..81c892e 100644
--- a/ua/node_id.go
+++ b/ua/node_id.go
@@ -78,6 +78,35 @@ func NewByteStringNodeID(ns uint16, id []byte) *NodeID {
 	}
 }
 
+// NewNodeIDFromExpandedNodeID returns a new NodeID derived from ExpandedNodeID
+func NewNodeIDFromExpandedNodeID(id *ExpandedNodeID) *NodeID {
+	bid := make([]byte, len(id.NodeID.bid))
+	copy(bid, id.NodeID.bid)
+	if len(bid) == 0 {
+		bid = nil
+	}
+	var gid *GUID
+	if egid := id.NodeID.gid; egid != nil {
+		var ngid GUID
+		ngid.Data1 = egid.Data1
+		ngid.Data2 = egid.Data2
+		ngid.Data3 = egid.Data3
+		ngid.Data4 = make([]byte, len(egid.Data4))
+		copy(ngid.Data4, egid.Data4)
+		if len(ngid.Data4) == 0 {
+			ngid.Data4 = nil
+		}
+		gid = &ngid
+	}
+	return &NodeID{
+		mask: id.NodeID.mask & 0b00111111, // expanded flag NamespaceURI and ServerIndex need to be unset
+		ns:   id.NodeID.ns,
+		nid:  id.NodeID.nid,
+		bid:  bid,
+		gid:  gid,
+	}
+}
+
 // MustParseNodeID returns a node id from a string definition
 // if it is parseable by ParseNodeID. Otherwise, the function
 // panics.


Test patch:
diff --git a/ua/node_id_test.go b/ua/node_id_test.go
index d4ff3c8..a4cc97b 100644
--- a/ua/node_id_test.go
+++ b/ua/node_id_test.go
@@ -516,3 +516,71 @@ func TestNodeIDToString(t *testing.T) {
 		})
 	}
 }
+
+func TestNewNodeIDFromExpandedNodeID(t *testing.T) {
+	type args struct {
+		id *ExpandedNodeID
+	}
+	tests := []struct {
+		name string
+		args args
+		want *NodeID
+	}{
+		{
+			name: "NewExpandedNodeID",
+			args: args{
+				id: NewExpandedNodeID(NewTwoByteNodeID(42), "someUri", 42),
+			},
+			want: NewTwoByteNodeID(42),
+		},
+		{
+			name: "NewTwoByteExpandedNodeID",
+			args: args{
+				id: NewTwoByteExpandedNodeID(42),
+			},
+			want: NewTwoByteNodeID(42),
+		},
+		{
+			name: "NewFourByteExpandedNodeID",
+			args: args{
+				NewFourByteExpandedNodeID(42, 24),
+			},
+			want: NewFourByteNodeID(42, 24),
+		},
+		{
+			name: "NewNumericExpandedNodeID",
+			args: args{
+				NewNumericExpandedNodeID(42, 24),
+			},
+			want: NewNumericNodeID(42, 24),
+		},
+		{
+			name: "NewStringExpandedNodeID",
+			args: args{
+				NewStringExpandedNodeID(42, "42"),
+			},
+			want: NewStringNodeID(42, "42"),
+		},
+		{
+			name: "NewGUIDExpandedNodeID",
+			args: args{
+				NewGUIDExpandedNodeID(42, "42"),
+			},
+			want: NewGUIDNodeID(42, "42"),
+		},
+		{
+			name: "NewByteStringExpandedNodeID",
+			args: args{
+				NewByteStringExpandedNodeID(42, []byte{0xAF, 0xFE}),
+			},
+			want: NewByteStringNodeID(42, []byte{0xAF, 0xFE}),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := NewNodeIDFromExpandedNodeID(tt.args.id); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("NewNodeIDFromExpandedNodeID() = %#v, want %#v", got, tt.want)
+			}
+		})
+	}
+}