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

Upgrade OpenAPI to 3.1.0 #1216

Closed
wants to merge 11 commits into from
Closed

Upgrade OpenAPI to 3.1.0 #1216

wants to merge 11 commits into from

Conversation

rojekp
Copy link
Contributor

@rojekp rojekp commented May 5, 2021

No description provided.

@@ -519,6 +521,32 @@ class VerifyYamlTest extends AnyFunSuite with Matchers {

noIndentation(actualYaml) shouldBe load("expected_extensions.yml")
}

test("should match the expected json with schema dialect") {
Copy link
Member

Choose a reason for hiding this comment

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

as these are not tests of the openapi interpreter, but for the serialisation of customly enhanced OpenAPI objects, I'd move those to the openapi-model module

Copy link
Member

Choose a reason for hiding this comment

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

or rather, since we are testing serialisation, the appropriate place would be openapi-circe-yaml - where the encoders/decoders for the OpenAPI data type are defined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to module openapi-circe - the encoders/decoders are there :)

@rojekp rojekp linked an issue May 5, 2021 that may be closed by this pull request
build.sbt Outdated
@@ -562,7 +562,9 @@ lazy val openapiCirce: ProjectMatrix = (projectMatrix in file("apispec/openapi-c
libraryDependencies ++= Seq(
"io.circe" %% "circe-core" % Versions.circe,
"io.circe" %% "circe-parser" % Versions.circe,
"io.circe" %% "circe-generic" % Versions.circe
"io.circe" %% "circe-generic" % Versions.circe,
"io.circe" %% "circe-yaml" % Versions.circeYaml,
Copy link
Member

Choose a reason for hiding this comment

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

why are we adding yaml dependencies to this module? there's a dedicated module w/ yaml dependencies just below


trait TapirOpenAPICirceToYaml {
implicit class RichOpenAPI(openAPI: OpenAPI) {
def convertToYaml: String = Printer(dropNullKeys = true, preserveOrder = true).pretty(openAPI.asJson)
Copy link
Member

Choose a reason for hiding this comment

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

isn't it the same functionality as .toYaml? Why would this be in the main sources?

rojekp and others added 8 commits May 7, 2021 16:37
If decoding of an endpoint's input may fail, the endpoint may
respond with a 400 status code. Provide a default 400 response
for each such endpoint in OpenAPI docs to make them more
consistent with the actual endpoint behavior.

* feat(openapi): add default 400 output to OpenAPIDocsOptions
* wip: make default 400 a func input => Option[output]
* wip: use FailureMessages.failureSourceMessage
* wip: refactor
* wip: rename to defaultDecodeFailureOutput
* wip: enhance fallible input detection
* wip: fix json and security tests
* wip: fix yaml coproduct test
* wip: fix all tests
* wip: refactor
* wip: extract tests
* wip: check for required inputs
* refactor: extract EndpointInputToDecodeFailureOutput
* wip: exclude auth headers
* wip: add test for no response
* docs: add info about defaultDecodeFailureOutput
@rojekp
Copy link
Contributor Author

rojekp commented May 10, 2021

I will close this one and open same changes in another PR: #1223
Also linked new PR to issue with upgrading OpenAPI 3.1.0.

@rojekp rojekp closed this May 10, 2021
@mergify mergify bot deleted the openapi-3.1.0-support branch May 10, 2021 07:39
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.

4 participants