Skip to content

Defer reading the OpenAPI spec until it is first requested - #569

Open
jonbartels wants to merge 1 commit into
OpenIntegrationEngine:mainfrom
jonbartels:defer-swagger-spec-read
Open

jonbartels wants to merge 1 commit into
OpenIntegrationEngine:mainfrom
jonbartels:defer-swagger-spec-read

Conversation

@jonbartels

Copy link
Copy Markdown
Contributor

What

SwaggerServlet.init no longer calls .read() on the OpenAPI context it builds. It still registers the configuration (title, description, and the resource classes to document).

Why

SwaggerServlet doesn't serve anything; its only job is to register the OpenAPI configuration. The spec is served by swagger's OpenApiResource inside Jersey. On its first request, OpenApiResource looks up a context under its own servlet's id. It doesn't find one, so it creates a child context whose parent is the default context SwaggerServlet registered, and reads the spec again. The spec read at startup was never used.

Evidence: with the current code, the first /api/openapi.json request already takes ~460 ms (and the first /api/4.7.0/openapi.json ~230 ms), even though SwaggerServlet had just read the spec during startup.

Results

Apple M4, JDK 25, default Derby config, no channels; warm medians of 5 runs:

Before After
SwaggerServlet.init ~660 ms ~120 ms
Jetty start() ~1170 ms ~620 ms
Launch → "Web server running at" ~4.46 s ~3.64 s
First /api/openapi.json request ~460 ms ~700 ms
Later /api/openapi.json requests ~15 ms ~15 ms

The cost of the first spec request moves from startup to whoever requests it first (normally someone opening the Swagger UI).

Verification

  • /api/openapi.json, /api/4.7.0/openapi.json and /api/openapi.yaml all return 200 with the "Open Integration Engine Client API" title.
  • The served JSON and YAML have identical content before and after once keys are sorted (jq -S). The raw bytes differ, but key order already varies between two runs of unmodified code.
  • ./gradlew :server:test: 834 of 836 pass. The two failures (TcpDispatcherTest.testMLLPMessageSent, testMessageSentWhenOneOfMultipleClientsClosed) are a BindException on the test's ports, which count up from 9000. On my machine IntelliJ listens on 9000/9003, so these fail on unmodified main as well.

🤖 Generated with Claude Code

Copilot AI 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.

🟢 Approval recommended

No active review issues were identified for the change deferring OpenAPI specification loading until the first request.

No open issues 🎉

What changed in this PR

Defers OpenAPI specification reading until the first request, reducing server startup work while preserving configuration registration in SwaggerServlet.

Changes:

  • Removes the eager .read() call after building the OpenAPI context.
  • Adds a comment explaining that OpenApiResource reads the specification on its first request.
File Description
server/​src/​main/​java/​com/​mirth/​connect/​server/​servlets/​SwaggerServlet.java Retains OpenAPI configuration registration without eagerly reading the specification.

Review effort: Balanced

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Test Results

  142 files  ±0    142 suites  ±0   3m 28s ⏱️ +41s
  937 tests ±0    937 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 099 runs  ±0  1 093 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit 142bb4e. ± Comparison against base commit 594c6fb.

♻️ This comment has been updated with latest results.

@tonygermano tonygermano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not approved yet. The change is good. I'll approve once the comment and the commit message describe it as a trade-off. The startup read wasn't wasted: it warmed up the first spec request (~460 ms → ~700 ms in your results).

  1. Code comment: please reword it, for example:

    		// Only register the configuration here. OpenApiResource builds its own child context from it
    		// and reads the spec on the first request; it never uses a read done here. Reading here would
    		// only warm up classes for that first request, at the cost of slower server startup.
    
  2. Commit message: please add a paragraph after "Register the configuration only", for example:

    The startup read did warm up class loading and Jackson introspection,
    so the first spec request now takes longer (about 460 ms to 700 ms on
    a warm start). That request normally comes from someone opening the
    Swagger UI, which is a better place for the cost than server startup.
    

FYI, no action needed: Claude checked the change against the swagger-core 2.2.41 sources. OpenApiResource reads the spec in its own child context with its own cache, so the served spec is unchanged. Claude also started the server built from this PR and from fdff780 and compared the responses: /api/openapi.json, /api/4.7.0/openapi.json and /api/openapi.yaml return the same content with keys sorted, and the Swagger UI page loads.


🤖 Drafted with Claude Code

SwaggerServlet read the full OpenAPI spec during server startup. Nothing
used that result: OpenApiResource, which serves /api/openapi.json and
/api/openapi.yaml, builds its own child context from the configuration
SwaggerServlet registers and reads the spec again on the first request.

Register the configuration only. The served spec is unchanged, and the
Swagger servlet's init drops from about 660 ms to about 120 ms on a warm
start, taking about 0.5 s off the time until the web server is up.

The startup read did warm up class loading and Jackson introspection,
so the first spec request now takes longer (about 460 ms to 700 ms on
a warm start). That request normally comes from someone opening the
Swagger UI, which is a better place for the cost than server startup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jon Bartels <jonathan.bartels@gmail.com>
@jonbartels
jonbartels force-pushed the defer-swagger-spec-read branch from 5ff7cdb to 142bb4e Compare November 10, 2026 14:16

@tonygermano tonygermano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the quick turnaround on my requests and good find!

@mgaffigan mgaffigan 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.

Can you add a call to the endpoint to CI? This would allow silent failures since nothing would be logged if the document cannot be generated on boot.

I agree with the change - but need testing here. I would not go wild. A 200 with assert contains x string is sufficient

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.

4 participants