Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support nullable ref in openapi schema #2901

Closed
wants to merge 1 commit into from

Conversation

cornerman
Copy link
Contributor

I was missing support for nullable refs in the openapi schema. That is usually some optional case classes in the json schema.

It is inspired by this stack overflow response: https://stackoverflow.com/a/48114924
See also: OAI/OpenAPI-Specification#1368

Related: #1147

@kciesielski
Copy link
Member

@cornerman Thanks for this proposition! I have just added an utility for convenient conversion between Tapir schemas and JSON schemas. It uses TSchemaToASchema underneath and I tried your change with it. It seems that the check you've implemented won't actually be applied to optional case class fields.
Could you add a test? For example in TapirSchemaToJsonSchemaTest.scala (please make sure you have the latest master).
Something simple like

    // given
    case class Parent(innerChildField: Option[Child])
    case class Child(childId: String)
    val tSchema = implicitly[Schema[Parent]]

    // when
    val result: ASchema = TapirSchemaToJsonSchema(tSchema, markOptionsAsNullable = true).value

    // then
    result.asJson.deepDropNullValues shouldBe json"""{}""" // expected json goes here

  }

The problem may be caused by nullable never being true, because pattern matching actually falls into this case:

case TSchemaType.SOption(nested @ TSchema(_, Some(name), _, _, _, _, _, _, _, _, _)) =>

@cornerman
Copy link
Contributor Author

This is related to #3331.

Closing this PR, because the code base has changed and the proposed change here did not really work.

@cornerman cornerman closed this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants