3234: use markOptionsAsNullable for Option reference types #3235
Conversation
|
Could you maybe add a test? seems like a similar one |
|
generated schema
|
done :) |
| @@ -666,6 +666,20 @@ class VerifyYamlTest extends AnyFunSuite with Matchers { | |||
| actualYamlNoIndent shouldBe expectedYaml | |||
| } | |||
|
|
|||
| test("should mark optional class fields as nullable by inlining them when configured to do so") { | |||
| case class Bar(bar: Int) | |||
| case class ClassWithOptionClassField(optionalIntField: Option[Bar], requiredStringField: String) | |||
eanea
force-pushed
the
3234/fix_markOptionsAsNullable_for_reference_types
branch
from
209e1eb to
2b34395
Compare
| type: object | ||
| properties: | ||
| optionalObjField: | ||
| required: |
There was a problem hiding this comment.
by inlining the schema we end up with an unused Bar component. Maybe it would be better to generate a one of, e.g.:
ClassWithOptionClassField:
required:
- requiredStringField
type: object
properties:
optionalObjField:
oneOf:
- type: 'null'
- $ref: '#/components/schemas/Bar'
requiredStringField:
type: string
?
There was a problem hiding this comment.
it'll require changes in the sttp-apispec for all encoders.
I could do it for the circe integration only. will it be acceptable?
There was a problem hiding this comment.
- PR for
sttp-apispeccirce is ready support nullable for anyOf & oneOf sttp-apispec#118 - updated current PR to generete
oneOfinstead of inlining
eanea
force-pushed
the
3234/fix_markOptionsAsNullable_for_reference_types
branch
from
5a7a72e to
3a39268
Compare
| optionalClassField: | ||
| oneOf: | ||
| - $ref: '#/components/schemas/Bar' | ||
| - type: 'null' |
There was a problem hiding this comment.
- type: 'null' requires to merge & use softwaremill/sttp-apispec#118
| val ref = toSchemaReference.map(nested, name) | ||
| if (!markOptionsAsNullable) ref | ||
| else | ||
| ASchema.oneOf(List(ref), ref.discriminator).copy(nullable = Some(true)) |
There was a problem hiding this comment.
maybe doing here simply ASchema.oneOf(List(ref, ASchema(SchemaType.Null), None) would work?
There was a problem hiding this comment.
I've added this change, let's see if things work :)
There was a problem hiding this comment.
great! thank you :)
what else is needed to merge the PR?
fixes #3234
markOptionsAsNullable = true