github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-05-07 03:58:09 -05:00
Code Issues 690 Packages Projects Releases 11 Wiki Activity

[PR #2659] [CLOSED] spike: serve OpenAPI 3.1 via Huma alongside legacy Swagger 2.0 #8436

New Issue
Closed
opened 2026-04-20 18:13:02 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-20 18:13:02 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/go-vikunja/vikunja/pull/2659
Author: @tink-bot
Created: 4/20/2026
Status: ❌ Closed

Base: main ← Head: spike-huma-openapi3

📝 Commits (10+)

  • 37adc38 chore: add github.com/danielgtaylor/huma/v2 dependency
  • 35f9c4b feat: vendor humaecho adapter for echo/v5
  • 1d6c1cf test: humaecho5 adapter roundtrip and spec serving
  • abc0cdf feat(auth): add GetAuthFromContext for Huma handlers
  • 00d394e feat(huma): error formatter matching legacy Vikunja JSON shape
  • 19fa92f refactor(handler): extract DoCreate from CreateWeb
  • 12feb63 refactor(handler): extract DoReadOne from ReadOneWeb
  • be988d4 refactor(handler): extract DoReadAll from ReadAllWeb
  • d677e40 refactor(handler): extract DoUpdate from UpdateWeb
  • 5ba404a refactor(handler): extract DoDelete from DeleteWeb

📊 Changes

20 files changed (+1356 additions, -182 deletions)

View changed files

📝 go.mod (+8 -7)
📝 go.sum (+17 -0)
📝 pkg/modules/auth/auth.go (+15 -0)
➕ pkg/modules/auth/auth_test.go (+34 -0)
➕ pkg/modules/humaecho5/humaecho5.go (+170 -0)
➕ pkg/modules/humaecho5/humaecho5_test.go (+86 -0)
➕ pkg/routes/api/v1/humaapi/crud.go (+239 -0)
➕ pkg/routes/api/v1/humaapi/crud_test.go (+105 -0)
➕ pkg/routes/api/v1/humaapi/errors.go (+45 -0)
➕ pkg/routes/api/v1/humaapi/errors_test.go (+46 -0)
➕ pkg/routes/api/v1/humaapi/labels.go (+38 -0)
➕ pkg/routes/api/v1/humaapi/spec_verification_test.go (+121 -0)
📝 pkg/routes/routes.go (+54 -2)
➕ pkg/web/handler/core.go (+214 -0)
📝 pkg/web/handler/create.go (+1 -38)
📝 pkg/web/handler/delete.go (+1 -36)
📝 pkg/web/handler/read_all.go (+1 -22)
📝 pkg/web/handler/read_one.go (+3 -40)
📝 pkg/web/handler/update.go (+1 -37)
➕ pkg/webtests/huma_label_test.go (+157 -0)

📄 Description

Proves end-to-end that Vikunja can serve an OpenAPI 3.1 spec via Huma alongside the existing Swagger 2.0 swaggo-driven spec, by porting Label through a generic CRUD registrar reusing the existing models.CObject + permissions interface.

Spike is additive: legacy /api/v1/docs.json and /api/v1/labels keep working. The Huma-served spec is at /api/v1/oas3/openapi.json; Huma Label routes at /api/v1/oas3/labels{,/{id}}.

What landed

  • pkg/modules/humaecho5/ — vendored Huma adapter for echo/v5 (upstream PR #959 not yet merged)
  • pkg/modules/auth.GetAuthFromContext — bridges Huma's plain context.Context to Vikunja's echo-based JWT flow
  • pkg/routes/api/v1/humaapi/errors.go — error formatter matching legacy {code, message} shape (no RFC 9457 leakage)
  • pkg/web/handler/core.go — DoCreate / DoReadOne / DoReadAll / DoUpdate / DoDelete extracted from the *Web wrappers so both Echo and Huma can drive the same lifecycle
  • pkg/routes/api/v1/humaapi/crud.go — generic Register[T] registrar producing five Huma operations from one Config
  • pkg/routes/api/v1/humaapi/labels.go — Label ported through the registrar
  • Three integration tests in pkg/webtests/huma_label_test.go proving round-trip, spec contents, and error shape

Findings

What worked on first try

  • The vendored echo/v5 adapter (pkg/modules/humaecho5) — upstream PR #959's 6-line diff applied cleanly; round-trip + spec-serving smoke tests passed on the first run.
  • huma.NewError global override — installing humaapi.Install() once at boot replaces error formatting everywhere with no per-handler ceremony.
  • auth.GetAuthFromContext bridge — stashing *echo.Context under a context key in the adapter's Context() lets all existing JWT auth flow through unchanged.
  • Extracting DoCreate/DoReadOne/DoUpdate/DoDelete from *Web handlers — the lifecycle (session, perm check, model op, commit, event dispatch) factored out cleanly with zero test regressions across mage test:web.
  • Generic Register[T] registrar — produced correct paths, methods, security refs, tags, and path/query params for Label from a single Config.

What needed deviation from the plan

  1. Mounted Huma under /api/v1/oas3/... instead of /api/v1/.... Echo v5 panics on duplicate (method, path) registrations and the legacy Label routes already own /api/v1/labels. To keep both alive simultaneously the Huma routes got a parallel prefix. Full migration will need to swap the legacy handlers out atomically to reclaim /api/v1/labels.
  2. Echo *HTTPError → vikunjaError translation (translateError in crud.go). Errors bubbling from Do* (e.g. echo.NewHTTPError(403, "Forbidden")) need an explicit errors.As translation layer so the wire shape matches legacy {"message":"Forbidden"}. Plan originally assumed huma.NewError override alone would suffice.
  3. humaConfig.FieldsOptionalByDefault = true. Without this Huma marks every non-pointer struct field as required and rejects legitimate partial Label PUTs at schema-validation time. Legacy handlers are permissive; govalidator enforces real rules later.
  4. Generic Register[T, P] simplified to Register[T] over SingleID only. Go does not permit embedding a type parameter in a struct, and Huma's parameter discovery only walks anonymous (embedded) fields. The generalised multi-path-shape registrar needs per-shape concrete wrappers. Spike only exercises SingleID; nested-route resources (/tasks/{task}/labels/{label}) will need hand-written wrapper types for the full migration.
  5. Echo v5 in go.mod is v5.0.3 — echo.Context is still a concrete struct; adapter uses Param(name) not PathParam(name) that the upstream PR assumes. Single-line deviation from upstream v5 diff.

Semantic spec differences (legacy Swagger 2.0 vs Huma OAS 3.1)

Real contract divergences, not cosmetic — flagging for anyone consuming the spec:

  1. Create returns 200 instead of 201. Huma's default success status is 200. Either set DefaultStatus: http.StatusCreated on the create op or accept the wire change. Frontend code asserting === 201 will break.
  2. JWTKeyAuth referenced by every op but never declared in components.securitySchemes. Huma won't generate the scheme definition unless registered via api.OpenAPI().Components.SecuritySchemes[...]. As-is the spec fails OAS validators (Spectral, Redoc, Swagger UI auth flows). Trivial one-time fix.
  3. All non-success responses collapse into a single default response. Legacy lists explicit 400/403/404/500 per op with distinct schemas; Huma emits one default referencing VikunjaError. OAS-3-idiomatic but client code generators branching on status codes lose granularity. Runtime error shape is identical — only spec descriptors differ.
  4. Request body moved from parameters[in:body] (Swagger 2) to requestBody (OAS 3.1). Expected, correct — flagging because anything regex-scraping the legacy spec for "in":"body" will need updating.

DoReadAll's result.([]T) cast — does NOT hold for Label

models.Label.ReadAll returns []*LabelWithTaskID (via GetLabelsByTaskIDs), not []*models.Label. The generic registrar's type assertion result.([]T) fails silently and returns an empty list. Integration tests exercise PUT + GET /{id} + DELETE but not GET list, so this was not caught until Phase F verification.

For the full migration, pick one of:

  • Make Config[T] accept a ListItem type parameter distinct from T
  • Hand-write the list operation per-resource via huma.Register directly (the comment in crud.go already documents this escape hatch)
  • Normalise model ReadAll returns to []T (semantic change — touches every consumer)

This is the single biggest scope risk for porting more resources. Most Vikunja list endpoints return wrapper types (*WithTaskID, *WithCounts, etc.).

Effort estimate for porting the next 5 resources

Tier: Medium — 1-2 dev-days per resource, pending the list-cast fix.

Main cost drivers:

  1. List operation type mismatch (above) — needs design decision before scaling (~1-2 days registrar refactor).
  2. Multi-path resources (/tasks/{task}/labels/{label}) — need a second concrete Config shape; ~50 LoC per shape, one-time cost.
  3. Custom non-CRUD endpoints (bulk ops, /labels/bulk) — outside the registrar; hand-written huma.Register per endpoint.
  4. valid: → JSON Schema migration — out of spike scope but eventually needed for Huma to enforce validation natively.
  5. The 200-vs-201 status fix and missing security scheme are global and should be addressed once before more resources are added.

Test plan

  • mage test:web — all existing webtests plus the three new Huma Label tests pass
  • mage build — clean build
  • mage lint — clean
  • Manual: /api/v1/docs.json still serves legacy Swagger 2.0 unchanged
  • Manual: /api/v1/oas3/openapi.json serves OAS 3.1 with Label paths
  • Manual: Label CRUD round-trips through the Huma routes using the same JWT as legacy routes
  • Review whether to address the semantic spec differences (201 status, security scheme declaration) inside this spike or leave for the migration PR

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/go-vikunja/vikunja/pull/2659 **Author:** [@tink-bot](https://github.com/tink-bot) **Created:** 4/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `spike-huma-openapi3` --- ### 📝 Commits (10+) - [`37adc38`](https://github.com/go-vikunja/vikunja/commit/37adc38df7fb93fd6c6c100fb16fcfec7b303589) chore: add github.com/danielgtaylor/huma/v2 dependency - [`35f9c4b`](https://github.com/go-vikunja/vikunja/commit/35f9c4b68416b4eadb98b132bb3dcb311036bc5d) feat: vendor humaecho adapter for echo/v5 - [`1d6c1cf`](https://github.com/go-vikunja/vikunja/commit/1d6c1cf8388bd44b963ace09412044a2b40436e4) test: humaecho5 adapter roundtrip and spec serving - [`abc0cdf`](https://github.com/go-vikunja/vikunja/commit/abc0cdfc6a4817e1312619dfab5bb7ea303223bb) feat(auth): add GetAuthFromContext for Huma handlers - [`00d394e`](https://github.com/go-vikunja/vikunja/commit/00d394ed9f54543d952a310a98b21653bf0e17d7) feat(huma): error formatter matching legacy Vikunja JSON shape - [`19fa92f`](https://github.com/go-vikunja/vikunja/commit/19fa92febf5bdc2729e4b4a205c01264ed887bd7) refactor(handler): extract DoCreate from CreateWeb - [`12feb63`](https://github.com/go-vikunja/vikunja/commit/12feb63e4ca666a1359e20cc7c3fac2579dea49e) refactor(handler): extract DoReadOne from ReadOneWeb - [`be988d4`](https://github.com/go-vikunja/vikunja/commit/be988d47e29d583e9a163dda3c6fc3ac053ddb91) refactor(handler): extract DoReadAll from ReadAllWeb - [`d677e40`](https://github.com/go-vikunja/vikunja/commit/d677e40597ddb7d4dc8a1ecd3f242accfde8aacc) refactor(handler): extract DoUpdate from UpdateWeb - [`5ba404a`](https://github.com/go-vikunja/vikunja/commit/5ba404ac584df22a3d0030f1305d37b6500f1781) refactor(handler): extract DoDelete from DeleteWeb ### 📊 Changes **20 files changed** (+1356 additions, -182 deletions) <details> <summary>View changed files</summary> 📝 `go.mod` (+8 -7) 📝 `go.sum` (+17 -0) 📝 `pkg/modules/auth/auth.go` (+15 -0) ➕ `pkg/modules/auth/auth_test.go` (+34 -0) ➕ `pkg/modules/humaecho5/humaecho5.go` (+170 -0) ➕ `pkg/modules/humaecho5/humaecho5_test.go` (+86 -0) ➕ `pkg/routes/api/v1/humaapi/crud.go` (+239 -0) ➕ `pkg/routes/api/v1/humaapi/crud_test.go` (+105 -0) ➕ `pkg/routes/api/v1/humaapi/errors.go` (+45 -0) ➕ `pkg/routes/api/v1/humaapi/errors_test.go` (+46 -0) ➕ `pkg/routes/api/v1/humaapi/labels.go` (+38 -0) ➕ `pkg/routes/api/v1/humaapi/spec_verification_test.go` (+121 -0) 📝 `pkg/routes/routes.go` (+54 -2) ➕ `pkg/web/handler/core.go` (+214 -0) 📝 `pkg/web/handler/create.go` (+1 -38) 📝 `pkg/web/handler/delete.go` (+1 -36) 📝 `pkg/web/handler/read_all.go` (+1 -22) 📝 `pkg/web/handler/read_one.go` (+3 -40) 📝 `pkg/web/handler/update.go` (+1 -37) ➕ `pkg/webtests/huma_label_test.go` (+157 -0) </details> ### 📄 Description Proves end-to-end that Vikunja can serve an OpenAPI 3.1 spec via Huma alongside the existing Swagger 2.0 swaggo-driven spec, by porting Label through a generic CRUD registrar reusing the existing `models.CObject` + permissions interface. Spike is **additive**: legacy `/api/v1/docs.json` and `/api/v1/labels` keep working. The Huma-served spec is at `/api/v1/oas3/openapi.json`; Huma Label routes at `/api/v1/oas3/labels{,/{id}}`. ## What landed - `pkg/modules/humaecho5/` — vendored Huma adapter for echo/v5 (upstream PR #959 not yet merged) - `pkg/modules/auth.GetAuthFromContext` — bridges Huma's plain `context.Context` to Vikunja's echo-based JWT flow - `pkg/routes/api/v1/humaapi/errors.go` — error formatter matching legacy `{code, message}` shape (no RFC 9457 leakage) - `pkg/web/handler/core.go` — `DoCreate` / `DoReadOne` / `DoReadAll` / `DoUpdate` / `DoDelete` extracted from the `*Web` wrappers so both Echo and Huma can drive the same lifecycle - `pkg/routes/api/v1/humaapi/crud.go` — generic `Register[T]` registrar producing five Huma operations from one `Config` - `pkg/routes/api/v1/humaapi/labels.go` — Label ported through the registrar - Three integration tests in `pkg/webtests/huma_label_test.go` proving round-trip, spec contents, and error shape ## Findings ### What worked on first try - The vendored echo/v5 adapter (`pkg/modules/humaecho5`) — upstream PR #959's 6-line diff applied cleanly; round-trip + spec-serving smoke tests passed on the first run. - `huma.NewError` global override — installing `humaapi.Install()` once at boot replaces error formatting everywhere with no per-handler ceremony. - `auth.GetAuthFromContext` bridge — stashing `*echo.Context` under a context key in the adapter's `Context()` lets all existing JWT auth flow through unchanged. - Extracting `DoCreate/DoReadOne/DoUpdate/DoDelete` from `*Web` handlers — the lifecycle (session, perm check, model op, commit, event dispatch) factored out cleanly with zero test regressions across `mage test:web`. - Generic `Register[T]` registrar — produced correct paths, methods, security refs, tags, and path/query params for Label from a single `Config`. ### What needed deviation from the plan 1. **Mounted Huma under `/api/v1/oas3/...` instead of `/api/v1/...`.** Echo v5 panics on duplicate `(method, path)` registrations and the legacy Label routes already own `/api/v1/labels`. To keep both alive simultaneously the Huma routes got a parallel prefix. Full migration will need to swap the legacy handlers out atomically to reclaim `/api/v1/labels`. 2. **Echo `*HTTPError` → `vikunjaError` translation (`translateError` in `crud.go`).** Errors bubbling from `Do*` (e.g. `echo.NewHTTPError(403, "Forbidden")`) need an explicit `errors.As` translation layer so the wire shape matches legacy `{"message":"Forbidden"}`. Plan originally assumed `huma.NewError` override alone would suffice. 3. **`humaConfig.FieldsOptionalByDefault = true`.** Without this Huma marks every non-pointer struct field as required and rejects legitimate partial Label PUTs at schema-validation time. Legacy handlers are permissive; govalidator enforces real rules later. 4. **Generic `Register[T, P]` simplified to `Register[T]` over `SingleID` only.** Go does not permit embedding a type parameter in a struct, and Huma's parameter discovery only walks anonymous (embedded) fields. The generalised multi-path-shape registrar needs per-shape concrete wrappers. Spike only exercises `SingleID`; nested-route resources (`/tasks/{task}/labels/{label}`) will need hand-written wrapper types for the full migration. 5. **Echo v5 in `go.mod` is v5.0.3** — `echo.Context` is still a concrete struct; adapter uses `Param(name)` not `PathParam(name)` that the upstream PR assumes. Single-line deviation from upstream v5 diff. ### Semantic spec differences (legacy Swagger 2.0 vs Huma OAS 3.1) Real contract divergences, not cosmetic — flagging for anyone consuming the spec: 1. **Create returns `200` instead of `201`.** Huma's default success status is 200. Either set `DefaultStatus: http.StatusCreated` on the create op or accept the wire change. Frontend code asserting `=== 201` will break. 2. **`JWTKeyAuth` referenced by every op but never declared in `components.securitySchemes`.** Huma won't generate the scheme definition unless registered via `api.OpenAPI().Components.SecuritySchemes[...]`. As-is the spec fails OAS validators (Spectral, Redoc, Swagger UI auth flows). Trivial one-time fix. 3. **All non-success responses collapse into a single `default` response.** Legacy lists explicit 400/403/404/500 per op with distinct schemas; Huma emits one `default` referencing `VikunjaError`. OAS-3-idiomatic but client code generators branching on status codes lose granularity. Runtime error shape is identical — only spec descriptors differ. 4. **Request body moved from `parameters[in:body]` (Swagger 2) to `requestBody` (OAS 3.1).** Expected, correct — flagging because anything regex-scraping the legacy spec for `"in":"body"` will need updating. ### `DoReadAll`'s `result.([]T)` cast — does NOT hold for Label `models.Label.ReadAll` returns `[]*LabelWithTaskID` (via `GetLabelsByTaskIDs`), not `[]*models.Label`. The generic registrar's type assertion `result.([]T)` **fails silently and returns an empty list**. Integration tests exercise PUT + GET /{id} + DELETE but not GET list, so this was not caught until Phase F verification. For the full migration, pick one of: - Make `Config[T]` accept a `ListItem` type parameter distinct from `T` - Hand-write the list operation per-resource via `huma.Register` directly (the comment in `crud.go` already documents this escape hatch) - Normalise model `ReadAll` returns to `[]T` (semantic change — touches every consumer) This is the single biggest scope risk for porting more resources. Most Vikunja list endpoints return wrapper types (`*WithTaskID`, `*WithCounts`, etc.). ### Effort estimate for porting the next 5 resources **Tier:** Medium — 1-2 dev-days per resource, pending the list-cast fix. Main cost drivers: 1. **List operation type mismatch** (above) — needs design decision before scaling (~1-2 days registrar refactor). 2. **Multi-path resources** (`/tasks/{task}/labels/{label}`) — need a second concrete `Config` shape; ~50 LoC per shape, one-time cost. 3. **Custom non-CRUD endpoints** (bulk ops, `/labels/bulk`) — outside the registrar; hand-written `huma.Register` per endpoint. 4. **`valid:` → JSON Schema migration** — out of spike scope but eventually needed for Huma to enforce validation natively. 5. **The 200-vs-201 status fix and missing security scheme** are global and should be addressed once before more resources are added. ## Test plan - [ ] `mage test:web` — all existing webtests plus the three new Huma Label tests pass - [ ] `mage build` — clean build - [ ] `mage lint` — clean - [ ] Manual: `/api/v1/docs.json` still serves legacy Swagger 2.0 unchanged - [ ] Manual: `/api/v1/oas3/openapi.json` serves OAS 3.1 with Label paths - [ ] Manual: Label CRUD round-trips through the Huma routes using the same JWT as legacy routes - [ ] Review whether to address the semantic spec differences (201 status, security scheme declaration) inside this spike or leave for the migration PR --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-20 18:13:02 -05:00
GiteaMirror changed title from [PR #2659] spike: serve OpenAPI 3.1 via Huma alongside legacy Swagger 2.0 to [PR #2659] [CLOSED] spike: serve OpenAPI 3.1 via Huma alongside legacy Swagger 2.0 2026-04-23 09:24:45 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#8436
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?