From 199ee3a680a0bedbaa6ba7f86956e07e5ed9081b Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <evgeniy@nspcc.ru>
Date: Mon, 6 Jun 2022 13:14:27 +0300
Subject: [PATCH] [#1481] pilorama: Fix `TreeApply`

Current implementation prevents invalid operations to become valid at
some later point (consider adding a child to the non-existent parent and
then adding the parent). This seems to diverge from the paper algorithm
and complicates implementation. Make it simpler.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
---
 pkg/local_object_storage/pilorama/boltdb.go      | 14 +-------------
 pkg/local_object_storage/pilorama/forest_test.go |  9 +++++----
 pkg/local_object_storage/pilorama/inmemory.go    | 14 +-------------
 3 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/pkg/local_object_storage/pilorama/boltdb.go b/pkg/local_object_storage/pilorama/boltdb.go
index 31aa36293..bb8b5c09d 100644
--- a/pkg/local_object_storage/pilorama/boltdb.go
+++ b/pkg/local_object_storage/pilorama/boltdb.go
@@ -270,9 +270,7 @@ func (t *boltForest) applyOperation(logBucket, treeBucket *bbolt.Bucket, lm *Log
 }
 
 func (t *boltForest) do(lb *bbolt.Bucket, b *bbolt.Bucket, key []byte, op *LogMove) error {
-	shouldPut := !t.isAncestor(b, key, op.Child, op.Parent) &&
-		!(op.Parent != 0 && op.Parent != TrashID && b.Get(timestampKey(key, op.Parent)) == nil)
-	shouldRemove := op.Parent == TrashID
+	shouldPut := !t.isAncestor(b, key, op.Child, op.Parent)
 
 	currParent := b.Get(parentKey(key, op.Child))
 	if currParent != nil { // node is already in tree
@@ -292,16 +290,6 @@ func (t *boltForest) do(lb *bbolt.Bucket, b *bbolt.Bucket, key []byte, op *LogMo
 		return nil
 	}
 
-	if shouldRemove {
-		if currParent != nil {
-			p := binary.LittleEndian.Uint64(currParent)
-			if err := b.Delete(childrenKey(key, op.Child, p)); err != nil {
-				return err
-			}
-		}
-		return t.removeNode(b, key, op.Child, op.Parent)
-	}
-
 	if currParent == nil {
 		if err := b.Put(timestampKey(key, op.Child), toUint64(op.Time)); err != nil {
 			return err
diff --git a/pkg/local_object_storage/pilorama/forest_test.go b/pkg/local_object_storage/pilorama/forest_test.go
index 909b16c10..5670a71c1 100644
--- a/pkg/local_object_storage/pilorama/forest_test.go
+++ b/pkg/local_object_storage/pilorama/forest_test.go
@@ -354,16 +354,17 @@ func testForestTreeApply(t *testing.T, constructor func(t testing.TB) Forest) {
 		testMeta(t, s, cid, treeID, 11, 10, meta)
 
 		testApply(t, s, 10, TrashID, Meta{Time: 2, Items: []KeyValue{{"parent", []byte{2}}}})
-		testMeta(t, s, cid, treeID, 11, 0, Meta{})
+		testMeta(t, s, cid, treeID, 11, 10, meta)
 	})
 	t.Run("add a child to non-existent parent, then add a parent", func(t *testing.T) {
 		s := constructor(t)
 
-		testApply(t, s, 11, 10, Meta{Time: 1, Items: []KeyValue{{"child", []byte{3}}}})
-		testMeta(t, s, cid, treeID, 11, 0, Meta{})
+		meta := Meta{Time: 1, Items: []KeyValue{{"child", []byte{3}}}}
+		testApply(t, s, 11, 10, meta)
+		testMeta(t, s, cid, treeID, 11, 10, meta)
 
 		testApply(t, s, 10, 0, Meta{Time: 2, Items: []KeyValue{{"grand", []byte{1}}}})
-		testMeta(t, s, cid, treeID, 11, 0, Meta{})
+		testMeta(t, s, cid, treeID, 11, 10, meta)
 	})
 }
 
diff --git a/pkg/local_object_storage/pilorama/inmemory.go b/pkg/local_object_storage/pilorama/inmemory.go
index c15de658b..b9a24158c 100644
--- a/pkg/local_object_storage/pilorama/inmemory.go
+++ b/pkg/local_object_storage/pilorama/inmemory.go
@@ -86,11 +86,7 @@ func (s *state) do(op *Move) LogMove {
 		},
 	}
 
-	_, parentInTree := s.tree.infoMap[op.Parent]
-	shouldPut := !s.tree.isAncestor(op.Child, op.Parent) &&
-		!(op.Parent != 0 && op.Parent != TrashID && !parentInTree)
-	shouldRemove := op.Parent == TrashID
-
+	shouldPut := !s.tree.isAncestor(op.Child, op.Parent)
 	p, ok := s.tree.infoMap[op.Child]
 	if ok {
 		lm.HasOld = true
@@ -101,14 +97,6 @@ func (s *state) do(op *Move) LogMove {
 		return lm
 	}
 
-	if shouldRemove {
-		if ok {
-			s.removeChild(op.Child, p.Parent)
-		}
-		delete(s.tree.infoMap, op.Child)
-		return lm
-	}
-
 	if !ok {
 		p.Timestamp = op.Time
 	} else {