Support for Specification Extensions #11 for v3.2.2 (with aeson 2 support) by alexbiehl · Pull Request #43 · biocad/openapi3

alexbiehl · 2022-03-24T14:15:20Z

This PR is a follow up from #42. Only change compared to #42 is the added support for Aeson 2.

maksbotan · 2022-03-24T14:47:19Z

Wonderful! Now tests pass. I will take a look at the code shortly, thanks!

maksbotan · 2022-03-24T14:55:24Z

src/Data/OpenApi/Internal.hs

    -- the examples value SHALL override the example provided by the schema.
  , _paramExamples :: InsOrdHashMap Text (Referenced Example)

+  , _paramExtensions :: SpecificationExtensions


missing haddock here?

maksbotan · 2022-03-24T14:58:19Z

src/Data/OpenApi/Lens.hs

 makeFields ''Example
 makeFields ''Discriminator
 makeFields ''Link
+makeLenses ''SpecificationExtensions


I believe corresponding changes should be done to Optics.hs as well?

maksbotan · 2022-03-24T14:59:48Z

stack.yaml.lock

@@ -0,0 +1,47 @@
+# This file was autogenerated by Stack.


Does this need to be in the repo? I'm not even sure if current stack.yaml works at all, I use only cabal...

I usually just use cabal too nowadays, but if there's a stack.yaml I'll usually use stack.

https://stackoverflow.com/questions/64682066/should-stack-yaml-lock-be-checked-into-source-control

Supposedly yes, they should be committed.

maksbotan · 2022-03-24T15:02:47Z

src/Data/OpenApi/Internal.hs

 instance ToJSON Schema where
  toJSON = sopSwaggerGenericToJSONWithOpts $
-      mkSwaggerAesonOptions "schema" & saoSubObject ?~ "items"
+      mkSwaggerAesonOptions "schema" & saoSubObject .~ ["items", "extensions"]


I wonder if it's possible to make extensions injection more implicit, like hardcode it into sopSwaggerGenericToJSON'' — check whether we have extensions in the type and then encode/decode it accordingly. What do you think?

With the current approach I fear we may forget to add this to saoSubObject for some of our types. On the other hand, it becomes more implicit, dunno if it's a good thing.

I suppose this one can be left for future improvement

alexbiehl · 2022-03-24T15:08:30Z

@PPKFS Do you want to carry out the requested changes or should I?

maksbotan · 2022-03-24T15:36:00Z

I've checked locally with our project that generated json does not change 👍
However servant-openapi3 and our internal library had to be updated a bit before they worked.

alexbiehl · 2022-03-25T08:32:52Z

I can also confirm that the mechanism works for our tool for generating Haskell servers from OpenAPI specs: scarf-sh/tie@d08d9a0#diff-b435607ddb81678ffc30b4fefe883599027eb7e487b8c735ded197ef07947efdR17

PPKFS · 2022-03-25T10:56:10Z

@PPKFS Do you want to carry out the requested changes or should I?

I'm happy to carry them out after work, unless you're already one step ahead of me :)

Also interesting to see we're not the only place with openapi->haskell server tools!

PPKFS · 2022-04-13T11:06:59Z

Oops, very sorry that I offered to make the changes and then promptly forgot.
I've got the changes done here, though I'm realising I'm not well versed in github PRs...

https://github.com/PPKFS/openapi3/tree/alex/aeson-2-support-for-extensions

With regards to the point about more implicit injection of extensions, I agree, but I realised after a while of trying that I don't know enough about sop-generics to do it properly.

alexbiehl · 2022-04-13T12:04:40Z

@PPKFS I think you can just open a new PR from your branch! We can then close this one in favour of the new one.

As to your second point, I agree it would be nice to have a more implicit injection but I hope we can postpone this kind of change to another contribution and get your work in ASAP.

ersran9 · 2022-11-18T11:20:49Z

Anything that can be done to merge this in? @alexbiehl @PPKFS

alexbiehl · 2022-11-18T13:11:17Z

@ersran9 @PPKFS @maksbotan I think we should probably merge as is. I can report I am using this in Production for some time and am not missing anything. We can improve incrementally for sure.

maksbotan

Whoops, I've reviewed the wrong one first :)

There's a couple of comments, once they are addressed I'd be happy to merge this.

maksbotan · 2022-11-19T08:32:56Z

src/Data/OpenApi.hs

  -- ** Miscellaneous
  MimeList(..),
  URL(..),
+  SpecificationExtensions (..),


Please copy this fix here :)

https://github.com/biocad/openapi3/pull/21/files#diff-72b8e7bcb5b302e7fe8867caf0eb76e8fc8c270acdcd5a6dc2fc1402a3904abcL187

maksbotan · 2022-11-19T08:33:13Z

src/Data/OpenApi/Internal.hs

+                                         sopSwaggerGenericToJSON, sopSwaggerGenericToJSONWithOpts, sopSwaggerGenericParseJSONWithOpts)
 import Data.OpenApi.Internal.Utils
 import Generics.SOP.TH                  (deriveGeneric)
+import Data.Maybe (catMaybes)


this one belongs in the first group

maksbotan · 2022-11-19T08:33:21Z

src/Data/OpenApi/Internal.hs

    -- the examples value SHALL override the example provided by the schema.
  , _paramExamples :: InsOrdHashMap Text (Referenced Example)

+  , _paramExtensions :: SpecificationExtensions


maksbotan · 2022-11-19T08:33:59Z

src/Data/OpenApi/Internal.hs

 instance ToJSON Schema where
  toJSON = sopSwaggerGenericToJSONWithOpts $
-      mkSwaggerAesonOptions "schema" & saoSubObject ?~ "items"
+      mkSwaggerAesonOptions "schema" & saoSubObject .~ ["items", "extensions"]


I suppose this one can be left for future improvement

maksbotan · 2022-11-19T08:35:10Z

src/Data/OpenApi/Internal/AesonUtils.hs

+  where
+    proxy = Proxy :: Proxy a
+
+    parseAdditionalField :: Object -> Pair -> Parser ()


there is a copy of this function on line 223 which is unused now, please remove

maksbotan · 2022-11-19T08:35:42Z

src/Data/OpenApi/Internal/AesonUtils.hs

+    in pairs (pairsToSeries (opts ^. saoAdditionalPairs) <> ps)
+  where
+    proxy = Proxy :: Proxy a
+    defs = hcpure (Proxy :: Proxy AesonDefaultValue) defaultValue


Copying my comment from #11:

Why is this different from (aesonDefaults proxy) in sopSwaggerGenericToEncoding?

potocpav · 2023-01-13T11:02:07Z

It would be amazing to get these improvements in. Is it possible to push it towards merging? 🙏 @alexbiehl @maksbotan

maksbotan · 2023-01-14T12:58:01Z

Hi @potocpav!
As far I see, my latest review comments have not been addressed yet? Once they are, I'm happy to merge this. Also, there is a merge conflict in src/Data/OpenApi/Internal.hs.

stepcut · 2023-06-02T15:48:24Z

I would love to be able to use this patch. Is there anything I can do to help get it into the mainline?

theophile-scrive · 2025-06-30T10:51:33Z

@maksbotan Any chance we can merge this? :)

alexbiehl changed the title ~~alex/aeson 2 support for extensions~~ Support for Specification Extensions #11 for v3.2.2 (with aeson 2) Mar 24, 2022

alexbiehl changed the title ~~Support for Specification Extensions #11 for v3.2.2 (with aeson 2)~~ Support for Specification Extensions #11 for v3.2.2 (with aeson 2 support) Mar 24, 2022

alexbiehl mentioned this pull request Mar 24, 2022

Support for Specification Extensions #11 for v3.2.2 #42

Open

maksbotan reviewed Mar 24, 2022

View reviewed changes

maksbotan reviewed Nov 19, 2022

View reviewed changes

maksbotan mentioned this pull request Nov 19, 2022

Fix bug parsing set of valid OpenApi spec versions #68

Merged

isomorpheme mentioned this pull request Apr 22, 2024

Add support for Specification Extensions #96

Closed

PPKFS and others added 9 commits May 28, 2024 15:33

tests pass

662b2b5

add param extension field

f33a35e

Add better extension lens support

5780c53

add header extensions and paramlocation ord

0ce05fb

add hashable instance

7471a6f

more gracefully handle optional extensions

169d249

Add better default value for extensions

2156a91

Oops, forgot some instances

86511ac

Aeson 2 for extensions

c2575df

alexbiehl force-pushed the alex/aeson-2-support-for-extensions branch from 4793b3c to c2575df Compare May 28, 2024 13:38

Conversation

alexbiehl commented Mar 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maksbotan commented Mar 24, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexbiehl commented Mar 24, 2022

Uh oh!

maksbotan commented Mar 24, 2022

Uh oh!

alexbiehl commented Mar 25, 2022

Uh oh!

PPKFS commented Mar 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PPKFS commented Apr 13, 2022

Uh oh!

alexbiehl commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ersran9 commented Nov 18, 2022

Uh oh!

alexbiehl commented Nov 18, 2022

Uh oh!

maksbotan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

potocpav commented Jan 13, 2023

Uh oh!

maksbotan commented Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stepcut commented Jun 2, 2023

Uh oh!

theophile-scrive commented Jun 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

alexbiehl commented Mar 24, 2022 •

edited

Loading

PPKFS commented Mar 25, 2022 •

edited

Loading

alexbiehl commented Apr 13, 2022 •

edited

Loading

maksbotan commented Jan 14, 2023 •

edited

Loading