quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Support overlapping paths on resource classes

Open Postremus opened this issue 7 months ago • 18 comments

Having partially matching paths on resource classes could lead to 404s, even though a matching resource method existed.

Lets take the example from #26496.

@Path("/base")
class Base {
   @GET
   @Path("{id}") 
   public Uni<RestResponse<?>> base() {..}
}
@Path("/base/{id}")
class Extension {
   @GET
   @Path("extension") 
   public Uni<RestResponse<?>> extension() {..}
}

Calling GET /base/123 would result in a 404. /base/{id} is a perfect match for that request, which result in the Extension resource class being used. Quarkus-rest always only works on the basis of one resource class - the one with the best match.

I however believe that is not up to spec.

https://jakarta.ee/specifications/restful-ws/4.0/jakarta-restful-ws-spec-4.0.pdf

Chapter 3.5.2 Request Matching Stage 1 only handles matching of the resource class path to the request uri -> both classes match. Stage 2 figures out which of all resource methods in all matched resource classes matches against the request uri.

So basically, the current implementation results in Stage 1 only reporting one matching resource class, altough it should report 2.

This PR adds additional logic, so that all matching resource classes are remembered, and the handler chain is restarted if the current resource class does not contain a matchin sub resource method, or sub resource locator.

  • Closes: #26496

Postremus avatar Apr 15 '25 18:04 Postremus


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 195d1a5db47b866fcba8a4ecb907ddeb8bf722aa.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
:heavy_check_mark: JVM Integration Tests - JDK 17 Logs Raw logs :mag:
JVM Integration Tests - JDK 17 Windows Build Failures Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 21 Logs Raw logs :mag:

You can consult the Develocity build scans.

Failures

:gear: JVM Integration Tests - JDK 17 Windows #

- Failing: integration-tests/oidc-wiremock-logout 

:package: integration-tests/oidc-wiremock-logout

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.2:test (default-test) on project quarkus-integration-test-oidc-wiremock-logout:

See D:\a\quarkus\quarkus\integration-tests\oidc-wiremock-logout\target\surefire-reports for the individual test results. See dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream. There was an error in the forked process

quarkus-bot[bot] avatar Apr 15 '25 20:04 quarkus-bot[bot]

cc @franz1981 as this one could potentially have a slight performance impact

geoand avatar Apr 16 '25 19:04 geoand

I have added few comments

franz1981 avatar Apr 16 '25 19:04 franz1981


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 943c446b8834a9c76e6b3af4a9239e5487b353e2.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
:heavy_check_mark: JVM Integration Tests - JDK 17 Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 17 Windows Logs Raw logs :mag:
JVM Integration Tests - JDK 21 Build Failures Logs Raw logs :mag:

Full information is available in the Build summary check run. You can consult the Develocity build scans.

Failures

:gear: JVM Integration Tests - JDK 21 #

- Failing: integration-tests/compose-devservices integration-tests/observability-lgtm 

:package: integration-tests/compose-devservices

io.quarkus.it.compose.devservices.kafka.KafkaServiceTest.test line 17 - History - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <204> but was <500>.

	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)

:package: integration-tests/observability-lgtm

io.quarkus.observability.test.LgtmReloadTest.testReload line 35 - History - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Lambda expression in io.quarkus.observability.test.LgtmTestHelper expected the predicate to return <true> but was null within 1 minutes  1 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AbstractHamcrestCondition.await(AbstractHamcrestCondition.java:86)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:712)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:729)
	at io.quarkus.observability.test.LgtmTestHelper.poke(LgtmTestHelper.java:25)
	at io.quarkus.observability.test.LgtmReloadTest.testReload(LgtmReloadTest.java:35)

Flaky tests - Develocity

:gear: JVM Tests - JDK 17

:package: extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario2 - History

  • io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor\#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7 at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:131) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732) at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856) - java.lang.RuntimeException
java.lang.RuntimeException: 
io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7
	at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:131)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:255)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

quarkus-bot[bot] avatar Apr 16 '25 20:04 quarkus-bot[bot]

Please @geoand wait to merge this till the comments are addressed or at least a round of profiling is performed 🙏

franz1981 avatar Apr 17 '25 07:04 franz1981

That's why I removed the waiting-for-ci label :)

geoand avatar Apr 17 '25 07:04 geoand


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit c24f1aca7f06e067a1e5aafb534167eac823649b.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
JVM Integration Tests - JDK 17 Build Failures Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 17 Windows Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 21 Logs Raw logs :mag:

Full information is available in the Build summary check run. You can consult the Develocity build scans.

Failures

:gear: JVM Integration Tests - JDK 17 #

- Failing: integration-tests/observability-lgtm 

:package: integration-tests/observability-lgtm

io.quarkus.observability.test.LgtmReloadTest.testReload line 35 - History - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Lambda expression in io.quarkus.observability.test.LgtmTestHelper expected the predicate to return <true> but was null within 1 minutes  1 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AbstractHamcrestCondition.await(AbstractHamcrestCondition.java:86)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:712)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:729)
	at io.quarkus.observability.test.LgtmTestHelper.poke(LgtmTestHelper.java:25)
	at io.quarkus.observability.test.LgtmReloadTest.testReload(LgtmReloadTest.java:35)

quarkus-bot[bot] avatar Apr 17 '25 20:04 quarkus-bot[bot]

@franz1981 PTAL

geoand avatar Apr 18 '25 06:04 geoand

I would suggest to just return the match without allocating any list, but just the top match 🙏 Sorry for not commenting but I'm on PTO till next week (Fri) so I can comment with phone :P

franz1981 avatar Apr 18 '25 06:04 franz1981


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit aeab4329fd12b32689509a96a199e85579f411ea.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Security2 Build Failures Logs Raw logs :mag:
Native Tests - Security3 Build Failures Logs Raw logs :mag:
JVM Integration Tests - JDK 17 Build Failures Logs Raw logs :mag:
JVM Integration Tests - JDK 17 Windows Build Failures Logs Raw logs :mag:
JVM Integration Tests - JDK 21 Build Failures Logs Raw logs :mag:

Full information is available in the Build summary check run. You can consult the Develocity build scans.

Failures

:gear: Native Tests - Security2 #

- Failing: integration-tests/oidc-wiremock 

:package: integration-tests/oidc-wiremock

io.quarkus.it.keycloak.CodeFlowAuthorizationInGraalITCase.testCodeFlowUserInfo - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-only
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.doTestCodeFlowUserInfo(CodeFlowAuthorizationTest.java:557)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo(CodeFlowAuthorizationTest.java:325)

io.quarkus.it.keycloak.CodeFlowAuthorizationInGraalITCase.testCodeFlowUserInfoCachedInIdToken - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-github-cached-in-idtoken
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken(CodeFlowAuthorizationTest.java:345)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

:gear: Native Tests - Security3 #

- Failing: integration-tests/keycloak-authorization 

:package: integration-tests/keycloak-authorization

io.quarkus.it.keycloak.PolicyEnforcerInGraalITCase.testUserHasAdminRoleServiceTenant - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: /api-permission-tenant ==> expected: <200> but was: <404>
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.assureGetPath(AbstractPolicyEnforcerTest.java:247)
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.testUserHasAdminRoleServiceTenant(AbstractPolicyEnforcerTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:872)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

:gear: JVM Integration Tests - JDK 17 #

- Failing: integration-tests/keycloak-authorization integration-tests/oidc-wiremock 

:package: integration-tests/keycloak-authorization

io.quarkus.it.keycloak.DynamicTenantConfigPolicyEnforcerTest.testUserHasAdminRoleServiceTenant - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: /api-permission-tenant ==> expected: <200> but was: <404>
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.assureGetPath(AbstractPolicyEnforcerTest.java:247)
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.testUserHasAdminRoleServiceTenant(AbstractPolicyEnforcerTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1036)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:883)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

io.quarkus.it.keycloak.StaticTenantConfigPolicyEnforcerTest.testUserHasAdminRoleServiceTenant - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: /api-permission-tenant ==> expected: <200> but was: <404>
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.assureGetPath(AbstractPolicyEnforcerTest.java:247)
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.testUserHasAdminRoleServiceTenant(AbstractPolicyEnforcerTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1036)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:883)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

:package: integration-tests/oidc-wiremock

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo line 325 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-only
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.doTestCodeFlowUserInfo(CodeFlowAuthorizationTest.java:557)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo(CodeFlowAuthorizationTest.java:325)

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken line 345 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-github-cached-in-idtoken
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken(CodeFlowAuthorizationTest.java:345)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

:gear: JVM Integration Tests - JDK 17 Windows #

- Failing: integration-tests/oidc-wiremock 

:package: integration-tests/oidc-wiremock

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo line 325 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-only
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.doTestCodeFlowUserInfo(CodeFlowAuthorizationTest.java:557)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo(CodeFlowAuthorizationTest.java:325)

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken line 345 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-github-cached-in-idtoken
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken(CodeFlowAuthorizationTest.java:345)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

:gear: JVM Integration Tests - JDK 21 #

- Failing: integration-tests/keycloak-authorization integration-tests/observability-lgtm integration-tests/oidc-wiremock 

:package: integration-tests/keycloak-authorization

io.quarkus.it.keycloak.DynamicTenantConfigPolicyEnforcerTest.testUserHasAdminRoleServiceTenant - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: /api-permission-tenant ==> expected: <200> but was: <404>
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.assureGetPath(AbstractPolicyEnforcerTest.java:247)
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.testUserHasAdminRoleServiceTenant(AbstractPolicyEnforcerTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1036)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:883)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

io.quarkus.it.keycloak.StaticTenantConfigPolicyEnforcerTest.testUserHasAdminRoleServiceTenant - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: /api-permission-tenant ==> expected: <200> but was: <404>
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.assureGetPath(AbstractPolicyEnforcerTest.java:247)
	at io.quarkus.it.keycloak.AbstractPolicyEnforcerTest.testUserHasAdminRoleServiceTenant(AbstractPolicyEnforcerTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1036)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:883)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

:package: integration-tests/observability-lgtm

io.quarkus.observability.test.LgtmReloadTest.testReload line 35 - History - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Lambda expression in io.quarkus.observability.test.LgtmTestHelper expected the predicate to return <true> but was null within 1 minutes  1 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AbstractHamcrestCondition.await(AbstractHamcrestCondition.java:86)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:712)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:729)
	at io.quarkus.observability.test.LgtmTestHelper.poke(LgtmTestHelper.java:25)
	at io.quarkus.observability.test.LgtmReloadTest.testReload(LgtmReloadTest.java:35)

:package: integration-tests/oidc-wiremock

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo line 325 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-only
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.doTestCodeFlowUserInfo(CodeFlowAuthorizationTest.java:557)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfo(CodeFlowAuthorizationTest.java:325)

io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken line 345 - History - More details - Source on GitHub

org.htmlunit.FailingHttpStatusCodeException: 404 Not Found for http://localhost:8081/code-flow-user-info-github-cached-in-idtoken
	at org.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:749)
	at org.htmlunit.WebClient.getPage(WebClient.java:503)
	at org.htmlunit.WebClient.getPage(WebClient.java:402)
	at org.htmlunit.WebClient.getPage(WebClient.java:538)
	at org.htmlunit.WebClient.getPage(WebClient.java:520)
	at io.quarkus.it.keycloak.CodeFlowAuthorizationTest.testCodeFlowUserInfoCachedInIdToken(CodeFlowAuthorizationTest.java:345)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

quarkus-bot[bot] avatar Apr 18 '25 07:04 quarkus-bot[bot]

Sorry for not commenting but I'm on PTO till next week (Fri) so I can comment with phone :P

@franz1981 no worries, I don't mind the back and forth as long as its not too nitpicky :)

Postremus avatar Apr 20 '25 16:04 Postremus

I would suggest to just return the match without allocating any list, but just the top match

I now return an Iterator, which allows to try the next match.

Postremus avatar Apr 20 '25 18:04 Postremus


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5d81443a77c7a0e8734b21793569e9a92ca14649.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

quarkus-bot[bot] avatar Apr 20 '25 20:04 quarkus-bot[bot]

drafting this PR, I might be able to simplify more in the common case (where only one path matches).

Postremus avatar Apr 24 '25 13:04 Postremus

BTW, I'm not expecting things to go overcomplicated for very minor gains. I.e. maybe keeping the iterator makes sense but making sure that we don't allocate additional things given there's only one item.

I will let you judge of finding the best compromise between readability and performance impact.

gsmet avatar Apr 24 '25 13:04 gsmet

@gsmet @franz1981 Next attempt. This time no additional allocations in the common path; simpler diff.

Postremus avatar Apr 28 '25 19:04 Postremus


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 80435fc7fa156c9589cdf7f0ef938ab0f645d6fa.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[!WARNING] There are other workflow runs running, you probably need to wait for their status before merging.

quarkus-bot[bot] avatar Apr 28 '25 19:04 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9b0f3e2b4666fa7a496f72144a14e56b34a733b4.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
JVM Integration Tests - JDK 17 Build Failures Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 17 Windows Logs Raw logs :mag:
:heavy_check_mark: JVM Integration Tests - JDK 21 Logs Raw logs :mag:

Full information is available in the Build summary check run. You can consult the Develocity build scans.

Failures

:gear: JVM Integration Tests - JDK 17 #

- Failing: integration-tests/compose-devservices 

:package: integration-tests/compose-devservices

io.quarkus.it.compose.devservices.HealthChecksTest.health line 16 - History - More details - Source on GitHub

java.net.SocketTimeoutException: Read timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:288)
	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:314)
	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)
	at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:808)
	at java.base/java.net.Socket$SocketInputStream.read(Socket.java:966)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:161)
	at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)

io.quarkus.it.compose.devservices.kafka.KafkaServiceTest.test line 17 - History - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <204> but was <500>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)

Flaky tests - Develocity

:gear: JVM Integration Tests - JDK 21

:package: integration-tests/smallrye-context-propagation

io.quarkus.context.test.SimpleContextPropagationTest.testArcTCContextPropagationDisabled - History

  • expected: <2> but was: <3> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <2> but was: <3>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
	at io.quarkus.context.test.SimpleContextPropagationTest.testArcTCContextPropagationDisabled(SimpleContextPropagationTest.java:92)

quarkus-bot[bot] avatar Apr 28 '25 20:04 quarkus-bot[bot]

Very clever, well done!

@FroMage do you also want to take a look at this?

I looked. I don't understand it anymore than the original implementation.

But the test is valid, the improvement is there, and if it doesn't break anything, it's progress, so +1 from me 🤷

FroMage avatar May 05 '25 09:05 FroMage

@franz1981 any final comments?

geoand avatar May 05 '25 17:05 geoand

Will take a look tomorrow morning 🙏

franz1981 avatar May 05 '25 17:05 franz1981

👌

geoand avatar May 05 '25 18:05 geoand

@franz1981 Sorry for the ping, but it has been a few days since the last update. And I kind of want to get this off my plate :)

Postremus avatar May 12 '25 09:05 Postremus

@franz1981 is on PTO, so if he doesn't respond this week (which I fully expect him not to), I will just go ahead and merge this

geoand avatar May 13 '25 05:05 geoand


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9b0f3e2b4666fa7a496f72144a14e56b34a733b4.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
JVM Integration Tests - JDK 17 Download previously uploaded .m2 content :warning: Check → Logs Raw logs :construction:
:heavy_check_mark: JVM Integration Tests - JDK 17 Windows Logs Raw logs :construction:
:heavy_check_mark: JVM Integration Tests - JDK 21 Logs Raw logs :construction:

quarkus-bot[bot] avatar May 13 '25 05:05 quarkus-bot[bot]

FYI: I rebased the PR onto main

geoand avatar May 13 '25 05:05 geoand

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6b67e052ff355782ac4eba5d5f392c702c594a7d.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

quarkus-bot[bot] avatar May 13 '25 07:05 quarkus-bot[bot]