Update state builder to use map and fix tests

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()

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 {
+		mapIds := map[string]struct{}{}
-			return id == d.settingsId
+		for _, id := range d.provider.AllIds() {
-		})
+			if id != d.settingsId {
-		d.deletionState.Add(allIds)
+				mapIds[id] = struct{}{}
+			}
+		}
+		d.deletionState.Add(mapIds)
 	}
 	d.onSpaceDelete()
 	return nil

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,

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,

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",

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
 		}

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)
 }

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)
 }

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
 }

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{
+		state = NewStateFromSnapshot(deleteChange.Snapshot, rootId)
-			DeletedIds:     deleteChange.Snapshot.DeletedIds,
-			DeleterId:      deleteChange.Snapshot.DeleterPeerId,
-			LastIteratedId: 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()
 		}

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{})
+		newSt := sb.processChange(ch, rootId, NewState())
-		require.Equal(t, []string{"id1", "id2"}, newSt.DeletedIds)
+		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{})
+		newSt := sb.processChange(ch, rootId, NewState())
-		require.Equal(t, []string{deletedId}, newSt.DeletedIds)
+		require.Equal(t, map[string]struct{}{deletedId: struct{}{}}, newSt.DeletedIds)
 	})
 }