Skip to content

Conversation

@bjhham
Copy link
Contributor

@bjhham bjhham commented Jan 29, 2026

Subsystem
Server, Swagger

Motivation
KTOR-9293 OpenAPI describe needs defaults

The Swagger front-end breaks when Operation is an empty object.

Solution
Added an empty string summary for the Operation type.

@bjhham bjhham requested a review from zibet27 January 29, 2026 13:38
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

Changes to how empty and null summaries are handled in OpenAPI operations. Operation.Builder.summary default changed from null to empty string, merging logic updated to prefer non-empty summaries, and test assertion added to verify empty string output.

Changes

Cohort / File(s) Summary
Operation Summary Default & Merging Logic
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt, ktor-server/ktor-server-plugins/ktor-server-routing-openapi/common/src/io/ktor/server/routing/openapi/OpenApiRoutes.kt
Modified Operation.Builder.summary default initialization from null to empty string (""). Updated Operation.plus merging logic to only propagate non-empty summaries, falling back to other.summary if primary is empty, resulting in empty string if both are empty/null.
Test Assertion
ktor-server/ktor-server-plugins/ktor-server-routing-openapi/jvm/test/io/ktor/server/routing/openapi/DescribeRouteTest.kt
Added assertion in annotateAddition test to verify generated OpenAPI JSON contains "summary" field with empty string value for /messages route.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding an empty summary default for operations, which aligns with the primary purpose of the PR.
Description check ✅ Passed The description follows the required template with all sections completed: Subsystem (Server, Swagger), Motivation (references KTOR-9293 and explains the Swagger front-end issue), and Solution (describes adding empty string summary).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

* [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.openapi.Operation.Builder.summary)
*/
public var summary: String? = null
public var summary: String? = ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make it non-nullable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be a breaking change 😞

@zibet27
Copy link
Collaborator

zibet27 commented Jan 29, 2026

Looks like io.ktor.openapi.OperationSerializationTests.operation with tags and external docs()[jvm] is failing

@bjhham
Copy link
Contributor Author

bjhham commented Jan 29, 2026

Yeah it seems that the empty string uncovered a bug in the YAML parser.

It creates something like:

summary:
responses:
  200: yadda yadda

Then interpreting responses as being part of summary. YAML is hard.

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.

3 participants