quarkus
quarkus copied to clipboard
Support overlapping paths on resource classes
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
: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
✖ 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 processFailed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.2:test (default-test) on project quarkus-integration-test-oidc-wiremock-logout:
cc @franz1981 as this one could potentially have a slight performance impact
I have added few comments
: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)
Please @geoand wait to merge this till the comments are addressed or at least a round of profiling is performed 🙏
That's why I removed the waiting-for-ci label :)
: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)
@franz1981 PTAL
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
: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)
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 :)
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.
: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.
drafting this PR, I might be able to simplify more in the common case (where only one path matches).
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 @franz1981 Next attempt. This time no additional allocations in the common path; simpler diff.
: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.
: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)
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 🤷
@franz1981 any final comments?
Will take a look tomorrow morning 🙏
👌
@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 :)
@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
: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: |
FYI: I rebased the PR onto main
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.