sttp-apispec 0.9.0 #3677
sttp-apispec 0.9.0 #3677
Conversation
docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TSchemaToASchema.scala
Outdated
Show resolved
Hide resolved
ghik
changed the title
| properties: | ||
| optionalObjField: | ||
| oneOf: | ||
| anyOf: |
There was a problem hiding this comment.
is this the same? oneOf vs anyOf?
There was a problem hiding this comment.
Not the same, but I believe anyOf is the right one to use here.
oneOf requires that a value matches exactly one of the listed schemas, which I think is needlessly strict and may be problematic if there's an overlap between values accepted by both schemas (e.g. the referred schema is already nullable).
There was a problem hiding this comment.
So this impacts only optional object schemas? We might consider this, but I think I would move this to a separate PR, separating the "update dependency" and "change the way tapir works" changes
There was a problem hiding this comment.
This was already done in sttp-apispec changes (the Schema.nullable method which replaced the nullable keyword).
There was a problem hiding this comment.
Hm maybe we need to fix this there then. If I have an Option[Bar], then the value must be either null or a Bar. It's a sealed type, after all.
There was a problem hiding this comment.
Yeah, it's possible that we might make a better choice there as well. But in the end, apart from sticking to the specification, it would be good to see what people actually use, so that the generated specs are understandable and useful :)
There was a problem hiding this comment.
it would be good to see what people actually use
It's not clear to me what you mean here. We are currently generating schemas which are technically incorrect. Since nobody ever complained about this, it probably doesn't make any difference for the ways that OpenAPIs are used in the wild, i.e. for generating docs and code stubs.
But that's also why I think it shouldn't hurt anybody when we change oneOf to anyOf - the tools most likely treat these in the same way.
On the other hand, this change may be more impactful for implementing things like #3645, and it would probably take off some mental load from me.
There was a problem hiding this comment.
I mean that if everybody uses oneOf in their schemas to represent hierarchies like this, it should also be an indication for us to follow suit. Specifications are great, if they are respected, otherwise you only have a living standard to follow.
There was a problem hiding this comment.
I believe that most of the time the schemas are actually disjoint (e.g. they have a discriminator field). In that case, it's completely fine and natural to use oneOf, and this is probably what "the world" does in most cases. I'm only suggesting a change for non-discriminated unions, which should be rare on its own.
I'll try to do some more research on that, though, to see how other similar frameworks handle this.
There was a problem hiding this comment.
Ok, thanks - we can try, and see how much of an impact this has (starting with our tests)
Integrating changes from softwaremill/sttp-apispec#152