diff --git a/commonspace/deletion_test.go b/commonspace/deletion_test.go index 3b5ad8b5..5829ac3b 100644 --- a/commonspace/deletion_test.go +++ b/commonspace/deletion_test.go @@ -16,7 +16,7 @@ import ( "time" ) -func addIncorrectSnapshot(settingsObject settings.SettingsObject, acc *accountdata.AccountKeys, partialIds []string, newId string) (err error) { +func addIncorrectSnapshot(settingsObject settings.SettingsObject, acc *accountdata.AccountKeys, partialIds map[string]struct{}, newId string) (err error) { factory := settingsstate.NewChangeFactory() bytes, err := factory.CreateObjectDeleteChange(newId, &settingsstate.State{DeletedIds: partialIds}, true) if err != nil { @@ -159,8 +159,12 @@ func TestSpaceDeleteIdsIncorrectSnapshot(t *testing.T) { err = spc.DeleteTree(ctx, id) require.NoError(t, err) } + mapIds := map[string]struct{}{} + for _, id := range ids[:partialObjs] { + mapIds[id] = struct{}{} + } // adding snapshot that breaks the state - err = addIncorrectSnapshot(settingsObject, acc, ids[:partialObjs], ids[partialObjs]) + err = addIncorrectSnapshot(settingsObject, acc, mapIds, ids[partialObjs]) require.NoError(t, err) // copying the contents of the settings tree treesCopy[settingsObject.Id()] = settingsObject.Storage() diff --git a/commonspace/settings/deletionmanager.go b/commonspace/settings/deletionmanager.go index 22a80531..eb66873d 100644 --- a/commonspace/settings/deletionmanager.go +++ b/commonspace/settings/deletionmanager.go @@ -4,7 +4,6 @@ import ( "context" "github.com/anytypeio/any-sync/commonspace/object/treemanager" "github.com/anytypeio/any-sync/commonspace/settings/settingsstate" - "github.com/anytypeio/any-sync/util/slice" "go.uber.org/zap" ) @@ -51,12 +50,16 @@ func (d *deletionManager) UpdateState(ctx context.Context, state *settingsstate. if state.DeleterId == "" { return nil } + // we should delete space log.Debug("deleting space") if d.isResponsible { - allIds := slice.DiscardFromSlice(d.provider.AllIds(), func(id string) bool { - return id == d.settingsId - }) - d.deletionState.Add(allIds) + mapIds := map[string]struct{}{} + for _, id := range d.provider.AllIds() { + if id != d.settingsId { + mapIds[id] = struct{}{} + } + } + d.deletionState.Add(mapIds) } d.onSpaceDelete() return nil diff --git a/commonspace/settings/deletionmanager_test.go b/commonspace/settings/deletionmanager_test.go index dce2c599..1f3afeae 100644 --- a/commonspace/settings/deletionmanager_test.go +++ b/commonspace/settings/deletionmanager_test.go @@ -19,7 +19,7 @@ func TestDeletionManager_UpdateState_NotResponsible(t *testing.T) { spaceId := "spaceId" settingsId := "settingsId" state := &settingsstate.State{ - DeletedIds: []string{"id"}, + DeletedIds: map[string]struct{}{"id": {}}, DeleterId: "deleterId", } deleted := false @@ -51,7 +51,7 @@ func TestDeletionManager_UpdateState_Responsible(t *testing.T) { spaceId := "spaceId" settingsId := "settingsId" state := &settingsstate.State{ - DeletedIds: []string{"id"}, + DeletedIds: map[string]struct{}{"id": struct{}{}}, DeleterId: "deleterId", } deleted := false @@ -64,7 +64,7 @@ func TestDeletionManager_UpdateState_Responsible(t *testing.T) { delState.EXPECT().Add(state.DeletedIds) provider.EXPECT().AllIds().Return([]string{"id", "otherId", settingsId}) - delState.EXPECT().Add([]string{"id", "otherId"}) + delState.EXPECT().Add(map[string]struct{}{"id": {}, "otherId": {}}) delManager := newDeletionManager(spaceId, settingsId, true, diff --git a/commonspace/settings/settingsstate/changefactory.go b/commonspace/settings/settingsstate/changefactory.go index a5a70dc5..03a8de2f 100644 --- a/commonspace/settings/settingsstate/changefactory.go +++ b/commonspace/settings/settingsstate/changefactory.go @@ -50,7 +50,7 @@ func (c *changeFactory) CreateSpaceDeleteChange(peerId string, state *State, isS func (c *changeFactory) makeSnapshot(state *State, objectId, deleterPeer string) *spacesyncproto.SpaceSettingsSnapshot { var ( - deletedIds = state.DeletedIds + deletedIds = make([]string, 0, len(state.DeletedIds)+1) deleterId = state.DeleterId ) if objectId != "" { @@ -59,6 +59,9 @@ func (c *changeFactory) makeSnapshot(state *State, objectId, deleterPeer string) if deleterPeer != "" { deleterId = deleterPeer } + for id := range state.DeletedIds { + deletedIds = append(deletedIds, id) + } return &spacesyncproto.SpaceSettingsSnapshot{ DeletedIds: deletedIds, DeleterPeerId: deleterId, diff --git a/commonspace/settings/settingsstate/changefactory_test.go b/commonspace/settings/settingsstate/changefactory_test.go index 41135b36..35cfbf55 100644 --- a/commonspace/settings/settingsstate/changefactory_test.go +++ b/commonspace/settings/settingsstate/changefactory_test.go @@ -4,13 +4,14 @@ import ( "github.com/anytypeio/any-sync/commonspace/spacesyncproto" "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" "testing" ) func TestChangeFactory_CreateObjectDeleteChange(t *testing.T) { factory := NewChangeFactory() state := &State{ - DeletedIds: []string{"1", "2"}, + DeletedIds: map[string]struct{}{"1": {}, "2": {}}, DeleterId: "del", } marshalled, err := factory.CreateObjectDeleteChange("3", state, false) @@ -26,6 +27,7 @@ func TestChangeFactory_CreateObjectDeleteChange(t *testing.T) { data = &spacesyncproto.SettingsData{} err = proto.Unmarshal(marshalled, data) require.NoError(t, err) + slices.Sort(data.Snapshot.DeletedIds) require.Equal(t, &spacesyncproto.SpaceSettingsSnapshot{ DeletedIds: []string{"1", "2", "3"}, DeleterPeerId: "del", @@ -36,7 +38,7 @@ func TestChangeFactory_CreateObjectDeleteChange(t *testing.T) { func TestChangeFactory_CreateSpaceDeleteChange(t *testing.T) { factory := NewChangeFactory() state := &State{ - DeletedIds: []string{"1", "2"}, + DeletedIds: map[string]struct{}{"1": {}, "2": {}}, } marshalled, err := factory.CreateSpaceDeleteChange("del", state, false) require.NoError(t, err) @@ -51,6 +53,7 @@ func TestChangeFactory_CreateSpaceDeleteChange(t *testing.T) { data = &spacesyncproto.SettingsData{} err = proto.Unmarshal(marshalled, data) require.NoError(t, err) + slices.Sort(data.Snapshot.DeletedIds) require.Equal(t, &spacesyncproto.SpaceSettingsSnapshot{ DeletedIds: []string{"1", "2"}, DeleterPeerId: "del", diff --git a/commonspace/settings/settingsstate/deletionstate.go b/commonspace/settings/settingsstate/deletionstate.go index 948c006b..4641832e 100644 --- a/commonspace/settings/settingsstate/deletionstate.go +++ b/commonspace/settings/settingsstate/deletionstate.go @@ -12,7 +12,7 @@ type StateUpdateObserver func(ids []string) type ObjectDeletionState interface { AddObserver(observer StateUpdateObserver) - Add(ids []string) + Add(ids map[string]struct{}) GetQueued() (ids []string) Delete(id string) (err error) Exists(id string) bool @@ -43,7 +43,7 @@ func (st *objectDeletionState) AddObserver(observer StateUpdateObserver) { st.stateUpdateObservers = append(st.stateUpdateObservers, observer) } -func (st *objectDeletionState) Add(ids []string) { +func (st *objectDeletionState) Add(ids map[string]struct{}) { var added []string st.Lock() defer func() { @@ -53,7 +53,7 @@ func (st *objectDeletionState) Add(ids []string) { } }() - for _, id := range ids { + for id := range ids { if _, exists := st.deleted[id]; exists { continue } diff --git a/commonspace/settings/settingsstate/deletionstate_test.go b/commonspace/settings/settingsstate/deletionstate_test.go index 937f4c94..2320bf98 100644 --- a/commonspace/settings/settingsstate/deletionstate_test.go +++ b/commonspace/settings/settingsstate/deletionstate_test.go @@ -38,7 +38,7 @@ func TestDeletionState_Add(t *testing.T) { id := "newId" fx.spaceStorage.EXPECT().TreeDeletedStatus(id).Return("", nil) fx.spaceStorage.EXPECT().SetTreeDeletedStatus(id, spacestorage.TreeDeletedStatusQueued).Return(nil) - fx.delState.Add([]string{id}) + fx.delState.Add(map[string]struct{}{id: {}}) require.Contains(t, fx.delState.queued, id) }) @@ -47,7 +47,7 @@ func TestDeletionState_Add(t *testing.T) { defer fx.stop() id := "newId" fx.spaceStorage.EXPECT().TreeDeletedStatus(id).Return(spacestorage.TreeDeletedStatusQueued, nil) - fx.delState.Add([]string{id}) + fx.delState.Add(map[string]struct{}{id: {}}) require.Contains(t, fx.delState.queued, id) }) @@ -56,7 +56,7 @@ func TestDeletionState_Add(t *testing.T) { defer fx.stop() id := "newId" fx.spaceStorage.EXPECT().TreeDeletedStatus(id).Return(spacestorage.TreeDeletedStatusDeleted, nil) - fx.delState.Add([]string{id}) + fx.delState.Add(map[string]struct{}{id: {}}) require.Contains(t, fx.delState.deleted, id) }) } @@ -96,7 +96,7 @@ func TestDeletionState_AddObserver(t *testing.T) { id := "newId" fx.spaceStorage.EXPECT().TreeDeletedStatus(id).Return("", nil) fx.spaceStorage.EXPECT().SetTreeDeletedStatus(id, spacestorage.TreeDeletedStatusQueued).Return(nil) - fx.delState.Add([]string{id}) + fx.delState.Add(map[string]struct{}{id: {}}) require.Contains(t, fx.delState.queued, id) require.Equal(t, []string{id}, queued) } diff --git a/commonspace/settings/settingsstate/mock_settingsstate/mock_settingsstate.go b/commonspace/settings/settingsstate/mock_settingsstate/mock_settingsstate.go index 41b17a90..b19a26ee 100644 --- a/commonspace/settings/settingsstate/mock_settingsstate/mock_settingsstate.go +++ b/commonspace/settings/settingsstate/mock_settingsstate/mock_settingsstate.go @@ -36,7 +36,7 @@ func (m *MockObjectDeletionState) EXPECT() *MockObjectDeletionStateMockRecorder } // Add mocks base method. -func (m *MockObjectDeletionState) Add(arg0 []string) { +func (m *MockObjectDeletionState) Add(arg0 map[string]struct{}) { m.ctrl.T.Helper() m.ctrl.Call(m, "Add", arg0) } diff --git a/commonspace/settings/settingsstate/settingsstate.go b/commonspace/settings/settingsstate/settingsstate.go index e2d410d0..f20463d7 100644 --- a/commonspace/settings/settingsstate/settingsstate.go +++ b/commonspace/settings/settingsstate/settingsstate.go @@ -1,15 +1,30 @@ package settingsstate -import "golang.org/x/exp/slices" +import "github.com/anytypeio/any-sync/commonspace/spacesyncproto" type State struct { - DeletedIds []string + DeletedIds map[string]struct{} DeleterId string LastIteratedId string } +func NewState() *State { + return &State{DeletedIds: map[string]struct{}{}} +} + +func NewStateFromSnapshot(snapshot *spacesyncproto.SpaceSettingsSnapshot, lastIteratedId string) *State { + st := NewState() + for _, id := range snapshot.DeletedIds { + st.DeletedIds[id] = struct{}{} + } + st.DeleterId = snapshot.DeleterPeerId + st.LastIteratedId = lastIteratedId + return st +} + func (s *State) Exists(id string) bool { // using map here will not give a lot of benefit, because this thing should be called only // when we are adding content, thus it doesn't matter - return slices.Contains(s.DeletedIds, id) + _, exists := s.DeletedIds[id] + return exists } diff --git a/commonspace/settings/settingsstate/statebuilder.go b/commonspace/settings/settingsstate/statebuilder.go index cc13bb24..7f171ec3 100644 --- a/commonspace/settings/settingsstate/statebuilder.go +++ b/commonspace/settings/settingsstate/statebuilder.go @@ -24,7 +24,7 @@ func (s *stateBuilder) Build(tr objecttree.ReadableObjectTree, oldState *State) ) state = oldState if state == nil { - state = &State{} + state = NewState() } else if state.LastIteratedId != "" { startId = state.LastIteratedId } @@ -55,11 +55,7 @@ func (s *stateBuilder) processChange(change *objecttree.Change, rootId string, s deleteChange := change.Model.(*spacesyncproto.SettingsData) // getting data from snapshot if we start from it if change.Id == rootId { - state = &State{ - DeletedIds: deleteChange.Snapshot.DeletedIds, - DeleterId: deleteChange.Snapshot.DeleterPeerId, - LastIteratedId: rootId, - } + state = NewStateFromSnapshot(deleteChange.Snapshot, rootId) return state } @@ -67,7 +63,7 @@ func (s *stateBuilder) processChange(change *objecttree.Change, rootId string, s for _, cnt := range deleteChange.Content { switch { case cnt.GetObjectDelete() != nil: - state.DeletedIds = append(state.DeletedIds, cnt.GetObjectDelete().GetId()) + state.DeletedIds[cnt.GetObjectDelete().GetId()] = struct{}{} case cnt.GetSpaceDelete() != nil: state.DeleterId = cnt.GetSpaceDelete().GetDeleterPeerId() } diff --git a/commonspace/settings/settingsstate/statebuilder_test.go b/commonspace/settings/settingsstate/statebuilder_test.go index e2ac4448..3119814b 100644 --- a/commonspace/settings/settingsstate/statebuilder_test.go +++ b/commonspace/settings/settingsstate/statebuilder_test.go @@ -17,9 +17,9 @@ func TestStateBuilder_ProcessChange(t *testing.T) { t.Run("empty model", func(t *testing.T) { ch := &objecttree.Change{} newSt := sb.processChange(ch, rootId, &State{ - DeletedIds: []string{deletedId}, + DeletedIds: map[string]struct{}{deletedId: struct{}{}}, }) - require.Equal(t, []string{deletedId}, newSt.DeletedIds) + require.Equal(t, map[string]struct{}{deletedId: struct{}{}}, newSt.DeletedIds) }) t.Run("changeId is equal to startId, LastIteratedId is equal to startId", func(t *testing.T) { @@ -34,10 +34,10 @@ func TestStateBuilder_ProcessChange(t *testing.T) { ch.Id = "startId" startId := "startId" newSt := sb.processChange(ch, rootId, &State{ - DeletedIds: []string{deletedId}, + DeletedIds: map[string]struct{}{deletedId: struct{}{}}, LastIteratedId: startId, }) - require.Equal(t, []string{deletedId}, newSt.DeletedIds) + require.Equal(t, map[string]struct{}{deletedId: struct{}{}}, newSt.DeletedIds) }) t.Run("changeId is equal to rootId", func(t *testing.T) { @@ -50,8 +50,8 @@ func TestStateBuilder_ProcessChange(t *testing.T) { }, } ch.Id = "rootId" - newSt := sb.processChange(ch, rootId, &State{}) - require.Equal(t, []string{"id1", "id2"}, newSt.DeletedIds) + newSt := sb.processChange(ch, rootId, NewState()) + require.Equal(t, map[string]struct{}{"id1": struct{}{}, "id2": struct{}{}}, newSt.DeletedIds) require.Equal(t, "peerId", newSt.DeleterId) }) @@ -66,8 +66,8 @@ func TestStateBuilder_ProcessChange(t *testing.T) { }, } ch.Id = "someId" - newSt := sb.processChange(ch, rootId, &State{}) - require.Equal(t, []string{deletedId}, newSt.DeletedIds) + newSt := sb.processChange(ch, rootId, NewState()) + require.Equal(t, map[string]struct{}{deletedId: struct{}{}}, newSt.DeletedIds) }) }