Skip to content

GO-5379 fix the undeletable space #2272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions core/acl/aclservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/anyproto/anytype-heart/pkg/lib/logging"
"github.com/anyproto/anytype-heart/pkg/lib/pb/model"
"github.com/anyproto/anytype-heart/space"
space2 "github.com/anyproto/anytype-heart/space/spacecore"
"github.com/anyproto/anytype-heart/space/spaceinfo"
"github.com/anyproto/anytype-heart/space/techspace"
)
Expand Down Expand Up @@ -78,6 +79,7 @@ type aclService struct {
nodeConfigGetter NodeConfGetter
joiningClient aclclient.AclJoiningClient
spaceService space.Service
spaceCoreService space2.SpaceCoreService
inviteService inviteservice.InviteService
accountService account.Service
coordClient coordinatorclient.CoordinatorClient
Expand All @@ -88,6 +90,7 @@ func (a *aclService) Init(ap *app.App) (err error) {
a.nodeConfigGetter = app.MustComponent[NodeConfGetter](ap)
a.joiningClient = app.MustComponent[aclclient.AclJoiningClient](ap)
a.spaceService = app.MustComponent[space.Service](ap)
a.spaceCoreService = app.MustComponent[space2.SpaceCoreService](ap)
a.accountService = app.MustComponent[account.Service](ap)
a.inviteService = app.MustComponent[inviteservice.InviteService](ap)
a.coordClient = app.MustComponent[coordinatorclient.CoordinatorClient](ap)
Expand Down Expand Up @@ -295,22 +298,23 @@ func (a *aclService) ApproveLeave(ctx context.Context, spaceId string, identitie
}

func (a *aclService) Leave(ctx context.Context, spaceId string) error {
removeSpace, err := a.spaceService.Get(ctx, spaceId)
status, err := a.spaceService.SpacePersistentStatus(ctx, spaceId)
if err != nil {
return err
}
if status != spaceinfo.AccountStatusActive && status != spaceinfo.AccountStatusUnknown {
return fmt.Errorf("space status not active or unknown")
}
removeSpace, err := a.spaceCoreService.Get(ctx, spaceId)
if err != nil {
// space storage missing can occur only in case of missing space
if errors.Is(err, space.ErrSpaceStorageMissig) || errors.Is(err, space.ErrSpaceDeleted) {
return nil
}
return convertedOrSpaceErr(err)
}
identity := a.accountService.Keys().SignKey.GetPublic()
if !removeSpace.GetAclIdentity().Equals(identity) {
// this is a streamable space
// we exist there under ephemeral guest identity and should not remove it
return nil
}

cl := removeSpace.CommonSpace().AclClient()
cl := removeSpace.AclClient()
err = cl.RequestSelfRemove(ctx)
if err != nil {
errs := []error{
Expand Down
91 changes: 31 additions & 60 deletions core/acl/aclservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import (
"github.com/anyproto/anytype-heart/space/clientspace"
"github.com/anyproto/anytype-heart/space/clientspace/mock_clientspace"
"github.com/anyproto/anytype-heart/space/mock_space"
"github.com/anyproto/anytype-heart/space/spacecore"
"github.com/anyproto/anytype-heart/space/spacecore/mock_spacecore"
"github.com/anyproto/anytype-heart/space/spaceinfo"
"github.com/anyproto/anytype-heart/space/techspace"
"github.com/anyproto/anytype-heart/space/techspace/mock_techspace"
Expand Down Expand Up @@ -69,6 +71,7 @@ type fixture struct {
ctrl *gomock.Controller
mockJoiningClient *mock_aclclient.MockAclJoiningClient
mockSpaceService *mock_space.MockService
mockSpaceCoreService *mock_spacecore.MockSpaceCoreService
mockAccountService *mock_account.MockService
mockInviteService *mock_inviteservice.MockInviteService
mockCoordinatorClient *mock_coordinatorclient.MockCoordinatorClient
Expand All @@ -89,6 +92,7 @@ func newFixture(t *testing.T) *fixture {
ctrl: ctrl,
mockJoiningClient: mock_aclclient.NewMockAclJoiningClient(ctrl),
mockSpaceService: mock_space.NewMockService(t),
mockSpaceCoreService: mock_spacecore.NewMockSpaceCoreService(t),
mockAccountService: mock_account.NewMockService(t),
mockInviteService: mock_inviteservice.NewMockInviteService(t),
mockCoordinatorClient: mock_coordinatorclient.NewMockCoordinatorClient(ctrl),
Expand All @@ -103,6 +107,7 @@ func newFixture(t *testing.T) *fixture {
fx.a.Register(testutil.PrepareMock(ctx, fx.a, fx.mockAccountService)).
Register(testutil.PrepareMock(ctx, fx.a, fx.mockJoiningClient)).
Register(testutil.PrepareMock(ctx, fx.a, fx.mockSpaceService)).
Register(testutil.PrepareMock(ctx, fx.a, fx.mockSpaceCoreService)).
Register(testutil.PrepareMock(ctx, fx.a, fx.mockInviteService)).
Register(testutil.PrepareMock(ctx, fx.a, fx.mockCoordinatorClient)).
Register(fx.mockConfig).
Expand Down Expand Up @@ -272,9 +277,8 @@ func TestService_ApproveLeave(t *testing.T) {
identityC := exec.ActualAccounts()["c"].Keys.SignKey.GetPublic()
acl := mockSyncAcl{exec.ActualAccounts()["a"].Acl}
mockCommonSpace.EXPECT().Acl().Return(acl)
aclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl)
mockCommonSpace.EXPECT().AclClient().Return(aclClient)
aclClient.EXPECT().RemoveAccounts(ctx, gomock.Any()).DoAndReturn(func(ctx context.Context, payload list.AccountRemovePayload) error {
mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient)
fx.mockSpaceClient.EXPECT().RemoveAccounts(ctx, gomock.Any()).DoAndReturn(func(ctx context.Context, payload list.AccountRemovePayload) error {
require.Equal(t, []crypto.PubKey{identityB, identityC}, payload.Identities)
return nil
}).Return(nil)
Expand Down Expand Up @@ -487,19 +491,13 @@ func TestService_Leave(t *testing.T) {
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
mockSpace := mock_clientspace.NewMockSpace(t)
mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl)
mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl)
fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil)
keys, err := accountdata.NewRandom()
require.NoError(t, err)
fx.mockAccountService.EXPECT().Keys().Return(keys)
mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic())

mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace)
mockCommonSpace.EXPECT().AclClient().Return(mockAclClient)
mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(nil)
err = fx.Leave(ctx, spaceId)
anySpace := &spacecore.AnySpace{}
anySpace.Space = fx.mockCommonSpace
fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil)
fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil)
fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient)
fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(nil)
err := fx.Leave(ctx, spaceId)
require.NoError(t, err)
})
t.Run("leave success if space service error is known", func(t *testing.T) {
Expand All @@ -512,7 +510,8 @@ func TestService_Leave(t *testing.T) {
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(nil, err)
fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusActive, nil)
fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(nil, err)
err = fx.Leave(ctx, spaceId)
require.NoError(t, err)
})
Expand All @@ -522,7 +521,8 @@ func TestService_Leave(t *testing.T) {
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(nil, fmt.Errorf("error"))
fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusActive, nil)
fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(nil, fmt.Errorf("error"))
err := fx.Leave(ctx, spaceId)
require.True(t, errors.Is(err, ErrInternal))
})
Expand All @@ -539,17 +539,12 @@ func TestService_Leave(t *testing.T) {
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
mockSpace := mock_clientspace.NewMockSpace(t)
mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl)
mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl)
fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil)
keys, err := accountdata.NewRandom()
require.NoError(t, err)
fx.mockAccountService.EXPECT().Keys().Return(keys)
mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic())
mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace)
mockCommonSpace.EXPECT().AclClient().Return(mockAclClient)
mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(err)
anySpace := &spacecore.AnySpace{}
anySpace.Space = fx.mockCommonSpace
fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil)
fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil)
fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient)
fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(err)
err = fx.Leave(ctx, spaceId)
require.NoError(t, err)
})
Expand All @@ -559,37 +554,13 @@ func TestService_Leave(t *testing.T) {
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
mockSpace := mock_clientspace.NewMockSpace(t)
mockCommonSpace := mock_commonspace.NewMockSpace(fx.ctrl)
mockAclClient := mock_aclclient.NewMockAclSpaceClient(fx.ctrl)
fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil)
mockSpace.EXPECT().CommonSpace().Return(mockCommonSpace)
mockCommonSpace.EXPECT().AclClient().Return(mockAclClient)
keys, err := accountdata.NewRandom()
require.NoError(t, err)
fx.mockAccountService.EXPECT().Keys().Return(keys)
mockSpace.EXPECT().GetAclIdentity().Return(keys.SignKey.GetPublic())
mockAclClient.EXPECT().RequestSelfRemove(ctx).Return(fmt.Errorf("error"))
err = fx.Leave(ctx, spaceId)
anySpace := &spacecore.AnySpace{}
anySpace.Space = fx.mockCommonSpace
fx.mockSpaceService.EXPECT().SpacePersistentStatus(ctx, spaceId).Return(spaceinfo.AccountStatusUnknown, nil)
fx.mockSpaceCoreService.EXPECT().Get(ctx, spaceId).Return(anySpace, nil)
fx.mockCommonSpace.EXPECT().AclClient().Return(fx.mockSpaceClient)
fx.mockSpaceClient.EXPECT().RequestSelfRemove(ctx).Return(fmt.Errorf("error"))
err := fx.Leave(ctx, spaceId)
require.True(t, errors.Is(err, ErrAclRequestFailed))
})

t.Run("leave if acl key is not account key", func(t *testing.T) {
// this is a case of guest user trying to leave the space
fx := newFixture(t)
defer fx.finish(t)
spaceId := "spaceId"
mockSpace := mock_clientspace.NewMockSpace(t)

fx.mockSpaceService.EXPECT().Get(ctx, spaceId).Return(mockSpace, nil)
accountKeys, err := accountdata.NewRandom()
require.NoError(t, err)
aclKeys, err := accountdata.NewRandom()
require.NoError(t, err)
fx.mockAccountService.EXPECT().Keys().Return(accountKeys)
mockSpace.EXPECT().GetAclIdentity().Return(aclKeys.SignKey.GetPublic())

err = fx.Leave(ctx, spaceId)
require.NoError(t, err)
})
}
59 changes: 59 additions & 0 deletions space/mock_space/mock_Service.go

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

9 changes: 9 additions & 0 deletions space/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type Service interface {
AddStreamable(ctx context.Context, id string, guestKey crypto.PrivKey) (err error)
Delete(ctx context.Context, id string) (err error)
TechSpaceId() string
SpacePersistentStatus(ctx context.Context, spaceId string) (spaceinfo.AccountStatus, error)
PersonalSpaceId() string
FirstCreatedSpaceId() string
TechSpace() *clientspace.TechSpace
Expand Down Expand Up @@ -473,6 +474,14 @@ func (s *service) TechSpaceId() string {
return s.techSpaceId
}

func (s *service) SpacePersistentStatus(ctx context.Context, spaceId string) (spaceinfo.AccountStatus, error) {
controller, isFound := s.spaceControllers[spaceId]
if !isFound {
return spaceinfo.AccountStatusUnknown, fmt.Errorf("controller not found")
}
return controller.GetStatus(), nil
}

func (s *service) PersonalSpaceId() string {
return s.personalSpaceId
}
Expand Down
Loading