From e22e169fb99fb75d5ea01f80927ee7d005b014be Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 4 Jun 2026 22:45:09 +0200 Subject: [PATCH] feat(api/v2): report max_permission on label and project-view reads Read/update use a per-resource struct that embeds the model by value and adds a readOnly max_permission field (labelReadBody, projectViewReadBody); Go and Huma promote the embedded fields, so the body stays flat with no custom marshaler and nothing on the shared models. The handler passes the model's Updated and the permission to conditionalReadResponse, which folds the permission into the ETag. Adds a webtest asserting two callers with different permission on the same label get different ETags, plus max_permission presence assertions. --- .claude/skills/api-v2-routes/SKILL.md | 6 ++--- pkg/routes/api/v2/labels.go | 30 +++++++++++++----------- pkg/routes/api/v2/project_views.go | 32 ++++++++++++++------------ pkg/webtests/huma_label_test.go | 17 ++++++++++++++ pkg/webtests/huma_project_view_test.go | 1 + 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/.claude/skills/api-v2-routes/SKILL.md b/.claude/skills/api-v2-routes/SKILL.md index 9301b2e4f..2644b4338 100644 --- a/.claude/skills/api-v2-routes/SKILL.md +++ b/.claude/skills/api-v2-routes/SKILL.md @@ -72,7 +72,7 @@ Use the package's `Register` wrapper, **not** `huma.Register` directly — it se Every handler: pull auth with `authFromCtx(ctx)`, call the matching `handler.Do*`, wrap returned errors in `translateDomainError`. Use the shared envelopes from `types.go` (`singleBody`, `singleReadBody`, `emptyBody`, `ListParams`, `Paginated`/`NewPaginated`). -- **List** takes `*ListParams` (gives you `page`/`per_page`/`q` for free) and returns `*fooListBody`. **You must type-assert the `DoReadAll` result to the concrete slice** — `result` is `any`, and a blind cast or a generic wrapper silently serialises `[]` (the "generic-any silent-empty trap"). Return a hard error on mismatch: +- **List** takes `*ListParams` (gives you `page`/`per_page`/`q` for free, already `doc:`-tagged in `types.go` — no need to re-document them) and returns `*fooListBody`. **You must type-assert the `DoReadAll` result to the concrete slice** — `result` is `any`, and a blind cast or a generic wrapper silently serialises `[]` (the "generic-any silent-empty trap"). Return a hard error on mismatch: ```go items, ok := result.([]*models.Foo) if !ok { @@ -81,8 +81,8 @@ Every handler: pull auth with `authFromCtx(ctx)`, call the matching `handler.Do* return &fooListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil ``` - **Extra query params go *directly* on the handler's input struct — not in a shared/embedded helper.** Beyond `ListParams`, if an operation needs its own query params (`expand`, `order_by`, `include_public`, …), declare each as a direct field with its own `query:"…"` tag on that operation's input struct, then bind it onto the model. A shared or embedded struct of query fields silently **fails to bind** under Huma when combined with other query params/embeds — the field arrives empty (hit while implementing Project's `expand`). Flatten them into the input struct. -- **Read** embeds `conditional.Params` in its input, builds an ETag from `id + Updated.UnixNano()`, calls `in.PreconditionFailed(etag, label.Updated)` when `in.HasConditionalParams()`, and returns `*singleReadBody[Model]` with the **quoted** ETag (`"`+etag+`"`). -- **Create / Update** take a `Body Model` input and return `*singleBody[Model]`. Update sets `in.Body.ID = in.ID` (URL wins over body). +- **Read** embeds `conditional.Params` in its input. To surface the caller's permission, define a small per-resource response struct that **embeds the model by value** and adds the permission: `type fooReadBody struct { models.Foo; MaxPermission models.Permission \`json:"max_permission" readOnly:"true" doc:"..."\` }`. Go and Huma both promote the embedded model's fields, so the wire shape is flat (model fields + `max_permission`) with no custom marshaler and nothing added to the shared model struct. Capture `DoReadOne`'s returned max permission (it is `0`/`1`/`2` on success — **never discard it as `_`**), build the body, and `return conditionalReadResponse(&in.Params, body, foo.Updated, maxPermission)`. The shared helper (in `types.go`) folds the permission into the ETag (so a share/role change invalidates the cache), applies the conditional precondition (304/412), and returns `*singleReadBody[fooReadBody]`. See `labels.go`/`project_views.go`. (A generic `struct{ T; ... }` is impossible — Go forbids embedding a type parameter — so the per-resource struct is the price of a flat shape without a marshaler.) +- **Create / Update** return `*singleBody[Model]` and set the model's `ID` from the path (URL wins over body). **Update's request body must be the same `fooReadBody` the read returns, not the bare model** — AutoPatch's GET→PUT round trip echoes the read body (max_permission included) into the PUT, and because `max_permission` is a declared `readOnly` property of `fooReadBody`'s schema, Huma accepts and ignores it on write rather than rejecting it. Take `&in.Body.Foo` (the embedded model — value-embedded, so never nil) and ignore the embedded `MaxPermission`. Create stays a bare `Body Model` (AutoPatch only round-trips into PUT). - **Delete** returns `*emptyBody`. ### 3. Self-register the resource diff --git a/pkg/routes/api/v2/labels.go b/pkg/routes/api/v2/labels.go index fcf0a9303..a9576bc50 100644 --- a/pkg/routes/api/v2/labels.go +++ b/pkg/routes/api/v2/labels.go @@ -103,26 +103,26 @@ func labelsList(ctx context.Context, in *ListParams) (*labelListBody, error) { return &labelListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil } +type labelReadBody struct { + models.Label + MaxPermission models.Permission `json:"max_permission" readOnly:"true" doc:"The maximum permission the requesting user has on this label (0=read, 1=read/write, 2=admin)."` +} + func labelsRead(ctx context.Context, in *struct { ID int64 `path:"id"` conditional.Params -}) (*singleReadBody[models.Label], error) { +}) (*singleReadBody[labelReadBody], error) { a, err := authFromCtx(ctx) if err != nil { return nil, err } label := &models.Label{ID: in.ID} - if _, err := handler.DoReadOne(ctx, label, a); err != nil { + maxPermission, err := handler.DoReadOne(ctx, label, a) + if err != nil { return nil, translateDomainError(err) } - // PreconditionFailed wants the unquoted etag; response header uses RFC 9110 quoted form. - etag := fmt.Sprintf("%d-%d", label.ID, label.Updated.UnixNano()) - if in.HasConditionalParams() { - if err := in.PreconditionFailed(etag, label.Updated); err != nil { - return nil, err - } - } - return &singleReadBody[models.Label]{ETag: `"` + etag + `"`, Body: label}, nil + body := &labelReadBody{Label: *label, MaxPermission: models.Permission(maxPermission)} + return conditionalReadResponse(&in.Params, body, label.Updated, maxPermission) } func labelsCreate(ctx context.Context, in *struct { @@ -138,19 +138,21 @@ func labelsCreate(ctx context.Context, in *struct { return &singleBody[models.Label]{Body: &in.Body}, nil } +// Body matches the read shape so AutoPatch's GET→PUT echo of max_permission validates. func labelsUpdate(ctx context.Context, in *struct { ID int64 `path:"id"` - Body models.Label + Body labelReadBody }) (*singleBody[models.Label], error) { a, err := authFromCtx(ctx) if err != nil { return nil, err } - in.Body.ID = in.ID // URL wins over body - if err := handler.DoUpdate(ctx, &in.Body, a); err != nil { + label := &in.Body.Label + label.ID = in.ID // URL wins over body + if err := handler.DoUpdate(ctx, label, a); err != nil { return nil, translateDomainError(err) } - return &singleBody[models.Label]{Body: &in.Body}, nil + return &singleBody[models.Label]{Body: label}, nil } func labelsDelete(ctx context.Context, in *struct { diff --git a/pkg/routes/api/v2/project_views.go b/pkg/routes/api/v2/project_views.go index 20d1b23ba..946b5aabf 100644 --- a/pkg/routes/api/v2/project_views.go +++ b/pkg/routes/api/v2/project_views.go @@ -107,11 +107,16 @@ func projectViewsList(ctx context.Context, in *struct { return &projectViewListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil } +type projectViewReadBody struct { + models.ProjectView + MaxPermission models.Permission `json:"max_permission" readOnly:"true" doc:"The maximum permission the requesting user has on this view (0=read, 1=read/write, 2=admin)."` +} + func projectViewsRead(ctx context.Context, in *struct { ProjectID int64 `path:"project"` ID int64 `path:"view"` conditional.Params -}) (*singleReadBody[models.ProjectView], error) { +}) (*singleReadBody[projectViewReadBody], error) { a, err := authFromCtx(ctx) if err != nil { return nil, err @@ -119,17 +124,12 @@ func projectViewsRead(ctx context.Context, in *struct { // ReadOne resolves the view via GetProjectViewByIDAndProject, which needs // both ids — the parent project scopes the lookup. view := &models.ProjectView{ID: in.ID, ProjectID: in.ProjectID} - if _, err := handler.DoReadOne(ctx, view, a); err != nil { + maxPermission, err := handler.DoReadOne(ctx, view, a) + if err != nil { return nil, translateDomainError(err) } - // PreconditionFailed wants the unquoted etag; response header uses RFC 9110 quoted form. - etag := fmt.Sprintf("%d-%d", view.ID, view.Updated.UnixNano()) - if in.HasConditionalParams() { - if err := in.PreconditionFailed(etag, view.Updated); err != nil { - return nil, err - } - } - return &singleReadBody[models.ProjectView]{ETag: `"` + etag + `"`, Body: view}, nil + body := &projectViewReadBody{ProjectView: *view, MaxPermission: models.Permission(maxPermission)} + return conditionalReadResponse(&in.Params, body, view.Updated, maxPermission) } func projectViewsCreate(ctx context.Context, in *struct { @@ -147,21 +147,23 @@ func projectViewsCreate(ctx context.Context, in *struct { return &singleBody[models.ProjectView]{Body: &in.Body}, nil } +// Body matches the read shape so AutoPatch's GET→PUT echo of max_permission validates. func projectViewsUpdate(ctx context.Context, in *struct { ProjectID int64 `path:"project"` ID int64 `path:"view"` - Body models.ProjectView + Body projectViewReadBody }) (*singleBody[models.ProjectView], error) { a, err := authFromCtx(ctx) if err != nil { return nil, err } - in.Body.ID = in.ID // URL wins over body - in.Body.ProjectID = in.ProjectID // parent from the path scopes the update - if err := handler.DoUpdate(ctx, &in.Body, a); err != nil { + view := &in.Body.ProjectView + view.ID = in.ID // URL wins over body + view.ProjectID = in.ProjectID // parent from the path scopes the update + if err := handler.DoUpdate(ctx, view, a); err != nil { return nil, translateDomainError(err) } - return &singleBody[models.ProjectView]{Body: &in.Body}, nil + return &singleBody[models.ProjectView]{Body: view}, nil } func projectViewsDelete(ctx context.Context, in *struct { diff --git a/pkg/webtests/huma_label_test.go b/pkg/webtests/huma_label_test.go index 237d85a04..02b314dc5 100644 --- a/pkg/webtests/huma_label_test.go +++ b/pkg/webtests/huma_label_test.go @@ -80,6 +80,7 @@ func TestHumaLabel(t *testing.T) { rec, err := testHandler.testReadOneWithUser(nil, map[string]string{"label": "1"}) require.NoError(t, err) assert.Contains(t, rec.Body.String(), `"title":"Label #1"`) + assert.Contains(t, rec.Body.String(), `"max_permission":`) assert.NotEmpty(t, rec.Result().Header.Get("ETag")) }) t.Run("Nonexisting", func(t *testing.T) { @@ -266,6 +267,22 @@ func TestHumaLabel_ETagReturns304(t *testing.T) { require.Equal(t, http.StatusNotModified, rec.Code, "body: %s", rec.Body.String()) } +func TestHumaLabel_ETagReflectsPermission(t *testing.T) { + // Label #4 is owned by user2 (admin) but readable by user1 only at read level; + // same label, so the per-caller ETag must differ — else a 304 serves stale perms. + e, err := setupTestEnv() + require.NoError(t, err) + + reader := humaRequest(t, e, http.MethodGet, "/api/v2/labels/4", "", humaTokenFor(t, &testuser1), "") + require.Equal(t, http.StatusOK, reader.Code, "body: %s", reader.Body.String()) + owner := humaRequest(t, e, http.MethodGet, "/api/v2/labels/4", "", humaTokenFor(t, &testuser2), "") + require.Equal(t, http.StatusOK, owner.Code, "body: %s", owner.Body.String()) + + assert.NotEmpty(t, reader.Header().Get("ETag")) + assert.NotEqual(t, reader.Header().Get("ETag"), owner.Header().Get("ETag"), + "same label, different caller permission must produce different ETags") +} + func TestHumaLabel_PATCHMergePatch(t *testing.T) { e, err := setupTestEnv() require.NoError(t, err) diff --git a/pkg/webtests/huma_project_view_test.go b/pkg/webtests/huma_project_view_test.go index 2d6a1c0a3..e7197ce6e 100644 --- a/pkg/webtests/huma_project_view_test.go +++ b/pkg/webtests/huma_project_view_test.go @@ -147,6 +147,7 @@ func TestProjectView(t *testing.T) { require.NoError(t, err) assert.Contains(t, rec.Body.String(), `"title":"List"`) assert.Contains(t, rec.Body.String(), `"id":1`) + assert.Contains(t, rec.Body.String(), `"max_permission":`) assert.NotEmpty(t, rec.Result().Header.Get("ETag")) }) t.Run("Read-only share can read", func(t *testing.T) {