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

BeanManager.getBeans() - assume the default qualifier if none specified #27582

Merged
merged 1 commit into from Aug 30, 2022

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Aug 30, 2022

  • this behavior is defined by the spec

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Aug 30, 2022
@mkouba
Copy link
Contributor Author

mkouba commented Aug 30, 2022

@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 30, 2022
@quarkus-bot

This comment has been minimized.

@mkouba
Copy link
Contributor Author

mkouba commented Aug 30, 2022

The CI failure seems to be related...

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 30, 2022

Failing Jobs - Building 60f62f7

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 18 Build Failures Logs Raw logs
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 18 #

- Failing: integration-tests/grpc-hibernate 

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.BlockingRawTest.shouldAdd line 61 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with com.example.grpc.hibernate.BlockingRawTest was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

@gsmet gsmet merged commit 9262a09 into quarkusio:main Aug 30, 2022
@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 30, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 30, 2022
@gsmet
Copy link
Member

gsmet commented Aug 30, 2022

@mkouba could you prepare a note about the potential compatibility issue? I see you had to add some qualifiers to some tests. Just add it as a comment here and I will include it in the 2.12.1 announcement blog post. Thanks!

@mkouba
Copy link
Contributor Author

mkouba commented Aug 31, 2022

@mkouba could you prepare a note about the potential compatibility issue? I see you had to add some qualifiers to some tests. Just add it as a comment here and I will include it in the 2.12.1 announcement blog post. Thanks!

So this change should only affect code that directly/indirectly makes use of the BeanManager.getBeans(Type, Annotation...) method and does not pass any required qualifiers, i.e. regular injection, programmatic lookup and code like beanManager.getBeans(Foo.class, MyQualifier.Literal.INSTANCE) is not affected. The difference is that previously if no qualifier was specified then the behavior was the same as if the @Any qualifier was used, i.e. all beans matching the required type were resolved. In general, a user can get an unsatisfied dependency exception at runtime (e.g. during tests when @InjectMock is used). The solution is to use an appropriate qualifier or @Any.

@manovotn
Copy link
Contributor

Note that this change is also aligning the behavior with CDI specification. Therefore, if users expected/relied on different behavior, it was not warranted by spec.
See https://github.com/jakartaee/cdi/blob/master/api/src/main/java/jakarta/enterprise/inject/spi/BeanContainer.java#L75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) release/breaking-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants