Skip to content

Commit

Permalink
Merge pull request #1358 from anyproto/fix-bugs
Browse files Browse the repository at this point in the history
Fix sync bugs and notification object indexing
  • Loading branch information
AnastasiaShemyakinskaya authored Jul 2, 2024
2 parents e6f684e + 4b584d2 commit e2c084a
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 77 deletions.
1 change: 1 addition & 0 deletions core/domain/syncstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const (
Syncing SpaceSyncStatus = 1
Error SpaceSyncStatus = 2
Offline SpaceSyncStatus = 3
Unknown SpaceSyncStatus = 4
)

type ObjectSyncStatus int32
Expand Down
15 changes: 15 additions & 0 deletions core/syncstatus/detailsupdater/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ func (u *syncStatusUpdater) setObjectDetails(syncStatusDetails *syncStatusDetail
if !changed {
return nil
}
if !u.isLayoutSuitableForSyncRelations(record) {
return nil
}
spc, err := u.spaceService.Get(u.ctx, syncStatusDetails.spaceId)
if err != nil {
return err
Expand Down Expand Up @@ -196,6 +199,18 @@ func (u *syncStatusUpdater) setObjectDetails(syncStatusDetails *syncStatusDetail
})
}

func (u *syncStatusUpdater) isLayoutSuitableForSyncRelations(details *types.Struct) bool {
layoutsWithoutSyncRelations := []float64{
float64(model.ObjectType_participant),
float64(model.ObjectType_dashboard),
float64(model.ObjectType_spaceView),
float64(model.ObjectType_space),
float64(model.ObjectType_date),
}
layout := details.Fields[bundle.RelationKeyLayout.String()].GetNumberValue()
return !slices.Contains(layoutsWithoutSyncRelations, layout)
}

func mapObjectSyncToSpaceSyncStatus(status domain.ObjectSyncStatus, syncError domain.SyncError) domain.SpaceSyncStatus {
switch status {
case domain.ObjectSynced:
Expand Down
38 changes: 25 additions & 13 deletions core/syncstatus/detailsupdater/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/anyproto/any-sync/app"
"github.com/anyproto/any-sync/app/ocache"
"github.com/cheggaaa/mb/v3"
"github.com/gogo/protobuf/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

Expand All @@ -18,6 +19,7 @@ import (
"github.com/anyproto/anytype-heart/pkg/lib/bundle"
coresb "github.com/anyproto/anytype-heart/pkg/lib/core/smartblock"
"github.com/anyproto/anytype-heart/pkg/lib/localstore/objectstore"
"github.com/anyproto/anytype-heart/pkg/lib/pb/model"
"github.com/anyproto/anytype-heart/space/clientspace/mock_clientspace"
"github.com/anyproto/anytype-heart/space/mock_space"
"github.com/anyproto/anytype-heart/tests/testutil"
Expand Down Expand Up @@ -256,23 +258,33 @@ func TestSyncStatusUpdater_setSyncDetails(t *testing.T) {
})
}

func TestSyncStatusUpdater_updateDetails(t *testing.T) {
t.Run("update sync status and date - no changes", func(t *testing.T) {
func TestSyncStatusUpdater_isLayoutSuitableForSyncRelations(t *testing.T) {
t.Run("isLayoutSuitableForSyncRelations - participant details", func(t *testing.T) {
// given
fixture := newFixture(t)
space := mock_clientspace.NewMockSpace(t)
fixture.service.EXPECT().Get(fixture.updater.ctx, "spaceId").Return(space, nil)
fixture.storeFixture.AddObjects(t, []objectstore.TestObject{
{
bundle.RelationKeyId: pbtypes.String("id"),
bundle.RelationKeySpaceId: pbtypes.String("spaceId"),
},
})
space.EXPECT().DoLockedIfNotExists("id", mock.Anything).Return(nil)

// when
fixture.statusUpdater.EXPECT().SendUpdate(domain.MakeSyncStatus("spaceId", domain.Synced, domain.Null, domain.Objects))
fixture.updater.updateDetails(&syncStatusDetails{nil, domain.ObjectSynced, domain.Null, "spaceId"})
details := &types.Struct{Fields: map[string]*types.Value{
bundle.RelationKeyLayout.String(): pbtypes.Float64(float64(model.ObjectType_participant)),
}}
isSuitable := fixture.updater.isLayoutSuitableForSyncRelations(details)

// then
assert.False(t, isSuitable)
})

t.Run("isLayoutSuitableForSyncRelations - basic details", func(t *testing.T) {
// given
fixture := newFixture(t)

// when
details := &types.Struct{Fields: map[string]*types.Value{
bundle.RelationKeyLayout.String(): pbtypes.Float64(float64(model.ObjectType_basic)),
}}
isSuitable := fixture.updater.isLayoutSuitableForSyncRelations(details)

// then
assert.True(t, isSuitable)
})
}

Expand Down
5 changes: 4 additions & 1 deletion core/syncstatus/spacesyncstatus/filestate.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ func (f *FileState) SetSyncStatusAndErr(status domain.SpaceSyncStatus, syncErr d
func (f *FileState) GetSyncStatus(spaceId string) domain.SpaceSyncStatus {
f.Lock()
defer f.Unlock()
return f.fileSyncStatusBySpace[spaceId]
if status, ok := f.fileSyncStatusBySpace[spaceId]; ok {
return status
}
return domain.Unknown
}

func (f *FileState) GetSyncObjectCount(spaceId string) int {
Expand Down
2 changes: 1 addition & 1 deletion core/syncstatus/spacesyncstatus/filestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestFileState_GetSyncStatus(t *testing.T) {
syncStatus := fileState.GetSyncStatus("spaceId")

// then
assert.Equal(t, domain.Synced, syncStatus)
assert.Equal(t, domain.Unknown, syncStatus)
})
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion core/syncstatus/spacesyncstatus/objectstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ func (o *ObjectState) SetSyncStatusAndErr(status domain.SpaceSyncStatus, syncErr
func (o *ObjectState) GetSyncStatus(spaceId string) domain.SpaceSyncStatus {
o.Lock()
defer o.Unlock()
return o.objectSyncStatusBySpace[spaceId]
if status, ok := o.objectSyncStatusBySpace[spaceId]; ok {
return status
}
return domain.Unknown
}

func (o *ObjectState) GetSyncObjectCount(spaceId string) int {
Expand Down
2 changes: 1 addition & 1 deletion core/syncstatus/spacesyncstatus/objectstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestObjectState_GetSyncStatus(t *testing.T) {
syncStatus := objectState.GetSyncStatus("spaceId")

// then
assert.Equal(t, domain.Synced, syncStatus)
assert.Equal(t, domain.Unknown, syncStatus)
})
}

Expand Down
34 changes: 23 additions & 11 deletions core/syncstatus/spacesyncstatus/spacestatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ type Updater interface {
type SpaceIdGetter interface {
app.Component
TechSpaceId() string
PersonalSpaceId() string
AllSpaceIds() []string
}

Expand Down Expand Up @@ -89,7 +88,7 @@ func (s *spaceSyncStatus) Run(ctx context.Context) (err error) {
close(s.finish)
return
} else {
s.sendStartEvent(s.spaceIdGetter.PersonalSpaceId())
s.sendStartEvent(s.spaceIdGetter.AllSpaceIds())
}
s.ctx, s.ctxCancel = context.WithCancel(context.Background())
go s.processEvents()
Expand All @@ -106,14 +105,16 @@ func (s *spaceSyncStatus) sendEventToSession(spaceId, token string) {
})
}

func (s *spaceSyncStatus) sendStartEvent(spaceId string) {
s.eventSender.Broadcast(&pb.Event{
Messages: []*pb.EventMessage{{
Value: &pb.EventMessageValueOfSpaceSyncStatusUpdate{
SpaceSyncStatusUpdate: s.makeSpaceSyncEvent(spaceId),
},
}},
})
func (s *spaceSyncStatus) sendStartEvent(spaceIds []string) {
for _, id := range spaceIds {
s.eventSender.Broadcast(&pb.Event{
Messages: []*pb.EventMessage{{
Value: &pb.EventMessageValueOfSpaceSyncStatusUpdate{
SpaceSyncStatusUpdate: s.makeSpaceSyncEvent(id),
},
}},
})
}
}

func (s *spaceSyncStatus) sendLocalOnlyEvent() {
Expand Down Expand Up @@ -183,7 +184,11 @@ func (s *spaceSyncStatus) isStatusNotChanged(status *domain.SpaceSync) bool {
return false
}
syncErrNotChanged := s.getError(status.SpaceId) == mapError(status.SyncError)
statusNotChanged := s.getSpaceSyncStatus(status.SpaceId) == status.Status
syncStatus := s.getSpaceSyncStatus(status.SpaceId)
if syncStatus == domain.Unknown {
return false
}
statusNotChanged := syncStatus == status.Status
if syncErrNotChanged && statusNotChanged {
return true
}
Expand Down Expand Up @@ -221,6 +226,9 @@ func (s *spaceSyncStatus) getSpaceSyncStatus(spaceId string) domain.SpaceSyncSta
filesStatus := s.filesState.GetSyncStatus(spaceId)
objectsStatus := s.objectsState.GetSyncStatus(spaceId)

if s.isUnknown(filesStatus, objectsStatus) {
return domain.Unknown
}
if s.isOfflineStatus(filesStatus, objectsStatus) {
return domain.Offline
}
Expand Down Expand Up @@ -276,6 +284,10 @@ func (s *spaceSyncStatus) getError(spaceId string) pb.EventSpaceSyncError {
return pb.EventSpace_Null
}

func (s *spaceSyncStatus) isUnknown(filesStatus domain.SpaceSyncStatus, objectsStatus domain.SpaceSyncStatus) bool {
return filesStatus == domain.Unknown && objectsStatus == domain.Unknown
}

func mapNetworkMode(mode pb.RpcAccountNetworkMode) pb.EventSpaceNetwork {
switch mode {
case pb.RpcAccount_LocalOnly:
Expand Down
36 changes: 33 additions & 3 deletions core/syncstatus/spacesyncstatus/spacestatus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestSpaceSyncStatus_Init(t *testing.T) {
// then
assert.Nil(t, err)

space.EXPECT().PersonalSpaceId().Return("personalId")
space.EXPECT().AllSpaceIds().Return([]string{"personalId"})
eventSender.EXPECT().Broadcast(&pb.Event{
Messages: []*pb.EventMessage{{
Value: &pb.EventMessageValueOfSpaceSyncStatusUpdate{
Expand Down Expand Up @@ -380,11 +380,11 @@ func TestSpaceSyncStatus_updateSpaceSyncStatus(t *testing.T) {
// when
assert.Equal(t, domain.Synced, status.objectsState.GetSyncStatus("spaceId"))
assert.Equal(t, 0, status.objectsState.GetSyncObjectCount("spaceId"))
assert.Equal(t, domain.Synced, status.filesState.GetSyncStatus("spaceId"))
assert.Equal(t, domain.Unknown, status.filesState.GetSyncStatus("spaceId"))
assert.Equal(t, 0, status.filesState.GetSyncObjectCount("spaceId"))
assert.Equal(t, domain.Synced, status.getSpaceSyncStatus(syncStatus.SpaceId))
})
t.Run("not send not needed synced event", func(t *testing.T) {
t.Run("send initial synced event", func(t *testing.T) {
// given
eventSender := mock_event.NewMockSender(t)
status := spaceSyncStatus{
Expand All @@ -394,8 +394,38 @@ func TestSpaceSyncStatus_updateSpaceSyncStatus(t *testing.T) {
filesState: NewFileState(objectstore.NewStoreFixture(t)),
objectsState: NewObjectState(objectstore.NewStoreFixture(t)),
}
eventSender.EXPECT().Broadcast(&pb.Event{
Messages: []*pb.EventMessage{{
Value: &pb.EventMessageValueOfSpaceSyncStatusUpdate{
SpaceSyncStatusUpdate: &pb.EventSpaceSyncStatusUpdate{
Id: "spaceId",
Status: pb.EventSpace_Synced,
Network: pb.EventSpace_SelfHost,
Error: pb.EventSpace_Null,
SyncingObjectsCounter: 0,
},
},
}},
})
// then
syncStatus := domain.MakeSyncStatus("spaceId", domain.Synced, domain.Null, domain.Objects)
status.updateSpaceSyncStatus(syncStatus)
})

t.Run("not send not needed synced event", func(t *testing.T) {
// given
eventSender := mock_event.NewMockSender(t)
status := spaceSyncStatus{
eventSender: eventSender,
networkConfig: &config.Config{NetworkMode: pb.RpcAccount_CustomConfig},
batcher: mb.New[*domain.SpaceSync](0),
filesState: NewFileState(objectstore.NewStoreFixture(t)),
objectsState: NewObjectState(objectstore.NewStoreFixture(t)),
}
status.objectsState.SetSyncStatusAndErr(domain.Synced, domain.Null, "spaceId")
status.filesState.SetSyncStatusAndErr(domain.Synced, domain.Null, "spaceId")

// then
syncStatus := domain.MakeSyncStatus("spaceId", domain.Synced, domain.Null, domain.Objects)
status.updateSpaceSyncStatus(syncStatus)

Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/core/smartblock/smartblock.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (sbt SmartBlockType) IsOneOf(sbts ...SmartBlockType) bool {
// Indexable determines if the object of specific type need to be proceeded by the indexer in order to appear in sets
func (sbt SmartBlockType) Indexable() (details, outgoingLinks bool) {
switch sbt {
case SmartBlockTypeDate, SmartBlockTypeAccountOld, SmartBlockTypeArchive, SmartBlockTypeHome:
case SmartBlockTypeDate, SmartBlockTypeAccountOld, SmartBlockTypeArchive, SmartBlockTypeHome, SmartBlockTypeNotificationObject:
return false, false
case SmartBlockTypeWidget:
return true, false
Expand Down

0 comments on commit e2c084a

Please sign in to comment.