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

EES-5057 Add missing public API Swagger docs for required properties and error responses #4749

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

ntsim
Copy link
Collaborator

@ntsim ntsim commented Apr 15, 2024

This PR adds the following to the public API Swagger docs:

  • Required schema properties. These are necessary so that users know which properties are actually needed for a request / response, otherwise, this is non-obvious to users and tools like OpenAPI client generators wouldn't work.
  • Error responses. These have been added for error status codes (400, 403, 404) for all endpoints, and we typically return ProblemDetailsViewModel or ValidationProblemViewModel in these cases.

Note that rather than try and extend Common's ErrorViewModel for the public API to make some of its properties required (e.g. code and message), we've just created an ErrorViewModelSchemaFilter that modifies the OpenAPI schema instead.

Several attempts were made, but trying to extend Common's ErrorViewModel provided a bit too difficult and intrusive in terms of code changes.

Related changes

  • Fixes missing required properties on various public API view models.

Other changes

  • Added missing cancellation tokens to ContentApiClient.

@ntsim ntsim changed the base branch from ees-5015 to dev April 15, 2024 10:56
ntsim added 5 commits April 16, 2024 16:31
…operties

This fixes the Swagger docs currently not being generated with required
properties. This meant that the docs were previously incomplete as we
specify if properties are required or not in the generated public API
docs.
This fixes various issues that were causing incorrect Swagger docs to
be generated for problem detail error responses.

Tins includes:

- Adding a `ErrorViewModelSchemaFilter` that fixes `code` and `message`
  properties to be required and non-nullable for `ErrorViewModel`.
- Including doc blocks from `Common` view models for Swagger docs.
- Using our own `ProblemDetailsViewModel` to control what Swagger docs
  are generated (rather than .NET's `ProblemDetails`).
- Specifying various problem detail view models for 400s and 404s.
Previously, .NET would not output a response body for a 403 Forbidden
as they leave it up to you do what you want with such a response (e.g.
redirect the browser to a login page).

However, this isn't helpful in the context of a REST API, so we've now
fixed this to return a default `ProblemDetails` response.
@ntsim ntsim merged commit 8118894 into dev Apr 16, 2024
7 checks passed
@ntsim ntsim deleted the ees-5057 branch April 16, 2024 15:48
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