From da0e4f148a01f4c4a5a9476053f5e8a16a16a178 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 27 Apr 2023 18:24:45 +0200 Subject: [PATCH] Fix empty data tree --- .../object/tree/objecttree/changebuilder.go | 24 ------- .../object/tree/objecttree/objecttree.go | 14 +++-- .../object/tree/objecttree/objecttree_test.go | 63 ++++++++++++++++++- .../tree/objecttree/objecttreefactory.go | 12 ++-- .../object/tree/objecttree/testutils.go | 6 +- .../object/tree/synctree/utils_test.go | 2 +- 6 files changed, 86 insertions(+), 35 deletions(-) diff --git a/commonspace/object/tree/objecttree/changebuilder.go b/commonspace/object/tree/objecttree/changebuilder.go index b3dc65a1..28b71ec9 100644 --- a/commonspace/object/tree/objecttree/changebuilder.go +++ b/commonspace/object/tree/objecttree/changebuilder.go @@ -52,18 +52,6 @@ func (c *nonVerifiableChangeBuilder) Marshall(ch *Change) (raw *treechangeproto. return c.ChangeBuilder.Marshall(ch) } -type emptyDataChangeBuilder struct { - ChangeBuilder -} - -func (c *emptyDataChangeBuilder) Build(payload BuilderContent) (ch *Change, raw *treechangeproto.RawTreeChangeWithId, err error) { - panic("should not be called") -} - -func (c *emptyDataChangeBuilder) Marshall(ch *Change) (raw *treechangeproto.RawTreeChangeWithId, err error) { - panic("should not be called") -} - type ChangeBuilder interface { Unmarshall(rawIdChange *treechangeproto.RawTreeChangeWithId, verify bool) (ch *Change, err error) Build(payload BuilderContent) (ch *Change, raw *treechangeproto.RawTreeChangeWithId, err error) @@ -79,18 +67,6 @@ type changeBuilder struct { newChange newChangeFunc } -func NewEmptyDataBuilder(keys crypto.KeyStorage, rootChange *treechangeproto.RawTreeChangeWithId) ChangeBuilder { - return &emptyDataChangeBuilder{&changeBuilder{ - rootChange: rootChange, - keys: keys, - newChange: func(id string, identity crypto.PubKey, ch *treechangeproto.TreeChange, signature []byte) *Change { - c := NewChange(id, identity, ch, nil) - c.Data = nil - return c - }, - }} -} - func NewChangeBuilder(keys crypto.KeyStorage, rootChange *treechangeproto.RawTreeChangeWithId) ChangeBuilder { return &changeBuilder{keys: keys, rootChange: rootChange, newChange: NewChange} } diff --git a/commonspace/object/tree/objecttree/objecttree.go b/commonspace/object/tree/objecttree/objecttree.go index 5f2d98e4..414ca7dc 100644 --- a/commonspace/object/tree/objecttree/objecttree.go +++ b/commonspace/object/tree/objecttree/objecttree.go @@ -96,10 +96,11 @@ type objectTree struct { treeBuilder *treeBuilder aclList list.AclList - id string - rawRoot *treechangeproto.RawTreeChangeWithId - root *Change - tree *Tree + removeDataOnAdd bool + id string + rawRoot *treechangeproto.RawTreeChangeWithId + root *Change + tree *Tree keys map[string]crypto.SymKey currentReadKey crypto.SymKey @@ -473,6 +474,11 @@ func (ot *objectTree) createAddResult(oldHeads []string, mode Mode, treeChangesA var added []*treechangeproto.RawTreeChangeWithId added, err = getAddedChanges(treeChangesAdded) + if ot.removeDataOnAdd { + for _, ch := range treeChangesAdded { + ch.Data = nil + } + } if err != nil { return } diff --git a/commonspace/object/tree/objecttree/objecttree_test.go b/commonspace/object/tree/objecttree/objecttree_test.go index 7ec9fac9..01fc8f7f 100644 --- a/commonspace/object/tree/objecttree/objecttree_test.go +++ b/commonspace/object/tree/objecttree/objecttree_test.go @@ -6,6 +6,7 @@ import ( "github.com/anytypeio/any-sync/commonspace/object/acl/list" "github.com/anytypeio/any-sync/commonspace/object/tree/treechangeproto" "github.com/anytypeio/any-sync/commonspace/object/tree/treestorage" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "testing" @@ -46,9 +47,17 @@ func prepareTreeDeps(aclList list.AclList) (*MockChangeCreator, objectTreeDeps) } func prepareTreeContext(t *testing.T, aclList list.AclList) testTreeContext { + return prepareContext(t, aclList, BuildTestableTree) +} + +func prepareEmptyDataTreeContext(t *testing.T, aclList list.AclList) testTreeContext { + return prepareContext(t, aclList, BuildEmptyDataTestableTree) +} + +func prepareContext(t *testing.T, aclList list.AclList, objTreeBuilder BuildObjectTreeFunc) testTreeContext { changeCreator := NewMockChangeCreator() treeStorage := changeCreator.CreateNewTreeStorage("0", aclList.Head().Id) - objTree, err := BuildTestableTree(aclList, treeStorage) + objTree, err := objTreeBuilder(treeStorage, aclList) require.NoError(t, err, "building tree should be without error") // check tree iterate @@ -266,6 +275,58 @@ func TestObjectTree(t *testing.T) { assert.Equal(t, true, objTree.(*objectTree).snapshotPathIsActual()) }) + t.Run("test empty data tree", func(t *testing.T) { + ctx := prepareEmptyDataTreeContext(t, aclList) + changeCreator := ctx.changeCreator + objTree := ctx.objTree + + rawChangesFirst := []*treechangeproto.RawTreeChangeWithId{ + changeCreator.CreateRawWithData("1", aclList.Head().Id, "0", false, []byte("1"), "0"), + changeCreator.CreateRawWithData("2", aclList.Head().Id, "0", false, []byte("2"), "1"), + changeCreator.CreateRawWithData("3", aclList.Head().Id, "0", false, []byte("3"), "2"), + } + rawChangesSecond := []*treechangeproto.RawTreeChangeWithId{ + changeCreator.CreateRawWithData("4", aclList.Head().Id, "0", false, []byte("4"), "2"), + changeCreator.CreateRawWithData("5", aclList.Head().Id, "0", false, []byte("5"), "1"), + changeCreator.CreateRawWithData("6", aclList.Head().Id, "0", false, []byte("6"), "3", "4", "5"), + } + + // making them to be saved in unattached + _, err := objTree.AddRawChanges(context.Background(), RawChangesPayload{ + NewHeads: []string{"6"}, + RawChanges: rawChangesSecond, + }) + require.NoError(t, err, "adding changes should be without error") + // attaching them + res, err := objTree.AddRawChanges(context.Background(), RawChangesPayload{ + NewHeads: []string{"3"}, + RawChanges: rawChangesFirst, + }) + + require.NoError(t, err, "adding changes should be without error") + require.Equal(t, "0", objTree.Root().Id) + require.Equal(t, []string{"6"}, objTree.Heads()) + require.Equal(t, 6, len(res.Added)) + + // checking that added changes still have data + for _, ch := range res.Added { + unmarshallRaw := &treechangeproto.RawTreeChange{} + proto.Unmarshal(ch.RawChange, unmarshallRaw) + treeCh := &treechangeproto.TreeChange{} + proto.Unmarshal(unmarshallRaw.Payload, treeCh) + require.Equal(t, ch.Id, string(treeCh.ChangesData)) + } + + // checking that the tree doesn't have data in memory + err = objTree.IterateRoot(nil, func(change *Change) bool { + if change.Id == "0" { + return true + } + require.Nil(t, change.Data) + return true + }) + }) + t.Run("changes from tree after common snapshot complex", func(t *testing.T) { ctx := prepareTreeContext(t, aclList) changeCreator := ctx.changeCreator diff --git a/commonspace/object/tree/objecttree/objecttreefactory.go b/commonspace/object/tree/objecttree/objecttreefactory.go index a913d183..f6089cdb 100644 --- a/commonspace/object/tree/objecttree/objecttreefactory.go +++ b/commonspace/object/tree/objecttree/objecttreefactory.go @@ -31,6 +31,7 @@ type objectTreeDeps struct { validator ObjectTreeValidator rawChangeLoader *rawChangeLoader aclList list.AclList + removeDataOnAdd bool } type BuildObjectTreeFunc = func(treeStorage treestorage.TreeStorage, aclList list.AclList) (ObjectTree, error) @@ -57,7 +58,7 @@ func emptyDataTreeDeps( rootChange *treechangeproto.RawTreeChangeWithId, treeStorage treestorage.TreeStorage, aclList list.AclList) objectTreeDeps { - changeBuilder := NewEmptyDataBuilder(crypto.NewKeyStorage(), rootChange) + changeBuilder := NewChangeBuilder(crypto.NewKeyStorage(), rootChange) treeBuilder := newTreeBuilder(treeStorage, changeBuilder) return objectTreeDeps{ changeBuilder: changeBuilder, @@ -66,6 +67,7 @@ func emptyDataTreeDeps( validator: newTreeValidator(), rawChangeLoader: newStorageLoader(treeStorage, changeBuilder), aclList: aclList, + removeDataOnAdd: true, } } @@ -107,7 +109,7 @@ func BuildEmptyDataObjectTree(treeStorage treestorage.TreeStorage, aclList list. return buildObjectTree(deps) } -func BuildTestableTree(aclList list.AclList, treeStorage treestorage.TreeStorage) (ObjectTree, error) { +func BuildTestableTree(treeStorage treestorage.TreeStorage, aclList list.AclList) (ObjectTree, error) { root, _ := treeStorage.Root() changeBuilder := &nonVerifiableChangeBuilder{ ChangeBuilder: NewChangeBuilder(newMockKeyStorage(), root), @@ -124,10 +126,10 @@ func BuildTestableTree(aclList list.AclList, treeStorage treestorage.TreeStorage return buildObjectTree(deps) } -func BuildEmptyDataTestableTree(aclList list.AclList, treeStorage treestorage.TreeStorage) (ObjectTree, error) { +func BuildEmptyDataTestableTree(treeStorage treestorage.TreeStorage, aclList list.AclList) (ObjectTree, error) { root, _ := treeStorage.Root() changeBuilder := &nonVerifiableChangeBuilder{ - ChangeBuilder: NewEmptyDataBuilder(newMockKeyStorage(), root), + ChangeBuilder: NewChangeBuilder(newMockKeyStorage(), root), } deps := objectTreeDeps{ changeBuilder: changeBuilder, @@ -136,6 +138,7 @@ func BuildEmptyDataTestableTree(aclList list.AclList, treeStorage treestorage.Tr rawChangeLoader: newStorageLoader(treeStorage, changeBuilder), validator: &noOpTreeValidator{}, aclList: aclList, + removeDataOnAdd: true, } return buildObjectTree(deps) @@ -251,6 +254,7 @@ func buildObjectTree(deps objectTreeDeps) (ObjectTree, error) { difSnapshotBuf: make([]*treechangeproto.RawTreeChangeWithId, 0, 10), notSeenIdxBuf: make([]int, 0, 10), newSnapshotsBuf: make([]*Change, 0, 10), + removeDataOnAdd: deps.removeDataOnAdd, } err := objTree.rebuildFromStorage(nil, nil) diff --git a/commonspace/object/tree/objecttree/testutils.go b/commonspace/object/tree/objecttree/testutils.go index d18eb859..1557ebe6 100644 --- a/commonspace/object/tree/objecttree/testutils.go +++ b/commonspace/object/tree/objecttree/testutils.go @@ -89,11 +89,15 @@ func (c *MockChangeCreator) CreateRoot(id, aclId string) *treechangeproto.RawTre } func (c *MockChangeCreator) CreateRaw(id, aclId, snapshotId string, isSnapshot bool, prevIds ...string) *treechangeproto.RawTreeChangeWithId { + return c.CreateRawWithData(id, aclId, snapshotId, isSnapshot, nil, prevIds...) +} + +func (c *MockChangeCreator) CreateRawWithData(id, aclId, snapshotId string, isSnapshot bool, data []byte, prevIds ...string) *treechangeproto.RawTreeChangeWithId { aclChange := &treechangeproto.TreeChange{ TreeHeadIds: prevIds, AclHeadId: aclId, SnapshotBaseId: snapshotId, - ChangesData: nil, + ChangesData: data, IsSnapshot: isSnapshot, } res, _ := aclChange.Marshal() diff --git a/commonspace/object/tree/synctree/utils_test.go b/commonspace/object/tree/synctree/utils_test.go index d0b207b4..92b7751c 100644 --- a/commonspace/object/tree/synctree/utils_test.go +++ b/commonspace/object/tree/synctree/utils_test.go @@ -296,7 +296,7 @@ func createStorage(treeId string, aclList list.AclList) treestorage.TreeStorage } func createTestTree(aclList list.AclList, storage treestorage.TreeStorage) (objecttree.ObjectTree, error) { - return objecttree.BuildEmptyDataTestableTree(aclList, storage) + return objecttree.BuildEmptyDataTestableTree(storage, aclList) } type fixtureDeps struct {