Repository: gopcua/opcua
Pull request #508 (2021-11-22 12:34:30)

client: set sensible defaults for Browse
This patch sets sensible defaults for the `Browse` method of the client.

See #506

kung-foo: These "sane defaults" are fine, however does this mean we should really do this for _all_ *Requests.
kung-foo: LGTM once merge conflict resolved.
magiconair: Well, we did this for `Read` and we have `client.Browse` and `node.Browse` which is confusing. I think other libraries solve this by providing defaults in arguments or helper methods. I'm OK with solving this in another way if you have a suggestion.

Patch:
diff --git a/client.go b/client.go
index d2f125f..4e5fc8c 100644
--- a/client.go
+++ b/client.go
@@ -847,8 +847,36 @@ func (c *Client) Write(req *ua.WriteRequest) (*ua.WriteResponse, error) {
 	return res, err
 }
 
+func cloneBrowseRequest(req *ua.BrowseRequest) *ua.BrowseRequest {
+	descs := make([]*ua.BrowseDescription, len(req.NodesToBrowse))
+	for i, d := range req.NodesToBrowse {
+		dc := &ua.BrowseDescription{}
+		*dc = *d
+		if dc.ReferenceTypeID == nil {
+			dc.ReferenceTypeID = ua.NewNumericNodeID(0, id.References)
+		}
+		descs[i] = dc
+	}
+	reqc := &ua.BrowseRequest{
+		View:                          req.View,
+		RequestedMaxReferencesPerNode: req.RequestedMaxReferencesPerNode,
+		NodesToBrowse:                 descs,
+	}
+	if reqc.View == nil {
+		reqc.View = &ua.ViewDescription{}
+	}
+	if reqc.View.ViewID == nil {
+		reqc.View.ViewID = ua.NewTwoByteNodeID(0)
+	}
+	return reqc
+}
+
 // Browse executes a synchronous browse request.
 func (c *Client) Browse(req *ua.BrowseRequest) (*ua.BrowseResponse, error) {
+	// clone the request and the NodesToBrowse to set defaults without
+	// manipulating them in-place.
+	req = cloneBrowseRequest(req)
+
 	var res *ua.BrowseResponse
 	err := c.Send(req, func(v interface{}) error {
 		return safeAssign(v, &res)


Test patch:
diff --git a/client_test.go b/client_test.go
index 85c8bfb..760225d 100644
--- a/client_test.go
+++ b/client_test.go
@@ -3,6 +3,7 @@ package opcua
 import (
 	"testing"
 
+	"github.com/gopcua/opcua/id"
 	"github.com/gopcua/opcua/ua"
 	"github.com/pascaldekloe/goe/verify"
 )
@@ -98,3 +99,84 @@ func TestCloneReadRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestCloneBrowseRequest(t *testing.T) {
+	tests := []struct {
+		name      string
+		req, want *ua.BrowseRequest
+	}{
+		{
+			name: "empty",
+			req:  &ua.BrowseRequest{},
+			want: &ua.BrowseRequest{
+				View: &ua.ViewDescription{
+					ViewID: ua.NewTwoByteNodeID(0),
+				},
+				NodesToBrowse: []*ua.BrowseDescription{},
+			},
+		},
+		{
+			name: "view id missing",
+			req: &ua.BrowseRequest{
+				View: &ua.ViewDescription{},
+			},
+			want: &ua.BrowseRequest{
+				View: &ua.ViewDescription{
+					ViewID: ua.NewTwoByteNodeID(0),
+				},
+				NodesToBrowse: []*ua.BrowseDescription{},
+			},
+		},
+		{
+			name: "keep view id",
+			req: &ua.BrowseRequest{
+				View: &ua.ViewDescription{
+					ViewID: ua.NewTwoByteNodeID(1),
+				},
+			},
+			want: &ua.BrowseRequest{
+				View: &ua.ViewDescription{
+					ViewID: ua.NewTwoByteNodeID(1),
+				},
+				NodesToBrowse: []*ua.BrowseDescription{},
+			},
+		},
+		{
+			name: "set reference id",
+			req: &ua.BrowseRequest{
+				NodesToBrowse: []*ua.BrowseDescription{
+					{
+						NodeID:          ua.MustParseNodeID("i=85"),
+						BrowseDirection: ua.BrowseDirectionForward,
+						ReferenceTypeID: nil,
+						IncludeSubtypes: true,
+						NodeClassMask:   1,
+						ResultMask:      2,
+					},
+				},
+			},
+			want: &ua.BrowseRequest{
+				View: &ua.ViewDescription{
+					ViewID: ua.NewTwoByteNodeID(0),
+				},
+				NodesToBrowse: []*ua.BrowseDescription{
+					{
+						NodeID:          ua.MustParseNodeID("i=85"),
+						BrowseDirection: ua.BrowseDirectionForward,
+						ReferenceTypeID: ua.NewNumericNodeID(0, id.References),
+						IncludeSubtypes: true,
+						NodeClassMask:   1,
+						ResultMask:      2,
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := cloneBrowseRequest(tt.req)
+			verify.Values(t, "", got, tt.want)
+		})
+	}
+}