Adding the change context to configuration to get valid leaf-selectio… (

onosproject#1627)

* Adding the change context to configuration to get valid leaf-selection values

* Added unit-test with context change

VERSION

@@ -1 +1 @@
-1.0.1-dev
+1.0.1

pkg/northbound/admin/admin.go

@@ -7,18 +7,23 @@ package admin
 
 import (
 	"context"
+	"fmt"
 	"github.com/google/uuid"
 	"github.com/grpc-ecosystem/go-grpc-middleware/util/metautils"
 	"github.com/onosproject/onos-api/go/onos/config/admin"
 	configapi "github.com/onosproject/onos-api/go/onos/config/v2"
 	"github.com/onosproject/onos-config/pkg/pluginregistry"
 	"github.com/onosproject/onos-config/pkg/store/configuration"
 	"github.com/onosproject/onos-config/pkg/store/transaction"
+	"github.com/onosproject/onos-config/pkg/utils"
+	pathutils "github.com/onosproject/onos-config/pkg/utils/path"
 	"github.com/onosproject/onos-config/pkg/utils/tree"
+	valueutils "github.com/onosproject/onos-config/pkg/utils/values/v2"
 	"github.com/onosproject/onos-lib-go/pkg/errors"
 	"github.com/onosproject/onos-lib-go/pkg/logging"
 	"github.com/onosproject/onos-lib-go/pkg/northbound"
 	"github.com/onosproject/onos-lib-go/pkg/uri"
+	"github.com/openconfig/gnmi/proto/gnmi"
 	"google.golang.org/grpc"
 	"strings"
 )
@@ -202,13 +207,55 @@ func (s Server) LeafSelectionQuery(ctx context.Context, req *admin.LeafSelection
 		return nil, errors.Status(err).Err()
 	}
 
+	modelPlugin, ok := s.pluginRegistry.GetPlugin(configType, configVersion)
+	if !ok {
+		return nil, errors.Status(errors.NewInvalid("error getting plugin for %s %s", configType, configVersion)).Err()
+	}
+
 	if req.ChangeContext != nil &&
 		len(req.ChangeContext.GetUpdate())+len(req.ChangeContext.GetReplace())+len(req.ChangeContext.GetDelete()) > 0 {
 
-		log.Warn("Ignoring change context for the moment")
-		// TODO if there is something in the req.ChangeContext then
-		//   a) convert it to Path-Value format like happens in gNMI Set (Updates, Replace and Delete elements)
-		//   b) overlay it on to the Path-Values from 1) like happens in gNMI Set Proposal Controller
+		log.Info("Getting change context from the request")
+		updates := make(configapi.TypedValueMap)
+		deletes := make([]string, 0)
+		for _, u := range req.ChangeContext.GetUpdate() {
+			if err := s.doUpdateOrReplace(ctx, req.ChangeContext.GetPrefix(), u, modelPlugin, updates); err != nil {
+				log.Warn(err)
+				return nil, errors.Status(err).Err()
+			}
+		}
+		for _, u := range req.ChangeContext.GetReplace() {
+			if err := s.doUpdateOrReplace(ctx, req.ChangeContext.GetPrefix(), u, modelPlugin, updates); err != nil {
+				log.Warn(err)
+				return nil, errors.Status(err).Err()
+			}
+		}
+		for _, u := range req.ChangeContext.GetDelete() {
+			deletedPaths, err := s.doDelete(req.ChangeContext.GetPrefix(), u, modelPlugin)
+			if err != nil {
+				log.Warn(err)
+				return nil, errors.Status(err).Err()
+			}
+			deletes = append(deletes, deletedPaths...)
+		}
+
+		// locally merge the change context into the stored configuration
+		newChanges := make(map[string]*configapi.PathValue)
+		for path, value := range updates {
+			updateValue, err := valueutils.NewChangeValue(path, *value, false)
+			if err != nil {
+				return nil, err
+			}
+			newChanges[path] = updateValue
+		}
+
+		for _, path := range deletes {
+			config.Values[path].Deleted = true
+		}
+
+		for path, value := range newChanges {
+			config.Values[path] = value
+		}
 	}
 
 	values := make([]*configapi.PathValue, 0, len(config.Values))
@@ -221,11 +268,6 @@ func (s Server) LeafSelectionQuery(ctx context.Context, req *admin.LeafSelection
 		return nil, errors.Status(errors.NewInternal("error converting configuration to JSON %v", err)).Err()
 	}
 
-	modelPlugin, ok := s.pluginRegistry.GetPlugin(configType, configVersion)
-	if !ok {
-		return nil, errors.Status(errors.NewInvalid("error getting plugin for %s %s", configType, configVersion)).Err()
-	}
-
 	selection, err := modelPlugin.LeafValueSelection(ctx, req.SelectionPath, jsonTree)
 	if err != nil {
 		return nil, errors.Status(errors.NewInvalid("error getting leaf selection for '%s'. %v", req.SelectionPath, err)).Err()
@@ -235,3 +277,64 @@ func (s Server) LeafSelectionQuery(ctx context.Context, req *admin.LeafSelection
 		Selection: selection,
 	}, nil
 }
+
+// This deals with either a path and a value (simple case) or a path with
+// a JSON body which implies multiple paths and values.
+func (s *Server) doUpdateOrReplace(ctx context.Context, prefix *gnmi.Path, u *gnmi.Update, plugin pluginregistry.ModelPlugin, updates configapi.TypedValueMap) error {
+	prefixPath := utils.StrPath(prefix)
+	path := utils.StrPath(u.Path)
+	if prefixPath != "/" {
+		path = fmt.Sprintf("%s%s", prefixPath, path)
+	}
+
+	jsonVal := u.GetVal().GetJsonVal()
+	if jsonVal != nil {
+		log.Debugf("Processing Json Value in set from base %s: %s", path, string(jsonVal))
+		pathValues, err := plugin.GetPathValues(ctx, prefixPath, jsonVal)
+		if err != nil {
+			return err
+		}
+
+		if len(pathValues) == 0 {
+			log.Warnf("no pathValues found for %s in %v", path, string(jsonVal))
+		}
+
+		for _, cv := range pathValues {
+			updates[cv.Path] = &cv.Value
+		}
+	} else {
+		_, rwPathElem, err := pathutils.FindPathFromModel(path, plugin.GetInfo().ReadWritePaths, true)
+		if err != nil {
+			return err
+		}
+		updateValue, err := valueutils.GnmiTypedValueToNativeType(u.Val, rwPathElem)
+		if err != nil {
+			return err
+		}
+		if err = pathutils.CheckKeyValue(path, rwPathElem, updateValue); err != nil {
+			return err
+		}
+		updates[path] = updateValue
+	}
+
+	return nil
+}
+
+func (s *Server) doDelete(prefix *gnmi.Path, gnmiPath *gnmi.Path, plugin pluginregistry.ModelPlugin) ([]string, error) {
+	deletes := make([]string, 0)
+	prefixPath := utils.StrPath(prefix)
+	path := utils.StrPath(gnmiPath)
+	if prefixPath != "/" {
+		path = fmt.Sprintf("%s%s", prefixPath, path)
+	}
+	// Checks for read only paths
+	isExactMatch, rwPath, err := pathutils.FindPathFromModel(path, plugin.GetInfo().ReadWritePaths, false)
+	if err != nil {
+		return nil, err
+	}
+	if isExactMatch && rwPath.IsAKey && !strings.HasSuffix(path, "]") { // In case an index attribute is given - take it off
+		path = path[:strings.LastIndex(path, "/")]
+	}
+	deletes = append(deletes, path)
+	return deletes, nil
+}

pkg/northbound/admin/admin_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/onosproject/onos-config/pkg/store/transaction"
 	"github.com/onosproject/onos-config/pkg/utils/path"
 	"github.com/onosproject/onos-lib-go/pkg/errors"
+	"github.com/openconfig/gnmi/proto/gnmi"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )
@@ -103,6 +104,8 @@ func setupTopoAndRegistry(t *testing.T, test *testContext, id string, model stri
 	plugin.EXPECT().LeafValueSelection(gomock.Any(), "/foo", gomock.Any()).AnyTimes().
 		Return([]string{id, model, version}, nil)
 	plugin.EXPECT().LeafValueSelection(gomock.Any(), "/bar", gomock.Any()).AnyTimes().
+		Return([]string{"bar1", "bar2", "bar3"}, nil)
+	plugin.EXPECT().LeafValueSelection(gomock.Any(), "/goo", gomock.Any()).AnyTimes().
 		Return([]string{}, nil)
 	plugin.EXPECT().LeafValueSelection(gomock.Any(), "", gomock.Any()).AnyTimes().
 		Return(nil, errors.NewInvalid("navigatedValue path cannot be empty"))
@@ -215,11 +218,169 @@ func TestServer_LeafSelectionQuery_NoChange(t *testing.T) {
 		},
 		{
 			name: "selection path no values - not an error",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "devicesim",
+				Version:       "1.0.0",
+				SelectionPath: "/goo",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		resp, err1 := test.server.LeafSelectionQuery(context.TODO(), &tc.req)
+		if tc.expectErr != "" {
+			if err1 != nil {
+				assert.Equal(t, tc.expectErr, err1.Error(), tc.name)
+				continue
+			}
+			t.Logf("%s Expected error %s", tc.name, tc.expectErr)
+			t.FailNow()
+		}
+		assert.NoError(t, err1, tc.name)
+		assert.NotNil(t, resp, tc.name)
+		assert.Equal(t, len(tc.expected), len(resp.Selection), tc.name)
+		if len(tc.expected) == 2 {
+			assert.Equal(t, tc.req.Version, resp.Selection[2])
+		}
+	}
+}
+
+func TestServer_LeafSelectionQuery_WithChange(t *testing.T) {
+	type testCase struct {
+		name      string
+		req       adminapi.LeafSelectionQueryRequest
+		expected  []string
+		expectErr string
+	}
+
+	test := createServer(t)
+	defer test.atomix.Close()
+	defer test.mctl.Finish()
+
+	// Create a plugin and a topo entry for target-1 with 1.0.0 model
+	setupTopoAndRegistry(t, test, "target-1", "devicesim", "1.0.0", false)
+	// Create another for the 2.0.0 model
+	setupTopoAndRegistry(t, test, "target-1", "devicesim", "2.0.0", false)
+
+	targetConfigValues := make(map[string]*configapi.PathValue)
+	targetConfigValues["/foo"] = &configapi.PathValue{
+		Path: "/foo",
+		Value: configapi.TypedValue{
+			Bytes: []byte("Hello world!"),
+			Type:  configapi.ValueType_STRING,
+		},
+	}
+
+	targetID := configapi.TargetID("target-1")
+	targetConfig := &configapi.Configuration{
+		ID:       configuration.NewID(targetID, "devicesim", "1.0.0"),
+		TargetID: targetID,
+		Values:   targetConfigValues,
+	}
+
+	err := test.server.configurationsStore.Create(context.TODO(), targetConfig)
+	assert.NoError(t, err)
+
+	targetConfigV2 := &configapi.Configuration{
+		ID:       configuration.NewID(targetID, "devicesim", "2.0.0"),
+		TargetID: targetID,
+		Values:   targetConfigValues,
+	}
+
+	err = test.server.configurationsStore.Create(context.TODO(), targetConfigV2)
+	assert.NoError(t, err)
+
+	// change context
+	gnmiSetrequest := &gnmi.SetRequest{
+		Update: []*gnmi.Update{
+			{
+				Path: &gnmi.Path{
+					Target: string(targetID),
+					Elem: []*gnmi.PathElem{
+						{
+							Name: "bar",
+						},
+					},
+				},
+				Val: &gnmi.TypedValue{Value: &gnmi.TypedValue_StringVal{StringVal: "Hello all!"}},
+			},
+		},
+	}
+
+	testCases := []testCase{
+		{
+			name: "valid path",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "devicesim",
+				Version:       "1.0.0",
+				SelectionPath: "/foo",
+			},
+			expected: []string{"target-1", "devicesim", "1.0.0"},
+		},
+		{
+			name: "alternate version",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "devicesim",
+				Version:       "2.0.0",
+				SelectionPath: "/foo",
+				ChangeContext: gnmiSetrequest,
+			},
+			expected: []string{"target-1", "devicesim", "2.0.0"},
+		},
+		{
+			name: "invalid target",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-2",
+				Type:          "devicesim",
+				Version:       "1.0.0",
+				SelectionPath: "/foo",
+				ChangeContext: gnmiSetrequest,
+			},
+			expectErr: "rpc error: code = NotFound desc = key target-2-devicesim-1.0.0 not found",
+		},
+		{
+			name: "invalid type",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "device-wrong",
+				Version:       "1.0.0",
+				SelectionPath: "/foo",
+				ChangeContext: gnmiSetrequest,
+			},
+			expectErr: "rpc error: code = NotFound desc = key target-1-device-wrong-1.0.0 not found",
+		},
+		{
+			name: "selection path empty",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "devicesim",
+				Version:       "1.0.0",
+				ChangeContext: gnmiSetrequest,
+			},
+			expectErr: "rpc error: code = InvalidArgument desc = error getting leaf selection for ''. navigatedValue path cannot be empty",
+		},
+		{
+			name: "selection path - change context not in the config store",
 			req: adminapi.LeafSelectionQueryRequest{
 				Target:        "target-1",
 				Type:          "devicesim",
 				Version:       "1.0.0",
 				SelectionPath: "/bar",
+				ChangeContext: gnmiSetrequest,
+			},
+			expected: []string{"bar1", "bar2", "bar3"},
+		},
+		{
+			name: "selection path no values - not an error",
+			req: adminapi.LeafSelectionQueryRequest{
+				Target:        "target-1",
+				Type:          "devicesim",
+				Version:       "1.0.0",
+				SelectionPath: "/goo",
+				ChangeContext: gnmiSetrequest,
 			},
 		},
 	}