Merge pull request #1728 from anyproto/GO-4338-fix-untitled-objects

Go 4338 fix untitled objects

core/indexer/reindex.go

@@ -46,35 +46,34 @@ const (
 
 	// ForceMarketplaceReindex forces to do reindex only for marketplace space
 	ForceMarketplaceReindex int32 = 1
+
+	ForceReindexDeletedObjectsCounter int32 = 1
 )
 
+type allDeletedIdsProvider interface {
+	AllDeletedTreeIds() (ids []string, err error)
+}
+
 func (i *indexer) buildFlags(spaceID string) (reindexFlags, error) {
 	checksums, err := i.store.GetChecksums(spaceID)
 	if err != nil && !errors.Is(err, anystore.ErrDocNotFound) {
 		return reindexFlags{}, err
 	}
 	if checksums == nil {
-		checksums, err = i.store.GetGlobalChecksums()
-		if err != nil && !errors.Is(err, anystore.ErrDocNotFound) {
-			return reindexFlags{}, err
-		}
-
-		if checksums == nil {
-			checksums = &model.ObjectStoreChecksums{
-				// per space
-				ObjectsForceReindexCounter: ForceObjectsReindexCounter,
-				// ?
-				FilesForceReindexCounter: ForceFilesReindexCounter,
-				// global
-				IdxRebuildCounter: ForceIdxRebuildCounter,
-				// per space
-				FilestoreKeysForceReindexCounter: ForceFilestoreKeysReindexCounter,
-				LinksErase:                       ForceLinksReindexCounter,
-				// global
-				BundledObjects:             ForceBundledObjectsReindexCounter,
-				AreOldFilesRemoved:         true,
-				AreDeletedObjectsReindexed: true,
-			}
+		checksums = &model.ObjectStoreChecksums{
+			// per space
+			ObjectsForceReindexCounter: ForceObjectsReindexCounter,
+			// ?
+			FilesForceReindexCounter: ForceFilesReindexCounter,
+			// global
+			IdxRebuildCounter: ForceIdxRebuildCounter,
+			// per space
+			FilestoreKeysForceReindexCounter: ForceFilestoreKeysReindexCounter,
+			LinksErase:                       ForceLinksReindexCounter,
+			// global
+			BundledObjects:        ForceBundledObjectsReindexCounter,
+			AreOldFilesRemoved:    true,
+			ReindexDeletedObjects: 0, // Set to zero to force reindexing of deleted objects when objectstore was deleted
 		}
 	}
 
@@ -106,7 +105,7 @@ func (i *indexer) buildFlags(spaceID string) (reindexFlags, error) {
 	if !checksums.AreOldFilesRemoved {
 		flags.removeOldFiles = true
 	}
-	if !checksums.AreDeletedObjectsReindexed {
+	if checksums.ReindexDeletedObjects != ForceReindexDeletedObjectsCounter {
 		flags.deletedObjects = true
 	}
 	if checksums.LinksErase != ForceLinksReindexCounter {
@@ -237,34 +236,18 @@ func (i *indexer) addSyncDetails(space clientspace.Space) {
 
 func (i *indexer) reindexDeletedObjects(space clientspace.Space) error {
 	store := i.store.SpaceIndex(space.Id())
-	recs, err := store.Query(database.Query{
-		Filters: []*model.BlockContentDataviewFilter{
-			{
-				RelationKey: bundle.RelationKeyIsDeleted.String(),
-				Condition:   model.BlockContentDataviewFilter_Equal,
-				Value:       pbtypes.Bool(true),
-			},
-			{
-				RelationKey: bundle.RelationKeySpaceId.String(),
-				Condition:   model.BlockContentDataviewFilter_Empty,
-			},
-		},
-	})
+	storage, ok := space.Storage().(allDeletedIdsProvider)
+	if !ok {
+		return fmt.Errorf("space storage doesn't implement allDeletedIdsProvider")
+	}
+	allIds, err := storage.AllDeletedTreeIds()
 	if err != nil {
-		return fmt.Errorf("query deleted objects: %w", err)
+		return fmt.Errorf("get deleted tree ids: %w", err)
 	}
-	for _, rec := range recs {
-		objectId := pbtypes.GetString(rec.Details, bundle.RelationKeyId.String())
-		status, err := space.Storage().TreeDeletedStatus(objectId)
+	for _, objectId := range allIds {
+		err = store.DeleteObject(objectId)
 		if err != nil {
-			log.With("spaceId", space.Id(), "objectId", objectId).Warnf("failed to get tree deleted status: %s", err)
-			continue
-		}
-		if status != "" {
-			err = store.DeleteObject(objectId)
-			if err != nil {
-				log.With("spaceId", space.Id(), "objectId", objectId).Errorf("failed to reindex deleted object: %s", err)
-			}
+			log.With("spaceId", space.Id(), "objectId", objectId, "error", err).Errorf("failed to reindex deleted object")
 		}
 	}
 	return nil
@@ -516,6 +499,7 @@ func (i *indexer) getLatestChecksums(isMarketplace bool) (checksums model.Object
 		AreOldFilesRemoved:               true,
 		AreDeletedObjectsReindexed:       true,
 		LinksErase:                       ForceLinksReindexCounter,
+		ReindexDeletedObjects:            ForceReindexDeletedObjectsCounter,
 	}
 	if isMarketplace {
 		checksums.MarketplaceForceReindexCounter = ForceMarketplaceReindex

core/indexer/reindex_test.go

@@ -4,13 +4,10 @@ import (
 	"context"
 	"testing"
 
-	"github.com/anyproto/any-sync/commonspace/spacestorage"
-	"github.com/anyproto/any-sync/commonspace/spacestorage/mock_spacestorage"
 	"github.com/gogo/protobuf/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/mock/gomock"
 
 	"github.com/anyproto/anytype-heart/core/block/editor"
 	"github.com/anyproto/anytype-heart/core/block/editor/smartblock/smarttest"
@@ -149,91 +146,6 @@ func TestReindexMarketplaceSpace(t *testing.T) {
 	})
 }
 
-func TestReindexDeletedObjects(t *testing.T) {
-	const (
-		spaceId1 = "spaceId1"
-		spaceId2 = "spaceId2"
-		spaceId3 = "spaceId3"
-	)
-	fx := NewIndexerFixture(t)
-	fx.sourceFx.EXPECT().IDsListerBySmartblockType(mock.Anything, mock.Anything).Return(idsLister{Ids: []string{}}, nil).Maybe()
-
-	fx.objectStore.AddObjects(t, spaceId1, []objectstore.TestObject{
-		{
-			bundle.RelationKeyId:        pbtypes.String("1"),
-			bundle.RelationKeyIsDeleted: pbtypes.Bool(true),
-		},
-	})
-	fx.objectStore.AddObjects(t, spaceId2, []objectstore.TestObject{
-		{
-			bundle.RelationKeyId:        pbtypes.String("2"),
-			bundle.RelationKeyIsDeleted: pbtypes.Bool(true),
-		},
-	})
-	fx.objectStore.AddObjects(t, spaceId3, []objectstore.TestObject{
-		{
-			bundle.RelationKeyId:        pbtypes.String("3"),
-			bundle.RelationKeyIsDeleted: pbtypes.Bool(true),
-			bundle.RelationKeySpaceId:   pbtypes.String(spaceId3),
-		},
-		{
-			bundle.RelationKeyId:   pbtypes.String("4"),
-			bundle.RelationKeyName: pbtypes.String("4"),
-		},
-	})
-
-	checksums := fx.getLatestChecksums(false)
-	checksums.AreDeletedObjectsReindexed = false
-
-	err := fx.objectStore.SaveChecksums(spaceId1, &checksums)
-	require.NoError(t, err)
-	err = fx.objectStore.SaveChecksums(spaceId2, &checksums)
-	require.NoError(t, err)
-
-	t.Run("reindex first space", func(t *testing.T) {
-		storage1 := mock_spacestorage.NewMockSpaceStorage(gomock.NewController(t))
-		storage1.EXPECT().TreeDeletedStatus("1").Return(spacestorage.TreeDeletedStatusDeleted, nil)
-		space1 := mock_space.NewMockSpace(t)
-		space1.EXPECT().Id().Return(spaceId1)
-		space1.EXPECT().Storage().Return(storage1)
-		space1.EXPECT().StoredIds().Return([]string{}).Maybe()
-
-		err = fx.ReindexSpace(space1)
-		require.NoError(t, err)
-
-		sums, err := fx.objectStore.GetChecksums(spaceId1)
-		require.NoError(t, err)
-
-		assert.True(t, sums.AreDeletedObjectsReindexed)
-	})
-
-	t.Run("reindex second space", func(t *testing.T) {
-		storage2 := mock_spacestorage.NewMockSpaceStorage(gomock.NewController(t))
-		storage2.EXPECT().TreeDeletedStatus("2").Return(spacestorage.TreeDeletedStatusDeleted, nil)
-		space2 := mock_space.NewMockSpace(t)
-		space2.EXPECT().Id().Return(spaceId2)
-		space2.EXPECT().Storage().Return(storage2)
-		space2.EXPECT().StoredIds().Return([]string{}).Maybe()
-
-		err = fx.ReindexSpace(space2)
-		require.NoError(t, err)
-
-		sums, err := fx.objectStore.GetChecksums(spaceId2)
-		require.NoError(t, err)
-
-		assert.True(t, sums.AreDeletedObjectsReindexed)
-	})
-
-	got := fx.queryDeletedObjectIds(t, spaceId1)
-	assert.Equal(t, []string{"1"}, got)
-
-	got = fx.queryDeletedObjectIds(t, spaceId2)
-	assert.Equal(t, []string{"2"}, got)
-
-	got = fx.queryDeletedObjectIds(t, spaceId3)
-	assert.Equal(t, []string{"3"}, got)
-}
-
 func TestIndexer_ReindexSpace_EraseLinks(t *testing.T) {
 	const (
 		spaceId1 = "space1"

docs/proto.md

@@ -27400,9 +27400,10 @@ Precondition: user A and user B opened the same block
 | bundledObjects | [int32](#int32) |  | anytypeProfile and maybe some others in the feature |
 | filestoreKeysForceReindexCounter | [int32](#int32) |  |  |
 | areOldFilesRemoved | [bool](#bool) |  |  |
-| areDeletedObjectsReindexed | [bool](#bool) |  |  |
+| areDeletedObjectsReindexed | [bool](#bool) |  | DEPRECATED |
 | linksErase | [int32](#int32) |  |  |
 | marketplaceForceReindexCounter | [int32](#int32) |  |  |
+| reindexDeletedObjects | [int32](#int32) |  |  |
 
 
 

pkg/lib/localstore/objectstore/indexer_store.go

@@ -110,10 +110,3 @@ func (s *dsObjectStore) SaveChecksums(spaceId string, checksums *model.ObjectSto
 	err = s.indexerChecksums.UpsertOne(s.componentCtx, it)
 	return err
 }
-
-// GetGlobalChecksums is a migration method, it returns checksums stored before we started to store them per space
-// it will be deleted after the first SaveChecksums() call
-func (s *dsObjectStore) GetGlobalChecksums() (checksums *model.ObjectStoreChecksums, err error) {
-	// TODO What to do?
-	return s.GetChecksums("global")
-}

pkg/lib/localstore/objectstore/indexer_store_test.go

@@ -55,13 +55,6 @@ func TestIndexerBatch(t *testing.T) {
 }
 
 func TestIndexerChecksums(t *testing.T) {
-	t.Run("previous checksums are not found", func(t *testing.T) {
-		s := NewStoreFixture(t)
-
-		_, err := s.GetGlobalChecksums()
-		require.Error(t, err)
-	})
-
 	t.Run("save and load checksums", func(t *testing.T) {
 		s := NewStoreFixture(t)
 

pkg/lib/localstore/objectstore/service.go

@@ -58,7 +58,6 @@ type IndexerStore interface {
 	AddToIndexQueue(ctx context.Context, id ...string) error
 	ListIdsFromFullTextQueue(limit int) ([]string, error)
 	RemoveIdsFromFullTextQueue(ids []string) error
-	GetGlobalChecksums() (checksums *model.ObjectStoreChecksums, err error)
 
 	// GetChecksums Used to get information about localstore state and decide do we need to reindex some objects
 	GetChecksums(spaceID string) (checksums *model.ObjectStoreChecksums, err error)