X wcfs/xbtree: root(a) must stay in trackSet even after treediff(a,ø)

wcfs/internal/xbtree/pptreesubset_test.go

@@ -50,6 +50,18 @@ func TestPPTreeSubSetOps(t *testing.T) {
 				S{},	// U
 				S{}),	// D
 
+		E(
+			S{a:{ø,0}},	// A
+			S{a:{ø,0}},	// B
+				S{a:{ø,0}},	// U
+				S{}),		// D
+
+		E(
+			S{a:{ø,0}},	// A
+			S{b:{ø,0}},	// B
+				S{a:{ø,0}, b:{ø,0}},	// U
+				S{a:{ø,0}}),		// D
+
 		E(
 			S{a:{ø,1}, b:{a,0}},	// A
 			S{a:{ø,1}, c:{a,0}},	// B

wcfs/internal/xbtree/treediff.go

@@ -464,28 +464,46 @@ func diffT(ctx context.Context, A, B *Tree, δZTC SetOid, trackSet PPTreeSubSet,
 	tracef("  diffT %s %s\n", xidOf(A), xidOf(B))
 	defer xerr.Contextf(&err, "diffT %s %s", xidOf(A), xidOf(B))
 
-	if A == nil { panic("A is nil") }	// XXX -> art. ø tree
-	Bempty := false
-	if B == nil {
-		// artificial empty tree
-		B = zodb.NewPersistent(reflect.TypeOf(Tree{}), /*jar*/nil).(*Tree)
-		Bempty = true
-	}
-
 	δ = map[Key]ΔValue{}
 	δtrack = &ΔPPTreeSubSet{Del: PPTreeSubSet{}, Add: PPTreeSubSet{}, δnchildNonLeafs: map[zodb.Oid]int{}}
 	defer tracef("  -> δ: %v\n", δ)
 
+	if A==nil && B==nil {
+		panic("TODO: verify it is covered by tests")
+		return δ, δtrack, nil // ø changes
+	}
+
+	// XXX A!=nil && B!=nil  ->  assert A.POid() == B.POid()
+
+	var ABoid zodb.Oid
+	if A != nil {
+		ABoid = A.POid()
+	}
+	if B != nil {
+		ABoid = B.POid()
+	}
+
 	// path prefix to A and B
-	prefix := []zodb.Oid{}
-	t := trackSet[A.POid()]
-	for t.parent != zodb.InvalidOid {
-		prefix = append([]zodb.Oid{t.parent}, prefix...)
-		t = trackSet[t.parent]
+	ABpath := trackSet.Path(ABoid)
+
+	if A == nil || B == nil {
+		// top of the subtree must stay in the tracking set even if the subtree is removed
+		// this way, if later, the subtree will be recreated, that change won't be missed
+		δtrack.Del.AddPath(ABpath)
+		δtrack.Add.AddPath(ABpath)
+	}
+
+	if A == nil { panic("A is nil") }	// TODO -> art. ø tree
+	Bempty := false
+	if B == nil {
+		// artificial empty tree
+		B = zodb.NewPersistent(reflect.TypeOf(Tree{}), /*jar*/nil).(*Tree)
+		Bempty = true
 	}
 
 	// initial split ranges for A and B
 	// XXX maybe walk till a from root to get more precise initial range?
+	prefix := ABpath[:len(ABpath)-1]
 	atop := &nodeInRange{prefix: prefix, lo: KeyMin, hi_: KeyMax, node: A} // [-∞, ∞)
 	btop := &nodeInRange{prefix: prefix, lo: KeyMin, hi_: KeyMax, node: B} // [-∞, ∞)
 	Av := rangeSplit{atop} // nodes expanded from A
@@ -510,10 +528,9 @@ func diffT(ctx context.Context, A, B *Tree, δZTC SetOid, trackSet PPTreeSubSet,
 	}
 
 	// {} oid -> parent for all nodes in Bv: current and previously expanded - up till top B
-	// XXX requires A.oid == B.oid
 	BtrackSet := PPTreeSubSet{}
 	if !Bempty {
-		BtrackSet.AddPath(trackSet.Path(B.POid()))
+		BtrackSet.AddPath(ABpath)
 	}
 
 	// phase 1: expand A top->down driven by δZTC.

wcfs/internal/xbtree/δbtail_test.go

@@ -342,25 +342,21 @@ func (rbs RBucketSet) trackSet(tracked SetKey) PPTreeSubSet {
 				ppoid = p.parent.oid
 			}
 
-			// skip ø (non-existing) tree
-			if p.oid != zodb.InvalidOid {
-				newParent := false
-				pt, already := trackSet[p.oid]
-				if !already {
-					pt = &nodeInTree{parent: ppoid, nchild: 0}
-					trackSet[p.oid] = pt
-					newParent = true
-				}
-				if pt.parent != ppoid {
-					panicf("BUG: %s: T%s -> multiple parents: %s %s", rbs.coverage(), p.oid, pt.parent, ppoid)
-				}
-
-				if newNode {
-					pt.nchild++
-				}
-				newNode = newParent
+			newParent := false
+			pt, already := trackSet[p.oid]
+			if !already {
+				pt = &nodeInTree{parent: ppoid, nchild: 0}
+				trackSet[p.oid] = pt
+				newParent = true
+			}
+			if pt.parent != ppoid {
+				panicf("BUG: %s: T%s -> multiple parents: %s %s", rbs.coverage(), p.oid, pt.parent, ppoid)
 			}
 
+			if newNode {
+				pt.nchild++
+			}
+			newNode = newParent
 			p = p.parent
 		}
 	}
@@ -715,13 +711,21 @@ func xverifyΔBTail_Update1(t *testing.T, subj string, db *zodb.DB, treeRoot zod
 		badf("δbtail.holeIdx1 wrong  ; holeIdx=%v  holeIdxOK=%v", holeIdx, holes1)
 	}
 
+	ø := PPTreeSubSet{}
+
 	// verify δbtail.trackSet against @at1
 	// trackSet1 = xkv1[tracked1]
 	trackSet1 := xkv1.trackSet(initialTrackedKeys)
 //	if !reflect.DeepEqual(trackSet1, δbtail.trackSet) {
 //		badf("δbtail.trackSet1 wrong:\n\thave: %v\n\twant: %v", δbtail.trackSet, trackSet1)
 //	}
-	δbtail.assertTrack(t, "1", /*ø*/PPTreeSubSet{}, trackSet1)
+	if !δbtail.trackSet.Equal(ø) {
+		badf("δbtail.trackSet1 wrong:\n\thave: %v\n\twant: %v", δbtail.trackSet, ø)
+	}
+	if !δbtail.trackNew.Equal(trackSet1) {
+		badf("δbtail.trackNew1 wrong:\n\thave: %v\n\twant: %v", δbtail.trackNew, trackSet1)
+	}
+//	δbtail.assertTrack(t, "1", /*ø*/PPTreeSubSet{}, trackSet1)
 
 
 	// δB <- δZ
@@ -744,7 +748,13 @@ func xverifyΔBTail_Update1(t *testing.T, subj string, db *zodb.DB, treeRoot zod
 //	if !reflect.DeepEqual(trackSet2, δbtail.trackSet) {
 //		badf("δbtail.trackSet2 wrong:\n\thave: %v\n\twant: %v", δbtail.trackSet, trackSet2)
 //	}
-	δbtail.assertTrack(t, "2", trackSet2, /*ø*/PPTreeSubSet{})
+	if !δbtail.trackSet.Equal(trackSet2) {
+		badf("δbtail.trackSet2 wrong:\n\thave: %v\n\twant: %v", δbtail.trackSet, trackSet2)
+	}
+	if !δbtail.trackNew.Equal(ø) {
+		badf("δbtail.trackNew2 wrong:\n\thave: %v\n\twant: %v", δbtail.trackNew, ø)
+	}
+//	δbtail.assertTrack(t, "2", trackSet2, /*ø*/PPTreeSubSet{})
 
 
 	// assert δB.ByRoot == {treeRoot -> ...}  if δTok != ø
@@ -1239,11 +1249,21 @@ func testΔBTail(t *testing.T, testq chan ΔBTestEntry) {
 		if tree != DEL {
 			xkv = XGetTree(db, δZ.Tid, tg.treeRoot)
 		} else {
+			// empty tree with real tg.treeRoot as oid even though the tree is
+			// deleted.  Having real oid in the root tests that after deletion,
+			// root of the tree stays in the tracking set. We need root to stay
+			// in trackSet because e.g. in
+			//
+			//	T1 -> ø -> T2
+			//
+			// where the tree is first deleted, then recreated, without root
+			// staying in trackSet after ->ø, treediff will notice nothing when
+			// it comes to ->T2.
 			xkv = RBucketSet{
 				&RBucket{
 					oid:    zodb.InvalidOid,
 					parent: &RTree{
-						oid:    zodb.InvalidOid,
+						oid:    tg.treeRoot, // NOTE oid is not InvalidOid
 						parent: nil,
 					},
 					lo:     KeyMin,