Skip to content

Commit

Permalink
permissions-center: make PermissionSyncCodeHostState's status typed. (#…
Browse files Browse the repository at this point in the history 
…49834)

This commit makes status typed instead of a raw string which will be
further used in API and UI layers as well for increased stability and
removal of possibility of unexpected string results being passed over
API.

API/UI changes will be made in a follow-up commit.

Test plan:
Unit tests updated.
  • Loading branch information
@sashaostrikov
sashaostrikov committed Mar 22, 2023
1 parent 21f4088 commit ccf9b20
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 35 deletions.
2 changes: 1 addition & 1 deletion enterprise/cmd/frontend/internal/authz/resolvers/permissions_sync_jobs.go
View file
Expand Up @@ -266,7 +266,7 @@ func (c codeHostStateResolver) ProviderType() string {
}

func (c codeHostStateResolver) Status() string {
return c.state.Status
return string(c.state.Status)
}

func (c codeHostStateResolver) Message() string {
Expand Down
12 changes: 6 additions & 6 deletions enterprise/cmd/repo-updater/internal/authz/integration_test.go
View file
Expand Up @@ -145,7 +145,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchRepoPerms",
}}, providerStates)

Expand Down Expand Up @@ -230,7 +230,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchRepoPerms",
}}, providerStates)

Expand All @@ -256,7 +256,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchRepoPerms",
}}, providerStates)

Expand Down Expand Up @@ -353,7 +353,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}}, providerStates)

Expand Down Expand Up @@ -441,7 +441,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}}, providerStates)

Expand All @@ -467,7 +467,7 @@ func TestIntegration_GitHubPermissions(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://github.com/",
ProviderType: "github",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}}, providerStates)

Expand Down
22 changes: 11 additions & 11 deletions enterprise/cmd/repo-updater/internal/authz/perms_syncer_test.go
View file
Expand Up @@ -99,11 +99,11 @@ func (p *mockProvider) URN() string { return extsvc.URN(p.serviceType, p

func (*mockProvider) ValidateConnection(context.Context) error { return nil }

func (p *mockProvider) FetchUserPerms(ctx context.Context, acct *extsvc.Account, opts authz.FetchPermsOptions) (*authz.ExternalUserPermissions, error) {
func (p *mockProvider) FetchUserPerms(ctx context.Context, acct *extsvc.Account, _ authz.FetchPermsOptions) (*authz.ExternalUserPermissions, error) {
return p.fetchUserPerms(ctx, acct)
}

func (p *mockProvider) FetchUserPermsByToken(ctx context.Context, token string, opts authz.FetchPermsOptions) (*authz.ExternalUserPermissions, error) {
func (p *mockProvider) FetchUserPermsByToken(ctx context.Context, token string, _ authz.FetchPermsOptions) (*authz.ExternalUserPermissions, error) {
return p.fetchUserPermsByToken(ctx, token)
}

Expand Down Expand Up @@ -201,7 +201,7 @@ func TestPermsSyncer_syncUserPerms(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}}, providers)
}
Expand Down Expand Up @@ -384,7 +384,7 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
statuses: database.CodeHostStatusesSet{{
ProviderID: p1.serviceID,
ProviderType: p1.serviceType,
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}},
},
Expand All @@ -393,12 +393,12 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
statuses: database.CodeHostStatusesSet{{
ProviderID: p2.serviceID,
ProviderType: p2.serviceType,
Status: "ERROR",
Status: database.CodeHostStatusError,
Message: "FetchAccount: no account found for this user",
}, {
ProviderID: p1.serviceID,
ProviderType: p1.serviceType,
Status: "SUCCESS",
Status: database.CodeHostStatusSuccess,
Message: "FetchUserPerms",
}},
fetchAccountError: errors.New("no account found for this user"),
Expand All @@ -408,7 +408,7 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
statuses: database.CodeHostStatusesSet{{
ProviderID: p1.serviceID,
ProviderType: p1.serviceType,
Status: "ERROR",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: horse error",
}},
fetchUserPermsError: errors.New("horse error"),
Expand All @@ -418,12 +418,12 @@ func TestPermsSyncer_syncUserPerms_fetchAccount(t *testing.T) {
statuses: database.CodeHostStatusesSet{{
ProviderID: p2.serviceID,
ProviderType: p2.serviceType,
Status: "ERROR",
Status: database.CodeHostStatusError,
Message: "FetchAccount: no account found for this user",
}, {
ProviderID: p1.serviceID,
ProviderType: p1.serviceType,
Status: "ERROR",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: horse error",
}},
fetchAccountError: errors.New("no account found for this user"),
Expand Down Expand Up @@ -549,7 +549,7 @@ func TestPermsSyncer_syncUserPermsTemporaryProviderError(t *testing.T) {
assert.Equal(t, database.CodeHostStatusesSet{{
ProviderID: "https://gitlab.com/",
ProviderType: "gitlab",
Status: "ERROR",
Status: database.CodeHostStatusError,
Message: "FetchUserPerms: context deadline exceeded",
}}, providers)
}
Expand Down Expand Up @@ -1106,7 +1106,7 @@ func TestPermsSyncer_syncRepoPerms(t *testing.T) {

assert.Greater(t, len(providerStates), 0)
for _, ps := range providerStates {
if ps.Status == "ERROR" {
if ps.Status == database.CodeHostStatusError {
t.Fatal("Did not expect provider status of ERROR")
}
}
Expand Down
2 changes: 1 addition & 1 deletion enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker.go
View file
Expand Up @@ -106,7 +106,7 @@ func (h *permsSyncerWorker) handlePermsSync(ctx context.Context, reqType request
if err == nil {
allProvidersFailedToSync := len(providerStates) > 0
for _, state := range providerStates {
if state.Status != "ERROR" {
if state.Status != database.CodeHostStatusError {
allProvidersFailedToSync = false
break
}
Expand Down
8 changes: 4 additions & 4 deletions enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker_test.go
View file
Expand Up @@ -502,10 +502,10 @@ func (d *dummySyncerWithErrors) syncRepoPerms(_ context.Context, repoID api.Repo
Options: options,
}

codeHostStates := database.CodeHostStatusesSet{{ProviderID: "id1", Status: "SUCCESS"}, {ProviderID: "id2", Status: "SUCCESS"}}
codeHostStates := database.CodeHostStatusesSet{{ProviderID: "id1", Status: database.CodeHostStatusSuccess}, {ProviderID: "id2", Status: database.CodeHostStatusSuccess}}
if typ, ok := d.repoIDErrors[repoID]; ok && typ == allProvidersFailed {
for idx := range codeHostStates {
codeHostStates[idx].Status = "ERROR"
codeHostStates[idx].Status = database.CodeHostStatusError
}
}

Expand All @@ -524,10 +524,10 @@ func (d *dummySyncerWithErrors) syncUserPerms(_ context.Context, userID int32, n
Options: options,
}

codeHostStates := database.CodeHostStatusesSet{{ProviderID: "id1", Status: "ERROR"}, {ProviderID: "id2", Status: "SUCCESS"}}
codeHostStates := database.CodeHostStatusesSet{{ProviderID: "id1", Status: database.CodeHostStatusError}, {ProviderID: "id2", Status: database.CodeHostStatusSuccess}}
if typ, ok := d.userIDErrors[userID]; ok && typ == allProvidersFailed {
for idx := range codeHostStates {
codeHostStates[idx].Status = "ERROR"
codeHostStates[idx].Status = database.CodeHostStatusError
}
}

Expand Down
25 changes: 15 additions & 10 deletions internal/database/permission_sync_code_host_state.go
View file
Expand Up @@ -12,14 +12,19 @@ import (

// PermissionSyncCodeHostState describes the state of a provider during an authz sync job.
type PermissionSyncCodeHostState struct {
ProviderID string `json:"provider_id"`
ProviderType string `json:"provider_type"`

// Status is one of "ERROR" or "SUCCESS".
Status string `json:"status"`
Message string `json:"message"`
ProviderID string `json:"provider_id"`
ProviderType string `json:"provider_type"`
Status CodeHostStatus `json:"status"`
Message string `json:"message"`
}

type CodeHostStatus string

const (
CodeHostStatusSuccess CodeHostStatus = "SUCCESS"
CodeHostStatusError CodeHostStatus = "ERROR"
)

func (e *PermissionSyncCodeHostState) Scan(value any) error {
b, ok := value.([]byte)
if !ok {
Expand All @@ -38,14 +43,14 @@ func NewProviderStatus(provider authz.Provider, err error, action string) Permis
return PermissionSyncCodeHostState{
ProviderID: provider.ServiceID(),
ProviderType: provider.ServiceType(),
Status: "ERROR",
Status: CodeHostStatusError,
Message: fmt.Sprintf("%s: %s", action, err.Error()),
}
} else {
return PermissionSyncCodeHostState{
ProviderID: provider.ServiceID(),
ProviderType: provider.ServiceType(),
Status: "SUCCESS",
Status: CodeHostStatusSuccess,
Message: action,
}
}
Expand All @@ -62,12 +67,12 @@ func (ps CodeHostStatusesSet) SummaryField() log.Field {
for _, p := range ps {
key := fmt.Sprintf("%s:%s", p.ProviderType, p.ProviderID)
switch p.Status {
case "ERROR":
case CodeHostStatusError:
errored = append(errored, log.String(
key,
p.Message,
))
case "SUCCESS":
case CodeHostStatusSuccess:
succeeded = append(errored, log.String(
key,
p.Message,
Expand Down
4 changes: 2 additions & 2 deletions internal/database/permission_sync_jobs_test.go
View file
Expand Up @@ -853,13 +853,13 @@ func getSampleCodeHostStates() []PermissionSyncCodeHostState {
{
ProviderID: "ID",
ProviderType: "Type",
Status: "SUCCESS",
Status: CodeHostStatusSuccess,
Message: "successful success",
},
{
ProviderID: "ID",
ProviderType: "Type",
Status: "ERROR",
Status: CodeHostStatusError,
Message: "unsuccessful unsuccess :(",
},
}
Expand Down

0 comments on commit ccf9b20

Please sign in to comment.