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) {