Support schema reference description overrides #299
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #298.
Implementation
This PR replaces the
JSONSchema
reference
case'sReferenceContext
with aCoreContext
(only for theOpenAPIKit
module, not theOpenAPIKit30
module) and it uses that core context'sdescription
to override thedescription
within a referenced schema when calling thedereferenced()
ordereferenced(in:)
methods onJSONSchema
. This carries through, naturally, to dereferencing performed on parent values all the way up to the execution oflocallyDereferenced()
onOpenAPI.Document
.There is also a new
openAPIReference(withDescription description: String? = nil)
method onJSONReference
to make it easy to elevate aJSONReference
to anOpenAPI.Reference
. OpenAPIKit does not use this internally at the moment, but turning aJSONReference
into anOpenAPI.Reference
is a really nice way to carry the "override the referenced description" around so that whenever the reference is looked up the description is overridden without further effort.Alternatives Considered
It was briefly considered to store an
OpenAPI.Reference
on aJSONSchema
directly in place of the currentJSONReference
. This would take care of packaging up thedescription
(if present) when parsing which would be the easiest solution from the perspective of downstream implementations. I decided against this implementation both because it is a larger breaking change and, at least as importantly, becauseOpenAPI.Reference
is not the same thing as aJSONReference
with an overridden description; it also parsessummary
and generally holds different semantics. It is elegant that the newest OpenAPI specification completely embeds the JSON Schema specification and can therefore say "anything in the schema is specified in this other specification." I wanted to embrace that here as well which means keeping "OpenAPI" things out of theJSONSchema
core representation.It was also considered to add
description
to theJSONReference
type. This was decided against because currently theJSONReference
type is only the$ref
part of a schema and given that JSON References are more broadly understood even than in the context of the JSON Schema specification, it is nice to keep reference stuff inJSONReference
and non reference stuff (like the behavior of overridingdescription
) outside ofJSONReference
. Adding more properties toJSONReference
also would have been a more disruptive change from a maintenance perspective.Lastly, it is more correct to allow for all of the
CoreContext
fields alongside schema references even though the built-in dereferencing logic does not incorporate all of the fields at this time. In fact, JSON Schema now allows even more than the core context fields and it has now gained a caveat in the specification that it is not even always possible to dereference documents and get reasonable results because of the full allowance of keywords alongside$ref
. OpenAPIKit will need to make some decisions in the future about how to operate in that reality.