Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (#1498) #2470

987Nabil · 2023-10-05T07:24:50Z

Todos before this is no longer draft:

Add more tests
More methods to call the OpenAPI generator that allows to hand over more meta infos and config params
tidy up the code

fixes #1498
/claim #1498

jdegoes · 2023-10-06T11:37:29Z

@987Nabil Can you fix build failures? 🙏

codecov-commenter · 2023-10-13T23:37:31Z

Codecov Report

Attention: 431 lines in your changes are missing coverage. Please review.

Comparison is base (88730b0) 64.95% compared to head (7852bc8) 64.27%.
Report is 19 commits behind head on main.

Files	Patch %	Lines
...n/scala/zio/http/endpoint/openapi/JsonSchema.scala	44.70%	214 Missing ⚠️
...main/scala/zio/http/endpoint/openapi/OpenAPI.scala	40.67%	105 Missing ⚠️
...n/scala/zio/http/endpoint/openapi/OpenAPIGen.scala	76.80%	87 Missing ⚠️
...p/src/main/scala/zio/http/codec/SegmentCodec.scala	52.17%	11 Missing ⚠️
...http/src/main/scala/zio/http/codec/PathCodec.scala	65.21%	8 Missing ⚠️
zio-http/src/main/scala/zio/http/codec/Doc.scala	66.66%	2 Missing ⚠️
...http/src/main/scala/zio/http/codec/HttpCodec.scala	50.00%	2 Missing ⚠️
zio-http/src/main/scala/zio/http/Status.scala	0.00%	1 Missing ⚠️
...tp/src/main/scala/zio/http/endpoint/Endpoint.scala	66.66%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2470      +/-   ##
==========================================
- Coverage   64.95%   64.27%   -0.69%     
==========================================
  Files         136      138       +2     
  Lines        7197     7977     +780     
  Branches     1296     1483     +187     
==========================================
+ Hits         4675     5127     +452     
- Misses       2522     2850     +328

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

987Nabil · 2023-10-15T17:26:16Z

Scala 3 build is failing due to a bug in the Schema derivation macro. I'll check this. There is already a known bug that might be related.

* Add a test of a middleware providing a context to a `Routes` * Add a test of a middleware providing a context to a `Routes` * scalafmt * scalafmt

* Update sbt-github-actions to 0.18.0 * Regenerate GitHub Actions workflow Executed command: sbt githubWorkflowGenerate

guersam · 2023-10-31T03:30:41Z

zio-http/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala

+      },
+    )
+
+  def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =


Would it be beneficial to support new zio.schema.annotation.description here like #2499 ?

I'd leave this PR as is and add it later.

runtologist · 2023-11-07T14:56:30Z

zio-http/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala

+  def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =
+    schema match {
+      case enum0: Schema.Enum[_] if refType != SchemaStyle.Inline && nominal(enum0).isDefined     =>
+        JsonSchema.RefSchema(nominal(enum0, refType).get)
+      case enum0: Schema.Enum[_]                                                                  =>
+        JsonSchema.Enum(enum0.cases.map(c => EnumValue.Str(c.id)))


ZIO Schema expresses ADTs as Enums, not just lists of possible values. If one of enum0 cases is not a literal / case object, then this needs to be translated into oneOf.

The id of the case should respect any caseName annotations on the schema.

Suggested change

def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =

schema match {

case enum0: Schema.Enum[_] if refType != SchemaStyle.Inline && nominal(enum0).isDefined =>

JsonSchema.RefSchema(nominal(enum0, refType).get)

case enum0: Schema.Enum[_] =>

JsonSchema.Enum(enum0.cases.map(c => EnumValue.Str(c.id)))

private def allPrimitive(enum: Schema.Enum[_]): Boolean =

enum.cases.forall(case_ => allPrimitive_(case_.schema))

@tailrec

private def allPrimitive_(schema: Schema[_]): Boolean =

schema match {

case _: Schema.Primitive[_] => true

case _: Schema.CaseClass0[_] => true

case Schema.Transform(schema, _, _, _, _) => allPrimitive_(schema)

case _ => false

}

private def enumCaseId(schema: Schema.Case[_, _]): String =

schema.annotations.collectFirst { case caseName(name) => name }.getOrElse(schema.id)

def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =

schema match {

case enum0: Schema.Enum[_] if refType != SchemaStyle.Inline && nominal(enum0).isDefined =>

JsonSchema.RefSchema(nominal(enum0, refType).get)

case enum0: Schema.Enum[_] if allPrimitive(enum0) =>

JsonSchema.Enum(enum0.cases.map { c => EnumValue.Str(enumCaseId(c)) })

case enum0: Schema.Enum[_] =>

JsonSchema

.OneOfSchema(enum0.cases.flatMap { case_ =>

case_.schema match {

case Schema.Lazy(schema0) => ; None

case schema => Some(fromZSchema(schema, refType))

}

})

.deprecated(deprecated(enum0))

// .discriminator(enum0.annotations.collectFirst { case discriminatorName(discriminator) =>

// OpenAPI.Discriminator(discriminator)

// })

You are right. I remember I had this on my list, but must have missed it. Also I missed a test for this. Idk if I will take your your code, but I'll tackle this.

@runtologist Can you tell me your reasoning behind case _: Schema.Primitive[_] => true ?
I think this should be false, since primitive schemas still accept a set of values and a fixed value.

I think you are right.

runtologist · 2023-11-07T14:58:04Z

zio-http/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala

+            field.name ->
+              fromZSchema(field.schema, refType).deprecated(deprecated(field.schema))
+          })
+          .required(record.fields.filterNot(_.schema.isInstanceOf[Schema.Optional[_]]).map(_.name))


Fields annotated as optional are still modeled as required in the openapi.

True. I missed that one. Also transient fields should actually be filtered out. Thanks.

runtologist · 2023-11-07T15:02:14Z

zio-http/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala

+      },
+    )
+
+  def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =


IMO fromZSchema should be able to accumulate component schemas. For example oneOf, allOf cases should be named. If they are not named, this prohibits proper use of discriminators. Also schemas that are used in multiple places should not be expanded multiple times.

One possible approach would be to allow fromZSchema to return multiple JsonSchemas, but I'm sure there are other approaches.

The case of expanding multiple times was intentionally ignored. As it does not change meaning, but is just an optimization. And also right now, we build schemas per endpoint and then accumulate. In this case, we can't be very efficient, since we don't know all Endpoints. Also I am not sure how efficient this needs to be, since I don't expect that building OpenAPI is happening more than once per application.
Actually, we could probably generate it at compile time 🤔
Your argument with the naming I don't get. Could you please give me more details? Or an example?

I am referring to this section https://swagger.io/specification/#discriminator-object, where I am not sure the example for oneOf with discriminator can be modelled without naming cases. One can probably get pretty far without, but I have seen it in the past.

My concern with duplication is not about efficiency, but ergonomics. Usually the oeverall openapi spec is not for a single endpoint, but for a more complex API, where lots of types appear in multiple places. A human reader of the API will need to read a very long documents with lots of repetitions. A client that is generated from the spec will have basically the same datatype repeated lots of times under different names. If everything was expanded, such types could be generated exactly once.

I think I get what you mean. You want the schemas not to be inlined, but part of components, so you can use a discriminator to reference the schema. I'll impl. this

Exactly. Wonderful. :)

@runtologist Thanks for the helpful review. Maybe you could check the changes 🙂

Sure. This is really a lot better! Using discriminator + references works really well. I love it. There are a few things I've noticed:

After the change caseNames on enums are not applied any more. All enum values are the name of the class/object.

Optional fields and fields with defaults are no longer listed as required, but their type is

type: - OriginalType - 'null'

AFAIU it should just be OriginalType.

Enums are still inlined instead of referenced.

Element types of arrays are still inlined instead of referenced.

Enums are still inlined instead of referenced

Element types of arrays are still inlined instead of referenced.

In the test I see something else. Maybe matter of what SchemaStyle is used. Do you have an example?

AFAIU it should just be OriginalType.

I am not so sure about that. I'll try to read specs again and think about what implications that has. Anyhow I do not see this as a show stopper, since it is not wrong. Option[Value] in json is null | Value.

After the change caseNames on enums are not applied any more. All enum values are the name of the class/object.

I'll check this

OpenAPI gen support for default values, optional and transient fields

runtologist · 2023-11-27T19:42:24Z

zio-http/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala

+      JsonSchemas(
+        JsonSchema.ArrayType(Some(nested.root)),
+        ref,
+        nested.children + (nested.rootRef.get -> nested.root),


This crashes with a None.get for me.

Suggested change

nested.children + (nested.rootRef.get -> nested.root),

nested.children ++ nested.rootRef.map(_ -> nested.root),

Can you give me a reproducer?

jdegoes · 2023-11-29T09:27:18Z

@987Nabil Please submit followups in a separate PR. Thanks for all your hard work on this PR!

algora-pbc bot mentioned this pull request Oct 5, 2023

Generate OpenAPI documentation from EndpointSpec data type #1498

Closed

algora-pbc bot added the 🙋 Bounty claim label Oct 5, 2023

987Nabil force-pushed the generate-open-api branch 2 times, most recently from 298ef04 to 3d8d743 Compare October 6, 2023 04:36

987Nabil force-pushed the generate-open-api branch 3 times, most recently from 0198828 to 8c2f923 Compare October 10, 2023 14:06

987Nabil added 3 commits October 14, 2023 01:13

Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (zio#1498)

2e04c85

Minimize schema for optional fields

c02a89b

Integrate main changes

690ad90

987Nabil force-pushed the generate-open-api branch from 8fb4437 to 690ad90 Compare October 13, 2023 23:26

Fix Scala 3 build

f8581d1

987Nabil force-pushed the generate-open-api branch 5 times, most recently from ba87eb1 to f0820eb Compare October 14, 2023 20:05

Fix exhaustive matching

c4c8ecc

987Nabil force-pushed the generate-open-api branch from f0820eb to c4c8ecc Compare October 14, 2023 23:45

987Nabil added 3 commits October 21, 2023 15:35

More OpenAPI generation tests

0143477

Use latest zio-schema snapshot for Scala 3 macro derivation fix

a5f1e4d

Formatting

63de2ea

987Nabil force-pushed the generate-open-api branch from d83981d to 63de2ea Compare October 21, 2023 20:34

OpenAPI tests now compare json ASTs, to avoid string render differences

426177b

987Nabil marked this pull request as ready for review October 22, 2023 00:22

987Nabil requested review from amitksingh1490 and jdegoes as code owners October 22, 2023 00:22

987Nabil requested review from vigoo and adamgfraser as code owners October 22, 2023 00:22

987Nabil and others added 7 commits October 24, 2023 05:34

Refactoring

3ab3e84

improve docs (zio#2482)

f9928ba

Add a test of a middleware providing a context to a Routes (zio#2487)

bcbadc7

* Add a test of a middleware providing a context to a `Routes` * Add a test of a middleware providing a context to a `Routes` * scalafmt * scalafmt

Remove usage of deprecated method in build.sbt (zio#2486)

6be820b

Update sbt-github-actions to 0.18.0 (zio#2484)

7dc301b

* Update sbt-github-actions to 0.18.0 * Regenerate GitHub Actions workflow Executed command: sbt githubWorkflowGenerate

Update netty-codec-http, ... to 4.1.100.Final (zio#2485)

9c99b95

Generate readme

0eba475

987Nabil mentioned this pull request Oct 24, 2023

Create Middleware to provide SwaggerUI OpenAPI endpoint #2494

Closed

Merge branch 'main' into generate-open-api

9f2555c

guersam reviewed Oct 31, 2023

View reviewed changes

Merge branch 'main' into generate-open-api

6e00a81

987Nabil mentioned this pull request Nov 7, 2023

Implement code generator that takes OpenAPI and generates zio.http.api.EndpointSpec code #1520

Closed

runtologist reviewed Nov 7, 2023

View reviewed changes

OpenAPI gen support for all kinds of enums with(out) discriminators

7852bc8

OpenAPI gen support for default values, optional and transient fields

987Nabil force-pushed the generate-open-api branch from be0e3de to 7852bc8 Compare November 27, 2023 06:14

runtologist reviewed Nov 27, 2023

View reviewed changes

jdegoes approved these changes Nov 29, 2023

View reviewed changes

jdegoes merged commit 58e0594 into zio:main Nov 29, 2023
14 checks passed

guersam mentioned this pull request Dec 8, 2023

Enhancement of JSON Schema generation #2549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (#1498) #2470

Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (#1498) #2470

987Nabil commented Oct 5, 2023 •

edited

Loading

jdegoes commented Oct 6, 2023

codecov-commenter commented Oct 13, 2023 •

edited

Loading

987Nabil commented Oct 15, 2023

guersam Oct 31, 2023

987Nabil Oct 31, 2023

runtologist Nov 7, 2023

987Nabil Nov 7, 2023

987Nabil Nov 25, 2023

runtologist Nov 28, 2023

runtologist Nov 7, 2023

987Nabil Nov 7, 2023

runtologist Nov 7, 2023

987Nabil Nov 7, 2023 •

edited

Loading

runtologist Nov 8, 2023

987Nabil Nov 8, 2023

runtologist Nov 8, 2023

987Nabil Nov 27, 2023

runtologist Nov 27, 2023

987Nabil Nov 28, 2023

runtologist Nov 27, 2023

987Nabil Nov 28, 2023

jdegoes commented Nov 29, 2023

-  def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =
-    schema match {
-      case enum0: Schema.Enum[_] if refType != SchemaStyle.Inline && nominal(enum0).isDefined     =>
-        JsonSchema.RefSchema(nominal(enum0, refType).get)
-      case enum0: Schema.Enum[_]                                                                  =>
-        JsonSchema.Enum(enum0.cases.map(c => EnumValue.Str(c.id)))
+    private def allPrimitive(enum: Schema.Enum[_]): Boolean =
+    enum.cases.forall(case_ => allPrimitive_(case_.schema))
+  @tailrec
+  private def allPrimitive_(schema: Schema[_]): Boolean =
+    schema match {
+      case _: Schema.Primitive[_]               => true
+      case _: Schema.CaseClass0[_]              => true
+      case Schema.Transform(schema, _, _, _, _) => allPrimitive_(schema)
+      case _                                    => false
+    }
+  private def enumCaseId(schema: Schema.Case[_, _]): String =
+    schema.annotations.collectFirst { case caseName(name) => name }.getOrElse(schema.id)
+  def fromZSchema(schema: Schema[_], refType: SchemaStyle = SchemaStyle.Inline): JsonSchema =
+    schema match {
+      case enum0: Schema.Enum[_] if refType != SchemaStyle.Inline && nominal(enum0).isDefined =>
+        JsonSchema.RefSchema(nominal(enum0, refType).get)
+      case enum0: Schema.Enum[_] if allPrimitive(enum0)                                       =>
+        JsonSchema.Enum(enum0.cases.map { c => EnumValue.Str(enumCaseId(c)) })
+      case enum0: Schema.Enum[_]                                                              =>
+        JsonSchema
+          .OneOfSchema(enum0.cases.flatMap { case_ =>
+            case_.schema match {
+              case Schema.Lazy(schema0) => ; None
+              case schema               => Some(fromZSchema(schema, refType))
+            }
+          })
+          .deprecated(deprecated(enum0))
+          // .discriminator(enum0.annotations.collectFirst { case discriminatorName(discriminator) =>
+          //   OpenAPI.Discriminator(discriminator)
+          // })

	nested.children + (nested.rootRef.get -> nested.root),
	nested.children ++ nested.rootRef.map(_ -> nested.root),

Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (#1498) #2470

Improve OpenAPI model; Add OpenAPI generator for EndpointAPI (#1498) #2470

Conversation

987Nabil commented Oct 5, 2023 • edited Loading

jdegoes commented Oct 6, 2023

codecov-commenter commented Oct 13, 2023 • edited Loading

Codecov Report

987Nabil commented Oct 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

987Nabil Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdegoes commented Nov 29, 2023

987Nabil commented Oct 5, 2023 •

edited

Loading

codecov-commenter commented Oct 13, 2023 •

edited

Loading

987Nabil Nov 7, 2023 •

edited

Loading