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