OpenAPI 3 #219

maksbotan · 2020-06-23T19:56:32Z

Hi,

I've started porting swagger2 to openapi3, I will track my progress here.

I am following the spec at https://swagger.io/specification/, version 3.0.3.

Currently it works for simple cases:

λ> (decs, resp) = runDeclare (declareResponse "application/json" (Proxy @Day)) mempty
λ> resps = mempty & responses.at 200 ?~ Inline resp :: Responses
λ> op = mempty & responses .~ resps :: Operation
λ> swag = mempty & components.schemas <>~ decs & paths.at "/day" ?~ pathItem :: Swagger
λ> BSL.putStrLn $ encode swag

{"openapi":"3.0.3","info":{"version":"","title":""},"paths":{"/day":{"get":{"responses":{"200":{"content":{"application/json":{"schema":{"$ref":"#/components/schemas/Day"}}},"description":""}}}}},"components":{"schemas":{"Day":{"example":"2016-07-22","format":"date","type":"string"}}}}

And it renders as expected on Swagger Editor:

Of course, servant-swagger will have to be updated as well... See haskell-servant/servant-swagger#99

Fixes #105.

maksbotan · 2020-06-26T14:27:01Z

So, test suite passes, doctests not yet, I will fix them later.

OpenAPI 3 pet store example roundtrips without losses, so that's something.

Of course, Generic generators should be updated to use new features like oneOf.

Any preliminary thoughts?

sir4ur0n · 2020-06-26T14:29:22Z

I am not qualified to review this PR, but I just wanted to let you know this work on OpenAPI is valuable (I understand this is a first step before adding support for sum types oneOf; a good step nonetheless) and I thank you/appreciate your work on this PR 🙇

maksbotan · 2020-07-04T15:52:34Z

Update: I've implemented schema generation for (all) sum types, using SumEncoding configuration from aeson.

Example using Light type from tests:

data Light
  = NoLight
  | LightFreq Double
  | LightColor Color
  | LightWaveLength { waveLength :: Double }
  deriving (Generic)

Light:
  type: object
  oneOf:
    - required:
        - tag
        - contents
      type: object
      properties:
        tag:
          type: string
          enum:
            - NoLight
        contents:
          example: []
          items: {}
          maxItems: 0
          type: array
    - required:
        - tag
        - contents
      type: object
      properties:
        tag:
          type: string
          enum:
            - LightFreq
        contents:
          format: double
          type: number
    - required:
        - tag
        - contents
      type: object
      properties:
        tag:
          type: string
          enum:
            - LightColor
        contents:
          $ref: '#/components/schemas/Color'
    - required:
        - waveLength
        - tag
      type: object
      properties:
        tag:
          type: string
          enum:
            - LightWaveLength
        waveLength:
          format: double
          type: number

Validation tests pass.

There are some TODOs remaining thoigh.

maksbotan · 2020-07-04T16:38:19Z

Yay, CI passes!

sir4ur0n · 2020-07-05T21:23:49Z

Hey, just wanted to let you know: as soon as you feel like it works (even if not polished/documented/etc. yet), feel free to ping me, and we will give it a try on my project. We have several sum types, which we currently can't display in Swagger, so we would love to modestly contribute by beta-testing this and provide feedback (or contribute on what we understand, e.g. the documentation).

Cheers and good luck!

maksbotan · 2020-07-06T08:08:12Z

Hi @sir4ur0n.

This mostly works now, I'm going to add more tests for new functionality (sum types) and recheck everything by the spec.

However, keep in mind that OpenAPI 3 changed a lot of things, so a migration is certainly needed. And if you are using servant-swagger: it won't work with this PR because of those changes.

But anyway, you are more than welcome to try this code :)

sir4ur0n · 2020-07-06T08:19:39Z

However, keep in mind that OpenAPI 3 changed a lot of things, so a migration is certainly needed. And if you are using servant-swagger: it won't work with this PR because of those changes.

This is exactly the piece I didn't have in mind 😅

I'm not familiar with the boundaries between this library and servant-swagger: will compilation break? Or runtime? Maybe adapting servant-swagger wouldn't be such a big deal?

It would be much smoother if we can test with servant-swagger, but I guess we could give it a manual shot by displaying the JSON in another UI 🤔

maksbotan · 2020-07-06T08:25:24Z

Sadly, compilation most likely will break. You can try though :)

Adapting servant-swagger is on my list, I'll go to it after this PR is in ready state.

OpenAPI 3.0 unified schemas for parameters, now Parameter object directly references Schema object, and ParamSchema becomes not needed. Moreover, encoding of arrays and objects in parameters is now more uniform, using `style` property on the parameter itself.

maksbotan · 2020-07-26T10:31:34Z

Hi @fizruk, @phadej, @fisx and everyone interested!

I've marked this PR as ready for review. I tried to clean up the history with a rebase.

Here are some thoughts.

This is a massive change, just as the change in the spec itself. I suppose that it would make sense to split it into a separate openapi3 package.
servant-swagger is migrated in OpenAPI 3 haskell-servant/servant-swagger#121, perhaps it should be called servant-openapi3 as well.
OpenAPI 3 got simplified with respect to parameters, so SwaggerKind and stuff are no longer required. Maybe there are some relics of it left though.
I'm ready to hear your suggestions on the code and work together to make this change happen.

roosemberth

Thank you for the amazing work!

I reviewed your types agains the OpenAPI v3.0.3 specification, in particular Data.Swagger.Internal and some of the spec tests (thank you). Unfortunatelly I don't know optics, so I can't poke there.

Cheers!

roosemberth · 2020-07-26T20:02:16Z

src/Data/Swagger/Internal.hs

+  , _componentsParameters :: Definitions Param
+  , _componentsExamples :: Definitions Example
+  , _componentsRequestBodies :: Definitions RequestBody
+  , _componentsHeader :: Definitions Header


Suggested change

, _componentsHeader :: Definitions Header

, _componentsHeaders :: Definitions Header

roosemberth · 2020-07-26T20:06:35Z

src/Data/Swagger/Internal.hs

@@ -248,41 +257,41 @@ data Operation = Operation
  , _operationSummary :: Maybe Text


-- For maximum readability in the swagger-ui, this field SHOULD be less than 120 characters.

I couldn't find this note in the OpenAPI specification v3.0.3.

roosemberth · 2020-07-26T20:32:09Z

src/Data/Swagger/Internal.hs

@@ -330,159 +457,130 @@ data Param = Param
    -- Otherwise, the property MAY be included and its default value is @False@.


Unfortunatelly Github won't allow me to put a comment on an unmodified line, but comment on line 452 should be updated from GFM to Commonmark.

roosemberth · 2020-07-26T20:36:31Z

src/Data/Swagger/Internal.hs

+    -- Furthermore, if referencing a schema that contains an example,
+    -- the examples value SHALL override the example provided by the schema.
+  , _paramExamples :: InsOrdHashMap Text (Referenced Example)
+  } deriving (Eq, Show, Generic, Data, Typeable)


Missing _paramAllowReserved. It would also be useful to change the order of the fields in this class to match that in the OpenAPI specification v3.0.3, but it's not necessary.

roosemberth · 2020-07-26T20:52:32Z

src/Data/Swagger/Internal.hs


  , _schemaMaxProperties :: Maybe Integer
  , _schemaMinProperties :: Maybe Integer

-  , _schemaParamSchema :: ParamSchema 'SwaggerKindSchema
+  , _schemaParamSchema :: ParamSchema


I could not find this property in the OpenAPI v3.0.3 specification. Can you please add a comment where it is coming from or what its purpose is?

This is a subset of Schema that is used for both parameters and request/response body.

roosemberth · 2020-07-26T21:12:55Z

src/Data/Swagger/Internal.hs


-  , _headerParamSchema :: ParamSchema ('SwaggerKindNormal Header)
+  , _headerSchema :: Maybe (Referenced Schema)
  } deriving (Eq, Show, Generic, Data, Typeable)


From the OpenAPI v3.0.3 specification.

The Header Object follows the structure of the Parameter Object with the following changes:

name MUST NOT be specified, it is given in the corresponding headers map.
in MUST NOT be specified, it is implicitly in header.
All traits that are affected by the location MUST be applicable to a location of header (for example, style).

The following properties seem to be missing:

required

deprecated

allowEmptyValue

style

explode

example

examples

A comment shall be added if these properties are not supported.

maksbotan · 2020-07-26T21:45:10Z

Thanks for your review, @roosemberth! I will look at this :)

fizruk · 2020-08-03T07:57:14Z

Hi @fizruk, @phadej, @fisx and everyone interested!

I've marked this PR as ready for review. I tried to clean up the history with a rebase.

@maksbotan Thank you for this amazing work!

Here are some thoughts.

This is a massive change, just as the change in the spec itself. I suppose that it would make sense to split it into a separate openapi3 package.

Yes, I think this is the most sensible thing to do, since it should be way easier to maintain that way! Feel free to just create a copy of this repo named openapi3.

servant-swagger is migrated in haskell-servant/servant-swagger#121, perhaps it should be called servant-openapi3 as well.

Yes, same goes for servant-openapi3.

OpenAPI 3 got simplified with respect to parameters, so SwaggerKind and stuff are no longer required. Maybe there are some relics of it left though.

That sounds great! Less code is better 👍

I'm ready to hear your suggestions on the code and work together to make this change happen.

One suggestion is to also adapt servant-swagger-ui, since it is the trio of packages I find extremely useful (and I think others do as well) 😊

Another suggestion might be to update doctests to use YAML, as I see that it is what people use often, and also it is easier to read in documentation, I think.

At lastly, perhaps a list of key changes since Swagger 2.0 (or a link to such a list) somewhere in the obvious place might be great for users to switch over :) Especially, I believe we should emphasize that sum types are now encoded (more) faithfully in OpenAPI 3.0 than in Swagger 2.0, which should be a great motivator to move to the new package. We could also add references openapi3, servant-openapi3 and servant-openapi3-ui to the front pages of swagger2 packages, for people to find new packages faster :)

maksbotan · 2020-08-03T08:26:59Z

Thanks for the response!

I will make new packages in the biocad/ org.

Are you going to review this PR first or I can just proceed with copying?

I will prepare a list of changes, as you suggested, once the repo is a new place.

roosemberth · 2020-08-03T08:44:53Z

IMHO: I think it would be a good idea to finish reviewing the PR here (maybe change the target branch?) and then fork-off the merged branch.

fizruk · 2020-08-03T09:25:42Z

Thanks for the response!

I will make new packages in the biocad/ org.

Perfect!

Are you going to review this PR first or I can just proceed with copying?

I glanced over it, it looks good to me, so I think you can proceed. You can take the suggestion of @roosemberth to complete merge here (in a separate branch), or proceed in any other way you want :)

I will prepare a list of changes, as you suggested, once the repo is a new place.

Great, thank you! 😊

maksbotan · 2020-08-03T14:21:02Z

Thanks @roosemberth for your review!

maksbotan · 2020-08-03T14:56:17Z

So, the new home is at https://github.com/biocad/openapi3/ 🎉

I hope I did not mess with copyrights and acknowledgments. I've also added @fizruk as a maintainer to that repo.

By all means, check it out and file issues :)

roosemberth

The types seems to match the OpenAPI v3.0.3 spec. Thank you!

sir4ur0n · 2020-08-13T08:43:39Z

Hi, I'm asking here pretty much the same questions I asked there:

Hi, so what's the status of this PR? Will this development land in swagger2 or remain in a dedicated openapi3 package?

If new package:

Shouldn't swagger2 readme/documentation state that it's a deprecated package, and users should migrate to openapi3? I don't see the point of maintaining both packages (except for a few months for retro-compatibility reasons)

If it lands in this package:

Are there remaining steps to merge it?

Thank you!

maksbotan mentioned this pull request Jul 9, 2020

OpenAPI 3 haskell-servant/servant-swagger#121

Closed

maksbotan force-pushed the maksbotan/openapi3 branch 2 times, most recently from f205201 to ff87915 Compare July 10, 2020 13:06

maksbotan added 19 commits July 26, 2020 13:00

Migrate toplevel schema to openapi3

821654c

Extend Schema

6ac3540

Set version to 3.0.3

c253c9f

Fix ref paths for components

6dc3170

Add lenses for new types

08444f2

Update Operations.hs

eadb30a

Add Example

5902310

Polish types

9aa51b3

Add Optics

5b5cfcb

Migrate OAuth2 flows

fec285b

Remove old types, migrate Param

0bf745e

SwaggerSpec passes

8087980

SchemaSpec passes

84f3156

Doctests pass

27079c7

Implement schemas for sum types

73de316

Drop GHC 8.0 & 8.2 from cabal

506f211

haskell-ci regenerate

5887ee3

Add tests for different SumEncoding flavors

55662c8

maksbotan added 2 commits July 26, 2020 13:18

Use MediaType instead of plain Text

c9830de

Add Callback and Link

23caa64

maksbotan force-pushed the maksbotan/openapi3 branch from ff87915 to 23caa64 Compare July 26, 2020 10:19

maksbotan marked this pull request as ready for review July 26, 2020 10:24

Document OpenAPI 3 oneOf in examples

6978807

roosemberth reviewed Jul 26, 2020

View reviewed changes

maksbotan added 3 commits August 3, 2020 16:19

Fix typos, add more fields

4fd02b4

Merge ParamSchema into Schema

11f8f97

More missed fields

d193795

roosemberth approved these changes Aug 5, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenAPI 3 #219

OpenAPI 3 #219

maksbotan commented Jun 23, 2020

maksbotan commented Jun 26, 2020

sir4ur0n commented Jun 26, 2020

maksbotan commented Jul 4, 2020

maksbotan commented Jul 4, 2020

sir4ur0n commented Jul 5, 2020

maksbotan commented Jul 6, 2020

sir4ur0n commented Jul 6, 2020

maksbotan commented Jul 6, 2020

maksbotan commented Jul 26, 2020

roosemberth left a comment

roosemberth Jul 26, 2020

roosemberth Jul 26, 2020

roosemberth Jul 26, 2020

roosemberth Jul 26, 2020

roosemberth Jul 26, 2020

fizruk Aug 3, 2020

roosemberth Jul 26, 2020

maksbotan commented Jul 26, 2020

fizruk commented Aug 3, 2020

maksbotan commented Aug 3, 2020

roosemberth commented Aug 3, 2020

fizruk commented Aug 3, 2020

maksbotan commented Aug 3, 2020

maksbotan commented Aug 3, 2020

roosemberth left a comment

sir4ur0n commented Aug 13, 2020

	, _componentsHeader :: Definitions Header
	, _componentsHeaders :: Definitions Header

		@@ -248,41 +257,41 @@ data Operation = Operation
		, _operationSummary :: Maybe Text

		@@ -330,159 +457,130 @@ data Param = Param
		-- Otherwise, the property MAY be included and its default value is @False@.

OpenAPI 3 #219

Are you sure you want to change the base?

OpenAPI 3 #219

Conversation

maksbotan commented Jun 23, 2020

maksbotan commented Jun 26, 2020

sir4ur0n commented Jun 26, 2020

maksbotan commented Jul 4, 2020

maksbotan commented Jul 4, 2020

sir4ur0n commented Jul 5, 2020

maksbotan commented Jul 6, 2020

sir4ur0n commented Jul 6, 2020

maksbotan commented Jul 6, 2020

maksbotan commented Jul 26, 2020

roosemberth left a comment

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

fizruk Aug 3, 2020

Choose a reason for hiding this comment

roosemberth Jul 26, 2020

Choose a reason for hiding this comment

maksbotan commented Jul 26, 2020

fizruk commented Aug 3, 2020

maksbotan commented Aug 3, 2020

roosemberth commented Aug 3, 2020

fizruk commented Aug 3, 2020

maksbotan commented Aug 3, 2020

maksbotan commented Aug 3, 2020

roosemberth left a comment

Choose a reason for hiding this comment

sir4ur0n commented Aug 13, 2020