Consider preserving referential transparency of OpenAPI schemas #888

harpocrates · 2021-08-17T12:34:30Z

Using withExample, withTitle, and withDescription combinators on a named OpenAPI schema mutate the schema. I expect that they would just make a new modified copy of the schema, not modify it! Motivating example:

import endpoints4s.openapi.model.{OpenApi, Info}
import endpoints4s.Encoder
import java.nio.file._

final case class Foo(name: String)

object Example
  extends endpoints4s.algebra.Endpoints
    with endpoints4s.algebra.JsonEntitiesFromSchemas
    with endpoints4s.generic.JsonSchemas
    with endpoints4s.openapi.Endpoints
    with endpoints4s.openapi.JsonEntitiesFromSchemas {

  implicit lazy val fooSchema: JsonSchema[Foo] = genericJsonSchema[Foo]

  // example for this endpoint should be `{ "name": "foo 1" }`
  lazy val getFoo1: Endpoint[Unit, Foo] =
    endpoint(
      request = get(path / "foo1"),
      response = ok(jsonResponse[Foo](fooSchema.withExample(Foo("foo 1"))))
    )

  // example for this endpoint should be `{ "name": "foo 2" }`
  lazy val getFoo2: Endpoint[Unit, Foo] =
    endpoint(
      request = get(path / "foo2"),
      response = ok(jsonResponse[Foo](fooSchema.withExample(Foo("foo 2"))))
    )

  lazy val schema = implicitly[Encoder[OpenApi, String]]
  lazy val api = openApi(Info(title = "API", version = "0.0.1"))(getFoo1, getFoo2)
}

Files.write(Paths.get("doc.json"), Example.schema.encode(Example.api).getBytes("UTF-8"))

However, in the output OpenAPI JSON, the output schema for both endpoints is identical

                "schema": {
                  "$ref": "#/components/schemas/.line5..read..iw..iw..iw..iw.Foo"
                }

The example that gets picked up is just whichever one was "seen" first in the schema traversal. In this case, it is "foo 2":

      ".line5..read..iw..iw..iw..iw.Foo": {
        "type": "object",
        "properties": {
          "name": {
            "type": "string"
          }
        },
        "required": [
          "name"
        ],
        "example": {
          "name": "foo 2"
        }
      }

If you agree this is an issue, I can put together a fix. This is fixable now that we have #885 merged - the two request schemas could be

                "schema": {
                  "allOf": [ { "$ref": "#/components/schemas/.line5..read..iw..iw..iw..iw.Foo" } ],
                  "example": "foo 1"
                }

and

                "schema": {
                  "allOf": [ { "$ref": "#/components/schemas/.line5..read..iw..iw..iw..iw.Foo" } ],
                  "example": "foo 2"
                }

The text was updated successfully, but these errors were encountered:

julienrf · 2021-08-17T13:54:56Z

Is this behavior present only on master or also in the existing releases?

harpocrates · 2021-08-17T13:56:55Z

Definitely existing releases. It isn't a recent regression though - it has been around since withTitle and withDescription, I think. Unless you get to it sooner, I aim to start trying to fix this in a couple days.

julienrf · 2021-08-17T13:58:30Z

Your help is definitively welcome!

harpocrates · 2021-08-22T15:57:35Z

It is looking like I won't get to this for at least another week. If someone else wants to pick it up, please feel free to do so. 😄

agboom · 2022-03-02T21:34:24Z

I was able to reproduce this bug in ReferencedSchemaTest.scala by expanding the test fixture a little and adding request body examples to two different endpoints.

I think I also found a fix for it, but I have to admit that OpenAPI schemas are fairly new to me. I'll push some results soon so that you can take a look at it.

- I.e. description, example, title and default - Fixes endpoints4s#888

harpocrates · 2022-03-24T15:37:17Z

I've been thinking about API evolution around this issue, and am coming to realize that fixing this will probably inadvertently break some existing uses. In particular, I'm thinking about:

calling withExample on a generically derived schema will no longer cause the example to be added to the named schema (since withExample is called after named, and if we want referential transparency, withExample shouldn't mutate the schema that was named).

calling withExample outside a lazyRecord/lazyTagged means the example won't be attached to the named schema anymore. In order to preserve the existing behaviour, users will need to move the withExample and co. into the lazy argument.

case class Recursive(next: Option[Recursive])

val recursiveSchemaOld: Record[Recursive] =
  lazyRecord("Rec")(
    optField("next")(recursiveSchema)
  ).xmap(Recursive(_))(_.next)
    .withDescription("Rec description")
    .withTitle("Rec title")
    .withExample(Recursive(None))

val recursiveSchemaNew: Record[Recursive] =
  lazyRecord("Rec") {
    optField("next")(recursiveSchema)
      .xmap(Recursive(_))(_.next)
      .withDescription("Rec description")
      .withTitle("Rec title")
      .withExample(Recursive(None))
  }

Now, more on the implementation side of things and still taking the Recursive type above as an example, what does it mean to override the description on a recursive schema? I think it means we'd end up with a top-level schema with a description, but then the schema of next should be the regular named schema "Rec" and shouldn't have the description. Adding examples, descriptions, titles, etc. is really overriding parts of the schema, which got me looking at https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/ again. I propose we handle withExample on an already named schema using allOf:

{
  "allOf": [  { "$ref": "#/Rec" } ]
  "description": "Rec description"
  "title": "Rec title"
  "example": { }
}

In a nutshell: - adds a new `DocumentedRecord` subclass for representing overrides of examples, descriptions, etc. on named schemas - renders that subclass using `allOf` with the base schema Note: this is a prototype - it doesn't solve referential transparency for names anywhere else (and it is likely that doing so would lead to more subclasses for the other documented JSON schema types). See endpoints4s#888

harpocrates · 2022-03-24T16:56:18Z

@julienrf more concrete example of how I ended up needing to tweak tests such that the expected output didn't change is in this commit - see in particular the tweaks to how the fixture schemas are created: harpocrates@ef9aaa7. I think this sort of change will be unavoidable if we want referential transparency. 😟

julienrf added the bug label Aug 17, 2021

agboom added a commit to agboom/endpoints4s that referenced this issue Mar 7, 2022

OpenAPI: support sibling values for reference schemas

0073d09

- I.e. description, example, title and default - Fixes endpoints4s#888

agboom linked a pull request Mar 7, 2022 that will close this issue

OpenAPI: support sibling values for reference schemas #1020

Open

agboom added a commit to agboom/endpoints4s that referenced this issue Mar 7, 2022

OpenAPI: support sibling values for reference schemas

90f27e3

- I.e. description, example, title and default - Fixes endpoints4s#888

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider preserving referential transparency of OpenAPI schemas #888

Consider preserving referential transparency of OpenAPI schemas #888

harpocrates commented Aug 17, 2021 •

edited

julienrf commented Aug 17, 2021

harpocrates commented Aug 17, 2021

julienrf commented Aug 17, 2021

harpocrates commented Aug 22, 2021

agboom commented Mar 2, 2022

harpocrates commented Mar 24, 2022

harpocrates commented Mar 24, 2022

Consider preserving referential transparency of OpenAPI schemas #888

Consider preserving referential transparency of OpenAPI schemas #888

Comments

harpocrates commented Aug 17, 2021 • edited

julienrf commented Aug 17, 2021

harpocrates commented Aug 17, 2021

julienrf commented Aug 17, 2021

harpocrates commented Aug 22, 2021

agboom commented Mar 2, 2022

harpocrates commented Mar 24, 2022

harpocrates commented Mar 24, 2022

harpocrates commented Aug 17, 2021 •

edited