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

Feature/update readme and swagger #151

Merged
merged 11 commits into from
Nov 20, 2024
Merged

Conversation

southeo
Copy link
Contributor

@southeo southeo commented Nov 15, 2024

Updates swagger endpoints to include request and response schemas. Adds more descriptions.

OAS = openApi standard

Design decisions

Internal classes
We follow JsonApi, which is a pretty nested standard. I nest classes where possible because there are a lot of domain objects being created, and I try to minimize that. Now I understand why GBIF wound up with so many...

Why are some internal classes private and some are public
Usually, we need to access the internal classes in the requests to get the data. We don't need to do so for the responses, so those can be private.

Two @RequestBody annotations.
There are two RequestBody annotations. One from Spring, one from OAS. The OpenAPI @RequestBody annotation specifies what class to document the body as, but if it conflicts with the Spring @RequestBody, swagger prioritizes the Spring @RequestBody, which is the runtime type. So that means we can't have generic request objects anymore.

Why use the @Schema annotation at all

  1. The @Schema Annotation is used for adding additional metadata
  2. You can use it in conjunction with keywords like oneOf, which will be more useful in the Handle API, and it's good to be consistent across APIs
  3. If you use generic Objects, OpenAPI will use the object in the @Schema implementation

Known issues

GET requests + request body The "schema" tab doesn't appear for GET requests that require a request body, (see /api/v1/annotation/batch as an example). This is because OpenApi doesn't want to encourage GET requests with a request body, so they don't support that.

@southeo southeo requested a review from samleeflang November 15, 2024 15:44
Copy link
Contributor

@samleeflang samleeflang left a comment

Choose a reason for hiding this comment

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

Yes this is as expected. A lot more domain classes but not too complicated. Will help with readability of the docs. At some places we might need to add some description but let's start with this.

@southeo southeo requested a review from samleeflang November 19, 2024 08:23
samleeflang
samleeflang previously approved these changes Nov 20, 2024
Copy link
Contributor

@samleeflang samleeflang left a comment

Choose a reason for hiding this comment

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

💺

@southeo southeo merged commit 62a0406 into main Nov 20, 2024
2 checks passed
@southeo southeo deleted the feature/update-readme-and-swagger branch November 20, 2024 10:20
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