fix schema key ordering being lost

pkg/tree/ops/filterchilds.go

@@ -2,7 +2,6 @@ package ops
 
 import (
 	"fmt"
-	"sort"
 
 	"github.com/sdcio/data-server/pkg/tree/api"
 	"github.com/sdcio/data-server/pkg/tree/types"
@@ -16,9 +15,8 @@ func FilterChilds(s api.Entry, keys map[string]string) ([]api.Entry, error) {
 		return nil, fmt.Errorf("error non schema level %s", s.SdcpbPath().ToXPath(false))
 	}
 
-	// Retrieve and sort schema keys to maintain insertion order
-	schemaKeys := GetSchemaKeys(s)
-	sort.Strings(schemaKeys)
+	// Walk key levels in tree order (alphabetical by key name).
+	schemaKeys := GetSchemaKeysAlphabeticalOrder(s)
 
 	// Start with the root entry
 	currentEntries := []api.Entry{s}

pkg/tree/ops/getschemakeys.go

@@ -1,8 +1,14 @@
 package ops
 
-import "github.com/sdcio/data-server/pkg/tree/api"
+import (
+	"slices"
+	"sort"
 
-// GetSchemaKeys checks for the schema of the entry, and returns the defined keys
+	"github.com/sdcio/data-server/pkg/tree/api"
+)
+
+// GetSchemaKeys returns list key leaf names in YANG schema declaration order
+// (the order of the bound schema's `key` statement).
 func GetSchemaKeys(e api.Entry) []string {
 	if e.GetSchema() != nil {
 		// if the schema is a container schema, we need to process the aggregation logic
@@ -18,3 +24,15 @@ func GetSchemaKeys(e api.Entry) []string {
 	}
 	return nil
 }
+
+// GetSchemaKeysAlphabeticalOrder returns list key leaf names sorted alphabetically,
+// matching tree key level order.
+func GetSchemaKeysAlphabeticalOrder(e api.Entry) []string {
+	keys := GetSchemaKeys(e)
+	if len(keys) == 0 {
+		return nil
+	}
+	keys = slices.Clone(keys)
+	sort.Strings(keys)
+	return keys
+}

pkg/tree/ops/getschemakeys_test.go

@@ -0,0 +1,57 @@
+package ops_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/sdcio/data-server/pkg/pool"
+	"github.com/sdcio/data-server/pkg/tree"
+	"github.com/sdcio/data-server/pkg/tree/ops"
+	"github.com/sdcio/data-server/pkg/utils/testhelper"
+	"go.uber.org/mock/gomock"
+)
+
+func TestGetSchemaKeysOrders(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	scb, err := testhelper.GetSchemaClientBound(t, mockCtrl)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx := context.Background()
+	tc := tree.NewTreeContext(scb, pool.NewSharedTaskPool(ctx, 1))
+	root, err := tree.NewTreeRoot(ctx, tc)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	doublekeyList, err := tree.NewSharedEntryAttributes(ctx, root.Entry, "doublekey", tc)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("schema declaration order", func(t *testing.T) {
+		got := ops.GetSchemaKeys(doublekeyList)
+		want := []string{"key2", "key1"}
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Fatalf("GetSchemaKeys() mismatch (-want +got):\n%s", diff)
+		}
+	})
+
+	t.Run("alphabetical tree key level order", func(t *testing.T) {
+		got := ops.GetSchemaKeysAlphabeticalOrder(doublekeyList)
+		want := []string{"key1", "key2"}
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Fatalf("GetSchemaKeysAlphabeticalOrder() mismatch (-want +got):\n%s", diff)
+		}
+	})
+
+	t.Run("non-list returns nil", func(t *testing.T) {
+		if got := ops.GetSchemaKeysAlphabeticalOrder(root.Entry); got != nil {
+			t.Fatalf("GetSchemaKeysAlphabeticalOrder(root) = %v, want nil", got)
+		}
+	})
+}

pkg/tree/ops/json.go

@@ -140,23 +140,21 @@ func jsonGetIetfPrefixConditional(key string, a api.Entry, b api.Entry, ietf boo
 	return fmt.Sprintf("%s:%s", aModule, key)
 }
 
-// xmlAddKeyElements determines the keys of a certain Entry in the tree and adds those to the
-// element if they do not already exist.
+// jsonAddKeyElements adds the list key/value pairs for entry s to dict.
+//
+// Tree key levels are ordered alphabetically by key name, so we pair each level
+// with the sorted key names to keep values matched to the right key. JSON member
+// order is insignificant (RFC 7951), so no schema-order emission is needed.
 func jsonAddKeyElements(s api.Entry, dict map[string]any) {
-	// retrieve the parent schema, we need to extract the key names
-	// values are the tree level names
 	parentSchema, levelsUp := GetFirstAncestorWithSchema(s)
-	// from the parent we get the keys as slice
-	schemaKeys := GetSchemaKeys(parentSchema)
+
+	alphaKeys := GetSchemaKeysAlphabeticalOrder(parentSchema)
+
 	treeElem := s
-	// the keys do match the levels up in the tree in reverse order
-	// hence we init i with levelUp and count down
 	for i := levelsUp - 1; i >= 0; i-- {
-		// skip if the element already exists
-		if _, exists := dict[schemaKeys[i]]; !exists {
-			// and finally we create the patheleme key attributes
-			dict[schemaKeys[i]] = treeElem.PathName()
-			treeElem = treeElem.GetParent()
+		if _, exists := dict[alphaKeys[i]]; !exists {
+			dict[alphaKeys[i]] = treeElem.PathName()
 		}
+		treeElem = treeElem.GetParent()
 	}
 }

pkg/tree/ops/xml.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"slices"
-	"sort"
 
 	"github.com/beevik/etree"
 	"github.com/sdcio/data-server/pkg/tree/api"
@@ -278,30 +277,36 @@ func xmlAddNamespaceConditional(a api.Entry, b api.Entry, elem *etree.Element, h
 	}
 }
 
-// xmlAddKeyElements determines the keys of a certain Entry in the tree and adds those to the
-// element if they do not already exist.
+// xmlAddKeyElements adds the list key elements for entry s to parent if they
+// are not already present.
+//
+// Tree key levels are ordered alphabetically by key name (see
+// PathElem.PathElemNames), but NETCONF requires keys in YANG `key` order
+// (RFC 7950 §7.8.5). We map each tree level to its key via the sorted names
+// (preserving the value/key pairing), then emit in schema
+// order.
 func xmlAddKeyElements(s api.Entry, parent *etree.Element) {
-	// retrieve the parent schema, we need to extract the key names
-	// values are the tree level names
 	parentSchema, levelsUp := GetFirstAncestorWithSchema(s)
 
-	// from the parent we get the keys as slice
 	schemaKeys := GetSchemaKeys(parentSchema)
-	//issue #364: sort the slice
-	sort.Strings(schemaKeys)
+	alphaKeys := GetSchemaKeysAlphabeticalOrder(parentSchema)
 
+	// Walk up the tree, pairing each level with its correct key name.
+	keyValues := make(map[string]string, levelsUp)
 	treeElem := s
-	// the keys do match the levels up in the tree in reverse order
-	// hence we init i with levelUp and count down
 	for i := levelsUp - 1; i >= 0; i-- {
-		// skip if the element already exists
-		existingElem := parent.SelectElement(schemaKeys[i])
-		if existingElem == nil {
-			// and finally we create the key elements in schema order
-			keyElem := etree.NewElement(schemaKeys[i])
-			keyElem.SetText(treeElem.PathName())
-			parent.InsertChildAt(0, keyElem) // we go backwards, so always add to front of parent
-			treeElem = treeElem.GetParent()
+		keyValues[alphaKeys[i]] = treeElem.PathName()
+		treeElem = treeElem.GetParent()
+	}
+
+	// Emit in schema order; reverse + InsertChildAt(0) puts the first key at the front.
+	for i := len(schemaKeys) - 1; i >= 0; i-- {
+		keyName := schemaKeys[i]
+		// the key leaves may already exist (e.g. emitted via child recursion)
+		if parent.SelectElement(keyName) == nil {
+			keyElem := etree.NewElement(keyName)
+			keyElem.SetText(keyValues[keyName])
+			parent.InsertChildAt(0, keyElem)
 		}
 	}
 }