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

Fix ambiguous bean resolution for sub resource interfaces with multiple implementations #47197

Merged
merged 1 commit into from
Apr 9, 2025
Merged

Fix ambiguous bean resolution for sub resource interfaces with multiple implementations #47197

merged 1 commit into from
Apr 9, 2025
+132 −1

Conversation

Postremus
Copy link
Member

@Postremus Postremus commented Apr 5, 2025 •
edited
Loading

Sub resource interfaces need to be scanned because of tests like this https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-reactive/rest/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/resource/basic/resource/ResourceLocatorBaseResource.java#L50

Essentially, no implemting class of the interface exists, only a Proxy.newProxyInstance. The spec does not mention that this is allowed. I have personally never seen anyone use this sort of pattern - but this is a test that was copied from resteasy classic. So propably someone does use this.

So instead of removing the test, and not scanning the sub resource interface anymore for the resource methods, no endpoint factory is created anymore for interfaces.
The result of a sub resource locator method, where the interface might have been used in practice, already returns an instance. So there is need for the endpoint factory for interfaces.

The same might be true for abstract classes with multiple implementations. However, #47003, prevents the ambiguous bean resolution error for them. I added a test case for abstract classes, to make sure they stay working at least.

closes #47107

@Postremus Postremus force-pushed the issues/47107-amb-res-bean-fact branch from 665f694 to 0dfd5fb Compare April 5, 2025 18:20
@quarkus-bot quarkus-bot bot added the area/rest label Apr 5, 2025
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@Postremus Postremus force-pushed the issues/47107-amb-res-bean-fact branch from 0dfd5fb to 558299b Compare April 7, 2025 16:21
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Apr 7, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

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

✅ 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.

Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/injectmock

✖ io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy - History

  • Mock accessed after inline mocks were cleared - org.mockito.exceptions.misusing.DisabledMockException
org.mockito.exceptions.misusing.DisabledMockException: Mock accessed after inline mocks were cleared
	at io.quarkus.it.mockbean.PerClassSpyTest$IdentityService.call(PerClassSpyTest.java:43)
	at io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy(PerClassSpyTest.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1026)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:873)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

⚙️ JVM Integration Tests - JDK 17 Windows

📦 integration-tests/injectmock

✖ io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy - History

  • Mock accessed after inline mocks were cleared - org.mockito.exceptions.misusing.DisabledMockException
org.mockito.exceptions.misusing.DisabledMockException: Mock accessed after inline mocks were cleared
	at io.quarkus.it.mockbean.PerClassSpyTest$IdentityService.call(PerClassSpyTest.java:43)
	at io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy(PerClassSpyTest.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1026)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:873)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/injectmock

✖ io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy - History

  • Mock accessed after inline mocks were cleared - org.mockito.exceptions.misusing.DisabledMockException
org.mockito.exceptions.misusing.DisabledMockException: Mock accessed after inline mocks were cleared
	at io.quarkus.it.mockbean.PerClassSpyTest$IdentityService.call(PerClassSpyTest.java:43)
	at io.quarkus.it.mockbean.PerClassSpyTest.testWithoutSpy(PerClassSpyTest.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1026)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:873)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

@Postremus Postremus requested a review from geoand April 8, 2025 13:34
geoand
geoand approved these changes Apr 9, 2025
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand geoand merged commit 13e2810 into quarkusio:main Apr 9, 2025
51 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.22 - main milestone Apr 9, 2025
@Postremus Postremus deleted the issues/47107-amb-res-bean-fact branch April 9, 2025 07:14
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

Assignees
No one assigned
Labels
area/rest kind/bugfix triage/backport triage/flaky-test
Projects
None yet
Milestone
3.22 - main
Development

Successfully merging this pull request may close these issues.

Ambiguous Bean Resolution in ResteasyReactiveCommonRecorder
3 participants
@Postremus @geoand @gsmet