Merge pull request #300 from sdcio/FixKeyDeletionIssue

Fixing the unintended deletion of key attributes

Author: hansthienpondt

pkg/tree/entry_test.go

@@ -340,7 +340,7 @@ func Test_Entry_Three(t *testing.T) {
 		highPri := LeafEntriesToUpdates(highPriLe)
 
 		// diff the result with the expected
-		if diff := testhelper.DiffUpdates([]*types.Update{n1, n2}, highPri); diff != "" {
+		if diff := testhelper.DiffUpdates([]*types.Update{n1, n2, u0, u1_1, u2_1}, highPri); diff != "" {
 			t.Errorf("root.GetByOwner() mismatch (-want +got):\n%s", diff)
 		}
 	})
@@ -374,11 +374,11 @@ func Test_Entry_Four(t *testing.T) {
 	u4 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "13", "description"}, desc3, prio50, owner1, ts1)
 	u4_1 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "13", "index"}, testhelper.GetUIntTvProto(13), prio50, owner1, ts1)
 
-	u1o2_0 := types.NewUpdate([]string{"interface", "ethernet-1/1", "name"}, testhelper.GetStringTvProto("ethernet-1/1"), prio55, owner2, ts1)
+	// u1o2_0 := types.NewUpdate([]string{"interface", "ethernet-1/1", "name"}, testhelper.GetStringTvProto("ethernet-1/1"), prio55, owner2, ts1)
 	u1o2 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "10", "description"}, desc3, prio55, owner2, ts1)
-	u1o2_1 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "10", "index"}, testhelper.GetUIntTvProto(10), prio55, owner2, ts1)
+	// u1o2_1 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "10", "index"}, testhelper.GetUIntTvProto(10), prio55, owner2, ts1)
 	u2o2 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "11", "description"}, desc3, prio55, owner2, ts1)
-	u2o2_1 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "11", "index"}, testhelper.GetUIntTvProto(11), prio55, owner2, ts1)
+	// u2o2_1 := types.NewUpdate([]string{"interface", "ethernet-1/1", "subinterface", "11", "index"}, testhelper.GetUIntTvProto(11), prio55, owner2, ts1)
 
 	ctx := context.TODO()
 
@@ -461,7 +461,7 @@ func Test_Entry_Four(t *testing.T) {
 		highPri := LeafEntriesToUpdates(highPriLe)
 
 		// diff the result with the expected
-		if diff := testhelper.DiffUpdates([]*types.Update{n1, n2}, highPri); diff != "" {
+		if diff := testhelper.DiffUpdates([]*types.Update{n1, n2, u1o1_0, u1o1_1, u2o1_1}, highPri); diff != "" {
 			t.Errorf("root.GetByOwner() mismatch (-want +got):\n%s", diff)
 		}
 	})
@@ -477,7 +477,7 @@ func Test_Entry_Four(t *testing.T) {
 	t.Run("Check the old entries are gone from highest (Only New Or Updated)", func(t *testing.T) {
 		highpri := root.GetHighestPrecedence(true)
 		// diff the result with the expected
-		if diff := testhelper.DiffUpdates([]*types.Update{u1o2_0, n1, n2, u2o2_1, u1o2_1}, highpri.ToUpdateSlice()); diff != "" {
+		if diff := testhelper.DiffUpdates([]*types.Update{u1o1_0, n1, n2, u2o1_1, u1o1_1}, highpri.ToUpdateSlice()); diff != "" {
 			t.Errorf("root.GetHighestPrecedence() mismatch (-want +got):\n%s", diff)
 		}
 	})

pkg/tree/leaf_entry.go

@@ -63,6 +63,10 @@ func (l *LeafEntry) MarkNew() {
 	l.IsNew = true
 }
 
+func (l *LeafEntry) RemoveDeleteFlag() {
+	l.Delete = false
+}
+
 func (l *LeafEntry) GetDeleteFlag() bool {
 	l.mu.RLock()
 	defer l.mu.RUnlock()

pkg/tree/sharedEntryAttributes.go

@@ -222,6 +222,7 @@ func (s *sharedEntryAttributes) checkAndCreateKeysAsLeafs(ctx context.Context, i
 				var result []*LeafEntry
 				lvs := child.GetByOwner(intentName, result)
 				if len(lvs) > 0 {
+					lvs[0].RemoveDeleteFlag()
 					continue
 				}
 			}
@@ -1608,7 +1609,7 @@ func (s *sharedEntryAttributes) TreeExport(owner string) ([]*tree_persist.TreeEl
 func (s *sharedEntryAttributes) BlameConfig(includeDefaults bool) (*sdcpb.BlameTreeElement, error) {
 	name := s.pathElemName
 	if s.GetLevel() == 0 {
-		name = fmt.Sprintf("root")
+		name = "root"
 	}
 	result := sdcpb.NewBlameTreeElement(name)
 

pkg/tree/sharedEntryAttributes_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/sdcio/data-server/pkg/config"
 	schemaClient "github.com/sdcio/data-server/pkg/datastore/clients/schema"
 	jsonImporter "github.com/sdcio/data-server/pkg/tree/importer/json"
+	"github.com/sdcio/data-server/pkg/tree/importer/proto"
 	"github.com/sdcio/data-server/pkg/tree/types"
 	"github.com/sdcio/data-server/pkg/utils/testhelper"
 	sdcio_schema "github.com/sdcio/data-server/tests/sdcioygot"
@@ -921,3 +922,116 @@ func Test_sharedEntryAttributes_BlameConfig(t *testing.T) {
 		})
 	}
 }
+
+func Test_sharedEntryAttributes_ReApply(t *testing.T) {
+	ctx := context.TODO()
+	owner1 := "owner1"
+	owner1Prio := int32(50)
+
+	tests := []struct {
+		name      string
+		r         func(t *testing.T) *sdcio_schema.Device
+		numDelete int
+	}{
+		{
+			name: "multiple keys",
+			r: func(t *testing.T) *sdcio_schema.Device {
+				conf1 := &sdcio_schema.Device{
+					Doublekey: map[sdcio_schema.SdcioModel_Doublekey_Key]*sdcio_schema.SdcioModel_Doublekey{
+						{
+							Key1: "k1.1",
+							Key2: "k1.2",
+						}: {
+							Key1: ygot.String("k1.1"),
+							Key2: ygot.String("k1.2"),
+							Cont: &sdcio_schema.SdcioModel_Doublekey_Cont{
+								Value1: ygot.String("containerval1.1"),
+								Value2: ygot.String("containerval1.2"),
+							},
+							Mandato: ygot.String("foo"),
+						},
+					},
+				}
+				return conf1
+			},
+			numDelete: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			defer mockCtrl.Finish()
+
+			scb, err := testhelper.GetSchemaClientBound(t, mockCtrl)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			tc := NewTreeContext(scb, owner1)
+			root, err := NewTreeRoot(ctx, tc)
+			if err != nil {
+				t.Fatal(err)
+			}
+			updSlice := types.UpdateSlice{
+				types.NewUpdate([]string{"doublekey", "k1.1", "k1.2", "mandato"}, testhelper.GetStringTvProto("TheMandatoryValue1"), owner1Prio, owner1, 0),
+			}
+
+			err = root.AddUpdatesRecursive(ctx, updSlice, flagsNew)
+			if err != nil {
+				t.Error(err)
+			}
+
+			fmt.Println(root.String())
+
+			treepersist, err := root.TreeExport(owner1, owner1Prio)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+
+			persistByte, err := protojson.Marshal(treepersist)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			fmt.Println("\nTreeExport:")
+			fmt.Println(string(persistByte))
+
+			tcNew := NewTreeContext(scb, owner1)
+			newRoot, err := NewTreeRoot(ctx, tcNew)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = newRoot.ImportConfig(ctx, types.PathSlice{}, proto.NewProtoTreeImporter(treepersist.Root), owner1, owner1Prio, flagsExisting)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+
+			// mark owner delete
+			newRoot.MarkOwnerDelete(owner1, false)
+
+			err = newRoot.AddUpdatesRecursive(ctx, updSlice, flagsNew)
+			if err != nil {
+				t.Error(err)
+			}
+
+			fmt.Println(newRoot.String())
+
+			err = newRoot.FinishInsertionPhase(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			deleteList, err := newRoot.GetDeletes(true)
+			if err != nil {
+				t.Error(err)
+			}
+			if len(deleteList) != tt.numDelete {
+				t.Errorf("%d deltes expected, got %d\n%s", tt.numDelete, len(deleteList), strings.Join(deleteList.PathSlices().StringSlice(), ", "))
+			}
+
+		})
+	}
+}