diff --git a/pkg/tree/ops/filterchilds.go b/pkg/tree/ops/filterchilds.go
index acf3042e..698621f9 100644
--- a/pkg/tree/ops/filterchilds.go
+++ b/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}
diff --git a/pkg/tree/ops/getschemakeys.go b/pkg/tree/ops/getschemakeys.go
index 11aefea0..25a39dde 100644
--- a/pkg/tree/ops/getschemakeys.go
+++ b/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
+}
diff --git a/pkg/tree/ops/getschemakeys_test.go b/pkg/tree/ops/getschemakeys_test.go
new file mode 100644
index 00000000..616ad21b
--- /dev/null
+++ b/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)
+ }
+ })
+}
diff --git a/pkg/tree/ops/json.go b/pkg/tree/ops/json.go
index f3b4a396..93f72d42 100644
--- a/pkg/tree/ops/json.go
+++ b/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()
}
}
diff --git a/pkg/tree/ops/xml.go b/pkg/tree/ops/xml.go
index 8b0ff576..03b7c32c 100644
--- a/pkg/tree/ops/xml.go
+++ b/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)
}
}
}
diff --git a/pkg/tree/ops/xml_test.go b/pkg/tree/ops/xml_test.go
index d76bcfb0..1951a5a4 100644
--- a/pkg/tree/ops/xml_test.go
+++ b/pkg/tree/ops/xml_test.go
@@ -52,12 +52,12 @@ func TestToXMLTable(t *testing.T) {
+ ethernet-1/1
enable
Foo
- ethernet-1/1
- Subinterface 0
0
+ Subinterface 0
sdcio_model_common:routed
@@ -66,9 +66,9 @@ func TestToXMLTable(t *testing.T) {
bar
+ default
disable
Default NI
- default
sdcio_model_ni:default
hallo 00
@@ -108,12 +108,12 @@ func TestToXMLTable(t *testing.T) {
ethernet-1/1
+ ethernet-1/2
enable
Foo
- ethernet-1/2
- Subinterface 5
5
+ Subinterface 5
sdcio_model_common:routed
@@ -122,9 +122,9 @@ func TestToXMLTable(t *testing.T) {
default
+ other
enable
Other NI
- other
sdcio_model_ni:ip-vrf
hallo 99
@@ -190,11 +190,11 @@ func TestToXMLTable(t *testing.T) {
return testhelper.ExpandUpdateFromConfig(ctx, c, converter)
},
expected: `
-
ethernet-1/1
+
-
0
+
sdcio_model_common:bridged
@@ -227,9 +227,9 @@ func TestToXMLTable(t *testing.T) {
ethernet-1/1
+ ethernet-1/2
enable
Test
- ethernet-1/2
`,
@@ -524,9 +524,9 @@ func TestToXMLTable(t *testing.T) {
},
expected: `
+ owner-service1
prefix-2001:db8::/32
nexthop-2001:db8::1
- owner-service1
`,
@@ -621,10 +621,6 @@ func TestToXMLTable(t *testing.T) {
t.Fatal(err)
}
- // Make sure the attributes are sorted, otherwise the comparison is an issue
- //TODO: Follow order of schema
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
-
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -701,7 +697,6 @@ func TestToXML_KeyLeafDeleteUpgradesToListEntryDelete(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -777,16 +772,17 @@ func TestToXML_DoubleKeyLeafDeleteUpgradesToListEntryDelete(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
t.Fatal(err)
}
+ // YANG schema declares `key "key2 key1"`, so key elements must follow that
+ // schema order (key2 first), not alphabetical order.
want := `
- k1bar
k2foo
+ k1bar
`
if diff := cmp.Diff(want, xmlDocStr); diff != "" {
@@ -869,7 +865,6 @@ func TestToXML_DoubleKeyLeafDeleteOnlyDeletesTargetEntry(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -877,9 +872,11 @@ func TestToXML_DoubleKeyLeafDeleteOnlyDeletesTargetEntry(t *testing.T) {
}
// Only [k1bar][k2foo] should be deleted; [k1bar][k2other] and [k1other][k2foo] must not appear.
+ // YANG schema declares `key "key2 key1"`, so key elements must follow that
+ // schema order (key2 first), not alphabetical order.
want := `
- k1bar
k2foo
+ k1bar
`
if diff := cmp.Diff(want, xmlDocStr); diff != "" {
@@ -953,7 +950,6 @@ func TestToXML_DoubleKeyLeafDeleteViaKey1(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -961,9 +957,11 @@ func TestToXML_DoubleKeyLeafDeleteViaKey1(t *testing.T) {
}
// Only [k1bar][k2foo] deleted; [k1bar][k2other] must not appear.
+ // YANG schema declares `key "key2 key1"`, so key elements must follow that
+ // schema order (key2 first), not alphabetical order.
want := `
- k1bar
k2foo
+ k1bar
`
if diff := cmp.Diff(want, xmlDocStr); diff != "" {
@@ -1032,7 +1030,6 @@ func TestToXML_DoubleKeyNonKeyLeafDeleteDoesNotPromote(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -1041,9 +1038,10 @@ func TestToXML_DoubleKeyNonKeyLeafDeleteDoesNotPromote(t *testing.T) {
// The list entry itself must NOT carry operation="delete";
// only the mandato child should carry it.
+ // YANG schema declares `key "key2 key1"`, so key elements must be in schema order.
want := `
- k1bar
k2foo
+ k1bar
`
@@ -1129,7 +1127,6 @@ func TestToXML_DoubleKeyTwoSimultaneousListEntryDeletes(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- utils.XmlRecursiveSortElementsByTagName(&xmlDoc.Element)
xmlDoc.Indent(2)
xmlDocStr, err := xmlDoc.WriteToString()
if err != nil {
@@ -1137,13 +1134,14 @@ func TestToXML_DoubleKeyTwoSimultaneousListEntryDeletes(t *testing.T) {
}
// Both targeted entries deleted; [k1bar][k2other] must not appear.
+ // YANG schema declares `key "key2 key1"`, so key elements must be in schema order.
want := `
- k1bar
k2foo
+ k1bar
- k1other
k2foo
+ k1other
`
if diff := cmp.Diff(want, xmlDocStr); diff != "" {
diff --git a/pkg/tree/processors/processor_blame_config.go b/pkg/tree/processors/processor_blame_config.go
index 66853018..cc970fdd 100644
--- a/pkg/tree/processors/processor_blame_config.go
+++ b/pkg/tree/processors/processor_blame_config.go
@@ -79,7 +79,7 @@ func (t *BlameConfigTask) Run(ctx context.Context, submit func(pool.Task) error)
// set KeyName for list element key levels
if t.selfEntry.GetSchema() == nil {
ancestorSchema, levelsUp := ops.GetFirstAncestorWithSchema(t.selfEntry)
- keys := ops.GetSchemaKeys(ancestorSchema)
+ keys := ops.GetSchemaKeysAlphabeticalOrder(ancestorSchema)
if len(keys) > 0 && levelsUp >= len(keys) {
t.self.KeyName = keys[levelsUp-1]
}
diff --git a/pkg/tree/sharedEntryAttributes.go b/pkg/tree/sharedEntryAttributes.go
index 7142ee11..a1a0ab5f 100644
--- a/pkg/tree/sharedEntryAttributes.go
+++ b/pkg/tree/sharedEntryAttributes.go
@@ -649,13 +649,9 @@ func (s *sharedEntryAttributes) SdcpbPath() *sdcpb.Path {
// 3. Add the key-value pair to the parent's last path element
parentSchema, levelsUp := ops.GetFirstAncestorWithSchema(s)
- // Get the list of keys defined in the parent schema.
- schemaKeys := ops.GetSchemaKeys(parentSchema)
- // Sort keys to match the tree's insertion order (consistent with how keys are organized in levels)
- slices.Sort(schemaKeys)
- // Select the key name based on how many levels up the schema is.
- // If levelsUp=1, we're one level below the schema, so we use schemaKeys[0], etc.
- keyName := schemaKeys[levelsUp-1]
+ alphaKeys := ops.GetSchemaKeysAlphabeticalOrder(parentSchema)
+ // If levelsUp=1, we're one level below the schema, so we use alphaKeys[0], etc.
+ keyName := alphaKeys[levelsUp-1]
// Set this entry's name as the value for the selected key in the parent's last element
newElems[len(newElems)-1].Key[keyName] = s.pathElemName