Split TreeContext into TreeConfig and OperationState

pkg/datastore/deviations.go

@@ -201,7 +201,7 @@ func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes.
 
 	if log := log.V(logger.VTrace); log.Enabled() {
 		log.Info("deviation tree", "content", deviationTree.String())
-		log.Info("nonrevertive infos", "data", deviationTree.GetTreeContext().NonRevertiveInfo().String())
+		log.Info("nonrevertive infos", "data", deviationTree.GetTreeContext().GetOperationState().NonRevertiveInfo().String())
 	}
 
 	deviationChan := make(chan *treetypes.DeviationEntry, 10)

pkg/datastore/target/gnmi/stream.go

@@ -157,7 +157,7 @@ func (s *StreamSync) buildTreeSyncWithDatastore(cUS <-chan *NotificationData, sy
 			if err != nil {
 				log.Error(err, "failed adding update to synctree")
 			}
-			syncTree.GetTreeContext().ExplicitDeletes().Add(consts.RunningIntentName, consts.RunningValuesPrio, noti.deletes)
+			syncTree.GetTreeContext().GetOperationState().ExplicitDeletes().Add(consts.RunningIntentName, consts.RunningValuesPrio, noti.deletes)
 		case <-syncResponse:
 			syncTree, err = s.syncToRunning(syncTree, syncTreeMutex, true)
 			tickerActive = true

pkg/datastore/transaction_rpc.go

@@ -241,7 +241,7 @@ func (d *Datastore) lowlevelTransactionSet(ctx context.Context, transaction *typ
 		}
 
 		// clear the owners existing explicit delete entries, retrieving the old entries for storing in the transaction for possible rollback
-		oldExplicitDeletes := root.GetTreeContext().ExplicitDeletes().Remove(intent.GetName())
+		oldExplicitDeletes := root.GetTreeContext().GetOperationState().ExplicitDeletes().Remove(intent.GetName())
 
 		priority := int32(math.MaxInt32)
 		if len(oldIntentContent) > 0 {
@@ -268,14 +268,14 @@ func (d *Datastore) lowlevelTransactionSet(ctx context.Context, transaction *typ
 			}
 
 			// add the explicit delete entries
-			treeContext.ExplicitDeletes().Add(intent.GetName(), intent.GetPriority(), intent.GetDeletes())
+			treeContext.GetOperationState().ExplicitDeletes().Add(intent.GetName(), intent.GetPriority(), intent.GetDeletes())
 		}
 
 		// add non-revertive info to tree context
-		treeContext.NonRevertiveInfo().Add(intent.GetName(), intent.NonRevertive(), intent.GetRevertPaths()...)
+		treeContext.GetOperationState().NonRevertiveInfo().Add(intent.GetName(), intent.NonRevertive(), intent.GetRevertPaths()...)
 	}
 
-	log.V(logger.VDebug).Info("nonrevertive infos", "data", treeContext.NonRevertiveInfo().String())
+	log.V(logger.VDebug).Info("nonrevertive infos", "data", treeContext.GetOperationState().NonRevertiveInfo().String())
 
 	les := ops.LeafsOfOwner(root.Entry, consts.RunningIntentName)
 

pkg/tree/api/leaf_entry.go

@@ -139,7 +139,7 @@ func (l *LeafEntry) NonRevertive() bool {
 	if l.parentEntry == nil {
 		return false
 	}
-	return l.parentEntry.GetTreeContext().NonRevertiveInfo().IsNonRevertive(l.Owner(), l)
+	return l.parentEntry.GetTreeContext().GetOperationState().NonRevertiveInfo().IsNonRevertive(l.Owner(), l)
 }
 
 // String returns a string representation of the LeafEntry

pkg/tree/api/leaf_variants.go

@@ -14,14 +14,14 @@ import (
 type LeafVariants struct {
 	les         LeafVariantSlice
 	lesMutex    sync.RWMutex
-	tc          TreeContext
+	os          OperationState
 	parentEntry Entry
 }
 
-func NewLeafVariants(tc TreeContext, parentEnty Entry) *LeafVariants {
+func NewLeafVariants(os OperationState, parentEnty Entry) *LeafVariants {
 	return &LeafVariants{
 		les:         make(LeafVariantSlice, 0, 2),
-		tc:          tc,
+		os:          os,
 		parentEntry: parentEnty,
 	}
 }
@@ -259,10 +259,10 @@ func (lv *LeafVariants) GetHighestPrecedenceValue(filter HighestPrecedenceFilter
 	return result
 }
 
-func (lv *LeafVariants) DeepCopy(tc TreeContext, parent Entry) *LeafVariants {
+func (lv *LeafVariants) DeepCopy(os OperationState, parent Entry) *LeafVariants {
 	result := &LeafVariants{
 		lesMutex:    sync.RWMutex{},
-		tc:          tc,
+		os:          os,
 		les:         make([]*LeafEntry, 0, len(lv.les)),
 		parentEntry: parent,
 	}
@@ -382,7 +382,7 @@ func (lv *LeafVariants) highestIsUnequalRunning(highest *LeafEntry) bool {
 	}
 
 	// if highest is not new or updated and highest is non-revertive
-	if !highest.IsNew && !highest.IsUpdated && lv.tc.NonRevertiveInfo().IsNonRevertive(highest.Update.Owner(), lv.parentEntry) {
+	if !highest.IsNew && !highest.IsUpdated && lv.os.NonRevertiveInfo().IsNonRevertive(highest.Update.Owner(), lv.parentEntry) {
 		return false
 	}
 

pkg/tree/api/leaf_variants_test.go

@@ -474,3 +474,53 @@ func TestLeafVariants_canDelete(t *testing.T) {
 		})
 	}
 }
+
+// TestNewLeafVariants_AcceptsOperationState verifies that NewLeafVariants accepts
+// an OperationState (not a full TreeContext) and returns a non-nil LeafVariants.
+func TestNewLeafVariants_AcceptsOperationState(t *testing.T) {
+	os := NewOperationState()
+	lv := NewLeafVariants(os, nil)
+	if lv == nil {
+		t.Fatal("expected NewLeafVariants to return non-nil LeafVariants")
+	}
+}
+
+// TestLeafVariants_NonRevertiveBehaviorPreservedAfterDeepCopy verifies that
+// when an owner is marked non-revertive in OperationState, GetHighestPrecedence
+// does not report the entry as changed — and that DeepCopy preserves this
+// behaviour with the copied state.
+func TestLeafVariants_NonRevertiveBehaviorPreservedAfterDeepCopy(t *testing.T) {
+	os := NewOperationState()
+	os.NonRevertiveInfo().Add("owner1", true)
+
+	// parentEntry nil is safe: NonRevertiveInfo.IsNonRevertive does not
+	// dereference the path when no revert-paths are configured.
+	lv := NewLeafVariants(os, nil)
+
+	// running entry: lower priority (higher number), value "v1"
+	leRun := NewLeafEntry(
+		types.NewUpdate(&mockUpdateParent{}, &sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: "v1"}}, RunningValuesPrio, RunningIntentName, 0),
+		types.NewUpdateInsertFlags(),
+		nil,
+	)
+	// owner1 entry: higher priority (lower number), different value "v2", not new/updated
+	leOwner := NewLeafEntry(
+		types.NewUpdate(&mockUpdateParent{}, &sdcpb.TypedValue{Value: &sdcpb.TypedValue_StringVal{StringVal: "v2"}}, 10, "owner1", 0),
+		types.NewUpdateInsertFlags(),
+		nil,
+	)
+	lv.Add(leRun)
+	lv.Add(leOwner)
+
+	// Non-revertive: even though owner1 has a different value, it is not "new or updated"
+	// so GetHighestPrecedence(onlyNewOrUpdated=true) should return nil.
+	if got := lv.GetHighestPrecedence(true, false, false); got != nil {
+		t.Errorf("expected nil from GetHighestPrecedence with non-revertive owner, got entry owned by %q", got.Owner())
+	}
+
+	// DeepCopy must carry the copied OperationState so the same behaviour holds.
+	lvCopy := lv.DeepCopy(os.DeepCopyState(), nil)
+	if got := lvCopy.GetHighestPrecedence(true, false, false); got != nil {
+		t.Errorf("after DeepCopy: expected nil from GetHighestPrecedence with non-revertive owner, got entry owned by %q", got.Owner())
+	}
+}

pkg/tree/api/operation_state.go

@@ -0,0 +1,39 @@
+package api
+
+// OperationState holds mutable state accumulated during a tree operation
+// (e.g. an import pass or transaction). It is created once per tree root and
+// deep-copied when the tree is branched.
+type OperationState interface {
+	ExplicitDeletes() *DeletePathSet
+	NonRevertiveInfo() NonRevertiveInfos
+	DeepCopyState() OperationState
+}
+
+// operationState is the concrete, package-private implementation.
+type operationState struct {
+	explicitDeletes  *DeletePathSet
+	nonRevertiveInfo NonRevertiveInfos
+}
+
+// NewOperationState creates a fresh OperationState with empty collections.
+func NewOperationState() OperationState {
+	return &operationState{
+		explicitDeletes:  NewDeletePaths(),
+		nonRevertiveInfo: NewNonRevertiveInfos(),
+	}
+}
+
+func (o *operationState) ExplicitDeletes() *DeletePathSet {
+	return o.explicitDeletes
+}
+
+func (o *operationState) NonRevertiveInfo() NonRevertiveInfos {
+	return o.nonRevertiveInfo
+}
+
+func (o *operationState) DeepCopyState() OperationState {
+	return &operationState{
+		explicitDeletes:  o.explicitDeletes.DeepCopy(),
+		nonRevertiveInfo: o.nonRevertiveInfo.DeepCopy(),
+	}
+}

pkg/tree/api/tree_config.go

@@ -0,0 +1,13 @@
+package api
+
+import (
+	schemaClient "github.com/sdcio/data-server/pkg/datastore/clients/schema"
+	"github.com/sdcio/data-server/pkg/pool"
+)
+
+// TreeConfig holds immutable tree-setup values. It is created once per tree
+// root and shared (by identity) across all DeepCopy calls.
+type TreeConfig interface {
+	SchemaClient() schemaClient.SchemaClientBound
+	PoolFactory() pool.VirtualPoolFactory
+}

pkg/tree/api/tree_context.go

@@ -1,14 +1,7 @@
 package api
 
-import (
-	schemaClient "github.com/sdcio/data-server/pkg/datastore/clients/schema"
-	"github.com/sdcio/data-server/pkg/pool"
-)
-
 type TreeContext interface {
-	PoolFactory() pool.VirtualPoolFactory
-	SchemaClient() schemaClient.SchemaClientBound
+	GetTreeConfig() TreeConfig
+	GetOperationState() OperationState
 	DeepCopy() TreeContext
-	ExplicitDeletes() *DeletePathSet
-	NonRevertiveInfo() NonRevertiveInfos
 }

pkg/tree/entry_test.go

@@ -1294,7 +1294,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// because thats an update.
 	t.Run("Delete Non New",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 
 			le1 := api.NewLeafEntry(types.NewUpdate(nil, nil, 2, owner1, ts), testhelper.FlagsExisting, nil)
 			lv.Add(le1)
@@ -1314,7 +1314,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// should return nil
 	t.Run("Single entry thats also marked for deletion",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, 1, owner1, ts), testhelper.FlagsDelete, nil))
 			le := lv.GetHighestPrecedence(true, false, false)
 
@@ -1328,7 +1328,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// on onlyIfPrioChanged == true we do not expect output.
 	t.Run("New Low Prio IsUpdate OnlyChanged True",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, 5, owner1, ts), testhelper.FlagsExisting, nil))
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, 6, owner2, ts), testhelper.FlagsNew, nil))
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, RunningValuesPrio, RunningIntentName, ts), testhelper.FlagsExisting, nil))
@@ -1345,7 +1345,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// on onlyIfPrioChanged == false we do not expect the highes prio update to be returned.
 	t.Run("New Low Prio IsUpdate OnlyChanged False",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			le1 := api.NewLeafEntry(types.NewUpdate(nil, nil, 5, owner1, ts), testhelper.FlagsExisting, nil)
 			lv.Add(le1)
 			le2 := api.NewLeafEntry(types.NewUpdate(nil, nil, 6, owner2, ts), testhelper.FlagsNew, nil)
@@ -1363,7 +1363,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// // on onlyIfPrioChanged == true we do not expect output.
 	t.Run("New Low Prio IsNew OnlyChanged == True",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, 5, owner1, ts), testhelper.FlagsExisting, nil))
 			lv.Add(api.NewLeafEntry(types.NewUpdate(nil, nil, 6, owner2, ts), testhelper.FlagsNew, nil))
@@ -1381,7 +1381,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// // on onlyIfPrioChanged == true we do not expect output.
 	t.Run("New Low Prio IsNew OnlyChanged == True, with running not existing",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			le1 := api.NewLeafEntry(types.NewUpdate(nil, nil, 5, owner1, ts), testhelper.FlagsExisting, nil)
 			lv.Add(le1)
 			le2 := api.NewLeafEntry(types.NewUpdate(nil, nil, 6, owner2, ts), testhelper.FlagsNew, nil)
@@ -1397,7 +1397,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 
 	t.Run("New Low Prio IsNew OnlyChanged == False",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			le1 := api.NewLeafEntry(types.NewUpdate(nil, nil, 5, owner1, ts), testhelper.FlagsExisting, nil)
 			lv.Add(le1)
 			le2 := api.NewLeafEntry(types.NewUpdate(nil, nil, 6, owner2, ts), testhelper.FlagsNew, nil)
@@ -1414,7 +1414,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// If no entries exist in the list nil should be returned.
 	t.Run("No Entries",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 
 			le := lv.GetHighestPrecedence(true, false, false)
 
@@ -1427,7 +1427,7 @@ func TestLeafVariants_GetHighesPrio(t *testing.T) {
 	// make sure the secondhighes is also populated if the highes was the first entry
 	t.Run("secondhighes populated if highes was first",
 		func(t *testing.T) {
-			lv := api.NewLeafVariants(&TreeContext{}, nil)
+			lv := api.NewLeafVariants(api.NewOperationState(), nil)
 			le1 := api.NewLeafEntry(types.NewUpdate(nil, nil, 1, owner1, ts), testhelper.FlagsDelete, nil)
 			lv.Add(le1)
 			le2 := api.NewLeafEntry(types.NewUpdate(nil, nil, 2, owner2, ts), testhelper.FlagsExisting, nil)
@@ -2224,21 +2224,21 @@ func Test_RevertNonRevertive(t *testing.T) {
 				t.Fatalf("failed to get existing config: %v", err)
 			}
 
-			_, err = root.ImportConfig(ctx, nil, runningConfig, types.NewUpdateInsertFlags(), tc.poolFactory)
+			_, err = root.ImportConfig(ctx, nil, runningConfig, types.NewUpdateInsertFlags(), tc.GetTreeConfig().PoolFactory())
 			if err != nil {
 				t.Fatalf("failed to import running config: %v", err)
 			}
 
 			for _, existing := range existingConfig {
-				_, err = root.ImportConfig(ctx, nil, existing, types.NewUpdateInsertFlags(), tc.poolFactory)
+				_, err = root.ImportConfig(ctx, nil, existing, types.NewUpdateInsertFlags(), tc.GetTreeConfig().PoolFactory())
 				if err != nil {
 					t.Fatalf("failed to import existing config: %v", err)
 				}
 			}
 
 			// adding paths to the non revertive info, this should mark the paths as non revertive, and thus not be deleted in the end.
 			for _, path := range tt.revertPaths {
-				tc.NonRevertiveInfo().Add(owner1, false, path)
+				tc.GetOperationState().NonRevertiveInfo().Add(owner1, false, path)
 			}
 
 			err = root.FinishInsertionPhase(ctx)