From 75a44c42814a558fb6c9cab96fd3655f9b35ecfc Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Wed, 26 Apr 2023 18:12:03 +0200 Subject: [PATCH 1/4] Remove error in broadcast --- commonspace/object/tree/synctree/synctree.go | 8 +++----- commonspace/object/tree/synctree/synctree_test.go | 6 +++--- .../objectsync/mock_objectsync/mock_objectsync.go | 6 ++---- commonspace/objectsync/syncclient.go | 10 +++++++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/commonspace/object/tree/synctree/synctree.go b/commonspace/object/tree/synctree/synctree.go index dc87a3c4..203d3cac 100644 --- a/commonspace/object/tree/synctree/synctree.go +++ b/commonspace/object/tree/synctree/synctree.go @@ -116,9 +116,7 @@ func buildSyncTree(ctx context.Context, isFirstBuild bool, deps BuildDeps) (t Sy if isFirstBuild { headUpdate := syncTree.syncClient.CreateHeadUpdate(t, nil) // send to everybody, because everybody should know that the node or client got new tree - if e := syncTree.syncClient.Broadcast(ctx, headUpdate); e != nil { - log.ErrorCtx(ctx, "broadcast error", zap.Error(e)) - } + syncTree.syncClient.Broadcast(ctx, headUpdate) } return } @@ -155,7 +153,7 @@ func (s *syncTree) AddContent(ctx context.Context, content objecttree.SignableCh } s.syncStatus.HeadsChange(s.Id(), res.Heads) headUpdate := s.syncClient.CreateHeadUpdate(s, res.Added) - err = s.syncClient.Broadcast(ctx, headUpdate) + s.syncClient.Broadcast(ctx, headUpdate) return } @@ -182,7 +180,7 @@ func (s *syncTree) AddRawChanges(ctx context.Context, changesPayload objecttree. s.notifiable.UpdateHeads(s.Id(), res.Heads) } headUpdate := s.syncClient.CreateHeadUpdate(s, res.Added) - err = s.syncClient.Broadcast(ctx, headUpdate) + s.syncClient.Broadcast(ctx, headUpdate) } return } diff --git a/commonspace/object/tree/synctree/synctree_test.go b/commonspace/object/tree/synctree/synctree_test.go index 9a9255ec..8b3b4865 100644 --- a/commonspace/object/tree/synctree/synctree_test.go +++ b/commonspace/object/tree/synctree/synctree_test.go @@ -73,7 +73,7 @@ func Test_BuildSyncTree(t *testing.T) { updateListenerMock.EXPECT().Update(tr) syncClientMock.EXPECT().CreateHeadUpdate(gomock.Eq(tr), gomock.Eq(changes)).Return(headUpdate) - syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)).Return(nil) + syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)) res, err := tr.AddRawChanges(ctx, payload) require.NoError(t, err) require.Equal(t, expectedRes, res) @@ -95,7 +95,7 @@ func Test_BuildSyncTree(t *testing.T) { updateListenerMock.EXPECT().Rebuild(tr) syncClientMock.EXPECT().CreateHeadUpdate(gomock.Eq(tr), gomock.Eq(changes)).Return(headUpdate) - syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)).Return(nil) + syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)) res, err := tr.AddRawChanges(ctx, payload) require.NoError(t, err) require.Equal(t, expectedRes, res) @@ -133,7 +133,7 @@ func Test_BuildSyncTree(t *testing.T) { Return(expectedRes, nil) syncClientMock.EXPECT().CreateHeadUpdate(gomock.Eq(tr), gomock.Eq(changes)).Return(headUpdate) - syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)).Return(nil) + syncClientMock.EXPECT().Broadcast(gomock.Any(), gomock.Eq(headUpdate)) res, err := tr.AddContent(ctx, content) require.NoError(t, err) require.Equal(t, expectedRes, res) diff --git a/commonspace/objectsync/mock_objectsync/mock_objectsync.go b/commonspace/objectsync/mock_objectsync/mock_objectsync.go index 29ea2717..cd9724ef 100644 --- a/commonspace/objectsync/mock_objectsync/mock_objectsync.go +++ b/commonspace/objectsync/mock_objectsync/mock_objectsync.go @@ -39,11 +39,9 @@ func (m *MockSyncClient) EXPECT() *MockSyncClientMockRecorder { } // Broadcast mocks base method. -func (m *MockSyncClient) Broadcast(arg0 context.Context, arg1 *treechangeproto.TreeSyncMessage) error { +func (m *MockSyncClient) Broadcast(arg0 context.Context, arg1 *treechangeproto.TreeSyncMessage) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Broadcast", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + m.ctrl.Call(m, "Broadcast", arg0, arg1) } // Broadcast indicates an expected call of Broadcast. diff --git a/commonspace/objectsync/syncclient.go b/commonspace/objectsync/syncclient.go index e6fd515e..5ba1ab0c 100644 --- a/commonspace/objectsync/syncclient.go +++ b/commonspace/objectsync/syncclient.go @@ -4,11 +4,12 @@ import ( "context" "github.com/anytypeio/any-sync/commonspace/object/tree/treechangeproto" "github.com/anytypeio/any-sync/commonspace/spacesyncproto" + "go.uber.org/zap" ) type SyncClient interface { RequestFactory - Broadcast(ctx context.Context, msg *treechangeproto.TreeSyncMessage) (err error) + Broadcast(ctx context.Context, msg *treechangeproto.TreeSyncMessage) SendWithReply(ctx context.Context, peerId, objectId string, msg *treechangeproto.TreeSyncMessage, replyId string) (err error) SendSync(ctx context.Context, peerId, objectId string, msg *treechangeproto.TreeSyncMessage) (reply *spacesyncproto.ObjectSyncMessage, err error) MessagePool() MessagePool @@ -31,12 +32,15 @@ func NewSyncClient( } } -func (s *syncClient) Broadcast(ctx context.Context, msg *treechangeproto.TreeSyncMessage) (err error) { +func (s *syncClient) Broadcast(ctx context.Context, msg *treechangeproto.TreeSyncMessage) { objMsg, err := MarshallTreeMessage(msg, s.spaceId, msg.RootChange.Id, "") if err != nil { return } - return s.messagePool.Broadcast(ctx, objMsg) + err = s.messagePool.Broadcast(ctx, objMsg) + if err != nil { + log.DebugCtx(ctx, "broadcast error", zap.Error(err)) + } } func (s *syncClient) SendSync(ctx context.Context, peerId, objectId string, msg *treechangeproto.TreeSyncMessage) (reply *spacesyncproto.ObjectSyncMessage, err error) { From da0e4f148a01f4c4a5a9476053f5e8a16a16a178 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 27 Apr 2023 18:24:45 +0200 Subject: [PATCH 2/4] 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 { From 6676480e1d419aba7d4939b051be2fa463026961 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 27 Apr 2023 18:43:04 +0200 Subject: [PATCH 3/4] Add sync protocol tests for empty data tree --- .../object/tree/synctree/syncprotocol_test.go | 42 +++++++++++++++---- .../object/tree/synctree/utils_test.go | 18 +++++--- .../object/tree/treestorage/inmemory.go | 20 ++++----- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/commonspace/object/tree/synctree/syncprotocol_test.go b/commonspace/object/tree/synctree/syncprotocol_test.go index d7c7a935..6cac042a 100644 --- a/commonspace/object/tree/synctree/syncprotocol_test.go +++ b/commonspace/object/tree/synctree/syncprotocol_test.go @@ -8,6 +8,7 @@ import ( "github.com/anytypeio/any-sync/commonspace/object/tree/treechangeproto" "github.com/anytypeio/any-sync/commonspace/object/tree/treestorage" "github.com/anytypeio/any-sync/util/slice" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/require" "math/rand" "testing" @@ -30,7 +31,8 @@ func TestEmptyClientGetsFullHistory(t *testing.T) { "peer1": []string{"peer2"}, "peer2": []string{"peer1"}, }, - emptyTrees: []string{"peer2"}, + emptyTrees: []string{"peer2"}, + treeBuilder: objecttree.BuildTestableTree, } fx := newProtocolFixture(t, spaceId, deps) fx.run(t) @@ -60,6 +62,11 @@ func TestEmptyClientGetsFullHistory(t *testing.T) { } func TestTreeFuzzyMerge(t *testing.T) { + testTreeFuzzyMerge(t, true) + testTreeFuzzyMerge(t, false) +} + +func testTreeFuzzyMerge(t *testing.T, withData bool) { var ( rnd = rand.New(rand.NewSource(time.Now().Unix())) levels = 20 @@ -67,20 +74,20 @@ func TestTreeFuzzyMerge(t *testing.T) { rounds = 10 ) for i := 0; i < rounds; i++ { - testTreeMerge(t, levels, perLevel, func() bool { + testTreeMerge(t, levels, perLevel, withData, func() bool { return true }) - testTreeMerge(t, levels, perLevel, func() bool { + testTreeMerge(t, levels, perLevel, withData, func() bool { return false }) - testTreeMerge(t, levels, perLevel, func() bool { + testTreeMerge(t, levels, perLevel, withData, func() bool { return rnd.Intn(10) > 8 }) levels += 2 } } -func testTreeMerge(t *testing.T, levels, perlevel int, isSnapshot func() bool) { +func testTreeMerge(t *testing.T, levels, perLevel int, hasData bool, isSnapshot func() bool) { treeId := "treeId" spaceId := "spaceId" keys, err := accountdata.NewRandom() @@ -88,15 +95,20 @@ func testTreeMerge(t *testing.T, levels, perlevel int, isSnapshot func() bool) { aclList, err := list.NewTestDerivedAcl(spaceId, keys) storage := createStorage(treeId, aclList) changeCreator := objecttree.NewMockChangeCreator() + builder := objecttree.BuildTestableTree + if hasData { + builder = objecttree.BuildEmptyDataTestableTree + } params := genParams{ prefix: "peer1", aclId: aclList.Id(), startIdx: 0, levels: levels, - perLevel: perlevel, + perLevel: perLevel, snapshotId: treeId, prevHeads: []string{treeId}, isSnapshot: isSnapshot, + hasData: hasData, } // generating initial tree initialRes := genChanges(changeCreator, params) @@ -110,7 +122,8 @@ func testTreeMerge(t *testing.T, levels, perlevel int, isSnapshot func() bool) { "peer2": []string{"node1"}, "node1": []string{"peer1", "peer2"}, }, - emptyTrees: []string{"peer2", "node1"}, + emptyTrees: []string{"peer2", "node1"}, + treeBuilder: builder, } fx := newProtocolFixture(t, spaceId, deps) fx.run(t) @@ -127,11 +140,12 @@ func testTreeMerge(t *testing.T, levels, perlevel int, isSnapshot func() bool) { aclId: aclList.Id(), startIdx: levels, levels: levels, - perLevel: perlevel, + perLevel: perLevel, snapshotId: initialRes.snapshotId, prevHeads: initialRes.heads, isSnapshot: isSnapshot, + hasData: hasData, } // generating different additions to the tree for different peers peer1Res := genChanges(changeCreator, params) @@ -156,4 +170,16 @@ func testTreeMerge(t *testing.T, levels, perlevel int, isSnapshot func() bool) { firstStorage := firstTree.Storage().(*treestorage.InMemoryTreeStorage) secondStorage := secondTree.Storage().(*treestorage.InMemoryTreeStorage) require.True(t, firstStorage.Equal(secondStorage)) + if hasData { + for _, ch := range secondStorage.Changes { + if ch.Id == treeId { + continue + } + unmarshallRaw := &treechangeproto.RawTreeChange{} + proto.Unmarshal(ch.RawChange, unmarshallRaw) + treeCh := &treechangeproto.TreeChange{} + proto.Unmarshal(unmarshallRaw.Payload, treeCh) + require.Equal(t, ch.Id, string(treeCh.ChangesData)) + } + } } diff --git a/commonspace/object/tree/synctree/utils_test.go b/commonspace/object/tree/synctree/utils_test.go index 92b7751c..bdbc0052 100644 --- a/commonspace/object/tree/synctree/utils_test.go +++ b/commonspace/object/tree/synctree/utils_test.go @@ -90,6 +90,7 @@ type testSyncHandler struct { aclList list.AclList log *messageLog syncClient objectsync.SyncClient + builder objecttree.BuildObjectTreeFunc } // createSyncHandler creates a sync handler when a tree is already created @@ -105,7 +106,7 @@ func createSyncHandler(peerId, spaceId string, objTree objecttree.ObjectTree, lo } // createEmptySyncHandler creates a sync handler when the tree will be provided later (this emulates the situation when we have no tree) -func createEmptySyncHandler(peerId, spaceId string, aclList list.AclList, log *messageLog) *testSyncHandler { +func createEmptySyncHandler(peerId, spaceId string, builder objecttree.BuildObjectTreeFunc, aclList list.AclList, log *messageLog) *testSyncHandler { factory := objectsync.NewRequestFactory() syncClient := objectsync.NewSyncClient(spaceId, newTestMessagePool(peerId, log), factory) @@ -116,6 +117,7 @@ func createEmptySyncHandler(peerId, spaceId string, aclList list.AclList, log *m aclList: aclList, log: log, syncClient: syncClient, + builder: builder, } } @@ -148,7 +150,7 @@ func (h *testSyncHandler) HandleMessage(ctx context.Context, senderId string, re } fullSyncResponse := unmarshalled.Content.GetFullSyncResponse() treeStorage, _ := treestorage.NewInMemoryTreeStorage(unmarshalled.RootChange, []string{unmarshalled.RootChange.Id}, nil) - tree, err := createTestTree(h.aclList, treeStorage) + tree, err := h.builder(treeStorage, h.aclList) if err != nil { return } @@ -304,6 +306,7 @@ type fixtureDeps struct { initStorage *treestorage.InMemoryTreeStorage connectionMap map[string][]string emptyTrees []string + treeBuilder objecttree.BuildObjectTreeFunc } // protocolFixture is the test environment for sync protocol tests @@ -326,10 +329,10 @@ func newProtocolFixture(t *testing.T, spaceId string, deps fixtureDeps) *protoco for peerId := range deps.connectionMap { var handler *testSyncHandler if slices.Contains(deps.emptyTrees, peerId) { - handler = createEmptySyncHandler(peerId, spaceId, deps.aclList, log) + handler = createEmptySyncHandler(peerId, spaceId, deps.treeBuilder, deps.aclList, log) } else { stCopy := deps.initStorage.Copy() - testTree, err := createTestTree(deps.aclList, stCopy) + testTree, err := deps.treeBuilder(stCopy, deps.aclList) require.NoError(t, err) handler = createSyncHandler(peerId, spaceId, testTree, log) } @@ -373,6 +376,7 @@ type genParams struct { snapshotId string prevHeads []string isSnapshot func() bool + hasData bool } // genResult is the result of genChanges @@ -399,7 +403,11 @@ func genChanges(creator *objecttree.MockChangeCreator, params genParams) (res ge ) newChange := func(isSnapshot bool, idx int, prevIds []string) string { newId := fmt.Sprintf("%s.%d.%d", params.prefix, params.startIdx+i, idx) - newCh := creator.CreateRaw(newId, params.aclId, snapshotId, isSnapshot, prevIds...) + var data []byte + if params.hasData { + data = []byte(newId) + } + newCh := creator.CreateRawWithData(newId, params.aclId, snapshotId, isSnapshot, data, prevIds...) res.changes = append(res.changes, newCh) return newId } diff --git a/commonspace/object/tree/treestorage/inmemory.go b/commonspace/object/tree/treestorage/inmemory.go index f5b192ba..af40561d 100644 --- a/commonspace/object/tree/treestorage/inmemory.go +++ b/commonspace/object/tree/treestorage/inmemory.go @@ -12,7 +12,7 @@ type InMemoryTreeStorage struct { id string root *treechangeproto.RawTreeChangeWithId heads []string - changes map[string]*treechangeproto.RawTreeChangeWithId + Changes map[string]*treechangeproto.RawTreeChangeWithId sync.RWMutex } @@ -22,7 +22,7 @@ func (t *InMemoryTreeStorage) TransactionAdd(changes []*treechangeproto.RawTreeC defer t.RUnlock() for _, ch := range changes { - t.changes[ch.Id] = ch + t.Changes[ch.Id] = ch } t.heads = append(t.heads[:0], heads...) return nil @@ -42,13 +42,13 @@ func NewInMemoryTreeStorage( id: root.Id, root: root, heads: append([]string(nil), heads...), - changes: allChanges, + Changes: allChanges, RWMutex: sync.RWMutex{}, }, nil } func (t *InMemoryTreeStorage) HasChange(ctx context.Context, id string) (bool, error) { - _, exists := t.changes[id] + _, exists := t.Changes[id] return exists, nil } @@ -81,14 +81,14 @@ func (t *InMemoryTreeStorage) AddRawChange(change *treechangeproto.RawTreeChange t.Lock() defer t.Unlock() // TODO: better to do deep copy - t.changes[change.Id] = change + t.Changes[change.Id] = change return nil } func (t *InMemoryTreeStorage) GetRawChange(ctx context.Context, changeId string) (*treechangeproto.RawTreeChangeWithId, error) { t.RLock() defer t.RUnlock() - if res, exists := t.changes[changeId]; exists { + if res, exists := t.Changes[changeId]; exists { return res, nil } return nil, fmt.Errorf("could not get change with id: %s", changeId) @@ -100,7 +100,7 @@ func (t *InMemoryTreeStorage) Delete() error { func (t *InMemoryTreeStorage) Copy() *InMemoryTreeStorage { var changes []*treechangeproto.RawTreeChangeWithId - for _, ch := range t.changes { + for _, ch := range t.Changes { changes = append(changes, ch) } other, _ := NewInMemoryTreeStorage(t.root, t.heads, changes) @@ -111,11 +111,11 @@ func (t *InMemoryTreeStorage) Equal(other *InMemoryTreeStorage) bool { if !slice.UnsortedEquals(t.heads, other.heads) { return false } - if len(t.changes) != len(other.changes) { + if len(t.Changes) != len(other.Changes) { return false } - for k, v := range t.changes { - if otherV, exists := other.changes[k]; exists { + for k, v := range t.Changes { + if otherV, exists := other.Changes[k]; exists { if otherV.Id == v.Id { continue } From 6efde255626295ce3651da57f7ed551e4d7d834c Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 27 Apr 2023 19:11:05 +0200 Subject: [PATCH 4/4] Keep inmemory data on load --- .../object/tree/objecttree/objecttree.go | 11 +- .../object/tree/objecttree/objecttree_test.go | 155 +++++++++++------- .../tree/objecttree/objecttreefactory.go | 14 +- .../object/tree/objecttree/treebuilder.go | 15 +- 4 files changed, 119 insertions(+), 76 deletions(-) diff --git a/commonspace/object/tree/objecttree/objecttree.go b/commonspace/object/tree/objecttree/objecttree.go index 414ca7dc..f81a9ddd 100644 --- a/commonspace/object/tree/objecttree/objecttree.go +++ b/commonspace/object/tree/objecttree/objecttree.go @@ -96,11 +96,10 @@ type objectTree struct { treeBuilder *treeBuilder aclList list.AclList - removeDataOnAdd bool - id string - rawRoot *treechangeproto.RawTreeChangeWithId - root *Change - tree *Tree + id string + rawRoot *treechangeproto.RawTreeChangeWithId + root *Change + tree *Tree keys map[string]crypto.SymKey currentReadKey crypto.SymKey @@ -474,7 +473,7 @@ func (ot *objectTree) createAddResult(oldHeads []string, mode Mode, treeChangesA var added []*treechangeproto.RawTreeChangeWithId added, err = getAddedChanges(treeChangesAdded) - if ot.removeDataOnAdd { + if !ot.treeBuilder.keepInMemoryData { for _, ch := range treeChangesAdded { ch.Data = nil } diff --git a/commonspace/object/tree/objecttree/objecttree_test.go b/commonspace/object/tree/objecttree/objecttree_test.go index 01fc8f7f..4d531d9d 100644 --- a/commonspace/object/tree/objecttree/objecttree_test.go +++ b/commonspace/object/tree/objecttree/objecttree_test.go @@ -28,7 +28,7 @@ func prepareAclList(t *testing.T) list.AclList { return aclList } -func prepareTreeDeps(aclList list.AclList) (*MockChangeCreator, objectTreeDeps) { +func prepareHistoryTreeDeps(aclList list.AclList) (*MockChangeCreator, objectTreeDeps) { changeCreator := NewMockChangeCreator() treeStorage := changeCreator.CreateNewTreeStorage("0", aclList.Head().Id) root, _ := treeStorage.Root() @@ -37,7 +37,7 @@ func prepareTreeDeps(aclList list.AclList) (*MockChangeCreator, objectTreeDeps) } deps := objectTreeDeps{ changeBuilder: changeBuilder, - treeBuilder: newTreeBuilder(treeStorage, changeBuilder), + treeBuilder: newTreeBuilder(true, treeStorage, changeBuilder), treeStorage: treeStorage, rawChangeLoader: newRawChangeLoader(treeStorage, changeBuilder), validator: &noOpTreeValidator{}, @@ -47,16 +47,25 @@ func prepareTreeDeps(aclList list.AclList) (*MockChangeCreator, objectTreeDeps) } func prepareTreeContext(t *testing.T, aclList list.AclList) testTreeContext { - return prepareContext(t, aclList, BuildTestableTree) + return prepareContext(t, aclList, BuildTestableTree, nil) } -func prepareEmptyDataTreeContext(t *testing.T, aclList list.AclList) testTreeContext { - return prepareContext(t, aclList, BuildEmptyDataTestableTree) +func prepareEmptyDataTreeContext(t *testing.T, aclList list.AclList, additionalChanges func(changeCreator *MockChangeCreator) RawChangesPayload) testTreeContext { + return prepareContext(t, aclList, BuildEmptyDataTestableTree, additionalChanges) } -func prepareContext(t *testing.T, aclList list.AclList, objTreeBuilder BuildObjectTreeFunc) testTreeContext { +func prepareContext( + t *testing.T, + aclList list.AclList, + objTreeBuilder BuildObjectTreeFunc, + additionalChanges func(changeCreator *MockChangeCreator) RawChangesPayload) testTreeContext { changeCreator := NewMockChangeCreator() treeStorage := changeCreator.CreateNewTreeStorage("0", aclList.Head().Id) + if additionalChanges != nil { + payload := additionalChanges(changeCreator) + err := treeStorage.TransactionAdd(payload.RawChanges, payload.NewHeads) + require.NoError(t, err) + } objTree, err := objTreeBuilder(treeStorage, aclList) require.NoError(t, err, "building tree should be without error") @@ -67,7 +76,9 @@ func prepareContext(t *testing.T, aclList list.AclList, objTreeBuilder BuildObje return true }) require.NoError(t, err, "iterate should be without error") - assert.Equal(t, []string{"0"}, iterChangesId) + if additionalChanges == nil { + assert.Equal(t, []string{"0"}, iterChangesId) + } return testTreeContext{ aclList: aclList, treeStorage: treeStorage, @@ -276,54 +287,86 @@ func TestObjectTree(t *testing.T) { }) t.Run("test empty data tree", func(t *testing.T) { - ctx := prepareEmptyDataTreeContext(t, aclList) - changeCreator := ctx.changeCreator - objTree := ctx.objTree + t.Run("empty tree add", func(t *testing.T) { + ctx := prepareEmptyDataTreeContext(t, aclList, nil) + 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 + 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("empty tree load", func(t *testing.T) { + ctx := prepareEmptyDataTreeContext(t, aclList, func(changeCreator *MockChangeCreator) RawChangesPayload { + rawChanges := []*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"), + 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"), + } + return RawChangesPayload{NewHeads: []string{"6"}, RawChanges: rawChanges} + }) + ctx.objTree.IterateRoot(nil, func(change *Change) bool { + if change.Id == "0" { + return true + } + require.Nil(t, change.Data) + return true + }) + rawChanges, err := ctx.objTree.ChangesAfterCommonSnapshot([]string{"0"}, []string{"6"}) + require.NoError(t, err) + for _, ch := range rawChanges { + unmarshallRaw := &treechangeproto.RawTreeChange{} + proto.Unmarshal(ch.RawChange, unmarshallRaw) + treeCh := &treechangeproto.TreeChange{} + proto.Unmarshal(unmarshallRaw.Payload, treeCh) + require.Equal(t, ch.Id, string(treeCh.ChangesData)) } - require.Nil(t, change.Data) - return true }) }) @@ -550,7 +593,7 @@ func TestObjectTree(t *testing.T) { }) t.Run("test history tree not include", func(t *testing.T) { - changeCreator, deps := prepareTreeDeps(aclList) + changeCreator, deps := prepareHistoryTreeDeps(aclList) rawChanges := []*treechangeproto.RawTreeChangeWithId{ changeCreator.CreateRaw("1", aclList.Head().Id, "0", false, "0"), @@ -581,7 +624,7 @@ func TestObjectTree(t *testing.T) { }) t.Run("test history tree include", func(t *testing.T) { - changeCreator, deps := prepareTreeDeps(aclList) + changeCreator, deps := prepareHistoryTreeDeps(aclList) rawChanges := []*treechangeproto.RawTreeChangeWithId{ changeCreator.CreateRaw("1", aclList.Head().Id, "0", false, "0"), @@ -612,7 +655,7 @@ func TestObjectTree(t *testing.T) { }) t.Run("test history tree root", func(t *testing.T) { - _, deps := prepareTreeDeps(aclList) + _, deps := prepareHistoryTreeDeps(aclList) hTree, err := buildHistoryTree(deps, HistoryTreeParams{ BeforeId: "0", IncludeBeforeId: true, diff --git a/commonspace/object/tree/objecttree/objecttreefactory.go b/commonspace/object/tree/objecttree/objecttreefactory.go index f6089cdb..dfa78f15 100644 --- a/commonspace/object/tree/objecttree/objecttreefactory.go +++ b/commonspace/object/tree/objecttree/objecttreefactory.go @@ -31,7 +31,6 @@ type objectTreeDeps struct { validator ObjectTreeValidator rawChangeLoader *rawChangeLoader aclList list.AclList - removeDataOnAdd bool } type BuildObjectTreeFunc = func(treeStorage treestorage.TreeStorage, aclList list.AclList) (ObjectTree, error) @@ -43,7 +42,7 @@ func verifiableTreeDeps( treeStorage treestorage.TreeStorage, aclList list.AclList) objectTreeDeps { changeBuilder := NewChangeBuilder(crypto.NewKeyStorage(), rootChange) - treeBuilder := newTreeBuilder(treeStorage, changeBuilder) + treeBuilder := newTreeBuilder(true, treeStorage, changeBuilder) return objectTreeDeps{ changeBuilder: changeBuilder, treeBuilder: treeBuilder, @@ -59,7 +58,7 @@ func emptyDataTreeDeps( treeStorage treestorage.TreeStorage, aclList list.AclList) objectTreeDeps { changeBuilder := NewChangeBuilder(crypto.NewKeyStorage(), rootChange) - treeBuilder := newTreeBuilder(treeStorage, changeBuilder) + treeBuilder := newTreeBuilder(false, treeStorage, changeBuilder) return objectTreeDeps{ changeBuilder: changeBuilder, treeBuilder: treeBuilder, @@ -67,7 +66,6 @@ func emptyDataTreeDeps( validator: newTreeValidator(), rawChangeLoader: newStorageLoader(treeStorage, changeBuilder), aclList: aclList, - removeDataOnAdd: true, } } @@ -76,7 +74,7 @@ func nonVerifiableTreeDeps( treeStorage treestorage.TreeStorage, aclList list.AclList) objectTreeDeps { changeBuilder := &nonVerifiableChangeBuilder{NewChangeBuilder(newMockKeyStorage(), rootChange)} - treeBuilder := newTreeBuilder(treeStorage, changeBuilder) + treeBuilder := newTreeBuilder(true, treeStorage, changeBuilder) return objectTreeDeps{ changeBuilder: changeBuilder, treeBuilder: treeBuilder, @@ -116,7 +114,7 @@ func BuildTestableTree(treeStorage treestorage.TreeStorage, aclList list.AclList } deps := objectTreeDeps{ changeBuilder: changeBuilder, - treeBuilder: newTreeBuilder(treeStorage, changeBuilder), + treeBuilder: newTreeBuilder(true, treeStorage, changeBuilder), treeStorage: treeStorage, rawChangeLoader: newRawChangeLoader(treeStorage, changeBuilder), validator: &noOpTreeValidator{}, @@ -133,12 +131,11 @@ func BuildEmptyDataTestableTree(treeStorage treestorage.TreeStorage, aclList lis } deps := objectTreeDeps{ changeBuilder: changeBuilder, - treeBuilder: newTreeBuilder(treeStorage, changeBuilder), + treeBuilder: newTreeBuilder(false, treeStorage, changeBuilder), treeStorage: treeStorage, rawChangeLoader: newStorageLoader(treeStorage, changeBuilder), validator: &noOpTreeValidator{}, aclList: aclList, - removeDataOnAdd: true, } return buildObjectTree(deps) @@ -254,7 +251,6 @@ 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/treebuilder.go b/commonspace/object/tree/objecttree/treebuilder.go index dd5d8622..0b7ffe62 100644 --- a/commonspace/object/tree/objecttree/treebuilder.go +++ b/commonspace/object/tree/objecttree/treebuilder.go @@ -21,18 +21,20 @@ type treeBuilder struct { treeStorage treestorage.TreeStorage builder ChangeBuilder - cache map[string]*Change - tree *Tree + cache map[string]*Change + tree *Tree + keepInMemoryData bool // buffers idStack []string loadBuffer []*Change } -func newTreeBuilder(storage treestorage.TreeStorage, builder ChangeBuilder) *treeBuilder { +func newTreeBuilder(keepData bool, storage treestorage.TreeStorage, builder ChangeBuilder) *treeBuilder { return &treeBuilder{ - treeStorage: storage, - builder: builder, + treeStorage: storage, + builder: builder, + keepInMemoryData: keepData, } } @@ -163,6 +165,9 @@ func (tb *treeBuilder) loadChange(id string) (ch *Change, err error) { if err != nil { return nil, err } + if !tb.keepInMemoryData { + ch.Data = nil + } tb.cache[id] = ch return ch, nil