From db7a95514dc00a4a3d7ddeb1f471feeb5500c24d Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Tue, 23 May 2023 11:34:24 +0200 Subject: [PATCH] Add mark deleted logic --- commonspace/deletion_test.go | 95 +++++++++++++++++-- .../mock_treemanager/mock_treemanager.go | 14 +++ commonspace/object/treemanager/treemanager.go | 1 + commonspace/settings/deleter.go | 38 ++++++-- commonspace/settings/deleter_test.go | 34 ++++++- commonspace/spaceutils_test.go | 6 ++ 6 files changed, 170 insertions(+), 18 deletions(-) diff --git a/commonspace/deletion_test.go b/commonspace/deletion_test.go index 5829ac3b..ad222db5 100644 --- a/commonspace/deletion_test.go +++ b/commonspace/deletion_test.go @@ -6,6 +6,7 @@ import ( "github.com/anytypeio/any-sync/commonspace/object/accountdata" "github.com/anytypeio/any-sync/commonspace/object/tree/objecttree" "github.com/anytypeio/any-sync/commonspace/object/tree/treechangeproto" + "github.com/anytypeio/any-sync/commonspace/object/tree/treestorage" "github.com/anytypeio/any-sync/commonspace/settings" "github.com/anytypeio/any-sync/commonspace/settings/settingsstate" "github.com/anytypeio/any-sync/commonspace/spacestorage" @@ -67,10 +68,10 @@ func TestSpaceDeleteIds(t *testing.T) { spc, err := fx.spaceService.NewSpace(ctx, sp) require.NoError(t, err) require.NotNil(t, spc) - err = spc.Init(ctx) - require.NoError(t, err) // adding space to tree manager fx.treeManager.space = spc + err = spc.Init(ctx) + require.NoError(t, err) var ids []string for i := 0; i < totalObjs; i++ { @@ -124,10 +125,10 @@ func TestSpaceDeleteIdsIncorrectSnapshot(t *testing.T) { spc, err := fx.spaceService.NewSpace(ctx, sp) require.NoError(t, err) require.NotNil(t, spc) - err = spc.Init(ctx) - require.NoError(t, err) // adding space to tree manager fx.treeManager.space = spc + err = spc.Init(ctx) + require.NoError(t, err) settingsObject := spc.(*space).settingsObject var ids []string @@ -177,12 +178,94 @@ func TestSpaceDeleteIdsIncorrectSnapshot(t *testing.T) { spc, err = fx.spaceService.NewSpace(ctx, sp) require.NoError(t, err) require.NotNil(t, spc) - err = spc.Init(ctx) - require.NoError(t, err) fx.treeManager.space = spc fx.treeManager.deletedIds = nil + err = spc.Init(ctx) + require.NoError(t, err) // waiting until everything is deleted time.Sleep(3 * time.Second) require.Equal(t, len(ids), len(fx.treeManager.deletedIds)) + + // TODO: check that new snapshot will have all the changes +} + +func TestSpaceDeleteIdsMarkDeleted(t *testing.T) { + fx := newFixture(t) + acc := fx.account.Account() + rk := crypto.NewAES() + ctx := context.Background() + totalObjs := 3000 + + // creating space + sp, err := fx.spaceService.CreateSpace(ctx, SpaceCreatePayload{ + SigningKey: acc.SignKey, + SpaceType: "type", + ReadKey: rk.Bytes(), + ReplicationKey: 10, + MasterKey: acc.PeerKey, + }) + require.NoError(t, err) + require.NotNil(t, sp) + + // initializing space + spc, err := fx.spaceService.NewSpace(ctx, sp) + require.NoError(t, err) + require.NotNil(t, spc) + // adding space to tree manager + fx.treeManager.space = spc + err = spc.Init(ctx) + require.NoError(t, err) + + settingsObject := spc.(*space).settingsObject + var ids []string + for i := 0; i < totalObjs; i++ { + // creating a tree + bytes := make([]byte, 32) + rand.Read(bytes) + doc, err := spc.CreateTree(ctx, objecttree.ObjectTreeCreatePayload{ + PrivKey: acc.SignKey, + ChangeType: "some", + SpaceId: spc.Id(), + IsEncrypted: false, + Seed: bytes, + Timestamp: time.Now().Unix(), + }) + require.NoError(t, err) + tr, err := spc.PutTree(ctx, doc, nil) + require.NoError(t, err) + ids = append(ids, tr.Id()) + tr.Close() + } + // copying storage, so we will have the same contents, except for empty trees + inmemory := spc.Storage().(*commonStorage).SpaceStorage.(*spacestorage.InMemorySpaceStorage) + storageCopy := inmemory.CopyStorage() + + // deleting trees, this will prepare the document to have all the deletion changes + for _, id := range ids { + err = spc.DeleteTree(ctx, id) + require.NoError(t, err) + } + treesMap := map[string]treestorage.TreeStorage{} + // copying the contents of the settings tree + treesMap[settingsObject.Id()] = settingsObject.Storage() + storageCopy.SetTrees(treesMap) + spc.Close() + time.Sleep(100 * time.Millisecond) + // now we replace the storage, so the trees are back, but the settings object says that they are deleted + fx.storageProvider.(*spacestorage.InMemorySpaceStorageProvider).SetStorage(storageCopy) + + spc, err = fx.spaceService.NewSpace(ctx, sp) + require.NoError(t, err) + require.NotNil(t, spc) + fx.treeManager.space = spc + fx.treeManager.deletedIds = nil + fx.treeManager.markedIds = nil + err = spc.Init(ctx) + require.NoError(t, err) + + // waiting until everything is deleted + time.Sleep(3 * time.Second) + require.Equal(t, len(ids), len(fx.treeManager.markedIds)) + require.Zero(t, len(fx.treeManager.deletedIds)) } diff --git a/commonspace/object/treemanager/mock_treemanager/mock_treemanager.go b/commonspace/object/treemanager/mock_treemanager/mock_treemanager.go index 68f1d579..8c0f12a9 100644 --- a/commonspace/object/treemanager/mock_treemanager/mock_treemanager.go +++ b/commonspace/object/treemanager/mock_treemanager/mock_treemanager.go @@ -93,6 +93,20 @@ func (mr *MockTreeManagerMockRecorder) Init(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Init", reflect.TypeOf((*MockTreeManager)(nil).Init), arg0) } +// MarkTreeDeleted mocks base method. +func (m *MockTreeManager) MarkTreeDeleted(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MarkTreeDeleted", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// MarkTreeDeleted indicates an expected call of MarkTreeDeleted. +func (mr *MockTreeManagerMockRecorder) MarkTreeDeleted(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkTreeDeleted", reflect.TypeOf((*MockTreeManager)(nil).MarkTreeDeleted), arg0, arg1, arg2) +} + // Name mocks base method. func (m *MockTreeManager) Name() string { m.ctrl.T.Helper() diff --git a/commonspace/object/treemanager/treemanager.go b/commonspace/object/treemanager/treemanager.go index 0573471e..0cc5513c 100644 --- a/commonspace/object/treemanager/treemanager.go +++ b/commonspace/object/treemanager/treemanager.go @@ -12,5 +12,6 @@ const CName = "common.object.treemanager" type TreeManager interface { app.ComponentRunnable GetTree(ctx context.Context, spaceId, treeId string) (objecttree.ObjectTree, error) + MarkTreeDeleted(ctx context.Context, spaceId, treeId string) error DeleteTree(ctx context.Context, spaceId, treeId string) error } diff --git a/commonspace/settings/deleter.go b/commonspace/settings/deleter.go index c50da268..6cc3c0b5 100644 --- a/commonspace/settings/deleter.go +++ b/commonspace/settings/deleter.go @@ -2,6 +2,7 @@ package settings import ( "context" + "github.com/anytypeio/any-sync/commonspace/object/tree/treestorage" "github.com/anytypeio/any-sync/commonspace/object/treemanager" "github.com/anytypeio/any-sync/commonspace/settings/settingsstate" "github.com/anytypeio/any-sync/commonspace/spacestorage" @@ -23,17 +24,40 @@ func newDeleter(st spacestorage.SpaceStorage, state settingsstate.ObjectDeletion } func (d *deleter) Delete() { - allQueued := d.state.GetQueued() + var ( + allQueued = d.state.GetQueued() + spaceId = d.st.Id() + ) for _, id := range allQueued { - err := d.getter.DeleteTree(context.Background(), d.st.Id(), id) - if err != nil && err != spacestorage.ErrTreeStorageAlreadyDeleted { - log.With(zap.String("id", id), zap.Error(err)).Error("failed to delete object") - continue + log := log.With(zap.String("treeId", id), zap.String("spaceId", spaceId)) + shouldDelete, err := d.tryMarkDeleted(spaceId, id) + if !shouldDelete { + if err != nil { + log.Error("failed to mark object as deleted", zap.Error(err)) + continue + } + } else { + err = d.getter.DeleteTree(context.Background(), spaceId, id) + if err != nil && err != spacestorage.ErrTreeStorageAlreadyDeleted { + log.Error("failed to delete object", zap.Error(err)) + continue + } } err = d.state.Delete(id) if err != nil { - log.With(zap.String("id", id), zap.Error(err)).Error("failed to mark object as deleted") + log.Error("failed to mark object as deleted", zap.Error(err)) } - log.With(zap.String("id", id), zap.Error(err)).Debug("object successfully deleted") + log.Debug("object successfully deleted", zap.Error(err)) } } + +func (d *deleter) tryMarkDeleted(spaceId, treeId string) (bool, error) { + _, err := d.st.TreeStorage(treeId) + if err == nil { + return true, nil + } + if err != treestorage.ErrUnknownTreeId { + return false, err + } + return false, d.getter.MarkTreeDeleted(context.Background(), spaceId, treeId) +} diff --git a/commonspace/settings/deleter_test.go b/commonspace/settings/deleter_test.go index fe6f7c51..21719f93 100644 --- a/commonspace/settings/deleter_test.go +++ b/commonspace/settings/deleter_test.go @@ -2,9 +2,9 @@ package settings import ( "fmt" + "github.com/anytypeio/any-sync/commonspace/object/tree/treestorage" "github.com/anytypeio/any-sync/commonspace/object/treemanager/mock_treemanager" "github.com/anytypeio/any-sync/commonspace/settings/settingsstate/mock_settingsstate" - "github.com/anytypeio/any-sync/commonspace/spacestorage" "github.com/anytypeio/any-sync/commonspace/spacestorage/mock_spacestorage" "github.com/golang/mock/gomock" "testing" @@ -18,23 +18,46 @@ func TestDeleter_Delete(t *testing.T) { deleter := newDeleter(st, delState, treeManager) - t.Run("deleter delete queued", func(t *testing.T) { + t.Run("deleter delete mark deleted success", func(t *testing.T) { id := "id" spaceId := "spaceId" delState.EXPECT().GetQueued().Return([]string{id}) st.EXPECT().Id().Return(spaceId) - treeManager.EXPECT().DeleteTree(gomock.Any(), spaceId, id).Return(nil) + st.EXPECT().TreeStorage(id).Return(nil, treestorage.ErrUnknownTreeId) + treeManager.EXPECT().MarkTreeDeleted(gomock.Any(), spaceId, id).Return(nil) delState.EXPECT().Delete(id).Return(nil) deleter.Delete() }) - t.Run("deleter delete already deleted", func(t *testing.T) { + t.Run("deleter delete mark deleted other error", func(t *testing.T) { id := "id" spaceId := "spaceId" delState.EXPECT().GetQueued().Return([]string{id}) st.EXPECT().Id().Return(spaceId) - treeManager.EXPECT().DeleteTree(gomock.Any(), spaceId, id).Return(spacestorage.ErrTreeStorageAlreadyDeleted) + st.EXPECT().TreeStorage(id).Return(nil, fmt.Errorf("unknown error")) + + deleter.Delete() + }) + + t.Run("deleter delete mark deleted fail", func(t *testing.T) { + id := "id" + spaceId := "spaceId" + delState.EXPECT().GetQueued().Return([]string{id}) + st.EXPECT().Id().Return(spaceId) + st.EXPECT().TreeStorage(id).Return(nil, treestorage.ErrUnknownTreeId) + treeManager.EXPECT().MarkTreeDeleted(gomock.Any(), spaceId, id).Return(fmt.Errorf("mark error")) + + deleter.Delete() + }) + //treeManager.EXPECT().DeleteTree(gomock.Any(), spaceId, id).Return(spacestorage.ErrTreeStorageAlreadyDeleted) + t.Run("deleter delete success", func(t *testing.T) { + id := "id" + spaceId := "spaceId" + delState.EXPECT().GetQueued().Return([]string{id}) + st.EXPECT().Id().Return(spaceId) + st.EXPECT().TreeStorage(id).Return(nil, nil) + treeManager.EXPECT().DeleteTree(gomock.Any(), spaceId, id).Return(nil) delState.EXPECT().Delete(id).Return(nil) deleter.Delete() @@ -45,6 +68,7 @@ func TestDeleter_Delete(t *testing.T) { spaceId := "spaceId" delState.EXPECT().GetQueued().Return([]string{id}) st.EXPECT().Id().Return(spaceId) + st.EXPECT().TreeStorage(id).Return(nil, nil) treeManager.EXPECT().DeleteTree(gomock.Any(), spaceId, id).Return(fmt.Errorf("some error")) deleter.Delete() diff --git a/commonspace/spaceutils_test.go b/commonspace/spaceutils_test.go index 7b025f73..04ff0f65 100644 --- a/commonspace/spaceutils_test.go +++ b/commonspace/spaceutils_test.go @@ -221,6 +221,12 @@ type mockTreeManager struct { space Space cache ocache.OCache deletedIds []string + markedIds []string +} + +func (t *mockTreeManager) MarkTreeDeleted(ctx context.Context, spaceId, treeId string) error { + t.markedIds = append(t.markedIds, treeId) + return nil } func (t *mockTreeManager) Init(a *app.App) (err error) {