permissions-center: make cancelPermissionsSyncJob mutation return a message. (#48999)

We need to return a message in case of a sync job being already dequeued
at least because of two reasons:
- Permissions sync jobs processing is fast, especially on smaller
instances, by the time user clicks "Cancel job" and request arrives to
backend, the job may already be dequeued for processing and telling the
user that "The job is canceled" would be lying;
- We have a "Cancel polling" button on Permissions Dashboard, which
means that the user can have "legit stale data" and they are able to
attempt to cancel a job which might have been processed a long time ago.
We don't want to lie in this case too.

Test plan:
Unit tests updated.
GraphQL tests updated.
Local sg run and API tests.
This commit is contained in:
Alex Ostrikov 2023-03-09 17:22:21 +04:00 committed by GitHub
parent 66b55f1eab
commit 64cd22d127
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 60 additions and 14 deletions

View File

@ -18,7 +18,7 @@ type AuthzResolver interface {
ScheduleUserPermissionsSync(ctx context.Context, args *UserPermissionsSyncArgs) (*EmptyResponse, error)
SetSubRepositoryPermissionsForUsers(ctx context.Context, args *SubRepoPermsArgs) (*EmptyResponse, error)
SetRepositoryPermissionsForBitbucketProject(ctx context.Context, args *RepoPermsBitbucketProjectArgs) (*EmptyResponse, error)
CancelPermissionsSyncJob(ctx context.Context, args *CancelPermissionsSyncJobArgs) (*EmptyResponse, error)
CancelPermissionsSyncJob(ctx context.Context, args *CancelPermissionsSyncJobArgs) (*string, error)
//AuthorizedUserRepositories and functions below are GraphQL Queries.
AuthorizedUserRepositories(ctx context.Context, args *AuthorizedRepoArgs) (RepositoryConnectionResolver, error)

View File

@ -108,7 +108,7 @@ extend type Mutation {
Optional cancellation reason.
"""
reason: String
): EmptyResponse!
): String
}
extend type Query {

View File

@ -342,7 +342,7 @@ func (r *Resolver) SetRepositoryPermissionsForBitbucketProject(
return &graphqlbackend.EmptyResponse{}, nil
}
func (r *Resolver) CancelPermissionsSyncJob(ctx context.Context, args *graphqlbackend.CancelPermissionsSyncJobArgs) (*graphqlbackend.EmptyResponse, error) {
func (r *Resolver) CancelPermissionsSyncJob(ctx context.Context, args *graphqlbackend.CancelPermissionsSyncJobArgs) (*string, error) {
if err := r.checkLicense(licensing.FeatureACLs); err != nil {
return nil, err
}
@ -365,10 +365,13 @@ func (r *Resolver) CancelPermissionsSyncJob(ctx context.Context, args *graphqlba
err = r.db.PermissionSyncJobs().CancelQueuedJob(ctx, reason, syncJobID)
// We shouldn't return an error when the job is already processing or not found
// by ID (might already be cleaned up).
if err != nil && !errcode.IsNotFound(err) {
if err != nil {
if errcode.IsNotFound(err) {
return strPtr("No job that can be canceled found."), nil
}
return nil, err
}
return &graphqlbackend.EmptyResponse{}, nil
return strPtr("Permissions sync job canceled"), nil
}
func (r *Resolver) AuthorizedUserRepositories(ctx context.Context, args *graphqlbackend.AuthorizedRepoArgs) (graphqlbackend.RepositoryConnectionResolver, error) {
@ -626,3 +629,5 @@ func (r *Resolver) PermissionsSyncJobs(ctx context.Context, args graphqlbackend.
return NewPermissionsSyncJobsResolver(r.db, args)
}
func strPtr(s string) *string { return &s }

View File

@ -714,8 +714,34 @@ func TestResolver_CancelPermissionsSyncJob(t *testing.T) {
},
)
require.Equal(t, &graphqlbackend.EmptyResponse{}, result)
require.NoError(t, err)
require.Equal(t, "No job that can be canceled found.", *result)
})
t.Run("SQL error", func(t *testing.T) {
users := database.NewStrictMockUserStore()
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{SiteAdmin: true}, nil)
permissionSyncJobStore := database.NewMockPermissionSyncJobStore()
const errorText = "oops"
permissionSyncJobStore.CancelQueuedJobFunc.SetDefaultReturn(errors.New(errorText))
db := edb.NewStrictMockEnterpriseDB()
db.UsersFunc.SetDefaultReturn(users)
db.PermissionSyncJobsFunc.SetDefaultReturn(permissionSyncJobStore)
r := &Resolver{db: db, logger: logger}
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
result, err := r.CancelPermissionsSyncJob(ctx,
&graphqlbackend.CancelPermissionsSyncJobArgs{
Job: marshalPermissionsSyncJobID(1337),
},
)
require.EqualError(t, err, errorText)
require.Nil(t, result)
})
t.Run("sync job successfully cancelled", func(t *testing.T) {
@ -739,7 +765,7 @@ func TestResolver_CancelPermissionsSyncJob(t *testing.T) {
},
)
require.Equal(t, &graphqlbackend.EmptyResponse{}, result)
require.Equal(t, "Permissions sync job canceled", *result)
require.NoError(t, err)
})
}
@ -755,7 +781,7 @@ func TestResolver_CancelPermissionsSyncJob_GraphQLQuery(t *testing.T) {
if jobID == 1 && reason == "because" {
return nil
}
return errors.New("Oh no, something's wrong.")
return database.MockPermissionsSyncJobNotFoundErr
})
db := edb.NewStrictMockEnterpriseDB()
@ -770,16 +796,31 @@ func TestResolver_CancelPermissionsSyncJob_GraphQLQuery(t *testing.T) {
cancelPermissionsSyncJob(
job: "%s",
reason: "because"
) {
alwaysNil
}
)
}
`, marshalPermissionsSyncJobID(1)),
ExpectedResult: `
{
"cancelPermissionsSyncJob": {
"alwaysNil": null
}
"cancelPermissionsSyncJob": "Permissions sync job canceled"
}
`,
})
})
t.Run("sync job is already dequeued", func(t *testing.T) {
graphqlbackend.RunTest(t, &graphqlbackend.Test{
Schema: mustParseGraphQLSchema(t, db),
Query: fmt.Sprintf(`
mutation {
cancelPermissionsSyncJob(
job: "%s",
reason: "cause"
)
}
`, marshalPermissionsSyncJobID(42)),
ExpectedResult: `
{
"cancelPermissionsSyncJob": "No job that can be canceled found."
}
`,
})