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

Openapi yaml #41

Merged
merged 38 commits into from
Jan 21, 2022
Merged

Openapi yaml #41

merged 38 commits into from
Jan 21, 2022

Conversation

mooreds
Copy link
Contributor

@mooreds mooreds commented Dec 23, 2021

This is a first pass at generating an openAPI yaml file from the API and domain JSON files.

This deploys files to the https://github.com/FusionAuth/fusionauth-openapi repository, where it can be versioned and released like any other client library package (not quite sure how the release plugin works, so I just copy pastaed it).

There are some flaws in the implementation. While the specification is valid, the generated client libraries haven't been fully exercised.

In particular:

  • polymorphic operations are not well supported by the client library generators. That means that identity provider requests and responses are not functional. I'm not sure if there are workarounds, but it seems like some work is being done. See [Java] Swagger oneOf type: Jackson trying to instantiate interface instead of implementation? swagger-api/swagger-codegen#10011 for example.
  • this file is generated from the fusionauth-client-builder JSON files, not from code. This means that there may be gaps when compared to the REST API.
  • there's no information about what parameters are required or not, because that is not part of the API JSON files.
  • there are certain operations, status codes and security mechanisms (JWT auth, cookies for auth) that are not currently supported, again, because they are not included in the API JSON files.
  • oauth specific operations are not currently supported

Rollout plan:

  • Review with eng team, determine if current state is shippable to alpha users.
  • Ask folks who commented on Investigate OpenAPI for documentation and client library generation fusionauth-issues#614 or otherwise expressed interest in this to kick the tires.
  • Determine what features need to be added to ship.
  • Fix any outstanding issues/add features.
  • Publish as 'tech preview' in docs. Close bugs for other SDKs.
  • Let it burn in for a few months, fix any issues.
  • Start publishing it as part of the release process. Maybe include it in the build? Maybe just on the download page.
  • Go back and provide specs for previous releases so that folks can use apidiff functionality.
  • Investigate bringing things closer to the code/fusionauth-app so that we don't need to maintain the API JSON files.

@mooreds mooreds requested a review from a team as a code owner December 23, 2021 21:54
@mooreds mooreds mentioned this pull request Jan 3, 2022
@mooreds mooreds requested a review from voidmain January 5, 2022 21:03
@voidmain
Copy link
Member

It's a lot of code to digest, but the first thing that jumped out was that the instants don't look to be correctly handled. We use integers for all instants (ZonedDateTime) and they represent the number of milliseconds since Epoch.

@mooreds
Copy link
Contributor Author

mooreds commented Jan 11, 2022

Okay, let me take another pass and re-request code review.

This makes it easier to understand.
This was a bit weird because the definition in the json file is java.time.zoneddatetime. This is pretty clearly an object:https://docs.oracle.com/javase/8/docs/api/java/time/ZonedDateTime.html
But in all the json we get back the value of these fields are longs.

So I just defined it as a long.
As documented here, there are workarounds for the compile errors I was getting:

OpenAPITools/openapi-generator#10880 (comment)
Now we postprocess operations at the same uri + method but with different params.
when the api path is complex enough, we get colliding ids.
if new has it, we're good. if both don't have it, we're good.
Copy link
Member

@voidmain voidmain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do this!

@mooreds mooreds merged commit 7432ae2 into master Jan 21, 2022
@mooreds mooreds deleted the build-openapi-yaml branch January 21, 2022 23:54
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