KTOR-8936 Improvements for the routing annotation API #5206
Conversation
WalkthroughAdds a JsonSchema-first OpenAPI model and inference, a composable OperationMapping pipeline (PopulateMediaTypeDefaults, CollectSchemaReferences), integrates operation post‑processing into route annotation/OpenAPI generation (Route.annotate, RoutingNode.findPathItems, generateOpenApiSpec), and updates many builders to accept JsonSchemaInference. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
6 times, most recently
from
772c348 to
5191ed1
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
1031-1156: Header extensions are discarded during build—implement Extensible pattern to preserve user-configured extensionsThe review comment is accurate. Verification confirms:
Header.Builderdefinesextensions: MutableMap<String, GenericElement>(line 1109) withextension()DSL method (lines 1127-1130)Header.build()(lines 1143-1155) constructsHeaderwithout passingextensionsHeaderdata class (lines 1064-1075) lacks theextensionsproperty and does not implementExtensible- In contrast,
Parameter,Response,Link,RequestBody,Server,PathItem,MediaType,ExampleObject,Encoding, and other classes in the module implementExtensiblewithoverride val extensions: ExtensionPropertiesand useExtensibleMixinSerializerThis silently discards any
x-*extensions configured via the DSL, breaking consistency with the OpenAPI spec model and established patterns in the codebase.To fix, make
Headerextensible by:
- Adding
override val extensions: ExtensionProperties = nullto the data class- Implementing
: Extensibleinterface- Setting up a companion
SerializerusingExtensibleMixinSerializer(as shown inParameter)- Updating
Header.build()to passextensions.ifEmpty { null }
🧹 Nitpick comments (10)
ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt (1)
130-139: Location header serialization looks correct; consider adding a shape assertionThe new
Locationheader uses the sameContentType.Text.Plain()pattern as other places and will be covered by the round‑tripcheckSerialization. If you want stronger regression protection around header modeling, you could add a focused test that asserts the serialized header structure explicitly, but the current change is fine as is.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
35-54: PopulateMediaTypeDefaults logic is sound; minor readability/refactor opportunitiesThe defaulting behavior for parameters (filling
inwithqueryandcontentwithMediaType.Textwhen bothschemaandcontentare null) is correct and matches OpenAPI expectations.Two small improvements you might consider:
- Make the predicate a bit clearer with explicit parentheses:
.any { (it.value.schema == null && it.value.content == null) || it.value.`in` == null }
- Use the new
mapValuehelper onReferenceOrto avoid manualvalueOrNullhandling:parameters = operation.parameters?.map { ref -> ref.mapValue { param -> param.copy( `in` = param.`in` ?: ParameterType.query, content = param.content ?: MediaType.Text.takeIf { param.schema == null }, ) } }ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt (1)
46-65: New ReferenceOr mapping helpers and serializer refactor look correct
mapValueandmapToReferencecorrectly:
- Leave
Referenceinstances untouched (benefiting fromoutvariance),- Apply the mapping only to
Valuebranches.- The
encodeReferencehelper keeps the old behavior of switching between$refand$recursiveRefshapes, just with clearer separation.Only a tiny nit: the KDoc for
mapToReferencecould explicitly say “Same asmapValue, but for functions returningReferenceOr<B>” to make the relationship clearer, but the implementation itself is solid.Also applies to: 77-108
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
28-36: Full OpenAPI spec golden string plus OpenApiSpecification decode provide strong coverageSwitching
routeAnnotationIntrospectionto:
- generate a full
OpenApiSpecificationviagenerateOpenApiSpec,- drop
/routesfrompaths,- assert exact equality against the pretty‑printed
expectedJSON, and- then decode back into
OpenApiSpecification,gives good end‑to‑end coverage of spec generation, including components and schema references. The strict string comparison is a bit brittle to future field‑ordering tweaks but acceptable here since you control both model and serializer configuration.
Also applies to: 37-76, 77-103, 113-131, 165-168
170-187: Header annotation and media type handling in annotateAddition are exercised appropriatelyThe
annotateAdditiontest now:
- Uses
generateOpenApiSpec(...).copy(paths = paths - "/routes")for consistency with the first test, and- Verifies that both
X-FirstandX-Secondannotated headers appear in the generated spec, withX-Secondexplicitly usingContentType.Text.Plain().This is a good sanity check on header aggregation. If you later need to assert the header content shapes (e.g., default media‑type map for
text/plain), you could extend the test to decode intoOpenApiSpecificationand inspect the response headers, but it’s not strictly necessary now.Also applies to: 189-202
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.kt (2)
20-34: Tighten invariants and document thread-safety of lazy schema cache
getrelies ontype!!being non-null whenever_schemais null, which currently holds only because the constructor is private and both factories enforce it. To make this more robust against future changes (or reflective construction) and easier to reason about, consider adding aninitinvariant and optionally clarifying thread-safety:public class JsonSchemaReference private constructor( public val type: KType?, private @Volatile var _schema: JsonSchema?, ) { init { require(type != null || _schema != null) { "JsonSchemaReference must have either a type or a schema" } } public fun get( inferenceFunction: (KType) -> JsonSchema = ::buildKotlinxSerializationSchema, ): JsonSchema = _schema ?: inferenceFunction(checkNotNull(type)).also { _schema = it } }This both encodes the invariant and makes the lazy cache semantics under concurrent access clearer;
@Volatileis optional but avoids stale reads when the same instance is shared across threads.
36-49: Clarify behavior when type-based references are not serializable
JsonSchemaReferenceSerializer.serializealways callsvalue.get(::buildKotlinxSerializationSchema). For instances created viaofTypewhoseKTypedoesn’t have a registered kotlinx-serialization serializer, this will throw instead of producing a fallback “error schema” (unlikeJsonSchemaInference.jsonSchema<T>(), which catchesSerializationExceptionand returns a descriptive placeholder schema).If this difference is intentional (i.e., failing hard when serializing an unsupported type reference), it would be helpful to document it on
JsonSchemaReference/JsonSchemaReferenceSerializer. If not, consider aligning the behavior withJsonSchemaInference.jsonSchemaby catchingSerializationExceptioningetor in the serializer and emitting an error-describingJsonSchemainstead.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (1)
118-135: operationFromAnnotateCalls assumes non-empty attribute lists
operationFromAnnotateCallsdoes:attributes.getOrNull(EndpointAnnotationAttributeKey) ?.map { function -> Operation.build(schemaInference, function) } ?.reduce(Operation::plus)Given
annotatealways stores a non-empty list, this is safe under normal usage. However, if user code ever setsEndpointAnnotationAttributeKeydirectly to an empty list,reducewill throw. If you expect 3rd-party tooling or future internal code to manipulate attributes more freely, you might guard this more defensively:attributes.getOrNull(EndpointAnnotationAttributeKey) ?.map { function -> Operation.build(schemaInference, function) } ?.takeIf { it.isNotEmpty() } ?.reduce(Operation::plus)Not strictly necessary today, but it would make the behavior more robust to attribute misuse.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
112-140: jsonSchema() provides nice fallback; consider reusing it from KotlinxJsonSchemaInference/buildKotlinxSerializationSchema
JsonSchemaInference.jsonSchema<T>()catchingSerializationExceptionand returning anOBJECTschema with a descriptivedescriptionis a good safeguard when a type is not serializable. However,KotlinxJsonSchemaInferenceandbuildKotlinxSerializationSchemacurrently callserializer(type)directly and will propagate the exception instead.If you want consistent behavior across all default inference paths, you could:
- Define
KotlinxJsonSchemaInferencein terms ofbuildKotlinxSerializationSchema, and- Make
buildKotlinxSerializationSchemaitself use the sametry/catchpattern asjsonSchema<T>(), or clearly document that it is a “fail hard” utility.This would reduce duplication and make the error behavior explicit in one place.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
11-55: Consider usingList<Tag>instead ofSet<Tag>fortagsto preserve declared order
tagsis modeled asSet<Tag>?, but the doc comment explicitly mentions that the order can be used by tools, and the OpenAPI spec treats it as an ordered list with uniqueness by convention. UsingSetmakes uniqueness explicit but loses strong ordering guarantees at the type level.If you care about reflecting author‑defined ordering (for UIs, docs, etc.), consider:
public val tags: List<Tag>? = nulland enforcing uniqueness either:
- In validation, or
- Via a helper that normalizes to a
LinkedHashSetinternally while still exposing aListin the public model.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(27 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🔇 Additional comments (26)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt (1)
7-8: LGTM! Rename successfully applied with all call sites updated.Verification confirms the rename from
maptomapValueis complete. Searches across the repository found no remaining references to the old.mappattern onPairtypes, and the newmapValueextension is actively used inktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt. The implementation is correct and the internal scope means no external impact.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
56-106: CollectSchemaReferences traversal is well‑structured and respects existing referencesThe schema reference collection correctly:
- Leaves existing
ReferenceOr.Referenceinstances untouched viamapValue/mapToReference.- Walks request bodies, responses, parameters, and media types to rewrite only inline
JsonSchemavalues.- Recursively rewrites
allOf/oneOf/not/properties/items, while delegating component naming toschemaToComponent.This looks consistent with the composable Operation model and the new
ReferenceOrhelpers.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
19-20: MediaType JsonSchema migration and builder wiring look consistentSwitching
MediaType.schematoReferenceOr<JsonSchema>?and havingBuilderdelegate toJsonSchemaInferencewith aJsonSchema?field that’s wrapped asValueinbuild()is internally consistent and matches the new JsonSchema‑centric model.This does change the public
MediaType.Builderconstructor signature; assuming the openapi‑schema module is still unreleased/experimental, that trade‑off seems acceptable.Also applies to: 52-55, 99-106
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt (1)
35-42: Reorderinggetchanges constructor and JSON field order; verify intendedMoving
getafteroptionsonly affects parameter/property order; semantics are otherwise unchanged. The main effects are:
- Positional constructor calls to
PathItemwill now bind differently.- Serialized JSON field order for operations changes.
Given this module is still evolving, that’s probably fine, but it’s worth double‑checking for any call sites using positional arguments. Based on learnings.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt (1)
31-33: AdditionalProperties.JsonSchema migration is coherent and symmetricUpdating
PSchemato holdReferenceOr<JsonSchema>and adjusting both deserialize and serialize paths to use the correspondingReferenceOr<JsonSchema>serializers keeps the wrapper behavior intact while aligning with the JsonSchema model. The booleanAllowedbranch remains unchanged, so existing behavior is preserved.Also applies to: 44-46, 55-58
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (1)
220-222: annotateMerging still validates merge semantics and now targets JsonSchemaInferenceThe merging test continues to validate that:
- Child and parent
qparameters are merged into a single parameter with the child’s description/required flag, and- Response metadata/extension merging behaves as expected.
Updating the schema assertions to compare against
KotlinxJsonSchemaInference.jsonSchema<String>()andKotlinxJsonSchemaInference.jsonSchema<List<Message>>()keeps the test aligned with the new JsonSchema inference model, while@Suppress("DEPRECATION")on the legacyparameter("q")DSL preserves coverage of the deprecated path. This all looks consistent.Also applies to: 231-236, 284-285, 294-296
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (2)
23-49: JsonSchema data class changes look consistent with JsonSchema-centric modelMigrating recursive fields (
allOf,oneOf,anyOf,not,properties,items) toReferenceOr<JsonSchema>is internally consistent with the new JsonSchema model and aligns with howSerialDescriptor.buildJsonSchema()now producesJsonSchemainstances. I don’t see correctness issues in these property type changes.
73-125: JsonType / SchemaType serialization logic preserves flexible typingThe new
JsonTypeenum andSchemaTypesealed interface (withAnyOf) plus the custom serializers correctly cover both single-type and multi-type (anyOf) JSON Schema representations. The serializer’s behavior of:
- decoding either a string or an array of strings into
JsonType/AnyOf, and- encoding
AnyOfas a list of lowercased type names andJsonTypeas a single lowercased stringmatches JSON Schema expectations and keeps the previous flexible typing model intact. The retained descriptor name for
SchemaType.Serializeralso helps with serialization compatibility.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (3)
20-38: EndpointAnnotationAttributeKey change and annotate accumulation look goodSwitching
EndpointAnnotationAttributeKeyto store aList<RouteAnnotationFunction>and lettingannotateappend (previous + configure) lets multiple annotations contribute composableOperation.Builderconfigurations for the same route. This aligns well with the composable Operation design noted for this module and doesn’t introduce correctness issues.
83-101: findPathItems and asPathItem correctly compose operations with mappings
findPathItemswalkingdescendants(), applyingasPathItem(onOperation), and folding withPathItem.plusgives a clean composition model: multiple handlers for the same path are merged, andonOperationis applied only when anOperationexists, falling back to an emptyOperation()otherwise.This respects the composable
Operationapproach (nullable intermediate states merged viaOperation.plus) used elsewhere in the OpenAPI schema module.
177-258: Operation / PathItem / Response / Parameter merge semantics align with composable designThe
plusoperators forOperation,PathItem,Responses,Response, andParameter, along withmergeParameters,mergeReferencesOr, andmergeElementsBy, implement sensible merge semantics:
- Prefer values from the left-hand side when non-null/non-empty (e.g.,
summary ?: other.summary), while merging collections and maps with additive or “other wins” rules (b + awhere appropriate).- Keep
responsesnullable and merged viamergeNullable, which matches the “composable / partially built Operation” intent from previous learnings.- Correctly merge
Parameter.contentandResponse.contentby combining maps (b + a) so that later contributions can override or extend earlier ones.These operators provide a coherent composition model for OpenAPI generation from multiple sources (selectors, annotations, mappings) without obvious correctness issues.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
49-109: SerialDescriptor.buildJsonSchema covers major descriptor kinds coherentlyThe new
buildJsonSchemaimplementation maps descriptor kinds to JSON Schema in a straightforward, predictable way:
- CLASS/OBJECT →
JsonSchemawithOBJECTtype, populatedproperties, and arequiredlist for non-null, non-optional fields.- LIST →
ARRAYwithitemsfrom the element descriptor.- MAP →
OBJECTwithadditionalPropertieskeyed by the value descriptor.- Primitive kinds → the expected
STRING,BOOLEAN,INTEGER, orNUMBERJsonTypes.- ENUM →
STRINGwithenumvalues derived from element names.- CONTEXTUAL and unknown kinds → generic
OBJECT.This looks correct and keeps responsibilities well-scoped to structural information from
SerialDescriptor.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api (1)
1-26: New OperationMapping-based API surface matches routing/OpenAPI composition goalsThe published API for:
OperationMapping(withmapand defaultplus),CollectSchemaReferences,PopulateMediaTypeDefaults, and- the updated
RouteAnnotationApiKtfunctions (findPathItemswith anOperationMappingparameter andgenerateOpenApiSpec)is consistent with the implementation in
RouteAnnotationApi.ktand provides a clean extension point for post-processing operations. This fits well with the composable OpenAPI generation model you’re building around routing annotations.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api (1)
9-27: OperationMapping / OpenAPI generation surface looks cohesive; note the EndpointAnnotationAttributeKey breakThe new
OperationMappingfun interface,CollectSchemaReferences,PopulateMediaTypeDefaults, and theRoutingNode.findPathItems/generateOpenApiSpecentry points form a clear, composable pipeline for runtime OpenAPI post‑processing. The design is coherent and gives you a nice hook to stack mappers viaplus.The one thing to keep in mind is that
EndpointAnnotationAttributeKeychanging fromAttributeKey<Operation>toAttributeKey<List<(Operation.Builder) -> Unit>>is an ABI/source breaking change for any code that read/wrote the attribute directly. Given learnings that unreleased APIs can break, this is probably fine, but it’s worth double‑checking that:
- All internal usages are updated, and
- If any external usage existed, the change is at least mentioned in the migration notes/changelog.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
149-180: Components + Extensible serializer pattern is consistent and flexibleThe
Componentsdata class with nullable maps,extensions: ExtensionProperties, and the customExtensibleMixinSerializerfollows the same pattern as other extensible OpenAPI types in this module. This gives you a clean way to surfacex-extensions without polluting the main schema structure and should interoperate well with the rest of the serialization pipeline.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (5)
58-76: JsonSchemaInference wiring throughOperation.build/Operation.Builderis consistent
Operation.build(schemaInference, configure)and theBuilder(private val schemaInference: JsonSchemaInference) : JsonSchemaInference by schemaInferencepattern give a single entry point for schema inference and cleanly propagate it into nested builders. This keeps the DSL surface unchanged for callers while making inference pluggable.
220-313: Parameter schema migration toJsonSchemais applied consistently across model and DSL
Parameter.schemaswitching toReferenceOr<JsonSchema>?together withParameter.Builderholding aJsonSchema?and wrapping it viaschema?.let(::Value)inbuild()matches the new JsonSchema‑centric model.
Parameters.Buildercorrectly threadsschemaInferenceinto eachParameter.Builderinstance (parameter,path,query,header,cookie), so all parameter schemas can be inferred using the same strategy.No functional or composability issues stand out here.
476-548: Responses / Response builders correctly propagate JsonSchemaInference
Responses.Builder(private val schemaInference: JsonSchemaInference)now usesResponse.Builder(schemaInference)for both per‑status responses and the default response. This ensures that any content/schema defined inside responses participates in the same inference pipeline as the parent operation.The
Responsesvalue object still keepsdefaultand theresponsesmap nullable, preserving the intermediate/composable semantics noted in previous learnings.
577-666: Response.Builder content +jsonSchemaconvenience are aligned with the JsonSchema model
Response.Builderdelegating toJsonSchemaInference, usingHeaders.Builder(schemaInference)for headers andMediaType.Builder(schemaInference)for content, and exposing ajsonSchema: JsonSchema?convenience aroundContentType.Application.Jsonall look consistent with the rest of the DSL.Callers still have access to the lower‑level
ContentType.invoke { ... }path if they need to control examples/encoding explicitly.
797-862: RequestBody.Builder’s JsonSchema conveniences are well‑integrated
RequestBody.BuildermirrorsResponse.Builder’s pattern:
- Delegates to
JsonSchemaInference,- Uses
MediaType.Builder(schemaInference)for content,- Adds
jsonSchemaandxml(schema: JsonSchema)helpers that wrap aJsonSchemainto aReferenceOr.Value.This keeps the DSL ergonomic for the common JSON/XML cases without constraining usage of more complex media types.
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (6)
67-112: Components ABI matches the new extensible root-components modelThe ABI for
io.ktor.openapi.Components(nullable maps for each component bucket plus an extensions map and companion serializer) matches the new implementation and keeps the shape close to the OpenAPI spec while allowing extension keys viaExtensible. Looks good from a public surface perspective.
415-552: JsonSchema and nested types provide a comprehensive schema modelThe exposed
JsonSchemadata class plus its nestedSchemaTypehierarchy (JsonType,AnyOf, etc.) andDiscriminatorgive a rich representation of JSON Schema 2020‑12/OpenAPI 3.1 features (anyOf, discriminator, nullable, numeric/string constraints, etc.). The serializers and companions are all wired, so consumers can treat this as the canonical schema model inside the module.No ABI inconsistencies stand out here.
621-652: JsonSchemaInference / JsonSchemaReference look like a solid hook; check cross‑platform reflection constraints
JsonSchemaInference(withbuildSchema(KType)),JsonSchemaInferenceKthelpers, andJsonSchemaReference/JsonSchemaReferenceSerializergive you a flexible way to plug in schema inference and lazy resolution.Because this ABI exposes
KTypein public signatures, it implicitly leans on Kotlin reflection support on each target. That’s fine if you confine actual reflection usage to platforms that support it, but it’s worth double‑checking:
- That the common API doesn’t force consumers on JS/Native into requiring full
kotlin-reflect, and- That the default
KotlinxJsonSchemaInferencepath behaves correctly on all supported targets.
759-910: OpenApiInfo / OpenApiSpecification ABI aligns with the new root spec modelThe ABI entries for
OpenApiInfo(with nestedContactandLicense) andOpenApiSpecification(openapi/version, info, servers, paths, webhooks, components, security, tags, externalDocs, extensions) reflect the same structure as the new Kotlin source.This gives downstream consumers a single, well‑typed root document to work with when integrating with routing annotations and OpenAPI generation.
961-1137: Operation / Parameter / RequestBody / Response / Responses builders now consistently carry JsonSchemaInferenceThe ABI shows that:
Operation$Builder,Parameter$Builder,Parameters$Builder,RequestBody$Builder,Response$Builder,Responses$Builder,MediaType$Builder,Header$Builder,Headers$Builderall take a
JsonSchemaInferencein their constructors and exposegetSchema/setSchemaorgetJsonSchema/setJsonSchemamethods returning/acceptingJsonSchema.This matches the source changes and ensures that the entire DSL, from operations down to media types and headers, participates in a single inference strategy. Given this module’s ongoing evolution, the ABI‑breaking constructor changes look intentional and cohesive.
1515-1545: Tag ABI addition is straightforward and matches the new modelThe new
io.ktor.openapi.Tagclass and its serializer companion are a simple, spec‑aligned addition (name, description, externalDocs). This pairs cleanly with thetagscollection onOpenApiSpecificationandOperation.
...-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
Show resolved
Hide resolved
...erver-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt
Show resolved
Hide resolved
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
3 times, most recently
from
b13f082 to
d03d25d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (1)
211-223:Responses.pluscan crash when all responses are references
Responses.plususesmergeReferencesOr { listOf(reduce { a, b -> a + b }) }and then.first(). When, for a given status code, all entries areReferenceOr.Reference(no inlineResponsevalues), thenonReferenceslist is empty, soreducethrows on an empty list.This is a real possibility when responses come only from
$refcomponents, and it will surface as a runtime exception during operation merging.A safe fix is to no-op on the non-reference list when it is empty, so we still prefer references but never call
reduceon an empty list:- statusCode to responseList.mergeReferencesOr { - listOf(reduce { responseA, responseB -> responseA + responseB }) - }.first() + statusCode to responseList.mergeReferencesOr { + if (isEmpty()) { + // Only references present; keep them as-is. + this + } else { + // Merge concrete responses for this status code into one. + listOf(reduce { responseA, responseB -> responseA + responseB }) + } + }.first()This keeps the “prefer references when present” behavior while making the merge safe for all combinations.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
579-665: Breaking change in Response.Builder.jsonSchema property.The
Response.Builder.jsonSchemaproperty type changed fromSchema?toJsonSchema?. This is a breaking change for consumers setting this property.Otherwise, the schema inference propagation to nested builders (Headers, MediaType) is correctly implemented.
799-862: Multiple breaking changes in RequestBody.Builder.Breaking changes identified:
RequestBody.Builder.jsonSchemaproperty type changed fromSchema?toJsonSchema?(line 827)RequestBody.Builder.xml()method parameter changed fromSchematoJsonSchema(line 838)The schema inference propagation to
MediaType.Builderis correctly implemented.
🧹 Nitpick comments (9)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (1)
73-126: SchemaType / JsonType serialization correctly models JSON Schema"type"behaviorEncoding
"type"as either a single string (JsonType) or an array of strings (AnyOf) with lowercase values aligns with JSON Schema, and theJsonTypeSerializerround‑trips case‑insensitively. One small nit:SchemaType.Serializer’s descriptor name still references"Schema.SchemaType"; if there is no need for descriptor‑name backward compatibility, consider updating it to"JsonSchema.SchemaType"for clarity.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.kt (2)
20-34: Lazy resolution logic is sound; invariant relies on factories
JsonSchemaReference’s private constructor plusofType/ofensure thattype!!ingetis safe as long as instances are only created via these factories, and the resolved schema is cached in_schema. If you foresee additional construction paths later (e.g., reflection or copy helpers), it may be worth documenting this invariant near the class declaration.
36-49: Custom serializer correctly flattens references to rawJsonSchema
JsonSchemaReferenceSerializerserializes as a plainJsonSchemaand deserializes back into an eagerJsonSchemaReference.of(...), which is a good fit for OpenAPI documents that don’t need to expose the indirection. Minor simplification option:serializecould callvalue.get()and rely on the default argument instead of passing::buildKotlinxSerializationSchemaexplicitly.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
49-109: Descriptor‑to‑JsonSchema mapping is reasonable; doc comment is slightly staleThe
buildJsonSchemawhenoverStructureKind/PrimitiveKind/SerialKindproduces sensible object/array/map/primitive/enum schemas, and the default/CONTEXTUAL branches falling back toOBJECTare a safe baseline. The KDoc still mentions a@param schemathat no longer exists on the function, which could be confusing.You can drop the stale parameter docs like this:
- * - * @param schema the base instance for the resulting schema, use this to include extra properties - * - * @return A [JsonSchema] object representing the JSON Schema for this descriptor. + * + * @return A [JsonSchema] object representing the JSON Schema for this descriptor.
112-140: Fallback JsonSchema description is helpful; update wording in KDocThe
jsonSchema<T>()extension’s catch ofSerializationExceptionwith a descriptiveOBJECTschema is a nice failure mode. The KDoc still talks about “returns a Schema” instead ofJsonSchema, which is slightly out of date.Suggested tweak:
- * If the type is not serializable, returns a Schema representing a non-serializable type. + * If the type is not serializable, returns a [JsonSchema] representing a non-serializable type.The
buildKotlinxSerializationSchemahelper is consistent withKotlinxJsonSchemaInferenceand keeps the default inference reusable.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
29-105: Expected OpenAPI JSON is precise but somewhat brittleThe updated
expectedstring now validates the full OpenAPI 3.1.1 document, including array item$refs and theMessagecomponent schema, which gives strong regression coverage. Just be aware this is sensitive to cosmetic changes (field ordering, pretty‑print config). If you start evolving the model frequently, consider switching to structural JSON assertions instead of full‑string equality.
180-193: Header/content annotation additions are covered adequatelyThe
annotateAdditiontest now drives OpenAPI generation throughgenerateOpenApiSpecand checks for both"X-First"and"X-Second"headers, including one with explicitContentType.Text.Plain()on the header parameter. That seems sufficient to exercise header accumulation; if you later start emitting more metadata forX-Sample-Message, you might want a dedicated assertion for that as well.Also applies to: 205-207
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
35-57: ClarifyhasMissingMediaInfopredicate for maintainabilityThe condition
.any { it.value.schema == null && it.value.content == null || it.value.`in` == null }relies on operator precedence. Wrapping the intended groups makes it easier to read and less error‑prone in future edits:
- val hasMissingMediaInfo = operation.parameters.orEmpty() - .filterIsInstance<ReferenceOr.Value<Parameter>>() - .any { it.value.schema == null && it.value.content == null || it.value.`in` == null } + val hasMissingMediaInfo = operation.parameters.orEmpty() + .filterIsInstance<ReferenceOr.Value<Parameter>>() + .any { (it.value.schema == null && it.value.content == null) || it.value.`in` == null }Behavior is unchanged but clearer.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
132-147: ConsiderList<Tag>instead ofSet<Tag>for predictable tag orderingThe KDoc for
tagsmentions that consumers may rely on the order of tags, but the property type isSet<Tag>?. While Kotlin’s defaultsetOfis insertion‑ordered, usingSetin the public API makes ordering less explicit and easier to break by swapping in a different implementation.If tag order is important for generated docs/clients, you might want to expose this as
List<Tag>?and enforce uniqueness elsewhere, or document that callers must preserve insertion order via aLinkedHashSet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(27 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt
- ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🧬 Code graph analysis (4)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (6)
ktor-utils/common/src/io/ktor/util/Attributes.kt (1)
AttributeKey(19-21)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
Components(154-199)ktor-utils/common/src/io/ktor/util/collections/TreeLike.kt (2)
descendants(26-30)lineage(19-21)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (2)
hasHandler(134-135)path(377-387)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
path(257-264)Operation(36-213)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RouteSelector.kt (1)
method(643-655)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (8)
schemaInference(72-212)schemaInference(227-312)schemaInference(359-457)schemaInference(499-548)schemaInference(578-665)schemaInference(798-862)schemaInference(1035-1056)schemaInference(1077-1156)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (1)
generateOpenApiSpec(46-73)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaReference.kt (1)
get(30-33)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
schemaInference(51-107)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/GenericElement.kt (3)
GenericElement(121-126)GenericElement(131-132)GenericElement(137-151)
🔇 Additional comments (21)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt (1)
7-8: No changes to Utils.kt in this PR.The file
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kthas not been modified in this pull request. ThemapValuefunction already exists unchanged in HEAD. There is no rename occurring in the current PR.Likely an incorrect or invalid review comment.
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api (7)
9-12: Well-designed composable abstraction.The
OperationMappinginterface provides a clean, composable pattern for post-processing OpenAPI operations. Theplusoperator enables chaining multiple mappings together.
14-18: LGTM - aligns with PR objectives.This class supports the PR's goal of extracting components by collecting schema references during operation processing. The constructor's naming function provides flexibility in how schemas are referenced.
22-23: LGTM - provides convenient default mapping.This property aligns with the PR's objective of "post-processing for defaults" by providing a pre-configured
OperationMappingfor populating media type defaults.
25-25: LGTM - idiomatic Ktor DSL design.The
Route.annotateextension provides a fluent DSL for adding OpenAPI metadata to routes. The return type enables method chaining, following Ktor's conventions.
26-26: LGTM - backward-compatible enhancement.The addition of an optional
OperationMappingparameter with a default value is non-breaking and allows callers to customize operation post-processing when generating path items.
27-27: LGTM - clean API for OpenAPI generation.This function provides a straightforward entry point for generating a complete OpenAPI specification from routing nodes, directly supporting the PR's objective of including full OpenAPI specification support.
21-21: Breaking change is acceptable — module is unreleased.The
EndpointAnnotationAttributeKeytype change fromAttributeKey<Operation>toAttributeKey<List<Function1<Operation.Builder, Unit>>>is a breaking change that enables accumulating multiple annotation functions instead of a single operation.Web search confirms the module "ktor-server-routing-annotate" is not published in Maven Central, and the directory does not exist in the repository state, indicating this is new unreleased code. Per established precedent in the codebase, breaking changes are acceptable for unreleased modules.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (1)
23-48: JsonSchema migration toReferenceOr<JsonSchema>looks internally consistent
allOf,oneOf,anyOf,not,properties, anditemsnow all useReferenceOr<JsonSchema>, matching the JsonSchema-centric model and avoiding mixedSchema/JsonSchemausage. Construction of composite schemas should remain straightforward and type‑safe with this change.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
20-29: JsonSchemaInference abstraction and default implementation are well‑factoredThe
JsonSchemaInferencefun interface with a sharedKotlinxJsonSchemaInferenceimplementation cleanly decouples schema derivation from the rest of the DSL, making it easy to swap in cached or customized inference strategies later.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
17-107: MediaType now aligns cleanly with JsonSchema‑based pipelineSwitching
MediaType.schematoReferenceOr<JsonSchema>?and havingMediaType.Builderdelegate toJsonSchemaInferencekeeps it consistent with Operation/Parameter/Response builders and the new inference API. Thebuild()method’s wrapping ofschemainValueand null‑stripping of maps is also coherent with the rest of the OpenAPI model.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt (1)
32-57: AdditionalProperties correctly migrated toJsonSchema
PSchemanow wrappingReferenceOr<JsonSchema>plus the updated (de)serialization usingReferenceOr.serializer(JsonSchema.serializer())bringsadditionalPropertiesin line with the JsonSchema‑centric model. The boolean vs. object discrimination viaGenericElementremains intact.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
124-133: UsinggenerateOpenApiSpecin tests exercises the full spec pipelineReplacing direct
findPathItems()usage withgenerateOpenApiSpec(...).copy(paths = paths - "/routes")and then decoding toOpenApiSpecificationverifies not only path extraction but also that components and other top‑level fields are wired correctly. The trimming of the/routespath still ensures a single/messagesentry, as asserted.Also applies to: 168-170
223-225: Merging semantics and schema inference assertions look correct
annotateMergingstill uses rawfindPathItems()(appropriate for focusing on merge behavior), while the assertions now check that:
- the merged parameter “q” keeps the child’s description/required flag, and
- its
schemamatchesKotlinxJsonSchemaInference.jsonSchema<String>(),- the OK response’s JSON content schema matches
KotlinxJsonSchemaInference.jsonSchema<List<Message>>().This ties the merge logic directly to the new
JsonSchemaInferencebehavior and validates that parent/child annotations compose as intended. The added@Suppress("DEPRECATION")around the legacyparameter("q")call keeps the test exercising the deprecated path without noise.Also applies to: 234-239, 283-299
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api (1)
20-26: Route annotation public surface looks consistentThe exposed APIs (
annotate,findPathItemswithOperationMapping, andgenerateOpenApiSpec) line up with the implementation inRouteAnnotationApi.ktand give a reasonable, composable entry point for OpenAPI generation.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (6)
73-75: LGTM: Clean delegation pattern.The Builder now accepts and delegates
JsonSchemaInference, enabling nested builders to access schema inference methods directly. This pattern is consistently applied throughout the file.
138-156: LGTM: Consistent schema inference propagation.All nested builder instantiations correctly propagate the
schemaInferenceinstance to child builders (Parameters, RequestBody, Responses).
228-306: LGTM: Complete schema inference propagation in Parameters DSL.All parameter creation methods (path, query, header, cookie) correctly propagate
schemaInferenceto their respectiveParameter.Builderinstances.
500-548: LGTM: Schema inference properly threaded through Responses DSL.The
Responses.Buildercorrectly propagatesschemaInferenceto allResponse.Builderinstances created viaresponse()anddefault()methods.
335-457: Breaking change fromSchematoJsonSchemais confirmed and acceptable for pre-release code.The review comment correctly identifies a real breaking change:
Parameter.schemachanged fromReferenceOr<Schema>?toReferenceOr<JsonSchema>?between the first commit (d8f58b0) and current state. The same change applies toParameter.Builder.schema. This is part of the module's JsonSchema adoption.The module is newly added with only 2 commits (KTOR-8936 related) and no release tags, confirming it's pre-release. Per established learnings in the Ktor codebase, breaking changes to public APIs are acceptable when the code hasn't been released yet.
58-61: Verification confirmed: This is unreleased code, breaking change is acceptable.The current version is 3.4.0-SNAPSHOT, confirming this is pre-release code. Per learnings from the Ktor development team, breaking changes are acceptable in unreleased versions. The
Operation.build()signature change with the defaultschemaInferenceparameter is already tracked in the API compatibility files and the default value provides backward compatibility for common usage patterns. This change can proceed.
...erver-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
1071-1169: Critical: Header extensions are collected but not preserved.The
Header.Buildercollects extensions via theextensionsfield (line 1121) and provides anextension()method (lines 1139-1142), but theHeaderdata class (lines 1076-1087) doesn't have anextensionsproperty and doesn't extendExtensible. Thebuild()method (lines 1156-1167) doesn't include extensions in the constructedHeader, causing data loss.All other similar classes (
Parameter,Response,RequestBody,Link,Server) extendExtensibleand include extensions in their data model.Apply this diff to fix the issue:
+@Serializable(Header.Companion.Serializer::class) +@OptIn(ExperimentalSerializationApi::class) +@KeepGeneratedSerializer public data class Header( /** A short description of the header. */ public val description: String? = null, public val required: Boolean = false, public val deprecated: Boolean = false, public val schema: ReferenceOr<JsonSchema>? = null, public val content: Map<@Serializable(ContentTypeSerializer::class) ContentType, MediaType>? = null, public val style: String? = null, public val explode: Boolean? = null, public val example: GenericElement? = null, public val examples: Map<String, ReferenceOr<ExampleObject>>? = null, + override val extensions: ExtensionProperties = null, -) { +) : Extensible { + public companion object { + internal object Serializer : ExtensibleMixinSerializer<Header>( + generatedSerializer(), + { h, extensions -> h.copy(extensions = extensions) } + ) + } + /** Builder for constructing a [Header] instance. */And update the
build()method:internal fun build(): Header { return Header( description = description, required = required, deprecated = deprecated, schema = schema?.let(::Value), content = _content.ifEmpty { null }, style = style, explode = explode, example = example, examples = examples.ifEmpty { null }, + extensions = extensions.ifEmpty { null }, ) }
🧹 Nitpick comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
839-843: Consider using the builder accumulation pattern for consistency.The
jsonSchemasetter directly creates a newMediaTypeinstance (line 842), which would replace any existingMediaTypeconfiguration (examples, encoding, extensions) rather than accumulating it. This differs fromResponse.Builder.jsonSchema(lines 636-644), which usesgetOrPutand preserves existing builder state.If a user configures JSON content with examples and then sets
jsonSchema, the examples would be lost:ContentType.Application.Json { example("test", ExampleObject(...)) } jsonSchema = someSchema // loses the exampleConsider aligning with the
Response.Builderpattern by storingMediaType.Builderinstances in_contentto support accumulation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
🧬 Code graph analysis (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
schemaInference(51-107)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/GenericElement.kt (3)
GenericElement(121-126)GenericElement(131-132)GenericElement(137-151)
🔇 Additional comments (7)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (7)
58-61: LGTM! Default parameter preserves backward compatibility.The addition of the
schemaInferenceparameter with a default value maintains backward compatibility while enabling custom schema inference when needed.
73-75: LGTM! Clean delegation pattern.The builder correctly accepts and delegates
JsonSchemaInference, making schema inference capabilities available throughout the DSL.
138-156: LGTM! Consistent schema inference propagation.The
schemaInferenceis correctly threaded through all nested builders (Parameters,RequestBody,Responses), ensuring consistent schema inference throughout the operation definition.
500-554: LGTM! Builder accumulation enables composability.The change to store
Response.Builderinstances (lines 501, 504) and usegetOrPut(lines 519-522, 539-543) enables incremental composition of responses across multiple DSL calls, aligning with the design goal that Operation objects should be composable.Based on learnings, this composability is intentional for the ktor-openapi-schema module.
585-677: LGTM! Consistent builder accumulation pattern.The
Response.Builderfollows the same accumulation pattern asResponses.Builder: storingMediaType.Builderinstances (line 593) and usinggetOrPut(lines 628-631, 639-643) to enable composable content configuration. ThejsonSchemasetter correctly preserves existing builder state.
228-306: LGTM! Consistent parameter builder initialization.All parameter factory methods (
parameter,path,query,header,cookie) consistently instantiateParameter.Builder(schemaInference), ensuring schema inference is available throughout parameter configuration.
341-341: Schema type change is properly shielded by builder pattern; no breaking surface detected.The
Parameter.schemaandHeader.schematype change fromJsonSchema?toReferenceOr<JsonSchema>?is internally managed through builder wrapping (schema?.let(::Value)). Verification shows:
- No direct
Parameter()orHeader()constructor calls outside builders- No code accesses
.schemaproperty directlyReferenceOr<T>custom serializer extractsValue<T>and encodes asTdirectly—JSON format is unchanged- Builder DSL shields consumers from the type transformation
The change is contained and poses no risk to existing code paths in the current codebase.
| * schema generation. | ||
| */ | ||
| @Serializable(JsonSchemaReferenceSerializer::class) | ||
| public class JsonSchemaReference private constructor( |
There was a problem hiding this comment.
Can it be a sealed class or enum?
There was a problem hiding this comment.
I actually ended up going in a different direction here and forgot to delete this 😄
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (3)
868-897: OpenApiSpecification modeling looks good; confirm Set vs List fortagsis intentionalThe new
OpenApiSpecificationclass plus serializer/companion nicely captureopenapi,info,servers,paths,components,security,webhooks,tags,externalDocs, and extensions. One detail to double‑check:tagsare modeled as aSet, whereas the OpenAPI document uses an array where order can be meaningful (e.g., for UI grouping). If you intend to deduplicate tags and don’t care about strict ordering,Setis fine; otherwise, consider aList<Tag>here for closer alignment with the spec and deterministic ordering in generated JSON.Also applies to: 899-912
1198-1203: New ReferenceOr mapping helpers are useful; consider clarifying/documenting semanticsAdding
mapToReferenceandmapValueonReferenceOrand providing concrete implementations onReferenceandValuemakes it much easier to transform payload vs reference representations. From the signatures:
Referenceoverrides both methods, presumably leaving references untouched or transforming the ref string.ValueimplementsmapToReference-impl/mapValue-implto transform the wrapped value.I’d suggest documenting (in KDoc) the exact semantics, especially for edge cases:
- Whether
Reference.mapValue(...)is a no‑op returningthis, and- Whether
Value.mapToReference(...)’s mapper is expected to be pure (no side effects) and how errors are handled.That will help consumers reason about these operations when composing behavior.
Also applies to: 1211-1214, 1228-1229, 1244-1247
1388-1398: Responses.Builder now depends on JsonSchemaInference and exposes default as Response.Builder
Responses$Buildertakes aJsonSchemaInferenceand itsdefaultaccessor now usesResponse$Builderinstead of a finalizedResponse. That’s powerful for nested DSL composition, but it also exposes the builder type on the public API. Two things to double‑check:
- Building a
Responsesinstance correctly finalizes anyResponse$Builderstored indefault/responsesentries.- You’re comfortable with callers holding on to
Response$Builderinstances obtained fromgetDefault()and mutating them afterResponseshas been built (or that you document this as undefined behavior).If that mutability is undesirable, you may want to keep the stored type as
Responseand use builders only transiently in the DSL.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🔇 Additional comments (13)
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (13)
67-99: Components API shape looks consistent with OpenAPI “components” objectThe new
Componentsdata class plusComponents$$serializer/Companion.serializer()provide a coherent container for schemas/responses/parameters/etc. The API surface (copy/isEmpty/isNotEmptyand per‑category getters) looks well-aligned with the OpenAPI 3.x components object and with the existingExtensiblepattern. I don’t see issues from the API signature alone.Also applies to: 101-114
310-323: Header now mixingReferenceOrin the model andJsonSchemain the builder—verify bridging logicThe
Headerctor and accessors now useReferenceOrforschema, whileHeader$BuilderexposesJsonSchemaand implementsJsonSchemaInference. That modeling split makes sense (builder works with concreteJsonSchema, final model usesReferenceOr<JsonSchema>), but it relies on correct conversion when the builder materializes aHeader. Please double‑check that the builder implementation consistently wraps the schema intoReferenceOr.value(...)(or the equivalent) when producing the immutable model.Also applies to: 325-332, 348-372
401-405: Headers.Builder ctor change introducesJsonSchemaInferencedependency
Headers$Buildernow requires aJsonSchemaInferencein its constructor, which is consistent with the other schema‑aware builders. This is fine, but it is a source/binary break for anyone directly instantiatingHeaders.Builder. Please confirm that this builder type is only constructed via your DSL/entry points (which you updated to pass an inference instance) and not by user code.
417-505: JsonSchema model and serializers look comprehensive and consistentThe new
JsonSchemaclass, its nestedSchemaTypehierarchy (JsonType,AnyOf, etc.), discriminator, and the associated serializers/companions form a rich JSON Schema representation. The field set (combinators, validation keywords, metadata, XML/externalDocs, nullable/readOnly/writeOnly, additionalProperties) corresponds well to JSON Schema/OpenAPI 3.x expectations, and the serializer layout (JsonSchema$$serializer,SchemaType$Serializer,JsonTypeSerializer) matches Kotlinx Serialization patterns. I don’t see any API‑level issues here.Also applies to: 507-520, 522-551, 552-604, 605-612, 614-621
623-631: JsonSchemaInference abstraction is cleanly defined
JsonSchemaInferencewith a singlebuildSchema(KType): JsonSchemaplus the helpers inJsonSchemaInferenceKt(buildJsonSchema,buildKotlinxSerializationSchema,getKotlinxJsonSchemaInference) provides a clear injection point for different inference strategies and for wiring Kotlinx Serialization. The signatures look good and should compose well with the various builders.
633-645: JsonSchemaReference API seems reasonable; ensure serializer’s wire format is stable
JsonSchemaReferenceplus itsCompanion.of/ofTypeandJsonSchemaReferenceSerializergive you a nice handle for deferring schema resolution byKTypeand/or wrapping an existingJsonSchema. From the signatures this looks fine; just ensure that the serializer’s JSON representation (e.g., whether it encodes type, id, or full schema) is considered stable enough for any persisted specs you might emit, since changing that later will be a wire‑compat break.Also applies to: 647-654
744-754: MediaType.Builder alignment with JsonSchemaInference
MediaType$Buildernow implementsJsonSchemaInferenceand usesJsonSchemain its schema accessors, whileMediaTypeitself still exposesReferenceOrforschema. This mirrors theHeaderpattern and makes the builder more powerful. Same caveat: verify the builder correctly converts fromJsonSchematoReferenceOrwhen building the finalMediaType.
963-993: Operation.Builder now depends on JsonSchemaInference; verify external call sites
Operation$BuilderimplementingJsonSchemaInferenceandOperation$Companion.build(JsonSchemaInference, Function1)centralize schema inference nicely and keep operations composable, which aligns with prior design for this module (Based on learnings, operations are built for composition). The newbuildsignature (with aJsonSchemaInferenceparameter + default) is, however, a behavioral/binary change compared to a simplerbuild(block)API. Please confirm:
- All internal usages are updated to pass the correct
JsonSchemaInference(or rely on the defaulted one), and- You’re comfortable with the impact on any external consumers that may have been using
Operation.build { ... }and constructingOperation.Builderdirectly.Also applies to: 995-998
1051-1082: Parameter.Builder schema type switched to JsonSchema—check model conversion
Parameter$Buildernow implementsJsonSchemaInferenceand exposesJsonSchemaviagetSchema/setSchema, while theParametermodel continues to useReferenceOrforschema. That’s consistent with the rest of the schema‑centric builders, but again relies on correct mapping when turning a builder into aParameter. Please verify that the builder’s build path wraps theJsonSchemain the expectedReferenceOrvalue and that there’s no accidental loss of any$refinformation.
1127-1138: Parameters.Builder ctor now requires JsonSchemaInference
Parameters$Builderacquiring aJsonSchemaInferencector parameter is consistent with the rest of the DSL. As with the other builders, this changes construction semantics for clients who might have usedParameters.Builder()directly; ensure those call sites are updated or that this builder type is considered internal/DSL‑only.
1286-1300: RequestBody.Builder JsonSchema integration—ensure consistency with media-type content
RequestBody$Buildernow implementsJsonSchemaInference, exposesgetJsonSchema/setJsonSchema, and adds anxml(JsonSchema)convenience. This is a nice upgrade, but there are a couple of consistency points to verify in the implementation:
- How
jsonSchemainteracts with the per‑ContentTypecontentmap—e.g., is it a shorthand for the JSON media type only, or a “canonical” schema synchronized with entries incontent?- That
xml(JsonSchema)properly sets up anapplication/xml(or similar) media type using the same schema instance.The API surface is fine; just make sure the underlying behavior is well‑defined and documented for callers using both
content{}and thejsonSchema/xml(...)helpers.
1338-1353: Response.Builder JsonSchema support aligns with RequestBody.Builder
Response$Buildernow implementsJsonSchemaInferenceand introducesgetJsonSchema/setJsonSchemaalongsidebuildSchema(KType), matching the request‑body side. This seems like the right abstraction for shared schema inference. Similar to the request case, please verify that:
- The
jsonSchemafield is wired consistently into thecontentmap so that the finalResponsemodel has matching media‑type schemas, and- Using multiple content types doesn’t introduce ambiguity about which one
jsonSchemarefers to.From the API surface alone there are no red flags.
1517-1532: Tag type and integration look goodThe new
Tagvalue type plus its serializer/companion give you a proper first‑class representation of OpenAPI tags for use inOpenApiSpecification.tags. The fields (name, description, externalDocs) align with the spec, and the API is straightforward. No issues from the signature perspective, and it complements the existingOperation.Builder.tag(String)usage well.Also applies to: 1534-1543, 1545-1547
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
b00e17b to
671d85a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (1)
28-105: Golden OpenAPI JSON greatly tightens coverage but is brittleThe embedded expected OpenAPI document captures the full 3.1.1 spec for
/messagesand is a strong regression guard, but it will require updates on any incidental formatting or ordering change. If this becomes noisy over time, consider asserting structure via decodedOpenApiSpecificationinstead of strict string equality, or at least confining the golden JSON to the minimal surface you care about.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
811-852: Minor asymmetry inRequestBody.Builder.jsonSchemavs. other content builders
RequestBody.BuilderusesMediaType.Builder(schemaInference)in theContentType.invokeoperator, but thejsonSchemasetter replaces the entry with a fully builtMediaType(schema = Value(schema))instead of updating/using a builder asResponse.Builder.jsonSchemadoes. That means any subsequent configuration viaContentType.Application.Json { ... }will discard the schema just set (and vice versa).Not a correctness bug given typical usage, but for consistency and flexibility you might want to mirror the
Response.Builderpattern and store a builder here too.- public var jsonSchema: JsonSchema? - get() = _content[ContentType.Application.Json]?.schema?.valueOrNull() - set(value) { - _content[ContentType.Application.Json] = MediaType(schema = value?.let(::Value)) - } + public var jsonSchema: JsonSchema? + get() = (_content[ContentType.Application.Json] as? MediaType.Builder)?.schema + set(value) { + _content.getOrPut(ContentType.Application.Json) { + MediaType.Builder(schemaInference) + }.apply { + schema = value + } + }ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (3)
14-54: Consider implementing the Extensible interface.The
OpenApiSpecificationclass has anextensionsfield but doesn't implement theExtensibleinterface, unlikeOpenApiInfo(line 91) andComponents(line 160), which both implement it. This inconsistency may prevent polymorphic handling or uniform processing of extensible objects.Apply this diff to implement the interface:
@OptIn(ExperimentalSerializationApi::class) @Serializable public data class OpenApiSpecification( @EncodeDefault(ALWAYS) public val openapi: String = "3.1.1", /** Provides metadata about the API. The metadata can be used by the clients if needed. */ public val info: OpenApiInfo, /** An array of Server Objects, which provide connectivity information to a target server. */ public val servers: List<Server>? = null, /** The available paths and operations for the API. */ public val paths: Map<String, PathItem> = emptyMap(), /** * The incoming webhooks that MAY be received as part of this API and that the API consumer MAY * choose to implement. Closely related to the callbacks feature, this section describes requests * initiated other than by an API call, for example by an out of band registration. The key name * is a unique string to refer to each webhook, while the (optionally referenced) Path Item Object * describes a request that may be initiated by the API provider and the expected responses. */ public val webhooks: Map<String, ReferenceOr<PathItem>>? = null, /** An element to hold various schemas for the specification. */ public val components: Components? = null, /** * A declaration of which security mechanisms can be used across the API. The list of values * includes alternative security requirement objects that can be used. Only one of the security * requirement objects need to be satisfied to authorize a request. Individual operations can * override this definition. To make security optional, an empty security requirement can be * included in the array. */ public val security: List<SecurityRequirement>? = null, /** * A list of tags used by the specification with additional metadata. The order of the tags can be * used to reflect on their order by the parsing tools. Not all tags that are used by the * 'Operation' Object must be declared. The tags that are not declared MAY be organized randomly * or based on the tools' logic. Each tag name in the list MUST be unique. */ public val tags: Set<Tag>? = null, /** Additional external documentation. */ public val externalDocs: ExternalDocs? = null, /** Any additional external documentation for this OpenAPI document. */ - public val extensions: Map<String, JsonElement>? = null, -) + public override val extensions: ExtensionProperties = null, +) : Extensible
86-86: Reconsider the default value for the contact field.The
contactfield defaults toContact()instead ofnull. This means an empty contact object{}will be serialized even when contact information is not provided. According to the OpenAPI specification, the contact field is optional and should be omitted when not needed.Apply this diff to change the default to null:
- public val contact: Contact? = Contact(), + public val contact: Contact? = null,
171-185: Consider whether extensions should be checked in isEmpty().The
isEmpty()method checks all component fields exceptextensions. If extensions are considered meaningful content (not just metadata), they should be included in the emptiness check.If extensions should count toward non-emptiness, apply this diff:
public fun isEmpty(): Boolean = schemas.isNullOrEmpty() && responses.isNullOrEmpty() && parameters.isNullOrEmpty() && examples.isNullOrEmpty() && requestBodies.isNullOrEmpty() && headers.isNullOrEmpty() && links.isNullOrEmpty() && callbacks.isNullOrEmpty() && - pathItems.isNullOrEmpty() + pathItems.isNullOrEmpty() && + extensions.isNullOrEmpty()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt
- ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🧬 Code graph analysis (1)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (1)
generateOpenApiSpec(46-73)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
OpenApiInfo(67-122)
🔇 Additional comments (18)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt (1)
7-8: The renamedPair<T, T>.mapValue()function is currently unused in the codebase.Verification results show that while the function has been successfully renamed from
maptomapValueinUtils.kt(line 7), it's not being called anywhere in the codebase. The.mapValue()calls found inbuild.gradle.ktstarget a different extension function onProvider<Iterable<T>>defined inbuild-logic/src/main/kotlin/ktorbuild/dsl/Collections.kt.Additionally, the initial search found zero references to the old
Pair.map()function name, confirming all potential call sites either don't exist or have been updated. However, the renamed utility function itself appears to be orphaned (defined but not used). Clarify whether this is:
- A new utility for future use
- A leftover from removed code that should be deleted
- A utility supporting external code outside this repository
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (3)
67-114: Acknowledge comprehensive JsonSchema migration.This PR introduces a large-scale migration from a previous schema system to a JsonSchema-centric OpenAPI model:
New core types:
- JsonSchema with 39 properties covering the full JSON Schema specification
- Components and OpenApiSpecification as top-level containers
- OpenApiInfo with nested Contact and License
- Tag for operation categorization
- JsonSchemaInference interface for schema generation
Breaking changes:
- Schema property types changed to
JsonSchema/ReferenceOr<JsonSchema>across Header, Parameter, MediaType, RequestBody, Response- Builder classes now implement/accept JsonSchemaInference
- Operation.Companion.build() signature updated to require JsonSchemaInference
These breaking changes are acceptable per project guidelines for unreleased code, and the new API surface appears well-structured with complete serialization support and consistent builder patterns.
Based on learnings about acceptable breaking changes for unreleased code.
Also applies to: 307-337, 419-657, 715-762, 763-910, 999-1036, 1253-1302, 1304-1355
419-657: LGTM: Comprehensive and well-structured new API surface.The new JsonSchema-based OpenAPI model is thorough and consistent:
Strengths:
- JsonSchema's 39 properties fully cover the JSON Schema specification
- JsonSchema$SchemaType sealed hierarchy (JsonType enum + AnyOf variant) with complete serialization support
- Components includes isEmpty/isNotEmpty helpers for practical conditional checks
- ReferenceOr.mapToReference/mapValue enable functional schema transformations
- All data classes expose complete APIs: componentN methods, copy, equals/hashCode, getters
- Comprehensive serialization support: $$serializer and Companion.serializer() for all new types
- JsonSchemaInference ecosystem with buildSchema, buildKotlinxSerializationSchema utilities
The API surface is production-ready for the OpenAPI 3.1 specification.
Also applies to: 67-114, 866-910, 1198-1199
836-849: The API is correct. Verify intentional design of privateidentifierfield.The previous review flagged an inconsistency, but the source code reveals the actual design:
public data class License( public val name: String, public val url: String? = null, private val identifier: String? = null, // ← private )The
identifieris markedprivate, which explains why the API dump shows:
- ✅ Constructor with 3 parameters
- ✅ Only
component1()andcomponent2()(notcomponent3(), becauseidentifieris private)- ✅
copy()with 3 parameters (includes private fields)- ✅ Only
getName()andgetUrl()getters (notgetIdentifier())This is consistent with Kotlin data class semantics: private constructor parameters don't generate
componentN()methods or property getters, but they remain part of thecopy()signature.Clarify: Is this intentional (cache
identifierfor serialization but keep it private) or should it bepubliclikeContact.email?ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
17-55: JsonSchema migration inMediaTypelooks consistent
MediaType.schemaandMediaType.Builder.schemanow useJsonSchemaand the builder correctly wrapsschemaintoReferenceOr.Valueinbuild(), while delegatingJsonSchemaInferencefor nested DSL use. This is a straightforward type migration with no behavioral regressions.Also applies to: 99-105
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
116-134: Use ofgenerateOpenApiSpecand path filtering is clear and correctBoth tests now derive the OpenAPI doc via
generateOpenApiSpec(info, routingRoot)and then strip/routesfrompaths, which mirrors the production API and keeps the test focused on the annotated routes rather than the helper endpoint. This matches the new OpenAPI‑centric flow.Also applies to: 173-190
216-306: Schema inference assertions are aligned with the newJsonSchemaInferenceAPI
- The suppression for deprecated
parameter("q")is appropriate inannotateMerging.- Comparing parameter and response schemas against
KotlinxJsonSchemaInference.jsonSchema<...>()is a good way to pin the inferred schemas to the same implementation used by the DSL.- The merged parameter expectations (description,
required, and schema) correctly validate the newOperation/Parametermerging behavior.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (1)
22-34:JsonType/SchemaTypeand serializers are wired correctlyThe move to
JsonSchemawith an internalJsonTypeenum implementingSchemaType, plus the contextualSchemaType.Serializerthat handles both single types andAnyOf, looks consistent. The dedicatedJsonTypeSerializercleanly maps between lowercase JSON strings and enum constants.Also applies to: 73-126
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (4)
17-38: Route annotation accumulation viaList<RouteAnnotationFunction>is a good improvementStoring a
List<RouteAnnotationFunction>underEndpointAnnotationAttributeKeyand appending inRoute.annotatelets multipleannotate {}blocks compose cleanly on the same route, deferringOperationconstruction to spec generation. That’s a nice simplification compared to storing prebuiltOperations.
40-73: Component name collision handling ingenerateOpenApiSpecnow looks correctThe
CollectSchemaReferencescallback now consistently uses either the unqualified title or the full title both for inserting intojsonSchemaand for the returned component name. This ensures that when simple names collide, the second schema is stored and referenced under its fully‑qualified name, keeping$refs andcomponents.schemaskeys aligned.
78-92: Operation mapping and composition pipeline is sound
findPathItemsnow threads anOperationMapping, enabling post‑processing likePopulateMediaTypeDefaultsand schema‑reference collection without changing core routing introspection.asPathItemcorrectly skips nodes without handlers or HTTP methods and still emits emptyOperation()when there’s a handler but no annotations, so undocumented routes remain visible if desired.operation()composes selector‑derived operations with annotation‑derived ones along the route lineage, andoperationFromAnnotateCallsfolds multipleRoute.annotatecalls per node viaOperation.plus, matching the test expectations for parent/child merging.Also applies to: 94-124, 126-130
237-253: Parameter content merge order favors more specific definitionsThe updated
Parameter.plusmergescontentandexamplesusing a merge that preserves entries from the more specific parameter when keys collide, which is consistent with the parent/child and multiple‑annotation merging semantics validated inannotateMerging.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (4)
58-62: JsonSchema inference is cleanly threaded throughOperationand nested DSLsPassing
JsonSchemaInferenceintoOperation.buildand delegating it throughOperation.BuilderintoParameters.Builder,RequestBody.Builder, andResponses.Builderenables consistent schema inference across the whole operation DSL. The defaultKotlinxJsonSchemaInferencekeeps existing behavior while allowing alternative inference strategies in the future.Also applies to: 69-76, 137-157
228-312: Parameter DSL migration toJsonSchemaand media-type builders looks correct
Parameter.schemanow usesReferenceOr<JsonSchema>andParameter.Builder.schemaisJsonSchema?, withbuild()wrapping it intoReferenceOr.Value, which matches the rest of the JsonSchema‑based model.Parameters.Builderconsistently passesschemaInferenceto eachParameter.Builder, andParameter.Builderitself delegatesJsonSchemaInference, so calls likeschema = jsonSchema<String>()resolve as expected.- Using
MediaType.Builder(schemaInference)in theContentType.invokeoperator keeps parameter content construction aligned with the newMediaTypebuilder.Also applies to: 335-457, 411-413
500-555: Response/Responses builders now compose nicely and preserve schema/context
Responses.BuilderholdingResponse.Builderinstances (plus adefaultbuilder) allows multiple configuration passes per status code before a single build, without losing modifications.Response.BuilderdelegatesJsonSchemaInference, threads it intoHeaders.BuilderandMediaType.Builder, and exposes ajsonSchemaconvenience that reads/writes the underlyingMediaType.Builderforapplication/json, which matches how tests use it and supports later content customization on the same media type.build()now maps the_contentbuilder map to builtMediaTypeinstances and preserves extensions/headers/links as before.Also applies to: 585-677
1043-1069: NewHeadermodel with schema inference and extensions looks solid
Headers.Buildernow takesschemaInferenceand usesHeader.Builder(schemaInference)so header definitions can also leveragejsonSchema<T>()and other inference helpers.- The new
Headerdata class mirrorsParameter(schema, content, examples, extensions) and implementsExtensiblewith the appropriate serializer mixin.Header.BuilderdelegatesJsonSchemaInference, usesMediaType.Builder(schemaInference)for content, and correctly carries through examples and extensions inbuild().This also addresses the previous issue where header extensions were collected but not preserved in the serialized model. Based on learnings, this mutable‑builder approach matches the existing patterns for other OpenAPI objects.
Also applies to: 1071-1180
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
18-29:JsonSchemaInferenceabstraction and default implementation integrate cleanlyThe
JsonSchemaInferencefun interface,KotlinxJsonSchemaInferencedefault, and theJsonSchemaInference.jsonSchema<T>()helper give a clear extension point while keeping existing behavior based on Kotlinx serializers.buildKotlinxSerializationSchema(type)mirrors the default implementation and is useful as a standalone helper; its slight duplication withKotlinxJsonSchemaInferenceis acceptable and keeps the API straightforward.Also applies to: 120-129, 136-140
49-110:buildJsonSchemacovers the main descriptor kinds with appropriate JsonSchema typesThe updated
SerialDescriptor.buildJsonSchema()now returnsJsonSchema, correctly mapping:
- classes/objects to
type = OBJECTwithpropertiesandrequired,- lists to
type = ARRAYwithitems,- maps to
type = OBJECTwithadditionalProperties,- primitives, enums, and contextual descriptors to the expected JsonSchema types.
The defaults for unsupported kinds (
OBJECT) provide a reasonable fallback for complex/custom serializers.
| @OptIn(ExperimentalSerializationApi::class) | ||
| @KeepGeneratedSerializer | ||
| public data class Components( | ||
| public val schemas: Map<String, JsonSchema>? = null, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify whether schemas should support references.
The schemas field is declared as Map<String, JsonSchema>?, while other component fields (responses, parameters, examples, requestBodies, headers, pathItems) consistently use ReferenceOr<T> to support OpenAPI reference objects. If schemas can reference other schemas using $ref, this field should also use ReferenceOr<JsonSchema> for consistency and correctness.
Run the following script to check if JsonSchema already supports references internally or if ReferenceOr wrapping is needed:
🏁 Script executed:
#!/bin/bash
# Search for JsonSchema definition and check if it has reference support
ast-grep --pattern 'data class JsonSchema'
ast-grep --pattern 'sealed class JsonSchema'
# Check ReferenceOr usage patterns with schemas
rg -n "ReferenceOr.*JsonSchema" --type=ktLength of output: 26
🏁 Script executed:
#!/bin/bash
# Search for JsonSchema definition - try without type filter
echo "=== Searching for JsonSchema definition ==="
rg -n "data class JsonSchema|sealed class JsonSchema" -g '*.kt'
echo ""
echo "=== Searching for ReferenceOr.*JsonSchema usage ==="
rg -n "ReferenceOr.*JsonSchema" -g '*.kt'
echo ""
echo "=== Check OpenApiSpecification for Components class around line 150 ==="
rg -n -A 50 "data class Components" -g '*.kt' | head -80Length of output: 7683
Change schemas field to use ReferenceOr<JsonSchema> to support schema references and maintain consistency.
At line 150 of OpenApiSpecification.kt, the schemas field is declared as Map<String, JsonSchema>?, but this breaks the established pattern:
- JsonSchema itself uses
ReferenceOr<JsonSchema>for nested schema fields (allOf, oneOf, anyOf, properties, items in JsonSchema.kt lines 29-33) - All other Components map fields use
ReferenceOr<T>(responses, parameters, examples, requestBodies, headers, pathItems at lines 151-155, 158) - The OpenAPI specification allows
$refin the schemas component, requiring reference object support
Update line 150 to: public val schemas: Map<String, ReferenceOr<JsonSchema>>? = null,
Additionally, consider whether the isEmpty() method (lines 171-180) should check the extensions field (line 159) for consistency, though this may be intentional design.
🤖 Prompt for AI Agents
In
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
around line 150, change the schemas property type from Map<String, JsonSchema>?
to Map<String, ReferenceOr<JsonSchema>>? so the components.schemas supports $ref
and matches other component maps and nested JsonSchema ReferenceOr usage; update
the declaration accordingly and run/adjust any usages or imports impacted by
this type change. Also review the isEmpty() method (lines ~171-180) to decide
whether to include extensions (line ~159) in the emptiness check for consistency
and update that logic if needed.
There was a problem hiding this comment.
According to the specification, the schemas field is Map[string, [Schema Object]] and does not allow references here.
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAPI 3.1 specification Components object schemas field
💡 Result:
-
In OpenAPI 3.1 the root components object contains a schemas field which is a map (object) whose keys are component names and whose values are Schema Objects (or Reference Objects pointing to Schema Objects). Each entry must be a Schema Object matching the JSON Schema (Draft 2020‑12) based Schema Object used by OAS 3.1. [1][2]
-
Typical form:
components:
schemas:
MyType:
type: object
properties: { ... }Those schema components are reusable anywhere a Schema Object is expected (e.g. request bodies, responses, parameters) and referenced with $ref: "#/components/schemas/MyType". [1][2]
-
Notes:
- The OpenAPI Schema Object in 3.1 is a superset of JSON Schema Draft 2020‑12 and the OAS base dialect id is https://spec.openapis.org/oas/3.1/dialect/base. [2][3]
Sources:
[1] OpenAPI Specification v3.1.x — Components / Schema examples.
[2] OpenAPI docs — Reusing Descriptions / Components Object.
[3] OpenAPI 3.1 spec — Schema Object (JSON Schema Draft 2020‑12 superset).
🌐 Web query:
OpenAPI 3.1 specification components schemas Reference Object $ref allowed
💡 Result:
Yes — OpenAPI 3.1 allows $ref inside components.schemas, but there are two distinct behaviors to be aware of:
-
Schema position (components.schemas and any Schema Object location): Schema $ref follows JSON Schema semantics (OAS 3.1 uses JSON Schema 2020‑12). A $ref in a Schema Object may have arbitrary sibling keywords (i.e., you can put other JSON Schema keywords alongside a $ref and they are combined as JSON Schema would). [1][2]
-
OpenAPI Reference Object (the generic Reference Object used in many non‑schema locations): that Reference Object is limited to the required $ref and may include only summary and description as siblings; other properties are ignored. This is a different construct than the JSON Schema $ref and applies where the spec explicitly expects a Reference Object rather than a Schema Object. [1][2]
Example (valid in OAS 3.1):
components:
schemas:
Pet:
$ref: '#/components/schemas/BasePet'
description: 'Extra description allowed as a schema sibling'
References:
[1] OpenAPI Specification v3.1.1 — Reference Object & Schema Object sections.
[2] OpenAPI Referencing Overview — table describing $ref behavior in 3.1 (schema vs. reference object).
🌐 Web query:
OpenAPI 3.1.1 specification components object field type definitions
💡 Result:
The Components Object (root.components) is a container of reusable named maps. Each fixed field is a map from string -> the indicated Object (or Reference Object). Key points and field-type definitions (from the OpenAPI 3.1.1 spec):
- schemas: Map[string, Schema Object]. (spec.openapis.org)
- responses: Map[string, Response Object | Reference Object]. (spec.openapis.org)
- parameters: Map[string, Parameter Object | Reference Object]. (spec.openapis.org)
- examples: Map[string, Example Object | Reference Object]. (spec.openapis.org)
- requestBodies: Map[string, Request Body Object | Reference Object]. (spec.openapis.org)
- headers: Map[string, Header Object | Reference Object]. (spec.openapis.org)
- securitySchemes: Map[string, Security Scheme Object | Reference Object]. (spec.openapis.org)
- links: Map[string, Link Object | Reference Object]. (spec.openapis.org)
- callbacks: Map[string, Callback Object | Reference Object]. (spec.openapis.org)
- pathItems: Map[string, Path Item Object]. (spec.openapis.org)
Additional notes:
- Component names (map keys) MUST match regex ^[a-zA-Z0-9.-_]+$. (spec.openapis.org)
- Schema Objects follow JSON Schema (OpenAPI 3.1 aligns with JSON Schema Draft 2020-12); schema.type may be an array in 3.1 to allow multiple types. (spec.openapis.org)
For full authoritative text and examples see the OpenAPI 3.1.1 specification (Components Object section) and the OpenAPI Learn page on Reusing Descriptions. (spec.openapis.org)
Citations:
- 1: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 2: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 3: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 4: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 5: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 6: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 7: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 8: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 9: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 10: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 11: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 12: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 13: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
@bjhham You're absolutely correct, and I apologize for the error in my review.
According to the official OpenAPI 3.1.1 specification, the Components object field types are explicitly defined as:
- schemas:
Map[string, Schema Object](no Reference Object) - responses:
Map[string, Response Object | Reference Object] - parameters:
Map[string, Parameter Object | Reference Object] - examples:
Map[string, Example Object | Reference Object] - requestBodies:
Map[string, Request Body Object | Reference Object] - headers:
Map[string, Header Object | Reference Object] - pathItems:
Map[string, Path Item Object](no Reference Object)
The schemas field is one of the few that does NOT permit Reference Objects at the top level. While JSON Schema $ref can be used within a Schema Object (following JSON Schema Draft 2020-12 semantics), the Components.schemas map itself expects Schema Objects directly.
The current implementation with Map<String, JsonSchema>? is correct. Thank you for the clarification!
✏️ Learnings added
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.445Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
671d85a to
8cccd84
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
149-180: Consider allowing references incomponents.schemasand whetherextensionsshould affectisEmpty().
Components.schemasis declared asMap<String, JsonSchema>?, while other component maps useReferenceOr<T>(responses, parameters, examples, requestBodies, headers, pathItems). If you want to support$refincomponents.schemassimilarly to other locations, it may be more consistent to useMap<String, ReferenceOr<JsonSchema>>?here and wrap inline schemas viaValue(...)like elsewhere.Also,
isEmpty()ignoresextensions, so aComponentsinstance that contains only extensions is treated as empty. If extensions are meant to count as “having content”, you might want to includeextensions.isNullOrEmpty()in the check; if not, a short comment explaining the intent would help future readers.Example for the schemas change:
public data class Components( - public val schemas: Map<String, JsonSchema>? = null, + public val schemas: Map<String, ReferenceOr<JsonSchema>>? = null,
🧹 Nitpick comments (3)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
50-53: ClarifyextensionsKDoc onOpenApiSpecification.
externalDocs(Line 51) andextensions(Line 53) currently both describe “additional external documentation”, butextensions: Map<String, JsonElement>?is really spec‑extensions (typicallyx-*keys), not documentation. Consider rewording the KDoc to match how other types describe extensions (e.g., “Specification‑extensions for this OpenAPI document (keys must start withx-).”).ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
56-61: JsonSchemaInference plumbing throughOperationDSL looks solid; consider updating KDoc.The new
schemaInference: JsonSchemaInference = KotlinxJsonSchemaInferenceparameter onOperation.buildand the delegation inOperation.Builder(and its use when constructingParameters.Builder,RequestBody.Builder, andResponses.Builder) make the schema‑inference strategy nicely configurable and composable.The only minor gap is that the KDoc for
buildstill only mentions theconfigureblock and not theschemaInferenceargument. Adding a short note (e.g., “@param schemaInference strategy used to infer JsonSchema for request/response bodies and parameters”) would keep the docs aligned with the new API.Also applies to: 73-76, 137-139, 146-148, 155-157
476-555: UnifyRequestBody.Buildercontent handling withResponse.Builderand fix minor DSL wording.
Response.Builderstores_contentasMutableMap<ContentType, MediaType.Builder>and exposescontent/jsonSchemavia builders so multiple calls (e.g.,ContentType.invoke { ... }followed byjsonSchema = ...) compose cleanly. In contrast,RequestBody.Builder:
- Stores
_contentasMutableMap<ContentType, MediaType>.jsonSchemaandxml()overwrite_content[type]with a newMediaType(...), discarding any previously configured examples/encoding/extensions for that content type.This is not a correctness bug today, but it creates surprising asymmetry between response and request body DSLs.
If you want consistency and safer composition, consider mirroring the
Response.Builderpattern inRequestBody.Builder:- private val _content = mutableMapOf<ContentType, MediaType>() + private val _content = mutableMapOf<ContentType, MediaType.Builder>() - public val content: Map<ContentType, MediaType> get() = _content + public val content: Map<ContentType, MediaType> get() = _content.mapValues { it.value.build() } - public operator fun ContentType.invoke(configure: MediaType.Builder.() -> Unit = {}) { - _content[this] = MediaType.Builder(schemaInference).apply(configure).build() - } + public operator fun ContentType.invoke(configure: MediaType.Builder.() -> Unit = {}) { + _content.getOrPut(this) { MediaType.Builder(schemaInference) }.apply(configure) + } - public var jsonSchema: JsonSchema? - get() = _content[ContentType.Application.Json]?.schema?.valueOrNull() - set(value) { - _content[ContentType.Application.Json] = MediaType(schema = value?.let(::Value)) - } + public var jsonSchema: JsonSchema? + get() = _content[ContentType.Application.Json]?.schema + set(value) { + _content.getOrPut(ContentType.Application.Json) { MediaType.Builder(schemaInference) } + .apply { schema = value } + } - public fun xml(schema: JsonSchema) { - _content[ContentType.Application.Xml] = MediaType(schema = ReferenceOr.Value(schema)) - } + public fun xml(schema: JsonSchema) { + _content.getOrPut(ContentType.Application.Xml) { MediaType.Builder(schemaInference) } + .apply { this.schema = schema } + } - return RequestBody( - description = description, - content = _content.ifEmpty { null }, + return RequestBody( + description = description, + content = _content.mapValues { it.value.build() }.ifEmpty { null }, required = required, extensions = extensions.ifEmpty { null }, )Also, several KDoc blocks in
Parameter.Builder.ContentType.invokeandHeader.Builder.ContentType.invokestill say “Adds a media type definition for the request body,” which is a bit misleading in the parameter/header context. You might want to tweak that wording to “for this parameter” / “for this header.”Also applies to: 583-677, 793-875
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
🧠 Learnings (10)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🧬 Code graph analysis (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
schemaInference(51-107)
🔇 Additional comments (12)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (10)
17-26: LGTM!The updated KDoc now correctly describes the attribute's purpose, and the
RouteAnnotationFunctiontypealias provides a clean abstraction for operation configuration functions. This enables deferred evaluation and composition of OpenAPI metadata.
28-38: LGTM!The accumulation logic correctly handles both initial and subsequent annotation calls, enabling multiple
annotate()calls on the same route to compose their configurations.
40-73: LGTM!The schema component naming logic correctly handles collisions by returning the same key used for storage. The first type with a given simple name gets the short name, while subsequent colliding types use their fully-qualified name. The
Components::isNotEmptycheck appropriately omits empty component sections.
75-92: LGTM!The function correctly discovers path items from route descendants and merges them when multiple HTTP methods share the same path. The
onOperationparameter provides extensibility for operation post-processing.
94-104: LGTM!The helper correctly filters routes that have handlers and HTTP methods, applies the operation mapping pipeline, and falls back to an empty
Operationfor endpoints without explicit annotations.
113-124: LGTM!The operation building logic correctly folds over the route lineage, combining annotations and selector-derived parameters. The TODO for KTOR-9086 appropriately documents the planned integration with ContentNegotiation plugin for schema inference.
126-157: LGTM!The annotation and selector processing correctly handles query parameters, path parameters, and headers. The required flag logic properly distinguishes between optional and required variants.
159-170: LGTM!All standard HTTP methods are correctly mapped to their corresponding
PathItemfields.
172-253: LGTM!The merge operators follow consistent patterns: first non-null value wins for scalar fields, collections are combined, and extensions use
{ b + a }to allow later values to override. TheParameter.plusupdate (line 245) correctly aligns with the content merging approach used elsewhere.
255-280: LGTM!The utility functions are well-implemented with clear documentation. The
mergeReferencesOrfunction correctly handles the separation of$refentries from inline values during merging.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
335-457: Parameter and Header schema handling is consistent and preserves extensions.The switch to
Parameter.schema: ReferenceOr<JsonSchema>?withParameter.Builder.schema: JsonSchema?and wrapping viaschema?.let(::Value)is consistent with the rest of the model. The same pattern inHeader/Header.Builder(includingJsonSchemaInferencedelegation,ContentType.invokecreatingMediaType.Builder(schemaInference), andbuild()wrapping schema/content/examples and preservingextensions.ifEmpty { null }) looks correct and matchesMediaType.Buildersemantics.No functional issues spotted here.
Also applies to: 1043-1068, 1071-1180
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (1)
67-115: Manual verification required—Gradle execution failed in sandbox environment.The ABI check command failed due to Gradle daemon connectivity issues in the sandbox. I cannot verify whether
checkLegacyAbipasses or detects any issues.Based on the learnings from this repository, breaking changes are acceptable in Ktor when code hasn't been released yet. However, to finalize this review, please confirm:
- Has
ktor-openapi-schemabeen released? If this is still pre-release, binary-incompatible changes may be intentional and acceptable.- Run the ABI check locally: Verify that
./gradlew :ktor-shared:ktor-openapi-schema:checkLegacyAbipasses on your machine. If it fails, run./gradlew :ktor-shared:ktor-openapi-schema:updateLegacyAbito sync the API signature files.
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
8cccd84 to
efa1c95
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
150-150: Consider usingReferenceOr<JsonSchema>forschemasfield consistency.The
schemasfield is declared asMap<String, JsonSchema>?while other component fields useReferenceOr<T>(responses, parameters, requestBodies, headers, pathItems). According to the OpenAPI 3.1 specification, schemas can reference other schemas using$ref. However, if inline schemas without references are the intended design here, this is acceptable.
🧹 Nitpick comments (5)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt (1)
40-59: Serialization logic is correct; minor stylistic inconsistency.The migration to
JsonSchemais properly applied in both deserialization and serialization paths. However, there's a minor inconsistency in how the serializer is obtained:
- Deserialization (line 44): uses
decoder.serializersModule.serializer<ReferenceOr<JsonSchema>>()- Serialization (line 56): uses
ReferenceOr.serializer(JsonSchema.serializer())Both approaches are functionally equivalent, but unifying them would improve consistency.
override fun serialize(encoder: Encoder, value: AdditionalProperties) { when (value) { is Allowed -> encoder.encodeBoolean(value.value) is PSchema -> encoder.encodeSerializableValue( - ReferenceOr.serializer(JsonSchema.serializer()), + encoder.serializersModule.serializer<ReferenceOr<JsonSchema>>(), value.value, ) } }ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
136-140: Redundant public function.
buildKotlinxSerializationSchemaduplicates the logic ofKotlinxJsonSchemaInference.buildSchema(). Consider whether both are needed in the public API, or if one could delegate to the other.public fun buildKotlinxSerializationSchema(type: KType): JsonSchema { - return serializer(type) - .descriptor - .buildJsonSchema() + return KotlinxJsonSchemaInference.buildSchema(type) }ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
97-109: AddanyOfhandling in collectSchema for consistency.The
JsonSchemaclass definesanyOfasList<ReferenceOr<JsonSchema>>?(ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt:32), identical in structure toallOfandoneOf. ThecollectSchemamethod currently processesallOfandoneOfrecursively but omitsanyOf, creating an inconsistency. Adding this handling ensures all schema collection types are processed uniformly.private fun collectSchema(schema: JsonSchema): ReferenceOr<JsonSchema> { return schemaToComponent(schema)?.let { ref -> ReferenceOr.schema(ref) } ?: ReferenceOr.value( schema.copy( allOf = schema.allOf?.map { it.mapToReference(::collectSchema) }, + anyOf = schema.anyOf?.map { it.mapToReference(::collectSchema) }, oneOf = schema.oneOf?.map { it.mapToReference(::collectSchema) }, not = schema.not?.mapToReference(::collectSchema), properties = schema.properties?.mapValues { (_, value) -> value.mapToReference(::collectSchema) }, items = schema.items?.mapToReference(::collectSchema), ) ) }ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (2)
113-121: VerifyLicense.identifierfield visibility is intentional.The
identifierfield is declared asprivate, making it inaccessible after construction despite being a valid OpenAPI 3.1info.license.identifierfield (SPDX license expression). If this is intentional for internal use only, consider adding a comment explaining why.@Serializable public data class License( /** The license name used for the API. */ public val name: String, /** A URL to the license used for the API. MUST be in the format of a URL. */ public val url: String? = null, - private val identifier: String? = null, + /** An SPDX license expression for the API. */ + public val identifier: String? = null, )
168-185:isEmpty()intentionally excludesextensions- document this behavior.The
isEmpty()method checks all content fields but excludesextensions. This is likely intentional since extensions are metadata rather than reusable components, but consider adding a brief comment explaining this design decision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt
- ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt
🧠 Learnings (11)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/test/**/*.kt : Follow test naming pattern: `DescribeWhatIsBeingTested`
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-11-25T09:38:19.380Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.380Z
Learning: Applies to **/*.kt : Mark internal APIs with `InternalAPI` annotation
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
🧬 Code graph analysis (3)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (2)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (1)
generateOpenApiSpec(46-73)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
OpenApiInfo(67-122)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (8)
schemaInference(72-212)schemaInference(227-312)schemaInference(359-457)schemaInference(499-554)schemaInference(584-677)schemaInference(810-874)schemaInference(1047-1068)schemaInference(1099-1179)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
schemaInference(51-107)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/GenericElement.kt (3)
GenericElement(121-126)GenericElement(131-132)GenericElement(137-151)
🔇 Additional comments (26)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt (1)
30-32: LGTM!The type change from
ReferenceOr<Schema>toReferenceOr<JsonSchema>aligns with the broader migration to theJsonSchema-based model across the OpenAPI schema module. Based on learnings, breaking changes are acceptable since this code hasn't been released yet.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt (1)
35-36: Property reordering noted.Moving
getafteroptionschanges the constructor parameter order. This is a binary-incompatible change, but acceptable since this API hasn't been released yet. Ensure the/api/signature files are updated via./gradlew updateLegacyAbi.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (2)
17-19: Schema type update to JsonSchema is consistent with the PR's design.The change from
ReferenceOr<Schema>?toReferenceOr<JsonSchema>?aligns with the broader JsonSchema-first approach introduced in this PR.
51-54: Builder delegation to JsonSchemaInference enables inline schema inference.The delegation pattern (
JsonSchemaInference by schemaInference) makesjsonSchema<T>()available within the builder DSL, consistent withOperation.BuilderandParameter.Builder.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (3)
29-105: Well-structured OpenAPI 3.1.1 expected document.The expected JSON correctly represents:
- OpenAPI version, info block
- Path item with operation, parameters, and responses
- Component schema extraction with
$refusage- Response extensions (
x-sample-message)
124-133: Proper integration of generateOpenApiSpec with path exclusion.The test correctly:
- Generates the full OpenAPI spec
- Excludes the
/routesintrospection endpoint from output- Uses the new
OpenApiInfomodel
287-298: Correct use of KotlinxJsonSchemaInference for schema assertions.Using
KotlinxJsonSchemaInference.jsonSchema<T>()directly in assertions matches the internal schema inference used by the route annotation system, ensuring test expectations align with actual behavior.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (4)
17-26: KDoc and type updates align with the new design.The attribute key now correctly documents storing route annotation handlers, and the
RouteAnnotationFunctiontypealias provides clear intent for the DSL pattern.
46-73: generateOpenApiSpec correctly handles schema component naming.The collision detection logic properly:
- Uses unqualified names when unique
- Falls back to fully-qualified names on collision
- Returns the same key used for storage (addressing the previous review)
113-129: Operation building correctly propagates schema inference.The lineage fold accumulates annotations from parent routes, and
Operation.build(schemaInference, function)ensures consistent schema inference across the operation tree. The TODO for KTOR-9086 appropriately tracks future ContentNegotiation integration.
237-253: Parameter merging now includes content field.The addition of
content = mergeNullable(content, other.content) { a, b -> b + a }at line 245 ensures parameter content definitions are properly merged when combining operations, consistent with other nullable field handling.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (2)
15-20: Clean functional interface design for operation post-processing.The
OperationMappingfun interface with composableplusoperator enables a flexible pipeline pattern for OpenAPI transformations.
22-33: JoinedOperationMapping correctly chains transformations.The loop now properly feeds
current(the progressively transformed operation) into each processor, fixing the previously reported composition bug.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
20-25: Clean JsonSchemaInference interface design.The fun interface provides a clear abstraction for schema generation, enabling custom implementations beyond the default Kotlinx Serialization-based approach.
120-129: Graceful error handling for non-serializable types.The try-catch wrapping
SerializationExceptionprovides a fallback schema with a descriptive error message rather than propagating the exception, which is appropriate for a documentation-generation context.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api (1)
1-26: LGTM! ABI changes are properly tracked.The new
OperationMappinginterface with itsmapandplusmethods enables composable post-processing of OpenAPI operations. ThefindPathItemssignature change maintains backward compatibility via the synthetic default method. The newgenerateOpenApiSpecfunction provides a clean entry point for full OpenAPI document generation.ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (3)
813-826: OpenApiInfo.Licenseidentifierfield is private - verify intentional design.The
Licenseclass has a 3-parameter constructor (name,url,identifier) but only exposesgetName()andgetUrl()getters, withcomponent1()andcomponent2()for destructuring. Theidentifierfield appears private, which is unusual for a data class. Per OpenAPI 3.1 spec,identifieris a valid SPDX license expression.If this is intentional (e.g., to support construction but prevent direct access), consider documenting this design decision. Otherwise, expose
identifierwith a getter andcomponent3().
625-633: JsonSchemaInference interface is well-designed for pluggable schema generation.The interface with
buildSchema(KType): JsonSchemaand theKotlinxJsonSchemaInferencegetter provides a clean extension point for custom schema inference strategies.
67-114: Components class provides comprehensive OpenAPI components support.The class properly implements
Extensibleand includes all standard OpenAPI component types (schemas, responses, parameters, examples, requestBodies, headers, links, callbacks, pathItems) with appropriate serialization support.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api (1)
9-27: Klib ABI properly mirrors JVM API surface.The multiplatform API declarations are consistent with the JVM ABI file. The
OperationMappinginterface withmapandplusmethods,CollectSchemaReferences, and the updatedfindPathItems/generateOpenApiSpecfunctions are correctly declared for all native targets.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
14-54: Well-documented OpenAPI root document model.The
OpenApiSpecificationclass provides comprehensive KDoc documentation matching the OpenAPI 3.1.1 specification. The use of@EncodeDefault(ALWAYS)for theopenapiversion field ensures it's always included in serialization.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (5)
58-75: Clean implementation of pluggable schema inference.The
Operation.buildfactory method now accepts aJsonSchemaInferenceparameter with a sensible default. TheBuilderclass properly delegatesJsonSchemaInferenceimplementation, enabling users to callbuildSchema(type)directly within the DSL.
1070-1180: Header class now properly implements Extensible with extensions preserved.The
Headerdata class correctly:
- Implements
Extensibleinterface (line 1090)- Includes
extensionsproperty (line 1089)- Uses
ExtensibleMixinSerializer(lines 1092-1095)- Preserves extensions in
build()(line 1176)This addresses the previous review concern about extensions being collected but not preserved.
500-554: Responses.Builder correctly uses lazy initialization pattern.The builder stores
Response.Builderinstances rather than builtResponseobjects, allowing incremental configuration when the same status code is configured multiple times. ThegetOrPutpattern (lines 519-521, 539-543) ensures builders are created once and reused.
585-677: Response.Builder properly propagates schemaInference to nested builders.The builder consistently passes
schemaInferencetoMediaType.Builder(lines 628-630, 639-641) andHeaders.Builder(line 617), maintaining the inference chain through all nested structures.
228-312: Parameters.Builder consistently propagates schemaInference to all parameter types.All parameter creation methods (
path,query,header,cookie, and deprecatedparameter) correctly instantiateParameter.Builder(schemaInference), ensuring schema inference is available throughout the parameter configuration DSL.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
Outdated
Show resolved
Hide resolved
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
Outdated
Show resolved
Hide resolved
| /** A URL to the Terms of Service for the API. MUST be in the format of a URL. */ | ||
| public val termsOfService: String? = null, | ||
| /** The contact information for the exposed API. */ | ||
| public val contact: Contact? = Contact(), |
There was a problem hiding this comment.
contact is an optional element, maybe better to use null as the default so the field is omitted unless set
| public val name: String, | ||
| /** A URL to the license used for the API. MUST be in the format of a URL. */ | ||
| public val url: String? = null, | ||
| private val identifier: String? = null, |
There was a problem hiding this comment.
Is private identifier needed?
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
Outdated
Show resolved
Hide resolved
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
efa1c95 to
4227d9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
85-85: Changecontactdefault tonullfor consistent optional field handling.The
contactfield defaults toContact(), creating an empty object that will serialize even when the user doesn't explicitly set contact information. Other optional fields inOpenApiInfo(description, termsOfService, license) default tonulland are omitted from serialization when not set.Apply this diff:
- public val contact: Contact? = Contact(), + public val contact: Contact? = null,This ensures the contact object is only included when explicitly configured.
🧹 Nitpick comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
16-53: Consider implementingExtensiblefor consistency with extension handling.The
OpenApiSpecificationclass has anextensionsfield (line 52) but doesn't implement theExtensibleinterface or useExtensibleMixinSerializer, unlikeOpenApiInfo(lines 66, 90) andComponents(lines 146, 160). This inconsistency may affect how extensions are serialized and deserialized.If extension support is intended, apply this pattern:
+@Serializable(OpenApiSpecification.Companion.Serializer::class) @OptIn(ExperimentalSerializationApi::class) +@KeepGeneratedSerializer public data class OpenApiSpecification( @EncodeDefault(ALWAYS) public val openapi: String = "3.1.1", ... - public val extensions: Map<String, GenericElement>? = null, -) + override val extensions: ExtensionProperties = null, +) : Extensible { + public companion object { + internal object Serializer : ExtensibleMixinSerializer<OpenApiSpecification>( + generatedSerializer(), + { spec, extensions -> spec.copy(extensions = extensions) } + ) + } +}If manual extension handling is intentional, please document the rationale.
Based on learnings, consistent patterns for extension handling across OpenAPI model classes improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
- ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
🧠 Learnings (12)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-26T11:34:00.445Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.445Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.apiktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🧬 Code graph analysis (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (8)
schemaInference(72-212)schemaInference(227-312)schemaInference(359-457)schemaInference(499-554)schemaInference(584-677)schemaInference(810-874)schemaInference(1047-1068)schemaInference(1099-1179)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (5)
ktor-utils/common/src/io/ktor/util/Attributes.kt (1)
AttributeKey(19-21)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
Components(146-187)ktor-utils/common/src/io/ktor/util/collections/TreeLike.kt (2)
descendants(26-30)lineage(19-21)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (2)
hasHandler(134-135)path(377-387)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
path(257-264)Operation(36-213)
🔇 Additional comments (33)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt (1)
7-8: LGTM! Clearer naming for the Pair transformation.The rename from
maptomapValueimproves clarity and aligns with themapValuemethods introduced inReferenceOr. Since this is an internal function, there's no API compatibility concern.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api (4)
9-12: Good functional composition design for OperationMapping.The
OperationMappinginterface provides a clean abstraction for composing operation transformations. Theplusmethod enables chaining mappings, which is demonstrated in the usage:PopulateMediaTypeDefaults + CollectSchemaReferences.
14-18: LGTM! Schema collection mechanism is well-designed.The
CollectSchemaReferencesclass provides a clear abstraction for extracting and naming schema components during OpenAPI generation.
20-21: API evolution tracked correctly.The change from
AttributeKey<Operation>toAttributeKey<List<Function1<Operation.Builder, Unit>>>is a breaking change but is properly tracked in this API file. This enables accumulating multiple route annotations, which is a sensible enhancement to the API.
22-27: New API additions look good.The new public APIs are well-designed:
PopulateMediaTypeDefaultsprovides a sensible default mappingRoute.annotateenables declarative OpenAPI annotationsfindPathItemsdefault parameter maintains backward compatibilitygenerateOpenApiSpecprovides the main entry point for spec generationktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (6)
67-114: Components structure aligns well with OpenAPI 3.1 specification.The
Componentsclass provides a clean container for reusable OpenAPI objects with appropriate Map-based properties. TheisEmpty()andisNotEmpty()methods enable conditional inclusion in the final specification.
419-522: Comprehensive JsonSchema implementation with proper OpenAPI 3.1 alignment.The
JsonSchemaclass (renamed fromSchema) provides extensive coverage of JSON Schema properties. The 39 component methods reflect the comprehensive data class structure needed for full OpenAPI/JSON Schema support. All nested schema references properly useReferenceOr<JsonSchema>.
625-633: Excellent abstraction for schema inference.The
JsonSchemaInferenceinterface provides a clean, extensible design for generating JSON schemas from type metadata. The defaultKotlinxJsonSchemaInferenceimplementation based on kotlinx.serialization is a sensible choice, and the design allows for alternative inference strategies if needed.
350-374: Consistent JsonSchemaInference integration across all builders.The builders have been systematically updated to:
- Accept a
JsonSchemaInferenceparameter- Delegate to it via
JsonSchemaInference by schemaInference- Use
JsonSchemainstead ofSchemafor all schema propertiesThis creates a consistent, composable pattern throughout the OpenAPI builder DSL.
Also applies to: 723-733, 938-973
1175-1228: Useful functional transformations for ReferenceOr.The addition of
mapValueandmapToReferencemethods enables elegant transformations of referenced or direct values. This is particularly useful when converting schemas to references during component extraction, as seen inCollectSchemaReferences.
740-887: Complete OpenAPI 3.1 model with proper serialization support.The additions of
OpenApiInfo,OpenApiSpecification,Components, andTagprovide a comprehensive OpenAPI 3.1 model. The nested classes (Contact, License), companion objects, and serializer integration are all properly structured.Also applies to: 843-887, 1492-1522
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (2)
19-19: Schema to JsonSchema migration applied correctly.The
schemafield type has been properly updated fromReferenceOr<Schema>?toReferenceOr<JsonSchema>?, consistent with the broader refactoring across the OpenAPI model.
50-107: MediaType.Builder properly integrates JsonSchemaInference.The builder has been correctly updated to:
- Accept and delegate to
JsonSchemaInference- Use
JsonSchemafor the schema property- Properly wrap the schema in
Value()during build()This enables schema inference to be available to nested DSL calls within media type configuration.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (4)
17-29: Clean functional interface for schema inference.The
JsonSchemaInferenceinterface provides an elegant abstraction for generating schemas from type metadata. The functional interface design with a singlebuildSchemamethod is appropriate and allows easy composition with default implementations.
31-108: Comprehensive schema generation from kotlinx.serialization descriptors.The
buildJsonSchema()extension provides thorough coverage of all serialization kinds:
- Structured types (CLASS, OBJECT, LIST, MAP)
- Primitives with correct JSON type mapping
- Enums with value enumeration
- Contextual serializers (fallback to OBJECT)
The implementation correctly:
- Identifies required fields (non-nullable, non-optional)
- Recursively builds nested schemas
- Uses
ReferenceOr.Valuefor nested referencesThe documentation correctly notes that circular references are not handled. For most use cases this is acceptable, as circular references should ideally use explicit
$refpointers in the OpenAPI components.
110-127: Good error handling for schema generation.The
jsonSchema<T>()extension provides a convenient reified API with appropriate error handling. CatchingSerializationExceptionand returning a schema with an error description ensures graceful degradation for non-serializable types.
129-138: Clean default implementation using kotlinx.serialization.The
buildKotlinxSerializationSchemafunction provides a straightforward implementation that chains serializer resolution through to schema building. This serves as the default inference strategy used byKotlinxJsonSchemaInference.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (7)
17-38: Clean API for accumulating route annotations.The introduction of
RouteAnnotationFunctionas a typealias and the updatedRoute.annotatemethod provide a clean way to accumulate multiple OpenAPI annotations on a single route. The list-based accumulation enables composing annotations from different sources (e.g., compiler plugin + manual annotations).
40-73: Well-designed OpenAPI spec generation with schema collection.The
generateOpenApiSpecfunction properly:
- Composes operation mappings (
PopulateMediaTypeDefaults + CollectSchemaReferences)- Handles schema naming with collision detection (short name when unique, full name otherwise)
- Conditionally includes components only when non-empty
The schema naming logic correctly returns the same key used for storage, avoiding the mismatch issue flagged in earlier reviews.
75-92: Efficient path item collection with composable transformations.The
findPathItemsfunction elegantly:
- Traverses the entire routing tree via
descendants()- Applies the operation mapping to each route's operation
- Merges path items for routes with the same path using the
+operatorThe default
OperationMappingparameter ensures backward compatibility while enabling customization.
113-124: Good operation composition with schema inference.The
operation()function properly:
- Uses
KotlinxJsonSchemaInferenceas the default (TODO notes future ContentNegotiation integration)- Folds across the route lineage to accumulate operations
- Merges annotation-based and selector-based operations
The lineage fold ensures child routes inherit and can override parent operations.
126-129: Clean transformation of annotation functions to operations.The
operationFromAnnotateCallsfunction elegantly transforms accumulated annotation functions into a single composedOperationusingOperation.buildwith schema inference andreducewith the merge operator.
237-253: Parameter merging enhanced with content support.The addition of content merging at line 245 properly extends the
Parametermerge operation to handle media type content, consistent with the broader OpenAPI model. The merge order (b + a) ensures the second parameter's content takes precedence, matching the behavior for other map fields likeextensions.
172-280: Comprehensive and consistent merge operations for OpenAPI objects.The suite of merge operators (
Operation.plus,PathItem.plus,Responses.plus,Response.plus,Parameter.plus) follow consistent patterns:
- Non-collection fields use first non-null value
- Collections are merged or deduplicated via helper functions
- Extensions prefer newer values (second parameter)
The utility functions (
mergeNullable,mergeReferencesOr,mergeElementsBy) are well-designed for handling nullable values, references, and element deduplication.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (3)
23-48: Consistent rename from Schema to JsonSchema throughout.The class has been properly renamed to
JsonSchema, and all nested schema references have been consistently updated to useReferenceOr<JsonSchema>. This includes:
allOf,oneOf,anyOflistsnotreferencepropertiesmapitemsreferenceThis rename better reflects that this is specifically a JSON Schema implementation per the OpenAPI/JSON Schema specifications.
73-82: Properly structured JsonType enum with correct JSON Schema types.The
JsonTypeenum provides all standard JSON Schema types (ARRAY, OBJECT, NUMBER, BOOLEAN, INTEGER, NULL, STRING). The placement withinJsonSchemaand implementation ofSchemaTypeenables polymorphic type handling. TheJsonTypeSerializerensures proper lowercase string serialization matching JSON Schema conventions.
84-126: Well-designed polymorphic SchemaType with proper serialization.The
SchemaTypesealed interface withJsonTypeandAnyOfvariants properly models JSON Schema's type system, which allows either:
- A single type (e.g.,
"string")- Multiple types (e.g.,
["string", "null"])The custom serializer correctly:
- Checks for array vs. single value during deserialization
- Serializes to lowercase strings (JSON Schema convention)
- Handles the
AnyOfcase by serializing to a JSON arrayktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt (1)
35-36: I'll help verify thegetfield reordering in PathItem.kt. Let me start by examining the full context and checking for any related changes.
<function_calls>
cat -n ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt | head -60
</function_calls>
<function_calls>
rg -n "PathItem(" --type kotlin -B 2 -A 5 | head -100
</function_calls>
<function_calls>
find . -name "PathItemapi*" -o -name "openapi.api" | head -20
</function_calls>
<function_calls>
OpenAPI 3.1 specification PathItem field order
</web_search>ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (5)
58-75: LGTM: Consistent schema inference propagation.The addition of the
schemaInferenceparameter tobuild()with a sensible default (line 59) and the constructor changes toOperation.Builder(lines 73-75) establish a clean pattern for propagating schema inference throughout the DSL hierarchy. The delegation patternJsonSchemaInference by schemaInferenceis an elegant solution.Note: While the
Builderconstructor signature change is breaking, this is acceptable for unreleased code per team policy.Based on learnings, API changes must be intentional and well-documented; this change serves the clear purpose of enabling schema inference throughout the OpenAPI builder DSL.
228-458: LGTM: Thorough and consistent parameter builder updates.All parameter builder instantiations correctly propagate
schemaInference(lines 244, 258, 273, 287, 301, 412), and the type migration fromSchematoJsonSchemais consistently applied throughoutParameter(line 341) andParameter.Builder(lines 360, 377).
500-678: LGTM: Builder accumulation pattern enables flexible configuration.The refactoring to store
Response.Builderinstances (line 501) andMediaType.Builderinstances (line 593) instead of final objects enables incremental configuration through multiple DSL calls. ThegetOrPutpattern (lines 519-522, 628-631) ensures that repeated calls toresponse(statusCode)orContentType.invokemodify the same builder, supporting composable operation definitions.Schema inference propagation is consistent throughout (lines 500, 520, 540, 585, 617, 629).
Based on learnings, Operation objects are designed to be composable and combined from multiple sources; this builder accumulation pattern supports that design goal.
811-875: LGTM: RequestBody builder correctly updated.Schema inference delegation (line 811), propagation to
MediaType.Builder(line 833), and the type migration toJsonSchema(lines 839, 850) are all correctly implemented.
1048-1180: LGTM: Header implementation is complete and consistent.The
Headerdata class and builder (lines 1071-1180) follow the established pattern:
- Proper
Extensibleimplementation withExtensibleMixinSerializer(lines 1090-1096)- Schema inference delegation in the builder (line 1100)
- Extensions properly collected and preserved in
build()(lines 1131, 1176)- Consistent schema type usage (
JsonSchemain builder line 1111,ReferenceOr<JsonSchema>in data class line 1083)The propagation of
schemaInferencethroughHeaders.Builder(line 1048) toHeader.Builder(line 1061) andMediaType.Builder(line 1140) is thorough.This resolves the past review comment about Header extensions not being preserved.
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
4227d9b to
b343b60
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
85-85: Consider usingnullas default forcontact.The
contactfield defaults toContact()(an empty object), which means it will always be serialized even when no contact information is provided. Per OpenAPI specification,contactis optional. Usingnullas the default would omit the field when not explicitly set, producing cleaner output.- public val contact: Contact? = Contact(), + public val contact: Contact? = null,
🧹 Nitpick comments (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
27-29: Consider removing duplicate implementation.
buildKotlinxSerializationSchemaduplicates the logic already present inKotlinxJsonSchemaInference. Consider delegating to avoid redundancy:public fun buildKotlinxSerializationSchema(type: KType): JsonSchema { - return serializer(type) - .descriptor - .buildJsonSchema() + return KotlinxJsonSchemaInference.buildSchema(type) }Also applies to: 134-138
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (1)
1118-1119: Minor: KDoc mentions incorrect style values for Header.The KDoc for
stylementions values like "matrix", "label", "form" which are only valid for certain parameter locations. For headers, only "simple" is allowed per OpenAPI spec (as noted in the class KDoc on line 1072-1073). Consider updating the property KDoc to reflect this constraint.- /** Describes how the header value will be serialized (e.g., "matrix", "label", "form", "simple", "spaceDelimited", "pipeDelimited", "deepObject"). */ + /** Describes how the header value will be serialized. For headers, only "simple" is valid. */ public var style: String? = null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt(6 hunks)ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt(9 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt(0 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt(4 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt(2 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt(29 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt(1 hunks)ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Header.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Utils.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/ReferenceOr.kt
- ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
**/test/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Follow test naming pattern:
DescribeWhatIsBeingTested
Files:
ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt
🧠 Learnings (16)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
📚 Learning: 2025-11-26T11:34:00.471Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.471Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.ktktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-10-15T16:56:46.844Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RouteSelector.kt:499-500
Timestamp: 2025-10-15T16:56:46.844Z
Learning: In Ktor's RouteSelector implementations, pathString() methods return OpenAPI-compliant path templates (e.g., `{parameterName}`) without optional markers like `?`, even for optional parameters. The optionality is expressed separately in the OpenAPI parameter definitions, not in the path template. This differs from toString() which may include optional markers for debugging purposes.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/test/**/*.kt : Follow test naming pattern: `DescribeWhatIsBeingTested`
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Mark internal APIs with `InternalAPI` annotation
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api
🧬 Code graph analysis (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (8)
schemaInference(72-212)schemaInference(227-312)schemaInference(359-457)schemaInference(499-554)schemaInference(584-677)schemaInference(810-874)schemaInference(1047-1068)schemaInference(1099-1179)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (1)
schemaInference(51-107)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/GenericElement.kt (3)
GenericElement(121-126)GenericElement(131-132)GenericElement(137-151)
🔇 Additional comments (33)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/test/io/ktor/annotate/RouteAnnotationApiTest.kt (4)
29-105: LGTM! Test expectations updated to full OpenAPI 3.1.1 spec.The expected JSON now properly represents a complete OpenAPI specification with all required top-level fields (openapi, info, paths, components). The Message schema is correctly defined in the components section with all fields marked as required.
124-133: LGTM! Clean approach to exclude introspection route from documentation.The pattern of generating the full OpenAPI spec and then filtering out the
/routespath itself prevents self-referential documentation, which is the correct approach.
234-234: LGTM! Appropriate use of deprecation suppression.The test intentionally exercises the deprecated
parameter()method to verify merging behavior with the new type-specific methods (path, query, header, cookie). The suppression is correctly applied.
287-287: LGTM! Tests verify schema inference consistency.The test correctly verifies that the merged parameter schema matches the expected
KotlinxJsonSchemaInference.jsonSchema<String>()output, ensuring schema inference works correctly through the merge pipeline.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (4)
10-20: LGTM! Well-designed composable interface.The SAM interface design with the
plusoperator enables clean pipeline composition (e.g.,PopulateMediaTypeDefaults + CollectSchemaReferences). The documentation clearly explains its use in post-processing.
22-33: LGTM! Correctly implements sequential composition.The loop correctly passes the progressively transformed
currentvalue to each processor (line 26), ensuring mappings compose properly. The optimizedplusmethod prevents unnecessary nesting ofJoinedOperationMappinginstances.
43-106: LGTM! Efficient default population with fast path optimization.The implementation correctly:
- Uses fast-path checks (lines 45-59) to avoid unnecessary copying when no defaults are needed
- Applies sensible defaults:
queryfor parameter location,text/plainfor content when schema is absent- Handles both parameters and response headers consistently
108-159: LGTM! Comprehensive schema reference collection.The implementation correctly:
- Traverses all schema locations: request body, responses, and parameters (lines 114-133)
- Uses the
schemaToComponentcallback to determine when to create a reference vs. inline the schema- Recursively processes nested schemas (allOf, oneOf, not, properties, items) to ensure complete transformation
ktor-shared/ktor-openapi-schema/common/test/io/ktor/openapi/OperationSerializationTests.kt (1)
134-139: LGTM! Appropriate test coverage for response headers.The addition of the
Locationheader in theCreatedresponse is standard REST practice and properly tests the headers DSL with content type specification.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/PathItem.kt (1)
35-36: Verify the rationale for reordering thegetproperty.The
getproperty has been moved from afterdescriptionto afteroptions. While this doesn't affect serialization (due to the custom serializer), it breaks the typical HTTP method ordering convention (GET typically comes first among methods).Please clarify if this reordering serves a specific purpose or if it should follow a more conventional order.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/MediaType.kt (2)
19-19: LGTM! Schema type updated to JsonSchema.This change aligns with the broader refactoring from
SchematoJsonSchemaacross the OpenAPI module.
52-54: LGTM! Builder enhanced with schema inference delegation.The Builder now accepts a
JsonSchemaInferenceparameter and delegates to it, enabling nested builders to generate schemas using thejsonSchema<T>()method. This is consistent with the pattern used inOperation.Builderand other builders.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/RouteAnnotationApi.kt (9)
17-26: LGTM! API improved with typealias and corrected documentation.The KDoc now correctly describes that this attribute stores OpenAPI metadata (not just an Operation), and the new
RouteAnnotationFunctiontypealias improves readability. The switch toList<RouteAnnotationFunction>enables composing multiple annotations.
31-38: LGTM! Simplified annotation accumulation.The function now properly accumulates multiple
RouteAnnotationFunctioncalls by building up a list, enabling composable route documentation.
40-73: LGTM! Proper OpenAPI spec generation with collision handling.The function correctly:
- Composes the post-processing pipeline:
PopulateMediaTypeDefaults + CollectSchemaReferences- Handles schema name collisions by using short names when unique, full names when there's a collision
- Constructs a complete
OpenApiSpecificationwith collected components
78-92: LGTM! Extensible path item collection with sensible default.Adding the
onOperationparameter with a default value ofPopulateMediaTypeDefaultsmaintains backward compatibility while enabling custom post-processing pipelines.
94-104: LGTM! Path item creation now applies operation mapping.The function correctly applies the
onOperationmapping to transform the operation before including it in the path item.
113-124: LGTM! Operation assembly with schema inference.The function correctly:
- Folds over the route lineage to merge operations from all ancestors
- Combines both explicit annotations and selector-derived operations
- Uses
KotlinxJsonSchemaInference(note the TODO at line 114 for future plugin integration)
126-129: LGTM! Clean functional composition of annotation operations.The function correctly maps each annotation function to an
Operationusing the provided schema inference, then merges them with theplusoperator.
141-150: LGTM! Improved readability with spacing.The added blank lines between selector cases improve code readability without changing behavior.
247-247: LGTM! Enhanced content merging for parameters.The parameter merging now properly combines content maps using
mergeNullable(content, other.content) { a, b -> b + a }, allowing content types to be merged instead of simply taking one or the other.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt (2)
23-48: LGTM! Schema renamed to JsonSchema with consistent type updates.The rename from
SchematoJsonSchemais more descriptive and all property types have been consistently updated. Per project learnings, breaking changes are acceptable for unreleased code.
73-82: LGTM! JsonType enum properly relocated.The
JsonTypeenum has been moved within theJsonSchemaclass structure while maintaining its serialization behavior and all enum values.ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.api (1)
1-26: LGTM! API changes properly tracked.All public API changes are correctly documented in the signature file:
- New
OperationMappinginterface and implementations- Updated
findPathItemssignature withOperationMappingparameter- New
generateOpenApiSpecfunctionThis satisfies the project's binary compatibility tracking requirements.
As per coding guidelines, binary compatibility changes are properly tracked in the
/api/directory.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
17-29: Well-designed schema inference abstraction.The
JsonSchemaInferencefun interface withKotlinxJsonSchemaInferenceimplementation provides a clean extension point for custom schema generation strategies while defaulting to kotlinx.serialization-based inference.
47-107: Schema generation logic looks correct.The
buildJsonSchema()function correctly maps kotlinx.serialization descriptor kinds to JSON Schema types. The handling of CLASS/OBJECT, LIST, MAP, primitives, ENUM, and CONTEXTUAL kinds is appropriate.One note: the function doesn't handle circular references (as documented in the KDoc). For deeply nested types with self-references, this could cause a stack overflow.
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/api/ktor-server-routing-annotate.klib.api (1)
9-27: API surface additions look intentional and well-structured.The new
OperationMappinginterface withplusfor composition,CollectSchemaReferencesfor schema extraction,PopulateMediaTypeDefaultsfor default population, and updatedfindPathItems/generateOpenApiSpecfunctions provide a clean, composable pipeline for OpenAPI post-processing.The
EndpointAnnotationAttributeKeytype change fromAttributeKey<Operation>toAttributeKey<List<Function1<Operation.Builder, Unit>>>is a breaking change, but this is acceptable since the code hasn't been released yet. Based on learnings, breaking changes are acceptable in Ktor codebase when the code hasn't been released yet.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
16-53: Comprehensive OpenAPI 3.1.1 model implementation.The
OpenApiSpecification,OpenApiInfo,Tag, andComponentsdata classes correctly model the OpenAPI 3.1.1 specification with proper serialization annotations, extension support viaExtensibleinterface, and appropriate nullability. TheComponents.schemasfield correctly usesMap<String, JsonSchema>?withoutReferenceOrwrapper, which aligns with the OpenAPI 3.1.1 specification. Based on learnings, this is the correct behavior.Also applies to: 66-122, 128-139, 146-187
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (1)
67-115: ABI changes correctly track new public API surface.The API dump properly reflects the new JsonSchema-centric OpenAPI model including:
Componentswith schema/response/parameter mapsJsonSchemawith comprehensive JSON Schema Draft 2020-12 propertiesJsonSchemaInferenceinterface andKotlinxJsonSchemaInferenceOpenApiInfo,OpenApiSpecification, andTagtypes- Updated Builder constructors accepting
JsonSchemaInferenceThis aligns with the coding guidelines requiring all public API changes to be tracked in
/api/directories.Also applies to: 419-633, 740-887, 1492-1522
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt (4)
58-75: Clean delegation pattern for schema inference.The
Operation.build()function acceptingJsonSchemaInferencewith a default, andBuilderimplementingJsonSchemaInference by schemaInference, provides a clean, composable API that propagates schema inference throughout the builder hierarchy without requiring explicit threading.
500-554: Builder composition pattern for Responses is well-implemented.Storing
Response.Builderinstances in_responsesand building on demand allows for incremental configuration through multiple DSL calls. ThegetOrPutpattern inresponse()anddefault()methods correctly accumulates configurations.
1071-1180: Header class now correctly implements Extensible.The
Headerdata class now includesextensions: ExtensionPropertiesproperty, implementsExtensible, and theBuilder.build()method properly passesextensions = extensions.ifEmpty { null }. This addresses the previously flagged issue about extensions being collected but not preserved.
593-644: Response content accumulation pattern is correct but may have edge case.The
Response.BuilderstoresMediaType.Builderinstances and builds them on demand. However, thejsonSchemasetter creates a newMediaType.Buildereach time, which means previous configurations forApplication.Jsoncontent type could be lost ifjsonSchemais set after callingContentType.Application.Json { ... }.Consider documenting this behavior or ensuring that
jsonSchemasetter also usesgetOrPut:public var jsonSchema: JsonSchema? get() = _content[ContentType.Application.Json]?.schema set(value) { - _content.getOrPut(ContentType.Application.Json) { - MediaType.Builder(schemaInference) - }.apply { + _content.getOrPut(ContentType.Application.Json) { + MediaType.Builder(schemaInference) + }.apply { schema = value } }Wait, looking more closely, the current implementation already uses
getOrPut, so this is fine. The setter correctly accumulates with existing builder.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
93-93: Consider: DefaultContact()may serialize as empty object.Defaulting
contacttoContact()means an empty contact object will be included in the serialized output even when no contact information is provided. Per a past review comment, usingnullas the default would omit the field when not set.This may be intentional if you want the contact structure always present, but if you want to omit it when empty, consider:
- public val contact: Contact? = Contact(), + public val contact: Contact? = null,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
🧠 Learnings (14)
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-11-26T11:34:00.471Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.471Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Validate ABI with `./gradlew checkLegacyAbi` and update with `./gradlew updateLegacyAbi`
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Run `./gradlew updateLegacyAbi` after making ABI changes to update ABI signature files
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
🔇 Additional comments (13)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (3)
10-20: LGTM! Clean functional interface design.The
OperationMappinginterface with SAM conversion support and theplusoperator for composition follows idiomatic Kotlin patterns well. The documentation clearly explains the purpose.
22-33: LGTM! Correct composition logic.The loop now correctly chains transformations using
current, and theplusoverride efficiently flattens the operations list.
80-92: Verify: Response default content may need similar treatment.The code applies header defaults for
responses.default, but does not apply any defaults to the response'scontentfield. If response content also requires default population similar to parameters (e.g., when bothschemaandcontentare missing), this may be an oversight.However, if response content is always expected to be explicitly set, this is fine.
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (4)
17-25: LGTM! Clean interface design.The
JsonSchemaInferencefunctional interface provides a clear abstraction for schema generation from type metadata, following Kotlin conventions.
27-29: LGTM!The
KotlinxJsonSchemaInferencecorrectly delegates to the serialization infrastructure.
47-117: LGTM! Comprehensive schema generation.The implementation covers all standard
SerialKindcases appropriately. The documented limitation regarding circular references is acceptable given the noted scope.
127-136: LGTM! Graceful error handling.Catching
SerializationExceptionand returning a descriptive error schema is a good defensive approach for runtime robustness.ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (3)
67-114: LGTM! Components ABI is consistent.The
Componentsclass ABI correctly reflects the data class with 10 properties (schemas through extensions), proper companion serializer, andisEmpty()/isNotEmpty()methods.
419-632: LGTM! JsonSchema and inference ABI is well-structured.The
JsonSchemaclass with its nested types (Discriminator,SchemaType,JsonType) and theJsonSchemaInferenceinterface are properly exposed in the public API.
812-842: LGTM! License ABI is now correct.The
OpenApiInfo$Licenseclass properly exposes all three properties (name,url,identifier) with consistent component functions and getters, addressing the past review concern.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (3)
13-61: LGTM! Well-documented root document model.The
OpenApiSpecificationclass correctly models the OpenAPI 3.1.1 root document with comprehensive KDoc for each field. The default version of "3.1.1" and@EncodeDefault(ALWAYS)ensure proper serialization.
132-147: LGTM!The
Tagdata class is clean and correctly models the OpenAPI Tag Object with appropriate nullable optional fields.
176-195: LGTM! Complete emptiness check.The
isEmpty()method correctly checks all component fields includingextensions, addressing the past review feedback.
...-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
Show resolved
Hide resolved
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
Outdated
Show resolved
Hide resolved
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
e9edbb8 to
0e0fe7e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
93-93: Usenulldefault for optionalcontactfield.The
contactfield defaults toContact(), which will serialize an empty contact object even when no contact info is provided. Per past review feedback, optional OpenAPI fields should default tonullso they're omitted from the output unless explicitly set.Apply this diff:
- public val contact: Contact? = Contact(), + public val contact: Contact? = null,ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
114-137: CollectSchemaReferences does not processresponses.defaultcontent schemasIn
CollectSchemaReferences.map, onlyresponses.responsesentries are passed throughcollectSchemaReferences, whileresponses.defaultis left unchanged. If a default response carries inline schemas in itscontent, those schemas will never be converted to component references, which is inconsistent with how non‑default responses are handled and with the class‑level intent to “replace all JSON class schema values with component references.”You can address this by also transforming
responses.defaultalongside the existingresponses.responsesmapping:- responses = operation.responses?.let { responses -> - responses.copy( - responses = responses.responses?.mapValues { (_, response) -> - response.mapValue { - it.copy(content = it.content?.let(::collectSchemaReferences)) - } - } - ) - }, + responses = operation.responses?.let { responses -> + responses.copy( + default = responses.default?.mapValue { response -> + response.copy( + content = response.content?.let(::collectSchemaReferences), + ) + }, + responses = responses.responses?.mapValues { (_, response) -> + response.mapValue { + it.copy(content = it.content?.let(::collectSchemaReferences)) + } + } + ) + },This keeps
responsesnullable and preserves the composable intermediate model, aligning with the existing design of the OpenAPI objects. As a minor follow‑up, you may also want to extend the KDoc to clarify whatschemaToComponentis expected to return (e.g., stable component names derived from schema titles). Based on learnings, this keeps the Operation model safely composable and nullable.
🧹 Nitpick comments (1)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
47-117: Consider tracking depth or implementing reference extraction for circular types.The KDoc at line 44-45 warns about circular references but the implementation doesn't guard against them. For recursive data structures (e.g., a
Nodewith achildren: List<Node>property), this will cause stack overflow.Consider one of these approaches:
- Add a
maxDepthparameter with a reasonable default (e.g., 10)- Track visited descriptors in a set to detect cycles and emit a reference
- Defer to the caller's
JsonSchemaInferencecontext to handle reference managementExample for approach #1:
public fun SerialDescriptor.buildJsonSchema(maxDepth: Int = 10, currentDepth: Int = 0): JsonSchema { if (currentDepth > maxDepth) { return JsonSchema( type = JsonSchema.JsonType.OBJECT, description = "Max depth exceeded for ${serialName}" ) } // ... rest of implementation, passing maxDepth and currentDepth + 1 to recursive calls }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
🧠 Learnings (15)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.apiktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Validate ABI with `./gradlew checkLegacyAbi` and update with `./gradlew updateLegacyAbi`
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Run `./gradlew updateLegacyAbi` after making ABI changes to update ABI signature files
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-26T11:34:00.471Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.471Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
🔇 Additional comments (13)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
17-29: LGTM! Clean interface design.The functional interface pattern with SAM conversion is idiomatic, and the default implementation using Kotlinx Serialization is well-suited for the use case.
119-136: LGTM! Good error handling.The SerializationException catch with a descriptive fallback schema is good defensive programming. The error message in the
descriptionfield provides useful debugging information.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (3)
13-61: LGTM! OpenAPI 3.1.1 root document structure is correct.The data class properly represents the OpenAPI specification root with appropriate required fields, optional fields, and extension support.
63-147: LGTM! SecurityRequirement and Tag types are well-defined.The typealias for
SecurityRequirementprovides clarity, and theTagdata class correctly represents the OpenAPI Tag Object specification.
149-195: LGTM! Components structure correctly implements OpenAPI 3.1.1 specification.The field types properly distinguish between schemas (no ReferenceOr) and other components (with ReferenceOr), matching the OpenAPI spec. The
isEmpty()method correctly checks all fields including extensions.Based on learnings: The schemas field correctly uses
Map<String, JsonSchema>without ReferenceOr per OpenAPI 3.1.1 specification.ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (6)
67-114: LGTM! Components ABI signatures are complete.The ABI correctly exposes all component fields, accessors, and utility methods (
isEmpty(),isNotEmpty()).
419-632: LGTM! JsonSchema and JsonSchemaInference ABIs are correctly defined.The interface, implementation, and all related types (Discriminator, JsonType, SchemaType) are properly exposed in the ABI with correct signatures.
350-374: LGTM! All builder classes correctly implement JsonSchemaInference.The ABI shows that Header$Builder, MediaType$Builder, Operation$Builder, Parameter$Builder, RequestBody$Builder, and Response$Builder all properly implement the JsonSchemaInference interface with the required
buildSchemamethod.Also applies to: 722-732, 939-941, 1027-1029, 1262-1264, 1314-1316
739-842: LGTM! OpenApiInfo and nested types ABIs are complete.The OpenApiInfo, Contact, and License types are fully exposed with all component accessors and getters. The previously flagged missing
component3()for License is now present.
844-888: LGTM! OpenApiSpecification ABI correctly reflects source.The constructor signature and all accessors match the source. The
getTags()method correctly returnsListas per the source definition.
1493-1523: LGTM! Tag ABI is complete and correct.The Tag type is fully exposed with all required accessors and serialization support.
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (2)
15-33: OperationMapping composition and JoinedOperationMapping implementation look correctThe SAM
OperationMappingplus-operator andJoinedOperationMapping.mapnow compose mappings properly by threading the progressively transformedcurrentthrough each processor, and the override ofplusonJoinedOperationMappingkeeps chained mappings efficient. No functional issues spotted here.
43-108: PopulateMediaTypeDefaults correctly applies minimal, targeted defaultsThe fast‑path checks and the subsequent copies for parameters and response headers are consistent with the documented behavior: only inline
Parameter/Headervalues are touched, defaults are added only when bothschemaandcontentare absent (for content) orinis missing (for parameters), and referenced components remain unchanged. This fits well with the composable, nullable OpenAPI model design.
bjhham
force-pushed
the
bjhham/openapi-schema-fixes
branch
from
0e0fe7e to
f8a2408
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (1)
120-128:responses.defaultcontent schemas are still not collected.The
responses.responsesmap is processed for schema references, butresponses.defaultis still skipped. This means inline schemas in the default response won't be replaced with component references.Apply this diff to also process the default response:
responses = operation.responses?.let { responses -> responses.copy( + default = responses.default?.mapValue { + it.copy(content = it.content?.let(::collectSchemaReferences)) + }, responses = responses.responses?.mapValues { (_, response) -> response.mapValue { it.copy(content = it.content?.let(::collectSchemaReferences)) } } ) },
🧹 Nitpick comments (2)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (2)
27-29: Consider adding KDoc forKotlinxJsonSchemaInference.This is a public API that users will reference. A brief KDoc explaining that this implementation uses kotlinx.serialization's
serializer(type)and that it may throwSerializationExceptionfor non-serializable types would be helpful.+/** + * A [JsonSchemaInference] implementation that uses kotlinx.serialization to build JSON schemas. + * + * This implementation delegates to [SerialDescriptor.buildJsonSchema] using the serializer + * resolved for the given type. May throw [SerializationException] if the type is not serializable. + */ public val KotlinxJsonSchemaInference: JsonSchemaInference = JsonSchemaInference { type -> serializer(type).descriptor.buildJsonSchema() }
96-100:PrimitiveKind.CHARfalls through to default case.The primitive kinds handled are STRING, BOOLEAN, BYTE/SHORT/INT/LONG, and FLOAT/DOUBLE. However,
PrimitiveKind.CHARis not explicitly handled and will fall through to the default case at line 115, resulting in an OBJECT type schema. A CHAR is typically represented as a STRING in JSON.PrimitiveKind.FLOAT, PrimitiveKind.DOUBLE -> JsonSchema(type = JsonSchema.JsonType.NUMBER) + PrimitiveKind.CHAR -> + JsonSchema(type = JsonSchema.JsonType.STRING) + SerialKind.ENUM -> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt(1 hunks)ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api(20 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt(3 hunks)ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Follow Kotlin official style guide (https://kotlinlang.org/docs/coding-conventions.html)
Use star imports forio.ktor.*packages
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with@InternalAPIannotation
Run./gradlew lintKotlinand fix all linting issues before giving control back to the user
Use./gradlew formatKotlinto automatically fix formatting issues
Run./gradlew updateLegacyAbiafter making ABI changes to update ABI signature files
Binary compatibility is enforced - all public API changes must be tracked in the/api/directories
Validate ABI with./gradlew checkLegacyAbiand update with./gradlew updateLegacyAbi
API changes must be intentional and well-documented
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt
🧠 Learnings (15)
📓 Common learnings
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
📚 Learning: 2025-10-15T16:46:27.493Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:43-43
Timestamp: 2025-10-15T16:46:27.493Z
Learning: In the ktor-openapi-schema module, Operation objects (and related OpenAPI objects) are designed to be composable and can be combined from multiple sources. Therefore, properties like `responses` should remain nullable to support intermediate states during object composition/merging.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : API changes must be intentional and well-documented
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Document all public APIs including parameters, return types, and exceptions
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Binary compatibility is enforced - all public API changes must be tracked in the `/api/` directories
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-10-15T16:47:28.272Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5125
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/Operation.kt:442-444
Timestamp: 2025-10-15T16:47:28.272Z
Learning: In the ktor-openapi-schema module, the maintainer prefers to keep certain properties mutable (using `var` instead of `val`) in data classes like Response, even when they are part of the public API, to allow flexibility during construction and modification.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-26T11:34:00.471Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 5206
File: ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt:150-150
Timestamp: 2025-11-26T11:34:00.471Z
Learning: In OpenAPI 3.1.1 specification, the Components.schemas field is defined as Map[string, Schema Object] and does NOT allow Reference Objects at the top level, unlike other component fields (responses, parameters, examples, requestBodies, headers, links, callbacks) which are defined as Map[string, <Type> | Reference Object].
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-09T07:08:35.085Z
Learnt from: bjhham
Repo: ktorio/ktor PR: 4916
File: ktor-server/ktor-server-core/api/ktor-server-core.api:151-151
Timestamp: 2025-06-09T07:08:35.085Z
Learning: Breaking changes are acceptable in Ktor codebase when the code hasn't been released yet, as confirmed by bjhham from the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.ktktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.ktktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-27T10:07:26.686Z
Learnt from: osipxd
Repo: ktorio/ktor PR: 5205
File: ktor-client/ktor-client-curl/desktop/interop/include/curl/multi.h:410-420
Timestamp: 2025-11-27T10:07:26.686Z
Learning: In the ktorio/ktor repository, do not review files under `ktor-client/ktor-client-curl/desktop/interop/include/curl/*` as these are upstream libcurl header files copied from the curl project and not maintained by the Ktor team.
Applied to files:
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Validate ABI with `./gradlew checkLegacyAbi` and update with `./gradlew updateLegacyAbi`
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-11-25T09:38:19.393Z
Learnt from: CR
Repo: ktorio/ktor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:38:19.393Z
Learning: Applies to **/*.kt : Run `./gradlew updateLegacyAbi` after making ABI changes to update ABI signature files
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-06-16T17:03:45.729Z
Learnt from: tre3p
Repo: ktorio/ktor PR: 4936
File: ktor-utils/jvm/src/io/ktor/util/ZstdEncoding.kt:28-35
Timestamp: 2025-06-16T17:03:45.729Z
Learning: In the Ktor codebase, encoder implementations consistently use GlobalScope.writer and GlobalScope.reader patterns for compression/decompression operations. This is the established architectural pattern used throughout the codebase for byte channel transformations.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
📚 Learning: 2025-05-30T06:45:52.309Z
Learnt from: rururux
Repo: ktorio/ktor PR: 4896
File: ktor-client/ktor-client-core/jvm/test/FileStorageTest.kt:1-12
Timestamp: 2025-05-30T06:45:52.309Z
Learning: The headersOf() function from io.ktor.http package is available through wildcard imports like `import io.ktor.http.*`, so no explicit import statement is needed when using wildcard imports from that package.
Applied to files:
ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api
🔇 Additional comments (6)
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchemaInference.kt (1)
130-134: Error details in schema description may be exposed in API documentation.The exception message is included in the schema description. If this schema ends up in a generated OpenAPI spec, the internal error details (class name, exception message) would be visible. This might be intentional for debugging, but consider if this is appropriate for production API documentation.
} catch (e: SerializationException) { JsonSchema( type = JsonSchema.JsonType.OBJECT, - description = "Failed to resolve schema for ${T::class.simpleName}. ${e::class.simpleName}: ${e.message}" + description = "Schema could not be resolved for ${T::class.simpleName}" ) }ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt (3)
10-20: Well-designed composable interface.The
fun interfacepattern enables clean SAM conversions, and theplusoperator provides intuitive composition semantics. The KDoc clearly explains the purpose.
22-33: LGTM!The composition logic correctly chains the transformed result through each processor (line 26 uses
current), and theplusoverride efficiently flattens nested mappings.
35-109: LGTM!The fast-path optimization avoids unnecessary object allocations, and the defaults logic correctly handles both
responses.defaultandresponses.responsesfor header content. The documentation accurately describes the applied defaults.ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt (1)
176-194:Components.isEmpty()/isNotEmpty()implementation looks solid.The helpers correctly consider all component maps and
extensionsin the emptiness check, andisNotEmpty()is just the negation, which keeps semantics clear and efficient.ktor-shared/ktor-openapi-schema/api/ktor-openapi-schema.api (1)
67-889: ABI surface for new OpenAPI/JsonSchema model is consistent with the Kotlin source and ready for ABI enforcement.The generated signatures for
Components,JsonSchema(and nested types),JsonSchemaInference,OpenApiInfo(+ nestedContact/License),OpenApiSpecification, and the various*Builderclasses all align correctly with the Kotlin declarations. Constructors match expected parameter counts, all major classes have proper$$serializerimplementations, andHeader$Builder/MediaType$Buildercorrectly implementJsonSchemaInference. Thetagsfield is now properly typed asListinstead ofSet. Ensure that./gradlew updateLegacyAbihas been run to generate the final ABI signatures, as per the learnings on binary compatibility enforcement in this codebase.
| private fun collectSchema(schema: JsonSchema): ReferenceOr<JsonSchema> { | ||
| return schemaToComponent(schema)?.let { ref -> | ||
| ReferenceOr.schema(ref) | ||
| } ?: ReferenceOr.value( | ||
| schema.copy( | ||
| allOf = schema.allOf?.map { it.mapToReference(::collectSchema) }, | ||
| anyOf = schema.anyOf?.map { it.mapToReference(::collectSchema) }, | ||
| oneOf = schema.oneOf?.map { it.mapToReference(::collectSchema) }, | ||
| not = schema.not?.mapToReference(::collectSchema), | ||
| properties = schema.properties?.mapValues { (_, value) -> value.mapToReference(::collectSchema) }, | ||
| items = schema.items?.mapToReference(::collectSchema), | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && fd -t f -n "*.kt" | xargs grep -l "class JsonSchema" 2>/dev/null | head -10Repository: ktorio/ktor
Length of output: 110
🏁 Script executed:
rg -n "(class|data class) JsonSchema" --type=kt -A 20Repository: ktorio/ktor
Length of output: 81
🏁 Script executed:
wc -l ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 167
🏁 Script executed:
rg "data class JsonSchema" --type=kt -A 30Repository: ktorio/ktor
Length of output: 81
🏁 Script executed:
rg "additionalProperties" --type=kt -B2 -A2Repository: ktorio/ktor
Length of output: 81
🏁 Script executed:
head -30 ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 895
🏁 Script executed:
git ls-files | grep -i "operationmapping"Repository: ktorio/ktor
Length of output: 163
🏁 Script executed:
git ls-files | grep -E "routing.*annotate" | head -20Repository: ktorio/ktor
Length of output: 846
🏁 Script executed:
git ls-files "*annotate*" | grep -i operationRepository: ktorio/ktor
Length of output: 163
🏁 Script executed:
rg "class JsonSchema" --type=ktRepository: ktorio/ktor
Length of output: 81
🏁 Script executed:
find . -type f -name "*.kt" | head -100 | xargs grep -l "JsonSchema" 2>/dev/nullRepository: ktorio/ktor
Length of output: 39
🏁 Script executed:
find . -name "OperationMapping.kt" -o -name "*JsonSchema*" 2>/dev/null | head -20Repository: ktorio/ktor
Length of output: 324
🏁 Script executed:
rg "data class JsonSchema\|class JsonSchema" --type=kt -B2 -A 15Repository: ktorio/ktor
Length of output: 81
🏁 Script executed:
git ls-files | grep -i "schema" | head -30Repository: ktorio/ktor
Length of output: 1749
🏁 Script executed:
git ls-files | head -50Repository: ktorio/ktor
Length of output: 1704
🏁 Script executed:
find . -type d -name "*ktor*" 2>/dev/null | head -10Repository: ktorio/ktor
Length of output: 420
🏁 Script executed:
find . -maxdepth 3 -type d | head -30Repository: ktorio/ktor
Length of output: 551
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "collectSchema" 2>/dev/nullRepository: ktorio/ktor
Length of output: 167
🏁 Script executed:
rg "fun collectSchema" --type=kt -B5 -A15Repository: ktorio/ktor
Length of output: 81
🏁 Script executed:
rg "collectSchema" --type=ktRepository: ktorio/ktor
Length of output: 81
🌐 Web query:
OpenAPI JsonSchema additionalProperties recursive collection
💡 Result:
Short answer: use $ref (in OpenAPI 3.1+ which embeds JSON Schema) and point additionalProperties at the recursive schema (or a $defs/$ref). Ensure your schema types are compatible (additionalProperties only applies to objects) and avoid accidental infinite containment by designing node shape (e.g. leaf vs node) or using oneOf/anyOf carefully.
Example (OpenAPI 3.1 / JSON Schema style):
components:
schemas:
Node:
type: object
properties:
value:
type: string
children:
type: object
additionalProperties:
$ref: '#/components/schemas/Node'
required: [value]
Notes:
- additionalProperties applies only when type is object (and defaults to true if omitted). (spec.openapis.org)
- When modelling maps/dictionaries, additionalProperties defines the value schema; use $ref to make it recursive. (docs.json-everything.net)
- Watch out for validation/serialization/tooling limits: some generators or validators restrict $ref usage (or require $ref to be the only keyword in a schema); test with your toolchain. (docs.json-everything.net)
If you want, tell me which OpenAPI version and toolchain you use and I’ll give a tailored schema and compatibility notes.
Citations:
- 1: https://spec.openapis.org/oas/3.1/schema/2025-02-13.html?utm_source=openai
- 2: https://docs.json-everything.net/schema/codegen/patterns/?utm_source=openai
- 3: https://docs.json-everything.net/schema/codegen/patterns/?utm_source=openai
🏁 Script executed:
rg "additionalProperties" -t kt -t javaRepository: ktorio/ktor
Length of output: 81
🏁 Script executed:
find . -name "*.kt" -type f 2>/dev/null | head -50Repository: ktorio/ktor
Length of output: 3974
🏁 Script executed:
cat -n ./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktRepository: ktorio/ktor
Length of output: 6058
🏁 Script executed:
sed -n '1,150p' ./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktRepository: ktorio/ktor
Length of output: 5169
🏁 Script executed:
grep -n "additionalProperties" ./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.ktRepository: ktorio/ktor
Length of output: 117
🏁 Script executed:
cat ./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt | head -200Repository: ktorio/ktor
Length of output: 5169
🏁 Script executed:
sed -n '140,175p' ./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 1078
🏁 Script executed:
wc -l ./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 169
🏁 Script executed:
python3 << 'EOF'
with open('./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines, 1):
print(f"{i:3d}: {line}", end='')
EOFRepository: ktorio/ktor
Length of output: 141
🏁 Script executed:
head -200 ./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 6547
🏁 Script executed:
# Check entire OperationMapping file structure
python3 << 'PYEOF'
try:
with open('./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt', 'r') as f:
content = f.read()
print(content)
except Exception as e:
print(f"Error: {e}")
PYEOFRepository: ktorio/ktor
Length of output: 141
🏁 Script executed:
python3 << 'PYEOF'
import os
files = [
'./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/JsonSchema.kt',
'./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.kt',
'./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt'
]
for filepath in files:
if os.path.exists(filepath):
print(f"\n{'='*60}\n{filepath}\n{'='*60}")
with open(filepath, 'r') as f:
print(f.read())
else:
print(f"File not found: {filepath}")
PYEOFRepository: ktorio/ktor
Length of output: 141
🏁 Script executed:
grep -n "fun.*mapToReference\|extension" ./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.ktRepository: ktorio/ktor
Length of output: 37
🏁 Script executed:
python3 << 'PYEOF'
with open('./ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt', 'r') as f:
lines = f.readlines()
# Find the mapToReference definition
for i, line in enumerate(lines):
if 'mapToReference' in line or 'private fun' in line or 'internal fun' in line:
start = max(0, i-2)
end = min(len(lines), i+10)
for j in range(start, end):
print(f"{j+1:3d}: {lines[j]}", end='')
print()
PYEOFRepository: ktorio/ktor
Length of output: 141
🏁 Script executed:
head -80 ./ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/AdditionalProperties.ktRepository: ktorio/ktor
Length of output: 2860
Handle additionalProperties schemas in recursive collection.
The collectSchema method does not process additionalProperties. While additionalProperties is of type AdditionalProperties (not ReferenceOr<JsonSchema>), it can contain a PSchema variant wrapping ReferenceOr<JsonSchema> that needs recursive schema collection. Nested schemas defined in additionalProperties won't be collected as component references.
When additionalProperties is PSchema, its schema should be recursively processed similar to how items and not are handled. This requires pattern matching on the sealed interface:
additionalProperties = schema.additionalProperties?.let { ap ->
when (ap) {
is AdditionalProperties.Allowed -> ap
is AdditionalProperties.PSchema -> AdditionalProperties.PSchema(ap.value.mapToReference(::collectSchema))
}
},
🤖 Prompt for AI Agents
In
ktor-server/ktor-server-plugins/ktor-server-routing-annotate/common/src/io/ktor/annotate/OperationMapping.kt
around lines 149 to 162, collectSchema currently ignores
schema.additionalProperties so nested schemas in AdditionalProperties.PSchema
are not recursively collected; update the returned schema copy to include
additionalProperties = schema.additionalProperties?.let { ap -> when (ap) { is
AdditionalProperties.Allowed -> ap; is AdditionalProperties.PSchema ->
AdditionalProperties.PSchema(ap.value.mapToReference(::collectSchema)) } } so
PSchema variants have their inner schema passed through
mapToReference(::collectSchema) just like items/not, preserving Allowed as-is.
| /** Additional external documentation. */ | ||
| public val externalDocs: ExternalDocs? = null, | ||
| /** Any additional external documentation for this OpenAPI document. */ | ||
| public override val extensions: Map<String, GenericElement>? = null, |
There was a problem hiding this comment.
Clarify KDoc for extensions (they're specification extensions, not documentation).
Both OpenApiSpecification.extensions (Line 53) and OpenApiInfo.extensions (Line 97) are documented as “additional external documentation”, but these fields actually carry specification extensions (x-* vendor fields).
Consider updating the comments to avoid confusion, e.g.:
- /** Any additional external documentation for this OpenAPI document. */
+ /** Specification extensions (`x-*` fields) for this OpenAPI document. */and similarly for OpenApiInfo.extensions (“for this Info object”).
Also applies to: 96-97
🤖 Prompt for AI Agents
In
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
around lines 50-53 and 96-97, the KDoc for the `extensions` properties
incorrectly describes them as “additional external documentation”; change the
comments to state these are OpenAPI specification/vendor extensions (x-*) rather
than documentation — e.g. “Specification extensions (vendor-specific x-* fields)
for this OpenAPI document.” and for the Info object: “Specification extensions
(vendor-specific x-* fields) for this Info object.” Ensure both KDoc comments
are updated accordingly.
| public data class Components( | ||
| public val schemas: Map<String, JsonSchema>? = null, | ||
| public val responses: Map<String, ReferenceOr<Response>>? = null, | ||
| public val parameters: Map<String, ReferenceOr<Parameter>>? = null, | ||
| public val examples: Map<String, ReferenceOr<ExampleObject>>? = null, | ||
| public val requestBodies: Map<String, ReferenceOr<RequestBody>>? = null, | ||
| public val headers: Map<String, ReferenceOr<Header>>? = null, | ||
| public val links: Map<String, ReferenceOr<Link>>? = null, | ||
| public val callbacks: Map<String, ReferenceOr<Callback>>? = null, | ||
| public val pathItems: Map<String, ReferenceOr<PathItem>>? = null, | ||
| public override val extensions: ExtensionProperties = null, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAPI 3.1.1 Components pathItems reference object specification
💡 Result:
Short answer
- In OpenAPI Specification v3.1.1 the Components Object supports a field named pathItems whose type is Map[string, Path Item Object]. Each entry is a reusable Path Item Object (the Path Item Object itself may use $ref). (spec.openapis.org)
Relevant spec excerpt (concept)
- components.pathItems: Map[string, Path Item Object] — an object to hold reusable Path Item Objects. (spec.openapis.org)
Minimal YAML example
components:
pathItems:
getStatus:
summary: "Get service status"
get:
responses:
'200':
description: "OK"
Notes
- Unlike many other component maps (responses, parameters, etc.) the components.pathItems map is declared as Path Item Objects; however Path Item Objects may include a $ref to external/other definitions per the Path Item Object rules. (spec.openapis.org)
Sources
- OpenAPI Specification v3.1.1 — Components Object; Path Item Object; Reference Object. [OpenAPI Spec]. (spec.openapis.org)
- Named Path Items (explanation / examples). (redocly.com)
Citations:
- 1: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 2: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 3: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 4: https://spec.openapis.org/oas/v3.1.1.html?utm_source=openai
- 5: https://redocly.com/learn/openapi/openapi-visual-reference/named-path-items?utm_source=openai
Fix Components.pathItems type to match OpenAPI 3.1.1 specification.
Components.pathItems should be Map<String, PathItem>? instead of Map<String, ReferenceOr<PathItem>>?. The OpenAPI 3.1.1 spec defines components.pathItems as Map[string, Path Item Object] without top-level Reference Objects, consistent with the schemas field. Reference Objects are handled within the PathItem itself via $ref, not at the map level.
public val pathItems: Map<String, PathItem>? = null,🤖 Prompt for AI Agents
In
ktor-shared/ktor-openapi-schema/common/src/io/ktor/openapi/OpenApiSpecification.kt
around lines 157 to 167, change the type of Components.pathItems from
Map<String, ReferenceOr<PathItem>>? to Map<String, PathItem>? to conform to
OpenAPI 3.1.1 (components.pathItems must be a map of Path Item objects, not
top-level ReferenceObjects); update the property declaration accordingly and
adjust any usages/serializers/tests that expect ReferenceOr<PathItem> to now
work with PathItem instances (remove or adapt reference handling at the map
level, keeping $ref support inside PathItem).
Subsystem
Server, Routing Annotations
Changes
Updates for: KTOR-8936 Routing documentation runtime API
These changes were required for completing the compiler plugin API work.
Also see current work for compiler plugin ktorio/ktor-build-plugins#253