Consider flattening allOf + 1 subschema as a way to add descriptions to properties

[apple]

Consider flattening allOf + 1 subschema as a way to add descriptions to properties

[liamnichols]

I agree with your proposal in the comment above @czechboy0 👍

From my time working on CreateAPI, I don’t think that I ever saw a reason to want to opt out of this behaviour.

This is particularly useful for making properties to reference schemas nullable too 🙏

[czechboy0]

Found that the k8s API is using this pattern all over the place, and not just for adding descriptions to properties, but also default fields, and I just realized that without this, deprecating individual properties will be difficult (as property values are plain old JSON schemas, not separate entities e.g. like a parameter, which has dedicated extra fields).

https://github.com/kubernetes/kubernetes/blob/v1.27.2/api/openapi-spec/v3/api__v1_openapi.json#L38

I'm starting to lean towards making flattening when N=1 the default behavior for single-child allOf's, due to how widespread this practice is, despite it being a workaround of a lacking feature in OpenAPI 3.0.x.

When considering the potential downsides of this approach, such as inconsistency with the 2+ allOfs, I see the upside of embracing the common practice as more valuable than to optimizing for the adopter who wants to evolve their allOf from N=1 to N=2+.

It all comes down to whether we believe that many OpenAPI authors evolve allOfs from having a single child schema to having 2+ child schemas. My hypothesis: no, I think most authors who put in an allOf with a single schema are doing it to work around the lack of description/default/deprecated/... on properties, rather than to represent any sort of composition at runtime. Looking at the k8s API schema above, all ~360 occurrences of allOf have N=1 schema, and a dedicated description.

This is different to e.g. OpenAPI docs with an allOf containing 2 child schemas, those I do believe that authors would reasonably want to go and add more child schemas, as the likely intent there is truly to get composition of multiple types.

To summarize my current proposal: always flatten for N=1 for allOf, don't even make it a configuration option, as I don't believe that anyone would complain about this and want to turn it off. If someone does come around needing an option, let's discuss then, but I wouldn't add a way to turn this off prematurely.

WDYT, folks?

[czechboy0]

Thinking about this a bit more, maybe we don't need this, here's an alternative idea:

as we embrace the design principles of this project outlined in Project scope and goals, we need to avoid bringing in functionality that doesn't have to be in the generator, and could be achieved by a tool that operates either on the source OpenAPI document, or on the generated Swift code.

In earlier comments above, I considered adding descriptions to object properties that are references as not covered by the default out of scope policy, because I was only thinking about OpenAPI 3.0.x, where it really is impossible, and the official recommendation as a workaround is to wrap the property in an allOf. Unfortunately, that leads to an extra property access, going from 1 to 2, which is less than ideal.

However, OpenAPI 3.1 does support adding descriptions to reference schemas, so the natural recommendation should be: use OpenAPI 3.1.

Originally, I wasn't sure how quickly we would be able to get to supporting 3.1, but based on recent discussions in #39, and OpenAPIKit now having alpha8, which allows us to migrate piece-meal, I think we should just prioritize supporting OpenAPI 3.1 as a way to resolve this issue without additional changes.

[czechboy0]

Blocking on OpenAPI 3.1 support for now, after that's done, we'll be in a better position to decide what to do with this.

[czechboy0]

Ok, when this PR lands, this will get closed - in OpenAPI 3.1, one can attach description (and other core properties, like deprecated) directly to a $ref schema as well, removing the need for this workaround.

Test here: e669276

[czechboy0]

Now that we support OpenAPI 3.1, descriptions on refs are supported. So the solution here is to migrate your document to 3.1, then descriptions will work.