diff --git a/core/types/optional.go b/core/types/optional.go new file mode 100644 index 00000000..09588d6c --- /dev/null +++ b/core/types/optional.go @@ -0,0 +1,63 @@ +package types + +import ( + "encoding/json" + "reflect" + + "github.com/swaggest/jsonschema-go" +) + +// Optional represents a JSON object field that may be absent, present with a +// null value, or present with a concrete value. The zero value is "absent". +// +// It is used on PATCH-style request structs to tell an omitted field apart from +// a field explicitly set to null, which a plain pointer cannot do. +// +// Use the `omitzero` JSON tag so an absent field is left out when encoding. +type Optional[T any] struct { + Value T + Present bool +} + +// UnmarshalJSON records that the field was present and decodes its value. +func (o *Optional[T]) UnmarshalJSON(data []byte) error { + o.Present = true + + var zero T + o.Value = zero + + return json.Unmarshal(data, &o.Value) +} + +// MarshalJSON encodes the underlying value. An absent optional is omitted from +// the object when the field uses the `omitzero` tag. +func (o Optional[T]) MarshalJSON() ([]byte, error) { + return json.Marshal(o.Value) +} + +// IsZero reports whether the field was absent so that encoding/json can omit it +// when the `omitzero` option is used. +func (o Optional[T]) IsZero() bool { + return !o.Present +} + +// PrepareJSONSchema makes the generated schema describe the inner type T +// instead of the wrapper struct. This keeps Optional invisible to API clients. +// +// It works for scalar, pointer, and map inner types. Named struct inner types +// can produce references that the outer reflector does not collect. +func (o Optional[T]) PrepareJSONSchema(schema *jsonschema.Schema) error { + innerType := reflect.TypeFor[T]() + + var reflector jsonschema.Reflector + reflector.InlineDefinition(reflect.Zero(innerType).Interface()) + + inner, err := reflector.Reflect(reflect.Zero(innerType).Interface()) + if err != nil { + return err + } + + *schema = inner + + return nil +} diff --git a/core/types/optional_test.go b/core/types/optional_test.go new file mode 100644 index 00000000..481abe90 --- /dev/null +++ b/core/types/optional_test.go @@ -0,0 +1,69 @@ +package types_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + coretypes "github.com/Authula/authula/core/types" +) + +func TestOptional_Unmarshal(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + wantPresent bool + wantValue *string + }{ + {name: "absent", body: `{}`, wantPresent: false, wantValue: nil}, + {name: "null", body: `{"logo":null}`, wantPresent: true, wantValue: nil}, + {name: "empty string", body: `{"logo":""}`, wantPresent: true, wantValue: new("")}, + {name: "value", body: `{"logo":"http://some/url/logo.svg"}`, wantPresent: true, wantValue: new("http://some/url/logo.svg")}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var payload struct { + Logo coretypes.Optional[*string] `json:"logo,omitzero"` + } + require.NoError(t, json.Unmarshal([]byte(tt.body), &payload)) + + assert.Equal(t, tt.wantPresent, payload.Logo.Present) + assert.Equal(t, tt.wantValue, payload.Logo.Value) + }) + } +} + +func TestOptional_Marshal(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value coretypes.Optional[*string] + want string + }{ + {name: "absent is omitted", value: coretypes.Optional[*string]{}, want: `{}`}, + {name: "null is kept", value: coretypes.Optional[*string]{Present: true, Value: nil}, want: `{"logo":null}`}, + {name: "value is kept", value: coretypes.Optional[*string]{Present: true, Value: new("http://some/url/logo.svg")}, want: `{"logo":"http://some/url/logo.svg"}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + payload := struct { + Logo coretypes.Optional[*string] `json:"logo,omitzero"` + }{Logo: tt.value} + + data, err := json.Marshal(payload) + require.NoError(t, err) + assert.JSONEq(t, tt.want, string(data)) + }) + } +} diff --git a/plugins/organizations/handlers/organization_handlers_test.go b/plugins/organizations/handlers/organization_handlers_test.go index 366388c3..3769b1e6 100644 --- a/plugins/organizations/handlers/organization_handlers_test.go +++ b/plugins/organizations/handlers/organization_handlers_test.go @@ -12,6 +12,7 @@ import ( coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/core/pagination" + coretypes "github.com/Authula/authula/core/types" internaltests "github.com/Authula/authula/internal/tests" "github.com/Authula/authula/models" orgconstants "github.com/Authula/authula/plugins/organizations/constants" @@ -410,7 +411,7 @@ func TestUpdateOrganizationHandler(t *testing.T) { organizationID: "org-1", body: internaltests.MarshalToJSON(t, orgtypes.UpdateOrganizationRequest{ Name: new("Acme Platform"), - Logo: new("http://some/url/logo.svg"), + Logo: coretypes.Optional[*string]{Present: true, Value: new("http://some/url/logo.svg")}, Metadata: map[string]any{"tier": "pro"}, }), prepare: func(f *organizationHandlerFixture) { @@ -443,7 +444,7 @@ func TestUpdateOrganizationHandler(t *testing.T) { organizationID: "org-1", body: internaltests.MarshalToJSON(t, orgtypes.UpdateOrganizationRequest{ Name: new("Acme Platform"), - Logo: new("http://some/url/logo.svg"), + Logo: coretypes.Optional[*string]{Present: true, Value: new("http://some/url/logo.svg")}, Metadata: map[string]any{"tier": "pro"}, }), prepare: func(f *organizationHandlerFixture) { @@ -470,6 +471,28 @@ func TestUpdateOrganizationHandler(t *testing.T) { assert.JSONEq(t, "{\"tier\":\"pro\"}", string(internaltests.MarshalToJSON(t, org.Metadata))) }, }, + { + name: "explicit null logo is treated as logo provided but resets the logo to nil", + userID: new("user-1"), + organizationID: "org-1", + body: []byte(`{"logo":null}`), + prepare: func(f *organizationHandlerFixture) { + f.service.On("UpdateOrganization", mock.Anything, "user-1", "org-1", mock.MatchedBy(func(request orgtypes.UpdateOrganizationRequest) bool { + return request.Logo.Present && request.Logo.Value == nil + })).Return(&orgtypes.Organization{ + ID: "org-1", + OwnerID: "user-1", + Name: "Acme", + Slug: "acme", + Logo: nil, + }, nil).Once() + }, + expectedStatus: http.StatusOK, + checkResponse: func(t *testing.T, reqCtx *models.RequestContext) { + org := internaltests.DecodeResponseJSON[orgtypes.Organization](t, reqCtx) + assert.Nil(t, org.Logo) + }, + }, } for _, tt := range tests { diff --git a/plugins/organizations/services/organization_service.go b/plugins/organizations/services/organization_service.go index e398b737..332bd63a 100644 --- a/plugins/organizations/services/organization_service.go +++ b/plugins/organizations/services/organization_service.go @@ -263,8 +263,12 @@ func (s *organizationService) UpdateOrganization(ctx context.Context, actor *mod organization.Name = *name } organization.Slug = slug - if request.Logo != nil { - organization.Logo = request.Logo + if request.Logo.Present { + if request.Logo.Value == nil || *request.Logo.Value == "" { + organization.Logo = nil + } else { + organization.Logo = request.Logo.Value + } } if request.Metadata != nil { organization.Metadata = request.Metadata diff --git a/plugins/organizations/services/organization_service_test.go b/plugins/organizations/services/organization_service_test.go index b05f1cde..e0200221 100644 --- a/plugins/organizations/services/organization_service_test.go +++ b/plugins/organizations/services/organization_service_test.go @@ -10,6 +10,7 @@ import ( coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/core/pagination" + coretypes "github.com/Authula/authula/core/types" internaltests "github.com/Authula/authula/internal/tests" "github.com/Authula/authula/models" "github.com/Authula/authula/plugins/organizations/constants" @@ -530,6 +531,32 @@ func TestOrganizationService_UpdateOrganization(t *testing.T) { })).Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme Platform", Slug: "acme"}, nil).Once() }, }, + { + name: "explicit null logo clears it", + actorUserID: "user-1", + organizationID: "org-1", + request: types.UpdateOrganizationRequest{Logo: coretypes.Optional[*string]{Present: true, Value: nil}}, + setup: func(repo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationHooks, serviceUtils *ServiceUtils) { + repo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme", Slug: "acme", Logo: new("http://old/logo.svg")}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() + repo.On("Update", mock.Anything, mock.MatchedBy(func(org *types.Organization) bool { + return org != nil && org.ID == "org-1" && org.Logo == nil + })).Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme", Slug: "acme", Logo: nil}, nil).Once() + }, + }, + { + name: "empty string logo clears it", + actorUserID: "user-1", + organizationID: "org-1", + request: types.UpdateOrganizationRequest{Logo: coretypes.Optional[*string]{Present: true, Value: new("")}}, + setup: func(repo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationHooks, serviceUtils *ServiceUtils) { + repo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme", Slug: "acme", Logo: new("http://old/logo.svg")}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() + repo.On("Update", mock.Anything, mock.MatchedBy(func(org *types.Organization) bool { + return org != nil && org.ID == "org-1" && org.Logo == nil + })).Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme", Slug: "acme", Logo: nil}, nil).Once() + }, + }, } for _, tt := range tests { diff --git a/plugins/organizations/types/api.go b/plugins/organizations/types/api.go index d10659c9..0105a26c 100644 --- a/plugins/organizations/types/api.go +++ b/plugins/organizations/types/api.go @@ -6,6 +6,7 @@ import ( coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/core/pagination" + coretypes "github.com/Authula/authula/core/types" "github.com/Authula/authula/models" ) @@ -141,10 +142,10 @@ func (r *CreateOrganizationRequest) Validate() error { } type UpdateOrganizationRequest struct { - Name *string `json:"name,omitempty" nullable:"true"` - Slug *string `json:"slug,omitempty" nullable:"true"` - Logo *string `json:"logo,omitempty" nullable:"true"` - Metadata map[string]any `json:"metadata,omitempty" nullable:"true"` + Name *string `json:"name,omitempty" nullable:"true"` + Slug *string `json:"slug,omitempty" nullable:"true"` + Logo coretypes.Optional[*string] `json:"logo,omitzero" nullable:"true"` + Metadata map[string]any `json:"metadata,omitempty" nullable:"true"` } func (r *UpdateOrganizationRequest) Validate() error { @@ -159,9 +160,9 @@ func (r *UpdateOrganizationRequest) Validate() error { value := strings.TrimSpace(*r.Slug) r.Slug = &value } - if r.Logo != nil { - value := strings.TrimSpace(*r.Logo) - r.Logo = &value + if r.Logo.Present && r.Logo.Value != nil { + value := strings.TrimSpace(*r.Logo.Value) + r.Logo.Value = &value } return nil }