Found the hiesenbug that causing random failures

pkg/modelregistry/jsonvalues/convertJsonDs1_test.go

@@ -22,7 +22,6 @@ import (
 	"gotest.tools/assert"
 	"io/ioutil"
 	"testing"
-	"time"
 )
 
 func Test_correctJsonPathValues2(t *testing.T) {
@@ -85,8 +84,6 @@ func Test_correctJsonPathRwValuesSubInterfaces(t *testing.T) {
 	assert.NilError(t, err)
 	readOnlyPaths, readWritePaths := modelregistry.ExtractPaths(ds1Schema["Device"], yang.TSUnset, "", "")
 
-	// All values are taken from testdata/sample-openconfig.json and defined
-	// here in the intermediate jsonToValues format
 	sampleTree, err := ioutil.ReadFile("./testdata/sample-openconfig-configuration.json")
 	assert.NilError(t, err)
 
@@ -131,7 +128,7 @@ func Test_correctJsonPathRwValuesSystemLogging(t *testing.T) {
 	// here in the intermediate jsonToValues format
 	sampleTree, err := ioutil.ReadFile("./testdata/sample-openconfig-double-index.json")
 	assert.NilError(t, err)
-	time.Sleep(10 * time.Millisecond)
+	assert.Equal(t, 843, len(sampleTree))
 
 	pathValues, err := DecomposeJSONWithPaths(sampleTree, readOnlyPaths, readWritePaths)
 	assert.NilError(t, err)

pkg/modelregistry/jsonvalues/convertJsonTd2_test.go

@@ -25,7 +25,6 @@ import (
 	"gotest.tools/assert"
 	"io/ioutil"
 	"testing"
-	"time"
 )
 
 // For testing TestDevice-2.0.0 - which has an augmented YANG with Choice and Case
@@ -71,7 +70,7 @@ func (m modelPluginTestDevice2) Schema() (map[string]*yang.Entry, error) {
 }
 
 // chocolate is a "case" within a "choice"
-func Test_JsonPathValuesTd2_config(t *testing.T) {
+func Test_DecomposeJSONWithPathsTd2_config(t *testing.T) {
 
 	var modelPluginTest modelPluginTestDevice2
 
@@ -84,6 +83,7 @@ func Test_JsonPathValuesTd2_config(t *testing.T) {
 
 	sampleTree, err := ioutil.ReadFile("./testdata/sample-testdevice2-config.json")
 	assert.NilError(t, err)
+	assert.Equal(t, 462, len(sampleTree))
 
 	pathValues, err := DecomposeJSONWithPaths(sampleTree, readOnlyPaths, readWritePaths)
 	assert.NilError(t, err)
@@ -131,7 +131,7 @@ func Test_JsonPathValuesTd2_config(t *testing.T) {
 }
 
 // chocolate is a "case" within a "choice"
-func Test_correctJsonPathValuesTd2(t *testing.T) {
+func Test_DecomposeJSONWithPathsTd2Choice(t *testing.T) {
 
 	var modelPluginTest modelPluginTestDevice2
 
@@ -168,7 +168,7 @@ func Test_correctJsonPathValuesTd2(t *testing.T) {
 
 // "chocolate" leaf is a "case" within a "choice". The other case has "beer" and "pretzel" together.
 // See config-models/modelplugin/testdevice-2.0.0/yang/test1-augmented@2020-02-29.yang
-func Test_correctJsonPathValuesTd2Wrong(t *testing.T) {
+func Test_ValidateTd2Wrong(t *testing.T) {
 	var modelPluginTest modelPluginTestDevice2
 
 	td2Schema, err := modelPluginTest.Schema()
@@ -191,7 +191,7 @@ func Test_correctJsonPathValuesTd2Wrong(t *testing.T) {
 	//assert.Error(t, err, "choice")
 }
 
-func Test_correctJsonPathValuesTd(t *testing.T) {
+func Test_DecomposeJSONWithPathsTd2OpState(t *testing.T) {
 
 	td2Schema, err := td2.UnzipSchema()
 	assert.NilError(t, err)
@@ -202,7 +202,6 @@ func Test_correctJsonPathValuesTd(t *testing.T) {
 
 	sampleTree, err := ioutil.ReadFile("./testdata/sample-testdevice2-opstate.json")
 	assert.NilError(t, err)
-	time.Sleep(10 * time.Millisecond)
 
 	pathValues, err := DecomposeJSONWithPaths(sampleTree, readOnlyPaths, nil)
 	assert.NilError(t, err)

pkg/modelregistry/jsonvalues/jsonToValues.go

@@ -16,13 +16,13 @@ package jsonvalues
 
 import (
 	"encoding/base64"
-	"encoding/binary"
 	"encoding/json"
 	"fmt"
 	devicechange "github.com/onosproject/onos-config/api/types/change/device"
 	"github.com/onosproject/onos-config/pkg/modelregistry"
 	"math"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 )
@@ -41,6 +41,7 @@ var rOnIndex = regexp.MustCompile(matchOnIndex)
 type indexValue struct {
 	name  string
 	value *devicechange.TypedValue
+	order int
 }
 
 // DecomposeJSONWithPaths - handling the decomposition and correction in one go
@@ -88,17 +89,20 @@ func extractValuesWithPaths(f interface{}, parentPath string,
 			}
 			for _, obj := range objs {
 				isIndex := false
-				for _, idxName := range indexNames {
+				for i, idxName := range indexNames {
 					if removePathIndices(obj.Path) == fmt.Sprintf("%s/%s", removePathIndices(parentPath), idxName) {
-						indices = append(indices, indexValue{name: idxName, value: obj.Value})
+						indices = append(indices, indexValue{name: idxName, value: obj.Value, order: i})
 						isIndex = true
-						continue
+						break
 					}
 				}
 				if !isIndex {
 					nonIndexPaths = append(nonIndexPaths, obj.Path)
 				}
 			}
+			sort.Slice(indices, func(i, j int) bool {
+				return indices[i].order < indices[j].order
+			})
 			// Now we have indices, need to go through again
 			for _, obj := range objs {
 				for _, nonIdxPath := range nonIndexPaths {
@@ -370,8 +374,7 @@ func findModelRwPathNoIndices(modelRWpaths modelregistry.ReadWritePathMap,
 
 	searchpathNoIndices := removePathIndices(searchpath)
 	for path, value := range modelRWpaths {
-		pathNoIndices := removePathIndices(path)
-		if pathNoIndices == searchpathNoIndices {
+		if removePathIndices(path) == searchpathNoIndices {
 			pathWithNumericalIdx, err := insertNumericalIndices(path, searchpath)
 			if err != nil {
 				return nil, fmt.Sprintf("could not replace wildcards in model path with numerical ids %v", err), false
@@ -394,9 +397,12 @@ func findModelRoPathNoIndices(modelROpaths modelregistry.ReadOnlyPathMap,
 			} else {
 				fullpath = fmt.Sprintf("%s%s", path, subpath)
 			}
-			pathNoIndices := removePathIndices(fullpath)
-			if pathNoIndices == searchpathNoIndices {
-				return &subpathValue, fullpath, true
+			if removePathIndices(fullpath) == searchpathNoIndices {
+				pathWithNumericalIdx, err := insertNumericalIndices(fullpath, searchpath)
+				if err != nil {
+					return nil, fmt.Sprintf("could not replace wildcards in model path with numerical ids %v", err), false
+				}
+				return &subpathValue, pathWithNumericalIdx, true
 			}
 		}
 	}
@@ -422,6 +428,7 @@ func indicesOfPath(modelROpaths modelregistry.ReadOnlyPathMap,
 	modelRWpaths modelregistry.ReadWritePathMap, searchpath string) []string {
 
 	searchpathNoIndices := removePathIndices(searchpath)
+	// First search through the RW paths
 	for path := range modelRWpaths {
 		pathNoIndices := removePathIndices(path)
 		// Find a short path
@@ -430,6 +437,7 @@ func indicesOfPath(modelROpaths modelregistry.ReadOnlyPathMap,
 		}
 	}
 
+	// If not found then search through the RO paths
 	for path, value := range modelROpaths {
 		for subpath := range value {
 			var fullpath string
@@ -508,14 +516,14 @@ func replaceIndices(path string, ignoreAfter int, indices []indexValue) (string,
 			}
 			index := indices[i-idxOffset-1]
 			if index.name != idxName {
-				continue
-				//return "", fmt.Errorf("unexpected index name %s", index.name)
+				//continue
+				return "", fmt.Errorf("unexpected index name %s", index.name)
 			}
 			switch index.value.Type {
 			case devicechange.ValueType_STRING:
 				actualValue = string(index.value.Bytes)
 			case devicechange.ValueType_UINT, devicechange.ValueType_INT:
-				actualValue = fmt.Sprintf("%d", binary.LittleEndian.Uint64(index.value.Bytes))
+				actualValue = fmt.Sprintf("%d", (*devicechange.TypedUint64)(index.value).Uint())
 			}
 			pathParts[i] = fmt.Sprintf("%s=%s%s", idxName, actualValue, pathPart[closeIdx:])
 		}

pkg/modelregistry/jsonvalues/jsonToValues_test.go

@@ -94,7 +94,7 @@ func Test_DecomposeTree(t *testing.T) {
 func Test_DecomposeTreeConfigOnly(t *testing.T) {
 	sampleTree, err := setUpJSONToValues("./testdata/sample-tree-double-key.json")
 	assert.NilError(t, err)
-	assert.Assert(t, len(sampleTree) > 0, "Empty sample tree", len(sampleTree))
+	assert.Equal(t, 762, len(sampleTree), "Empty sample tree", len(sampleTree))
 
 	_, ds1RwPaths := setUpRwPaths()
 	pathValues, err := DecomposeJSONWithPaths(sampleTree, nil, ds1RwPaths)
@@ -133,7 +133,7 @@ func Test_findModelRwPathNoIndices(t *testing.T) {
 func Test_findModelRoPathNoIndices(t *testing.T) {
 	ds1RoPaths, _ := setUpRwPaths()
 	const jsonPath = "/system/logging/remote-servers/remote-server[0]/state/host"
-	const modelPath = "/system/logging/remote-servers/remote-server[host=*]/state/host"
+	const modelPath = "/system/logging/remote-servers/remote-server[host=0]/state/host"
 
 	roAttr, fullpath, ok := findModelRoPathNoIndices(ds1RoPaths, jsonPath)
 	assert.Equal(t, true, ok)
@@ -217,12 +217,12 @@ func Test_replaceIndices(t *testing.T) {
 	const modelPathExpected = "/p/q/r[a=12]/s/t[b=34][c=56]/u/v[d=78][e=9][f=10]/w"
 
 	indices := make([]indexValue, 0)
-	indices = append(indices, indexValue{"a", devicechange.NewTypedValueString("12")})
-	indices = append(indices, indexValue{"b", devicechange.NewTypedValueUint64(34)})
-	indices = append(indices, indexValue{"c", devicechange.NewTypedValueInt64(56)})
-	indices = append(indices, indexValue{"d", devicechange.NewTypedValueString("78")})
-	indices = append(indices, indexValue{"e", devicechange.NewTypedValueString("9")})
-	indices = append(indices, indexValue{"f", devicechange.NewTypedValueString("10")})
+	indices = append(indices, indexValue{"a", devicechange.NewTypedValueString("12"), 0})
+	indices = append(indices, indexValue{"b", devicechange.NewTypedValueUint64(34), 1})
+	indices = append(indices, indexValue{"c", devicechange.NewTypedValueInt64(56), 2})
+	indices = append(indices, indexValue{"d", devicechange.NewTypedValueString("78"), 3})
+	indices = append(indices, indexValue{"e", devicechange.NewTypedValueString("9"), 4})
+	indices = append(indices, indexValue{"f", devicechange.NewTypedValueString("10"), 5})
 	replaced, err := replaceIndices(modelPathNumericalIdx, len(modelPathNumericalIdx), indices)
 	assert.NilError(t, err, "unexpected error replacing numbers")
 	assert.Equal(t, modelPathExpected, replaced, "unexpected value after replacing numbers")
@@ -234,9 +234,9 @@ func Test_replaceIndices2(t *testing.T) {
 	const modelPathExpected = "/p/q/r[name=10]/s/t[b=20][name=20]/u/v[d=78][e=9][f=10]/w/x[name=40]/y"
 
 	indices := make([]indexValue, 0)
-	indices = append(indices, indexValue{"d", devicechange.NewTypedValueString("78")})
-	indices = append(indices, indexValue{"e", devicechange.NewTypedValueString("9")})
-	indices = append(indices, indexValue{"f", devicechange.NewTypedValueString("10")})
+	indices = append(indices, indexValue{"d", devicechange.NewTypedValueString("78"), 4})
+	indices = append(indices, indexValue{"e", devicechange.NewTypedValueString("9"), 5})
+	indices = append(indices, indexValue{"f", devicechange.NewTypedValueString("10"), 6})
 
 	replaced, err := replaceIndices(modelPathNumericalIdx, 57, indices)
 	assert.NilError(t, err, "unexpected error replacing numbers")