Skip to content

Opt-in OpenAPI 3.0 root document (#53)#84

Merged
milmazz merged 16 commits into
mainfrom
openapi-30-emitter
Jul 21, 2026
Merged

Opt-in OpenAPI 3.0 root document (#53)#84
milmazz merged 16 commits into
mainfrom
openapi-30-emitter

Conversation

@milmazz

@milmazz milmazz commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Closes item 1 of #53. Stacked on #83 — re-target to main once that merges.

What's here

  • openapi_version config option"2.0" (default, byte-identical to today's PostgREST-parity Swagger document) or "3.0". A Bier extension: PostgREST core has never shipped OpenAPI 3.x (OpenAPI 3.0.0 PostgREST/postgrest#932, open since 2017). db_root_spec still overrides the document entirely.
  • Bier.OpenAPI.V3.convert/1 — converts the finished Swagger 2.0 map into OpenAPI 3.0.3: definitionscomponents.schemas, shared params → components.parameters/components.requestBodies with schema nesting, $ref rewriting, body params → requestBody, collectionFormat: multistyle: form + explode: true, response content wrapping, servers folding from scheme/host/basePath, securityDefinitionscomponents.securitySchemes. Converting the finished 2.0 doc (rather than emitting 3.0 in parallel from the internal model) keeps one wire-format source of truth: every future 2.0 parity fix flows into the 3.0 output for free.
  • Root wiringbuild_openapi_document/2 pipes through the converter only when the instance sets openapi_version: "3.0"; negotiation, HEAD, openapi-mode, and db-root-spec precedence are untouched. An HTTP test boots a real instance and asserts the 3.0.3 wire behavior.

Notes

  • The conformance instance never sets the option, so all frozen cases keep hitting the 2.0 default (suite green: 876 passed, 5 excluded; full mix precommit clean).
  • 3.1 deliberately deferred: the converter architecture makes it a follow-up (one more convert variant) — per the plan's scoping decision.
  • Known limitation (documented in the moduledoc): component keys inherit relation/column names verbatim; names outside OAS 3.0.3's component-key charset would produce technically invalid keys. Sanitization is out of scope for a parity-driven converter.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vdh3cbYLtKJfGkZ1ijrYGt

milmazz and others added 14 commits July 16, 2026 01:04
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the preferParams $ref on RPC POST (issue #53 item 3) and the rest of
the makeParamDefs parity: preferParams def with suppressed empty enum,
preferPost resolution enum values, on_conflict def, args body required:true,
PATCH/DELETE parameter lists without select, rowFilter description over
format. Each cited to Response/OpenAPI.hs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The most-parameters overload supplies the whole path item, mirroring
PostgREST's overload sort + last-insert-wins assembly (issue #53 item 2).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET / in follow-privileges mode no longer runs two catalog queries per
request: results are cached per {instance, role} in a public ETS table and
stamped with the schema-cache snapshot generation, so a reload invalidates
them with zero coordination (issue #53 item 4).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rescue ArgumentError around the ETS lookup/insert path so a table that died
between a GenServer crash and its restart degrades to calling the loader
directly instead of raising into the request; trap exits and erase the
persistent_term tid in terminate/2 so a clean shutdown never leaves a stale
handle published. Mirrors the identical defense in Bier.JwtCache.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- lib/bier.ex: document that follow-privileges caches per-role privilege
  filtering and refreshes on schema-cache reload; add the same one-liner
  to README.md's config-options table.
- test/bier/privileges_cache_test.exs: add on_exit cleanup for the dead
  tid the table-gone test re-publishes into :persistent_term.
- lib/bier/privileges_cache.ex: add an explicit fetch/4 clause that
  bypasses the cache for a nil generation (pre-first-snapshot-load
  caller), instead of leaving it an unreachable implicit invariant.
- CLAUDE.md: note the conditional Bier.JwtCache child in the
  boot-sequence list, matching lib/bier.ex's actual children.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add moduledoc note on component key charset limitations
- Add inline comment in convert/1 about eight top-level keys
- Replace weak map-equality assertion with map_size check

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vdh3cbYLtKJfGkZ1ijrYGt
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Base automatically changed from openapi-53-followups to main July 17, 2026 01:33
Resolves conflicts with the SSE events feature (#81):
- README.md: keep the openapi_version option row alongside main's table
- lib/bier/plugs/action_controller.ex: keep the extended
  build_openapi_document comment covering openapi_version
- .credo.exs: raise Bier.Config StructFieldAmount cap to 50 (the two
  branches together grew the struct to 46 fields, past the old 45 cap)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@milmazz

milmazz commented Jul 17, 2026

Copy link
Copy Markdown
Owner Author

@claude review once.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @milmazz's task in 3m 9s —— View job


Review: Opt-in OpenAPI 3.0 root document (#53 item 1)

Todo list

  • Gather context (read diff, changed files, CLAUDE.md)
  • Review lib/bier/openapi/v3.ex converter logic
  • Review root wiring in action_controller.ex / lib/bier.ex / config.ex
  • Review tests (openapi_v3_test.exs, openapi_v3_http_test.exs, config_test.exs)
  • Post final review feedback

Nice, focused implementation — converting the finished 2.0 map instead of forking the emitter is the right call for keeping one wire-format source of truth. I traced the converter against every top-level/operation-level key the 2.0 emitter (lib/bier/openapi.ex) actually produces and didn't find a correctness bug. Elixir CI is green (so mix precommit, including credo --strict and mix docs --warnings-as-errors, already passed) — I wasn't able to re-run mix test/mix compile myself in this sandbox (no approval for mix/DB access), so I relied on that CI run plus static review.

Findings (both minor, non-blocking):

  1. Small duplicationconvert_body/1 (lib/bier/openapi/v3.ex:70-75) and the inline-body branch of extract_body/2 (lib/bier/openapi/v3.ex:150-153) build the exact same %{"required" => ..., "content" => %{"application/json" => %{"schema" => rewrite_refs(...)}}} shape. Since the inline RPC body (rpc_body_param/3 in openapi.ex) never sets "description", the inline branch could just call convert_body(inline) and drop the duplicate construction. Cosmetic only — behavior is identical either way.

  2. collectionFormat case match is closed-world by designconvert_param/1's case Map.pop(schema_keys, "collectionFormat") do {"multi", rest} -> ...; {nil, rest} -> ... end (lib/bier/openapi/v3.ex:92-95) would raise CaseClauseError on any other collectionFormat value. This is explicitly disclosed in the moduledoc ("intentionally NOT general purpose... collectionFormat: multi only on query params") and today's emitter (Bier.OpenAPI.Types.query_param/2) only ever emits "multi", so this is a deliberate, documented scope boundary rather than a bug — just flagging it so it's a conscious tripwire if a future collectionFormat value is ever added to the 2.0 emitter without updating this converter too.

Everything else looks solid:

  • $ref rewriting, definitionscomponents.schemas, shared parameters/body splitting, collectionFormat: multistyle/explode, response content wrapping, and servers folding all match the OAS 3.0.3 shape and are backed by good unit coverage (test/bier/openapi_v3_test.exs), including the RPC overload-winner, variadic, and security-definitions paths.
  • Root wiring (action_controller.ex:197-227) cleanly gates the conversion behind config.openapi_version, leaving negotiation/HEAD/db_root_spec precedence untouched, and the HTTP test (openapi_v3_http_test.exs) exercises it against a real booted instance.
  • openapi_version config option, docs (README.md, moduledocs), and .credo.exs struct-field-count bump are all consistent with the new field.
  • The documented known limitation (component keys not sanitized to the OAS 3.0.3 charset) is a reasonable, clearly-scoped tradeoff for a parity-driven converter.

No blocking issues. This looks ready to merge once #83 lands and this is re-targeted to main.

Review follow-up on #84: the inline-body branch of extract_body/2
duplicated the requestBody shape convert_body/1 already builds; the
inline RPC args param never carries a description, so put_nonempty
makes the swap behavior-identical.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@milmazz
milmazz merged commit 472ad6f into main Jul 21, 2026
3 checks passed
@milmazz
milmazz deleted the openapi-30-emitter branch July 21, 2026 20:43
milmazz added a commit that referenced this pull request Jul 21, 2026
Resolves the build_openapi_document conflict by combining both sides:
the server_scheme/server_host/server_port build inputs from this branch
plus the openapi_version 3.0 conversion gate from #84.

Semantic follow-through for the merge: the V3 converter now drops the
2.0-only operation-level produces lists this branch added (its 3.0
output stays application/json-only per its moduledoc), and the #84 unit
tests are updated to the emitter's new required server inputs and the
always-present schemes/host/basePath server URL.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant