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.

Sign up for GitHub

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

Jump to bottom

SmallRye OpenAPI 3.7.0 and Tests to check content type between Services and OpenAPI #36514

Merged
merged 1 commit into from
Nov 2, 2023
Merged

SmallRye OpenAPI 3.7.0 and Tests to check content type between Services and OpenAPI #36514

merged 1 commit into from
Nov 2, 2023

Conversation

phillip-kruger
Copy link
Member

@phillip-kruger phillip-kruger commented Oct 17, 2023 •
edited

This started with this issue: #34700 and some initial fixes when into OpenAPI (some more needed) but we decided to first do a good analysis for the issue.

As discussed with @FroMage and @geoand :

  • Default values of content types for both Producing (return object) and Consuming (input parameters) in Services should be aligned.
  • Different Services (RESTEasy, RESTEasy-Reacitve, Spring Web, Reactive Routes) behavior needs to be consistent
  • Handling of content type and content type wrapped in an holder (except Collection) should be consistent

The fixes happened in SmallRye OpenAPI and this PR is mostly tests and the upgrade of SmallRye OpenAPI to 3.7.0.

SmallRye OpenAPI 3.7.0 also
Fix #36677
Fix #36646

//cc @MikeEdgar

Extensions to analyze

  • resteasy-reactive
  • resteasy-reactive-jackson
  • resteasy-reactive-jsonb
  • resteasy-reactive-jaxb
  • resteasy
  • resteasy-jackson
  • resteasy-jsonb
  • resteasy-jaxb
  • spring-web
  • reactive-routes

Types to consider

  • String
  • Integer/ int / OptionalInt
  • Boolean
  • Pojo
  • Byte / byte
  • Short / short
  • Long / long
  • Float / float
  • Double / double
  • Character / char
  • File
  • InputStream
  • Reader
  • BigDecimal
  • BigInteger
  • URL
  • Buffer (Vertx)
  • JsonObject
  • JsonArray
  • JsonStructure
  • JsonValue

Wrappers

  • RestResponse<?>
  • ResponseEntity<?>
  • Optional<?>
  • Uni<?>
  • CompletionStage<?>
  • CompletedFuture<?>
  • List<?>
  • [?]
  • Map

Notes so far:

@phillip-kruger
Copy link
Member Author

//cc @cescoffier @FroMage @geoand @rsvoboda

@phillip-kruger phillip-kruger mentioned this pull request Oct 18, 2023
Merged
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Oct 18, 2023
@phillip-kruger phillip-kruger force-pushed the content-type-tests branch 8 times, most recently from c48bdd8 to c3ffe67 Compare October 24, 2023 06:01
@phillip-kruger phillip-kruger force-pushed the content-type-tests branch 2 times, most recently from b3857b5 to a209ce4 Compare October 25, 2023 23:01
@MikeEdgar
Copy link
Contributor

Also closes #36646

@phillip-kruger phillip-kruger marked this pull request as ready for review October 25, 2023 23:01
@phillip-kruger
Copy link
Member Author

@FroMage @geoand - this is now ready for review. Please have a look.
@rsvoboda - this might break beefy scenarios, please update those tests

@phillip-kruger phillip-kruger changed the title Tests to check content type between services and openapi Tests to check content type between Services and OpenAPI Oct 25, 2023
@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Oct 26, 2023

I'll try and have a look tomorrow.

The test failures do seem relevant thought, right?

@phillip-kruger phillip-kruger force-pushed the content-type-tests branch 2 times, most recently from abc5ec9 to 316f120 Compare October 27, 2023 00:05
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Oct 31, 2023

I am kind of reluctant to merge any PR if there are no passing JDK jobs.

Wanna restart CI?

@phillip-kruger
Copy link
Member Author

Let me rebate, as it has already restarted with no effect

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Oct 31, 2023

@yrodiere are these tests known to be failing lately?

@yrodiere
Copy link
Member

@yrodiere are these tests known to be failing lately?

Yes, see #36794

Give me a minute y'all xD

@geoand
Copy link
Contributor

geoand commented Oct 31, 2023

Gotcha, sorry for asking. I've been out of the loop the past few days :)

@yrodiere
Copy link
Member

Gotcha, sorry for asking. I've been out of the loop the past few days :)

No problem, no need to be sorry :) Anyway, I'm on it...

@yrodiere
Copy link
Member

@yrodiere are these tests known to be failing lately?

Yes, see #36794

Give me a minute y'all xD

The fix is there: #36802

@phillip-kruger
Tests to check content type between Services and OpenAPI
3dad6f5 
Signed-off-by: Phillip Kruger <phillip.kruger@gmail.com>
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 2, 2023

Failing Jobs - Building 3dad6f5

Status Name Step Failures Logs Raw logs Build scan
✖ Devtools Tests - JDK 11 Build Failures Logs Raw logs
✖ Devtools Tests - JDK 11 Windows Build Failures Logs Raw logs
✖ Devtools Tests - JDK 17 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Devtools Tests - JDK 11 #

- Failing: integration-tests/devtools

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.AmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): AmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_lambda_LambdaHandlerTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyAmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyAmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyGoogleCloudFunctionsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyGoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_funqygooglecloudfunctions_GreetingFunctionsCloudEventsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyHttpCodestartTest.testContent line 22 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyHttpCodestartTest/testContent/src_test_java_ilove_quark_us_MyFunctionsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyKnativeEventsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyKnativeEventsCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_cloudevent_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GoogleCloudFunctionsCodestartTest.testContent line 24 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_googlecloudfunctions_HelloWorldCloudEventsFunctionTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GrpcCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GrpcCodestartTest/testContent/src_test_java_ilove_quark_us_HelloGrpcServiceTest.java] 
Path:

⚙️ Devtools Tests - JDK 11 Windows #

- Failing: integration-tests/devtools

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.AmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): AmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_lambda_LambdaHandlerTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyAmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyAmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyGoogleCloudFunctionsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyGoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_funqygooglecloudfunctions_GreetingFunctionsCloudEventsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyHttpCodestartTest.testContent line 22 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyHttpCodestartTest/testContent/src_test_java_ilove_quark_us_MyFunctionsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyKnativeEventsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyKnativeEventsCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_cloudevent_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GoogleCloudFunctionsCodestartTest.testContent line 24 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_googlecloudfunctions_HelloWorldCloudEventsFunctionTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GrpcCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GrpcCodestartTest/testContent/src_test_java_ilove_quark_us_HelloGrpcServiceTest.java] 
Path:

⚙️ Devtools Tests - JDK 17 #

- Failing: integration-tests/devtools

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.AmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): AmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_lambda_LambdaHandlerTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyAmazonLambdaCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyAmazonLambdaCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyGoogleCloudFunctionsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyGoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_funqygooglecloudfunctions_GreetingFunctionsCloudEventsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyHttpCodestartTest.testContent line 22 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyHttpCodestartTest/testContent/src_test_java_ilove_quark_us_MyFunctionsTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.FunqyKnativeEventsCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): FunqyKnativeEventsCodestartTest/testContent/src_test_java_ilove_quark_us_funqy_cloudevent_FunqyTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GoogleCloudFunctionsCodestartTest.testContent line 24 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GoogleCloudFunctionsCodestartTest/testContent/src_test_java_ilove_quark_us_googlecloudfunctions_HelloWorldCloudEventsFunctionTest.java] 
Path:

✖ io.quarkus.devtools.codestarts.quarkus.GrpcCodestartTest.testContent line 23 - More details - Source on GitHub

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically): GrpcCodestartTest/testContent/src_test_java_ilove_quark_us_HelloGrpcServiceTest.java] 
Path:

@geoand
Copy link
Contributor

geoand commented Nov 2, 2023

I doubt the devtools failures are related.

@ia3andy can you take a look and make sure please?

@ia3andy
Copy link
Contributor

ia3andy commented Nov 2, 2023

ok it's due to this PR #33181, @gsmet didn't you fix the snapshots? I saw a comment about it.. I'll fix it now

@ia3andy ia3andy mentioned this pull request Nov 2, 2023
Merged
@ia3andy
Copy link
Contributor

ia3andy commented Nov 2, 2023 •
edited

@geoand I made the PR to fix the codestarts snapshots: #36824

@yrodiere
Copy link
Member

yrodiere commented Nov 2, 2023

ok it's due to this PR #33181, @gsmet didn't you fix the snapshots? I saw a comment about it.. I'll fix it now

@ia3andy Guillaume is currently on PTO.

@geoand
Copy link
Contributor

geoand commented Nov 2, 2023

@phillip-kruger I would say go ahead and merge this if you are happy with it

@phillip-kruger phillip-kruger merged commit 24a77b8 into quarkusio:main Nov 2, 2023
48 of 51 checks passed
@quarkus-bot quarkus-bot bot added kind/bugfix kind/enhancement New feature or request labels Nov 2, 2023
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Nov 2, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 2, 2023
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.1 Nov 6, 2023
@rsvoboda rsvoboda changed the title Tests to check content type between Services and OpenAPI SmallRye OpenAPI 3.7.0 and Tests to check content type between Services and OpenAPI Nov 8, 2023
@rsvoboda rsvoboda mentioned this pull request Nov 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

@FroMage FroMage Awaiting requested review from FroMage

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/openapi area/smallrye kind/bugfix kind/enhancement New feature or request
Projects
None yet
Milestone
3.5.1
Development

Successfully merging this pull request may close these issues.

OpenAPI.yaml with recursion OpenAPI support to register schema for a type from 3rd party lib
6 participants
@phillip-kruger @MikeEdgar @geoand @yrodiere @ia3andy @gsmet