implement sort elements based on schema keys

Author: alexandernorth

pkg/datastore/target/nc.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/beevik/etree"
+	"github.com/sdcio/data-server/pkg/tree"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	log "github.com/sirupsen/logrus"
 
@@ -268,7 +269,7 @@ func (t *ncTarget) reconnect() {
 
 func (t *ncTarget) setRunning(source TargetSource) (*sdcpb.SetDataResponse, error) {
 
-	xtree, err := source.ToXML(true, t.sbiConfig.NetconfOptions.IncludeNS, t.sbiConfig.NetconfOptions.OperationWithNamespace, t.sbiConfig.NetconfOptions.UseOperationRemove, false)
+	xtree, err := source.ToXML(true, t.sbiConfig.NetconfOptions.IncludeNS, t.sbiConfig.NetconfOptions.OperationWithNamespace, t.sbiConfig.NetconfOptions.UseOperationRemove, tree.SchemaBound)
 	if err != nil {
 		return nil, err
 	}
@@ -328,7 +329,7 @@ func filterRPCErrors(xml *etree.Document, severity string) ([]string, error) {
 }
 
 func (t *ncTarget) setCandidate(source TargetSource) (*sdcpb.SetDataResponse, error) {
-	xtree, err := source.ToXML(true, t.sbiConfig.NetconfOptions.IncludeNS, t.sbiConfig.NetconfOptions.OperationWithNamespace, t.sbiConfig.NetconfOptions.UseOperationRemove, false)
+	xtree, err := source.ToXML(true, t.sbiConfig.NetconfOptions.IncludeNS, t.sbiConfig.NetconfOptions.OperationWithNamespace, t.sbiConfig.NetconfOptions.UseOperationRemove, tree.SchemaBound)
 	if err != nil {
 		return nil, err
 	}

pkg/datastore/target/target.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/beevik/etree"
+	"github.com/sdcio/data-server/pkg/tree"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	"google.golang.org/grpc"
 
@@ -74,7 +75,7 @@ type TargetSource interface {
 	// ToJsonIETF returns the Tree contained structure as JSON_IETF
 	// use e.g. json.MarshalIndent() on the returned struct
 	ToJsonIETF(onlyNewOrUpdated bool, ordered bool) (any, error)
-	ToXML(onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordered bool) (*etree.Document, error)
+	ToXML(onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordering tree.OrderingMethod) (*etree.Document, error)
 	ToProtoUpdates(ctx context.Context, onlyNewOrUpdated bool) ([]*sdcpb.Update, error)
 	ToProtoDeletes(ctx context.Context) ([]*sdcpb.Path, error)
 }

pkg/tree/entry.go

@@ -19,6 +19,14 @@ const (
 	RunningIntentName  = "running"
 )
 
+type OrderingMethod uint
+
+const (
+	None OrderingMethod = iota
+	Alphabetical
+	SchemaBound
+)
+
 type EntryImpl struct {
 	*sharedEntryAttributes
 }
@@ -120,8 +128,8 @@ type Entry interface {
 	// toJsonInternal the internal function that produces JSON and JSON_IETF
 	// Not for external usage
 	toJsonInternal(onlyNewOrUpdated bool, ietf bool, ordered bool) (j any, err error)
-	ToXML(onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordered bool) (*etree.Document, error)
-	toXmlInternal(parent *etree.Element, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordered bool) (doAdd bool, err error)
+	ToXML(onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordering OrderingMethod) (*etree.Document, error)
+	toXmlInternal(parent *etree.Element, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordering OrderingMethod) (doAdd bool, err error)
 	// ImportConfig allows importing config data received from e.g. the device in different formats (json, xml) to be imported into the tree.
 	ImportConfig(ctx context.Context, t importer.ImportConfigAdapter, intentName string, intentPrio int32) error
 }

pkg/tree/xml.go

@@ -1,6 +1,7 @@
 package tree
 
 import (
+	"cmp"
 	"fmt"
 	"slices"
 
@@ -13,16 +14,16 @@ import (
 // If honorNamespace is set, the xml elements will carry their respective namespace attributes.
 // If operationWithNamespace is set, the operation attributes added to the to be deleted alements will also carry the Netconf Base namespace.
 // If useOperationRemove is set, the remove operation will be used for deletes, instead of the delete operation.
-func (s *sharedEntryAttributes) ToXML(onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove bool, ordered bool) (*etree.Document, error) {
+func (s *sharedEntryAttributes) ToXML(onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove bool, ordering OrderingMethod) (*etree.Document, error) {
 	doc := etree.NewDocument()
-	_, err := s.toXmlInternal(&doc.Element, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordered)
+	_, err := s.toXmlInternal(&doc.Element, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordering)
 	if err != nil {
 		return nil, err
 	}
 	return doc, nil
 }
 
-func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordered bool) (doAdd bool, err error) {
+func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUpdated bool, honorNamespace bool, operationWithNamespace bool, useOperationRemove bool, ordering OrderingMethod) (doAdd bool, err error) {
 
 	switch s.schema.GetSchema().(type) {
 	case nil:
@@ -42,31 +43,45 @@ func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUp
 
 		childs := s.filterActiveChoiceCaseChilds()
 
-		if ordered {
-			keys := make([]string, 0, len(childs))
-			for k := range childs {
-				keys = append(keys, k)
-			}
-			slices.Sort(keys)
+		keys := make([]string, 0, len(childs))
+		for k := range childs {
+			keys = append(keys, k)
+		}
 
-			for _, k := range keys {
-				// recurse the call
-				// no additional element is created, since we're on a key level, so add to parent element
-				doAdd, err := childs[k].toXmlInternal(parent, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordered)
-				if err != nil {
-					return false, err
-				}
-				// only if there was something added in the childs, the element itself is meant to be added.
-				// we keep track of that via overAllDoAdd.
-				overallDoAdd = doAdd || overallDoAdd
+		switch ordering {
+		case Alphabetical:
+			slices.Sort(keys)
+		case None:
+		case SchemaBound:
+			schemaParent, _ := s.GetFirstAncestorWithSchema()
+			if schemaParent == nil {
+				return false, fmt.Errorf("no ancestor has schema for %v", s)
 			}
-			return overallDoAdd, nil
+			schemaKeys := schemaParent.GetSchemaKeys()
+			slices.SortFunc(keys, func(a, b string) int {
+				aIdx := slices.Index(schemaKeys, a)
+				bIdx := slices.Index(schemaKeys, b)
+				switch {
+				case aIdx == -1 && bIdx == -1:
+					// if neither are keys, sort them against each other
+					return cmp.Compare(a, b)
+				case aIdx == -1:
+					return 1
+				case bIdx == -1:
+					return -1
+				default:
+					return cmp.Compare(aIdx, bIdx)
+				}
+			})
+
+		default:
+			return false, fmt.Errorf("unknown ordering method: %v", ordering)
 		}
 
-		for _, c := range childs {
+		for _, k := range keys {
 			// recurse the call
 			// no additional element is created, since we're on a key level, so add to parent element
-			doAdd, err := c.toXmlInternal(parent, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordered)
+			doAdd, err := childs[k].toXmlInternal(parent, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordering)
 			if err != nil {
 				return false, err
 			}
@@ -87,8 +102,16 @@ func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUp
 				return false, err
 			}
 
-			if ordered {
+			switch ordering {
+			case Alphabetical:
+				slices.SortFunc(childs, func(a, b Entry) int {
+					return cmp.Compare(a.PathName(), b.PathName())
+				})
+			case SchemaBound:
 				slices.SortFunc(childs, getListEntrySortFunc(s))
+			case None:
+			default:
+				return false, fmt.Errorf("unknown ordering method: %v", ordering)
 			}
 
 			// go through the childs creating the xml elements
@@ -98,7 +121,7 @@ func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUp
 				// process the honorNamespace instruction
 				xmlAddNamespaceConditional(s, s.parent, newElem, honorNamespace)
 				// recurse the call
-				doAdd, err := child.toXmlInternal(newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordered)
+				doAdd, err := child.toXmlInternal(newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordering)
 				if err != nil {
 					return false, err
 				}
@@ -147,8 +170,21 @@ func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUp
 				keys = append(keys, k)
 			}
 
-			if ordered {
+			switch ordering {
+			case Alphabetical:
 				slices.Sort(keys)
+			case SchemaBound:
+				if s.parent == nil {
+					slices.Sort(keys)
+				} else {
+					cldrn := s.schema.GetContainer().GetChildren()
+					slices.SortFunc(keys, func(a, b string) int {
+						return cmp.Compare(slices.Index(cldrn, a), slices.Index(cldrn, b))
+					})
+				}
+			case None:
+			default:
+				return false, fmt.Errorf("unknown ordering method: %v", ordering)
 			}
 
 			// iterate through all the childs
@@ -165,7 +201,7 @@ func (s *sharedEntryAttributes) toXmlInternal(parent *etree.Element, onlyNewOrUp
 					newElem = parent
 				}
 				// recurse the call to all the children
-				doAdd, err := s.childs[k].toXmlInternal(newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordered)
+				doAdd, err := s.childs[k].toXmlInternal(newElem, onlyNewOrUpdated, honorNamespace, operationWithNamespace, useOperationRemove, ordering)
 				if err != nil {
 					return false, err
 				}