diff --git a/internal/api/v1/entries.go b/internal/api/v1/entries.go index 4c84908d..e71d5b30 100644 --- a/internal/api/v1/entries.go +++ b/internal/api/v1/entries.go @@ -216,11 +216,6 @@ func (routes *Routes) updateEntryClaims(w http.ResponseWriter, r *http.Request) return } - if err := service.ValidateEntryType(entryType); err != nil { - common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) - return - } - var req updateEntryClaimsRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { common.WriteErrorResponse(w, "invalid request body: "+err.Error(), http.StatusBadRequest) @@ -239,6 +234,10 @@ func (routes *Routes) updateEntryClaims(w http.ResponseWriter, r *http.Request) } if err := routes.service.UpdateEntryClaims(r.Context(), opts...); err != nil { + if errors.Is(err, service.ErrInvalidEntryType) { + common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest) + return + } if errors.Is(err, service.ErrClaimsInsufficient) { common.WriteErrorResponse(w, err.Error(), http.StatusForbidden) return diff --git a/internal/api/v1/entries_test.go b/internal/api/v1/entries_test.go index c002eea1..3b3b956f 100644 --- a/internal/api/v1/entries_test.go +++ b/internal/api/v1/entries_test.go @@ -377,12 +377,15 @@ func TestUpdateEntryClaims(t *testing.T) { wantStatus: http.StatusNoContent, }, { - name: "unsupported entry type", - path: "/entries/unknown/test%2Fentry/claims", - body: mustMarshal(map[string]any{"claims": map[string]any{}}), - setupMock: func(_ *mocks.MockRegistryService) {}, + name: "unsupported entry type", + path: "/entries/unknown/test%2Fentry/claims", + body: mustMarshal(map[string]any{"claims": map[string]any{}}), + setupMock: func(m *mocks.MockRegistryService) { + m.EXPECT().UpdateEntryClaims(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(fmt.Errorf("invalid option: %w", service.ErrInvalidEntryType)) + }, wantStatus: http.StatusBadRequest, - wantError: "unsupported entry type", + wantError: "invalid entry type", }, { name: "invalid JSON body", diff --git a/internal/service/db/impl_entries.go b/internal/service/db/impl_entries.go index 449fb4a7..456dedd0 100644 --- a/internal/service/db/impl_entries.go +++ b/internal/service/db/impl_entries.go @@ -134,18 +134,15 @@ func (s *dbService) executeUpdateClaimsTransaction( } // mapEntryType converts a validated entry type string to the corresponding sqlc.EntryType. -// Relies on service.ValidateEntryType as the single source of truth for valid inputs. +// service.WithEntryType is the single source of truth for accepted values, so +// reaching the default branch indicates a caller bypassed the option setter. func mapEntryType(entryType string) (sqlc.EntryType, error) { - if err := service.ValidateEntryType(entryType); err != nil { - return "", err - } switch entryType { case service.EntryTypeServer: return sqlc.EntryTypeMCP, nil case service.EntryTypeSkill: return sqlc.EntryTypeSKILL, nil default: - // Unreachable: ValidateEntryType guards the set of accepted values. - return "", fmt.Errorf("unsupported entry type: %s", entryType) + return "", fmt.Errorf("%w: %s", service.ErrInvalidEntryType, entryType) } } diff --git a/internal/service/entry_types.go b/internal/service/entry_types.go index 8f88e8d2..ea326bd7 100644 --- a/internal/service/entry_types.go +++ b/internal/service/entry_types.go @@ -1,21 +1,7 @@ package service -import "fmt" - // Supported entry types for registry operations. const ( EntryTypeServer = "server" EntryTypeSkill = "skill" ) - -// ValidateEntryType returns nil if entryType is a supported entry type, -// or an error otherwise. It is the single source of truth for the set of -// valid entry type strings used across the API, options, and service layers. -func ValidateEntryType(entryType string) error { - switch entryType { - case EntryTypeServer, EntryTypeSkill: - return nil - default: - return fmt.Errorf("unsupported entry type: must be %q or %q", EntryTypeServer, EntryTypeSkill) - } -} diff --git a/internal/service/options.go b/internal/service/options.go index d276ed20..9225b882 100644 --- a/internal/service/options.go +++ b/internal/service/options.go @@ -217,7 +217,7 @@ func WithJWTClaims(claims map[string]any) Option { // WithEntryType sets the entry type for the UpdateEntryClaims operation. // Valid values are defined in entry_types.go (EntryTypeServer, EntryTypeSkill). -// The setter fully validates the value via ValidateEntryType. +// Invalid values produce an error wrapping ErrInvalidEntryType. func WithEntryType(entryType string) Option { return func(o any) error { switch o := o.(type) { diff --git a/internal/service/options_entries.go b/internal/service/options_entries.go index 93a1c898..330317a4 100644 --- a/internal/service/options_entries.go +++ b/internal/service/options_entries.go @@ -1,5 +1,7 @@ package service +import "fmt" + // UpdateEntryClaimsOptions is the options for the UpdateEntryClaims operation. type UpdateEntryClaimsOptions struct { EntryType string // EntryTypeServer or EntryTypeSkill @@ -9,11 +11,13 @@ type UpdateEntryClaimsOptions struct { } func (o *UpdateEntryClaimsOptions) setEntryType(entryType string) error { - if err := ValidateEntryType(entryType); err != nil { - return err + switch entryType { + case EntryTypeServer, EntryTypeSkill: + o.EntryType = entryType + return nil + default: + return fmt.Errorf("%w: must be %q or %q", ErrInvalidEntryType, EntryTypeServer, EntryTypeSkill) } - o.EntryType = entryType - return nil } //nolint:unparam diff --git a/internal/service/options_entries_test.go b/internal/service/options_entries_test.go index 80fd2d33..480c65d8 100644 --- a/internal/service/options_entries_test.go +++ b/internal/service/options_entries_test.go @@ -32,6 +32,7 @@ func TestWithEntryType(t *testing.T) { if tt.wantErr { require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidEntryType) assert.Empty(t, opts.EntryType, "EntryType must remain unset on error") return } @@ -94,32 +95,3 @@ func TestUpdateEntryClaimsOptions_Setters(t *testing.T) { assert.Equal(t, map[string]any{"sub": "u1"}, opts.JWTClaims) }) } - -func TestValidateEntryType(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - input string - wantErr bool - }{ - {name: "server", input: EntryTypeServer, wantErr: false}, - {name: "skill", input: EntryTypeSkill, wantErr: false}, - {name: "empty", input: "", wantErr: true}, - {name: "unknown", input: "widget", wantErr: true}, - {name: "uppercase server", input: "SERVER", wantErr: true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := ValidateEntryType(tt.input) - if tt.wantErr { - require.Error(t, err) - assert.Contains(t, err.Error(), "unsupported entry type") - } else { - require.NoError(t, err) - } - }) - } -} diff --git a/internal/service/service.go b/internal/service/service.go index e54b85c3..08033fbb 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -61,6 +61,8 @@ var ( ErrClaimsMismatch = errors.New("claims mismatch") // ErrClaimsInsufficient is returned when the caller's JWT claims do not cover a resource's claims ErrClaimsInsufficient = errors.New("insufficient claims") + // ErrInvalidEntryType is returned when an unsupported entry type string is supplied to an option + ErrInvalidEntryType = errors.New("invalid entry type") ) //go:generate mockgen -destination=mocks/mock_service.go -package=mocks -source=service.go Service