Skip to content

🔥 feat: Add OpenAPI middleware - #3702

Draft
gaby wants to merge 96 commits into
mainfrom
2025-08-21-14-48-18
Draft

gaby wants to merge 96 commits into
mainfrom
2025-08-21-14-48-18

Conversation

@gaby

@gaby gaby commented Aug 21, 2025

Copy link
Copy Markdown
Member

Description

This PR introduces an OpenAPI middleware that auto-generates OpenAPI 3.0 specifications from registered Fiber routes. The middleware provides comprehensive support for documenting APIs through both fluent route methods and middleware configuration, making it easy to maintain up-to-date API documentation.

Screenshots

The Swagger UI page the middleware serves at GET /swagger, generated entirely
from registered routes and Config. The tag groups, server list and
Authorize button all come from route metadata and middleware config:

Swagger UI page served by the OpenAPI middleware, listing the documented operations grouped by tag

An expanded operation, showing where each route helper lands — Description
under the summary, RequestBodyWithExample as the Example Value,
ResponseWithExample as the response body, and ResponseHeader as the headers
table:

An expanded operation in Swagger UI showing the request body example, the response schema and the documented Location response header

These are also in docs/middleware/openapi.md, beside the example code that
produces them.

Changes introduced

  • OpenAPI Middleware Package: New middleware that automatically generates OpenAPI 3.0 JSON specifications from your Fiber application routes

  • Route Metadata Support: Extended the Route struct with OpenAPI-specific fields including Summary, Description, Tags, Parameters, RequestBody, Responses, Consumes, Produces, and Deprecated

  • Fluent API Methods: Added chainable methods to App, Group, and domainRouter for documenting routes inline (e.g., .Summary(), .Description(), .Tags(), .Parameter(), .Response(), .RequestBody())

  • Schema References: Support for OpenAPI schema references ($ref) and examples at the parameter, request body, and response levels

  • Auto-filtering: Automatically filters out Fiber's auto-generated HEAD routes (via Route.IsAutoHead()) and middleware routes registered with Use() (via Route.IsMiddleware()) to avoid cluttering the spec with synthetic operations

  • Route Introspection Methods: Added IsMiddleware() and IsAutoHead() public methods on Route to allow middleware and external consumers to distinguish middleware/auto-generated routes from user-defined routes

  • Flexible Configuration: Per-route metadata can be provided via fluent API or global middleware config (keyed by Fiber route syntax, e.g. GET /users/:id), with config taking precedence

  • Explicit Request Body Suppression: A non-nil config RequestBody with an empty Content map is treated as an explicit "no request body" override, preventing the default auto-insertion for POST/PUT/PATCH methods

  • Group Support: Correctly handles grouped routes and mounted sub-apps with proper path resolution

  • Domain Router Support: All OpenAPI fluent methods are implemented on domainRouter, ensuring domain-scoped routes can be documented identically to standard routes

  • Safe Route Cloning: copyRoute() deep-clones all OpenAPI-related fields including Tags, Parameters, Responses, and RequestBody to prevent shared backing arrays between mounted/cloned apps

  • Immutable Route Metadata: App.Tags() defensive-copies the incoming variadic slice before storing, preventing caller-side mutations from affecting route metadata

  • OpenAPI Spec Validity: buildRequestBody() omits the request body entirely when content is empty, preventing invalid OpenAPI documents with "content":null

  • Merge Conflict Fixes: Resolved duplicate field declarations in Route struct, handler type conversion issues, semantic conflicts in test files, and integrated parallel benchmark tests from main branch

  • Code Quality Improvements: Fixed all lint issues (deprecated utils.ToLower replaced with utilsstrings.ToLower, 28 httpNoBody warnings, 5 whyNoLint warnings, 4 paramTypeCombine warnings, 2 hugeParam warnings), applied struct alignment optimizations (reduced Operation struct from 136 to 128 bytes, Media struct from 48 to 40 bytes), and ensured code passes all quality checks with 0 issues

  • Security Hardening:

    • Input Validation: Consumes() and Produces() now trim whitespace before validation, preventing unexpected panics from inputs like " application/json" or trailing spaces
    • OpenAPI Path Template Generation: Implemented convertToOpenAPIPath() function that properly converts Fiber route patterns to valid OpenAPI path templates by stripping type constraints (:id{id}), handling regex constraints, converting wildcards (* and +{wildcard}), and skipping optional markers (?)
    • Nil Pointer Protection: Added defensive nil check in appendOrReplaceParameter() to prevent potential runtime panics if code is refactored
    • Bounds Checking: All array/string indexing operations in convertToOpenAPIPath() properly guarded with length checks to prevent index out of bounds errors
    • Comprehensive Testing: Added 9 test cases covering simple paths, parameters with constraints, regex constraints, optional parameters, wildcards, plus params, multiple parameters, and various delimiters
  • Documentation Improvements:

    • Caching Behavior: Added explicit documentation explaining that the OpenAPI spec is generated once on the first matching request and cached for the process lifetime, warning users to register the middleware after all routes
    • Markdown Compliance: All documentation properly formatted and passing markdown linting with 0 errors
  • Test Coverage Improvements: Comprehensive test suite with 93.1% code coverage (exceeding 90% goal)

    • Added 10 new test functions covering request body merge scenarios, media content defaults, path resolution edge cases, parameter merging, schema handling, HTTP method logic, nil parameter handling, marshal errors, and empty media types
    • All tests use t.Parallel() for concurrent execution
    • Per-function coverage improvements: mergeConfigParameters (76.9% → 92.3%), buildRequestBody (58.8% → 94.1%), schemaFrom (70.0% → 90.0%), shouldIncludeRequestBody (77.8% → 88.9%), resolvedSpecPath (70.6% → 82.4%), convertMediaContent (63.2% → 78.9%)
  • Benchmarks: No performance impact as spec generation happens once on first request via sync.Once. Merged 17 parallel benchmark tests from main branch to ensure thread-safety of router operations.

  • Documentation Update: Added comprehensive documentation at docs/middleware/openapi.md with examples and configuration options. Operations key format clarified to use Fiber route syntax (e.g. GET /users/:id). Added explicit caching behavior warnings. All markdown properly formatted and passing linting.

  • Changelog/What's New: OpenAPI middleware enables automatic API documentation generation from route definitions. Default responses documented as 200 OK for most methods, 204 No Content for DELETE and HEAD. Properly handles Fiber route constraints and wildcards in generated OpenAPI paths.

  • Migration Guide: No migration needed - this is a new opt-in middleware

  • API Alignment with Express: Not applicable - OpenAPI specification is framework-agnostic

  • API Longevity: The middleware uses OpenAPI 3.0 standard with extensible configuration structures to accommodate future enhancements. Security hardening ensures production stability.

  • Examples: Documentation includes examples for basic usage, custom metadata, schema references, grouped routes, and proper middleware registration order

Type of change

  • New feature (non-breaking change which adds functionality)
  • Code consistency (non-breaking change which improves code reliability and robustness)
  • Performance improvement (non-breaking change which improves efficiency)

Checklist

  • Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Updated the documentation in the /docs/ directory for Fiber's documentation.
  • Added or updated unit tests to validate the effectiveness of the changes or new features.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • Aimed for optimal performance with minimal allocations in the new code.
  • Provided benchmarks for the new code to analyze and improve upon.
  • Completed comprehensive security audit to prevent runtime panics and ensure production stability.
  • Achieved 93.1% test coverage with comprehensive test suite covering all edge cases.

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

@coderabbitai

coderabbitai Bot commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2025-08-21-14-48-18

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gaby gaby changed the title feat: add openapi middleware 🔥 feat: Add OpenAPI middleware Aug 21, 2025
@gaby gaby added the v3 label Aug 21, 2025
@gaby gaby added this to v3 Aug 21, 2025
@gaby gaby added this to the v3 milestone Aug 21, 2025
@codecov

codecov Bot commented Aug 21, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.24190% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.79%. Comparing base (ea98ecc) to head (e69a9a4).

Files with missing lines Patch % Lines
middleware/openapi/openapi.go 96.01% 20 Missing and 13 partials ⚠️
app.go 93.89% 14 Missing and 10 partials ⚠️
middleware/openapi/schema.go 95.08% 9 Missing and 3 partials ⚠️
router.go 96.63% 5 Missing and 6 partials ⚠️
middleware/openapi/config.go 94.06% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3702      +/-   ##
==========================================
+ Coverage   94.55%   94.79%   +0.24%     
==========================================
  Files         140      144       +4     
  Lines       17010    19143    +2133     
==========================================
+ Hits        16084    18147    +2063     
- Misses        581      625      +44     
- Partials      345      371      +26     
Flag Coverage Δ
unittests 94.79% <96.24%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gaby gaby moved this to In Progress in v3 Aug 21, 2025
@gaby

gaby commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@gaby

gaby commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new OpenAPI middleware for auto-generating API specifications. The implementation is solid, with good test coverage and documentation. I've identified a potential improvement to prevent the middleware from documenting its own endpoint in the generated spec, which would make the output cleaner for API consumers. I also found a minor formatting issue in the documentation. Overall, this is a great feature addition.

Comment thread docs/middleware/openapi.md Outdated
Comment thread middleware/openapi/openapi.go Outdated
@ReneWerner87

Copy link
Copy Markdown
Member

nice feature, thx @gaby
can you update and convert the DRAFT to READY (when it is ready for you)

@gaby
gaby requested a review from Copilot October 25, 2025 16:44
@gaby

gaby commented Oct 25, 2025

Copy link
Copy Markdown
Member Author

/gemini review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces an OpenAPI middleware that auto-generates OpenAPI 3.0 specifications from registered Fiber routes. The implementation adds per-route metadata capabilities (summary, description, tags, parameters, request/response bodies, deprecation) that can be configured either through fluent route builders or global middleware configuration.

Key changes:

  • New OpenAPI middleware package with spec generation and JSON serving
  • Extended Route struct with OpenAPI-specific metadata fields
  • Added fluent API methods to both App and Group for route documentation

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
middleware/openapi/openapi.go Core middleware implementation generating OpenAPI spec from routes
middleware/openapi/config.go Configuration types for middleware and operation metadata
middleware/openapi/openapi_test.go Comprehensive test suite covering spec generation scenarios
middleware/openapi/testdata/openapi.json Golden file fixture for JSON output validation
router.go Extended Route struct with OpenAPI fields and helper types
router_test.go Tests for new route metadata methods
app.go Fluent API methods for documenting routes
group.go Group delegation methods for route documentation
group_test.go Tests for group-level route documentation
docs/middleware/openapi.md User-facing middleware documentation
docs/whats_new.md Release notes entry

@gaby

gaby commented Oct 25, 2025

Copy link
Copy Markdown
Member Author

@codex review and suggest feature improvements.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new OpenAPI middleware to auto-generate API specifications from routes. The implementation is comprehensive, adding new methods to App and Group for route-level documentation, and providing configuration options for global overrides. The code is well-structured and includes extensive tests.

I have two main points of feedback. The most critical one is that the current implementation incorrectly includes middleware routes (from app.Use) in the generated OpenAPI spec, which should be fixed to ensure a clean and accurate API definition. The other is a minor improvement to an error message for better debugging. Overall, this is a great feature addition.

Comment thread middleware/openapi/openapi.go Outdated
Comment thread router.go Outdated
Comment thread app.go Outdated
@gofiber gofiber deleted a comment from chatgpt-codex-connector Bot Oct 25, 2025
@gaby

gaby commented Oct 25, 2025

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread middleware/openapi/openapi.go
@ReneWerner87

Copy link
Copy Markdown
Member

@gaby can you update this PR

@gaby

gaby commented Mar 27, 2026

Copy link
Copy Markdown
Member Author

Fixing conflicts this weekend

claude and others added 12 commits August 15, 2026 18:13
Copy depth (router.go). copyCompositeValue restarted the depth counter at zero,
so a cycle reachable only through a named map or slice type never reached
maxCopyDepth. Reproduced: `type metadata map[string]any` holding itself
stack-overflowed the process, while the same cycle in a plain map[string]any
returned fine. Depth is now threaded through the reflected copier.

Typed securityScheme maps (middleware/openapi). buildComponents asserted
Components["securitySchemes"] to map[string]any; a typed map such as
map[string]MyScheme failed the assertion and was then overwritten wholesale,
silently dropping the caller's schemes and any $ref pointing at them. Reproduced
against a typed map, which lost its entry while the untyped control kept both.
Now read through a reflective string-keyed accessor.

Compression and scoped helpers (router.go, app.go). Two routers registering the
same method and path share one stack entry, and the entry was restamped with the
later regID, leaving the earlier scope's helpers unable to find it. Reproduced:
after two groups registered GET /g/same, g1.Summary landed nowhere while
g2.Description applied. The restamp is kept so the batch fast path and the stack
scan agree, and the superseded ID is now aliased onto the survivor.

Group mounts (mount.go). Group.Use discarded the mount's registration ID, so a
chained helper retargeted the route registered before the mount. Reproduced:
grp.Use("/api", sub).Tags("mounted") tagged /v1/health. The cursor now advances
onto the mount, matching App.Use, making the helper a no-op.

Two further findings did not reproduce and are left unchanged: auto-HEAD twins
for domain-mounted sub-apps already answer HEAD, and pruning a twin on one
domain does not disturb another's.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
The reflective accessor added with the merge fix landed at 33% statement
coverage: the probe that drove it out was a throwaway. Covers the branches it
actually has — nil, a non-map, non-string keys, a nil typed map, the
map[string]any fast path and the typed path — plus an end-to-end assertion that
a typed Components map merges with Config.SecuritySchemes rather than being
replaced by it.

Package coverage holds at 97.7%; stringKeyedEntries and buildComponents are
both at 100%.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
Brings in #4620, the fix for the nested-domain-mount panic this branch reported
as #4618. Main reworked mount.go, domain.go and router.go — the same files this
branch changes — so three files conflicted.

domain.go: took main's side. cloneRoutesForDomain replaces the inline clone loop
and already snapshots under the sub-app's lock, which is what this branch's
locking was for, and does it without ever holding two apps' locks.

mount.go: took main's structure, restored the regID clearing a clone still needs,
and rebuilt the loop around a locked snapshot. Taking main's version verbatim
reintroduced a data race that Test_Mount_StartupConcurrentSubAppDocHelpers
catches: processSubAppsRoutes read the sub-app's stack while another goroutine
registered on it. The snapshot follows the pattern main uses in domainRoutes —
clone under the lock, re-parse outside it — so only clones cross the boundary.

router.go, auto-HEAD keying: combined rather than picked. Main keys twins by
{owner, path}, which separates mounted apps behind one host-scoped wrapper.
This branch keys by domain+path, which separates routes registered on different
domain routers — a case main cannot express, since Route.domain does not exist
there. Both are needed, so the key now carries owner, domain and path.

router.go, copyRoute: kept this branch's copyRouteValue indirection and carried
main's comment across. The comment matters now: omitting group is load-bearing
for the #4618 fix, and this branch's copyRouteInto already clears it.

router.go, elsewhere: took main's Params rebuild (a superset that also preserves
parameter-name case) and merged the twin construction, which now marks owner and
constraints as main does and blanks documentation as this branch does.

Verified: build, vet, golangci-lint (0 issues), full go test ./..., -race on root
and middleware. Behaviour checked from both sides — two domain routers sharing a
path each keep their HEAD twin, and a domain mount whose sub-app mounts another
app answers 200 on the matching host and 404 elsewhere instead of panicking.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
…chain

The repeated CI job runs go-version "stable", now Go 1.27, where all three
shards failed Test_SchemaOf_InvalidJSONTagName while every pinned unit leg
(1.25.x and 1.26.x) passed. The job's -race -count=5 -shuffle=on flags were a
red herring: the flags reproduce nothing, the toolchain reproduces it every
time.

encoding/json changed which struct tag names it accepts. Diffing every printable
ASCII name across 1.25 and 1.27 turns up four divergences, not one:

    a'b    rejected, field name used  ->  truncated to "a"
    a`b    rejected, field name used  ->  truncated to "a"
    a\b    rejected, field name used  ->  accepted
    a<tab>b rejected, field name used ->  accepted

isValidJSONTagName reimplemented encoding/json's private isValidTag, so it now
disagreed with the runtime and SchemaOf documented a property the wire did not
carry — the same class of defect the function was added to prevent.

Rather than chase those rules per release, the plain names every version has
taken as written keep the static fast path, and anything else is resolved by
asking encoding/json directly: marshal a one-field probe struct and read the key
back. Real struct tags never reach it, so no hot path is involved.

The probe has to quote the name into the tag rather than splice it — a tag value
is an unquoted Go string literal, so a spliced backslash round-trips through
StructTag.Get as an escape and asks about a different name entirely. That bug
was in the first version of this fix and is what the 1.27 run then caught.

The test no longer hard-codes names whose handling is version-dependent; it pins
the invariant that matters, that the schema names match the wire, and the probe
is checked against encoding/json on whichever toolchain compiles it.

Verified on 1.25.0, 1.26.0 and 1.27.0: full go test ./..., plus the repeated
job's exact ./... -race -count=5 -shuffle=on under 1.27. Lint clean.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
Router:
- Compression merges now record the shared entries per superseded
  registration instead of aliasing the whole registration ID, so a scope's
  helpers reach exactly the entries it registered: no more skipping an
  unmerged entry or documenting a method another scope added. Removed
  entries are forgotten again.
- Concurrent multi-method registrations no longer leave a partial batch;
  only the newest registration owns it.
- Automatic HEAD twins carry no registration ID, so a stack scan cannot
  document them, and Name() propagates to a GET route's existing twin again.
- App.Name fires OnName only when a route was actually named, not for a
  mount placeholder.
- app.Group(prefix, mw) and domainRouter.Group keep the middleware
  registration on the new scope, matching Group.Group.
- RemoveRouteFunc evaluates the matcher without the router lock, and a
  mounted app's startup twin hooks fire after the parent unlocks, so both may
  call GetRoute/GetRoutes without deadlocking.
- Redirect().Route and GetRouteURL use a routing-only lookup instead of a
  full documentation deep copy per request.
- Nested empty maps in documentation are kept as {} instead of null, and a
  batch's routes each get their own response Example copy.

OpenAPI middleware:
- A self-referential pointer field type no longer hangs SchemaOf.
- Constraint parsing mirrors path.go: the span closes at the first unescaped
  '>', a regex argument is kept whole (commas included) and other arguments
  are unescaped.
- A trailing slash no longer defeats prefix resolution under a dynamic mount.
- "*" is documented like an optional parameter, since the router also serves
  the path without it.
- AddParameter with only a description keeps the constraint-derived schema.
- Configuration deep copies are depth-bounded against cyclic values.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
- Drop the doc comments added alongside new struct fields (App.latestBatch,
  mergedEntries, latestBatchID, routesRevision, registrationID; Route.domain
  and regID; the per-router lastRegID; the internal openapi structs), matching
  the terse style of the fields already there.
- Remove the comments from the test files this branch adds, keeping only
  //nolint directives.
- Condense the remaining prose: the Router and Register interfaces carry one
  group header instead of a comment per documentation method, and the long
  explanations in app.go, router.go, mount.go, schema.go and constraint.go
  are cut to what the reader cannot get from the code.

Config field comments keep the "Optional. Default:" form the other middleware
packages use.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
main now requires Go 1.26, which turns on modernize's newexpr check and made
the lint job fail on openapiBoolPtr. Drop the helper for the built-in, the same
substitution be8f165 made for the client package's ptrInt and ptrString.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
gaby and others added 4 commits September 5, 2026 13:14
main's #4652 landed a per-registration Route.id and an auto-HEAD scan skip that
overlap with this branch's own bookkeeping, so the two had to be reconciled
rather than taken side by side:

- Route.regID is gone. main's id is the same notion — one value per register()
  call, shared by that call's per-method entries — so the scoped helpers key off
  it and the hot struct keeps a single counter. App.registrationID gives way to
  main's process-wide routeIDs, which also removes the cross-app collision the
  clone in processSubAppsRoutes used to clear its id to avoid.
- Auto-HEAD twins keep the id they copy from their GET route, because
  routeIndexInTree finds a route in another method's tree by it. Documentation
  is kept off them by their autoHead flag instead, which is what the stack scan
  in applyToRegIDLocked now skips.
- Twin OnRoute hooks still fire unlocked, so a sub-app hook may call back into
  the parent. To keep main's guarantee that an aborted pass is retried,
  fireOnRouteHooks clears the scan markers unless every hook returned, and
  RebuildTree fires the twins it used to discard.
- App.Name keeps this branch's registration-scoped form, which already covers
  the id match main added and still names the GET route's HEAD twin.
- copyRoute keeps the single-struct-copy form: it preserves every field main
  lists, id included, and additionally clones the documentation containers.
- buildTree returns nothing now that RebuildTree no longer forwards its result.

Verified on the merged tree: build, vet, golangci-lint (0 issues), go test ./...,
and -race on the core and openapi packages. openapi coverage unchanged at 97.3%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
Registration bookkeeping keeps one index instead of three mechanisms. A
registration's live entries are recorded under its id (a compression-merged
entry under both ids), so the batch fast path, the stack scan with its autoHead
and mount exclusions, the mergedEntries side map and the id restamp on merge
all go. Entries keep the id main assigned them, which is what routeIndexInTree
finds them by. A registration that lists a method twice is indexed once, where
it used to apply an appending helper twice.

App is now the same kind of scope as Group and Registering: Name and every doc
helper go through the registration path, so latestRoute, its sentinel in New,
the reset in deleteRoute and applyToLatestRouteLocked are gone, and the OnName
hook protocol lives in one place.

domainRegistering is folded into Registering, which takes an optional handler
wrapper and host pattern; that removes 217 lines that mirrored register.go. The
scoped Parameter/RequestBody/Response forms delegate to their full forms as App
does, and docResponseHeader/docResponseLink share one factory.

The doc factories no longer deep-copy in both the factory and the per-route
closure; the closure's copy is the only one. GetRoutes sizes its result and
fills each slot in place. copyRouteBase blanks the documentation scalars, so
the auto-HEAD twin no longer does it by hand. fireOnRouteHooks drops the
scan-marker reset: twins are in the stack and the scan recorded before any
hook runs, so a panicking hook could only force a no-op rescan.

The middleware resolves the app's case rule once per app rather than copying
the 624-byte Config on every request, builds the spec from GetRoutes(true)
since middleware routes are skipped anyway, references the private route
snapshot instead of cloning it a second time, appends path parameters in place
and forks only for optional ones, and loses a dead predicate, two no-op
fallbacks, a dead branch and a duplicate pointer-deref loop. domainMatcher
carries its joined pattern instead of re-joining it per registration.

Two tests pin the new behaviour: one handler shared by two apps with different
CaseSensitive settings, and a duplicate-method registration indexed once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Blocking API-compatibility, concurrency, specification-validity, and credential-persistence issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

router.go:52

  • Adding documentation methods to the exported Router interface is source-breaking for every external type that currently implements fiber.Router; those implementations will stop compiling even though this PR is described as non-breaking. Please expose these helpers through a separate capability/interface or explicitly treat and document this as a breaking API change.
  • Files reviewed: 25/33 changed files
  • Comments generated: 10
  • Review effort level: Balanced

Comment thread middleware/openapi/openapi.go Outdated
Comment thread register.go
Comment thread router.go
Comment thread router.go
Comment thread app.go
Comment thread app.go Outdated
Comment thread middleware/openapi/config.go
Comment thread middleware/openapi/openapi.go
Comment thread app.go
Comment thread docs/whats_new.md Outdated
claude and others added 9 commits September 6, 2026 23:15
- deleteRoute now hands matchFunc a snapshot taken under app.mutex instead
  of the live stack entry, so a user matcher cannot read a route a
  concurrent registration is still writing to. The live pointers travel
  alongside it for identity-based removal, so the callback still runs
  unlocked and may call locking methods such as GetRoute.
- pruneAutoHeadRouteLocked compares the full autoHeadKey rather than the
  path alone. Automatic HEAD twins are created per key, so path-only
  matching let an explicit HEAD registration on one domain drop another
  domain's twin and leave a duplicate ahead of the explicit handler.
- ResponseHeader falls back to a string schema when none is given. A Header
  Object follows the Parameter Object and carries a schema or a content
  map, so description-only headers produced an invalid document.
- Content maps are keyed by the media type that was validated. A padded key
  such as " application/json " passed validation but was emitted verbatim.
  A collision after trimming panics rather than dropping an entry.
- Swagger UI no longer forces persistAuthorization: it stores credentials
  across browser restarts, and Swagger UI itself defaults it off. Users can
  still opt in through SwaggerOptions.
- whats_new no longer claims automatic HEAD routes carry no name; they
  mirror the name of the GET route they were built from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
Covers the empty-content early return in sanitizeContentMediaTypes and the
blank-name panic in docResponseHeader.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
main's #4663 fixed route naming with a Route.latestID that a merged route
adopts from the merging registration. This branch already reaches the same
routes through the regEntries registration index, which covers merges by
indexing the shared entry under both ids, so latestID is dropped and
App.Name keeps the nameRegistrationLocked path. The group hand-off that
came with it stays: a merged entry takes the group of the registration
that merged into it, so Name prefixes with the group it was written
through. All four naming tests main added pass unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
- RemoveRoute and RemoveRouteByName match under the lock again: their
  matchers only compare a field, so the per-route documentation deep copy
  deleteRoute made for every candidate was wasted work held under
  app.mutex. Only RemoveRouteFunc, whose matcher is user code, goes through
  the snapshot path, which now fills each slot in place. The removal loop
  is shared by both.
- The automatic HEAD twin for a key is found in one place,
  autoHeadTwinLocked, used by both pruning and Name propagation. It drops
  the re-normalization of an already canonical path, which could miss a
  twin whose stored path ends in an escaped slash, and rejects on the
  string fields before the owner lookup.
- sanitizeContentMediaTypes validates once and rebuilds the map only when
  a key actually changed.
- The header schema comment no longer claims parity with the parameter
  rule, which merges a default type into a supplied schema; headers store a
  supplied schema as given.
- RemoveRouteFunc documents that its matcher sees a copy, in code and in
  docs/api/app.md.
- Tests: the domain-scoped prune test builds twins through RebuildTree
  rather than a request, header assertions compare whole maps, and the
  Swagger UI opt-in check joins the existing options test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
main's #4666 dropped the strconv import from app.go once its last use
became utils.FormatInt. The merge is textually clean but no longer
compiled: responseKey and defaultResponseDescription, added on this
branch, still called strconv.Itoa. Both now use utils.FormatInt, the
helper main adopted, which formats every int the same way.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XHncVMdy1VPWCTp53PQoYG
@github-actions

Copy link
Copy Markdown
Contributor
4 benchmarks faster (up to 1.71x)
Benchmark Base → Current
Benchmark_ShutdownServices/no-services v3 ⚡ 1.71x 6496 → 3802 ns/op
Benchmark_isValidToken68 v3/extractors ⚡ 1.70x 125 → 73.65 ns/op
Benchmark_ShutdownServices/single-service v3 ⚡ 1.59x 7731 → 4852 ns/op
Benchmark_ShutdownServices/multiple-services v3 ⚡ 1.59x 10018 → 6302 ns/op

e69a9a4 vs main@ef013fa · 1699/1822 results compared · 123 new, 0 gone · retest: 4/4 improvements reproduced · noise-aware thresholds · full results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

7 participants