slachiewicz commented on code in PR #13327:
URL: https://github.com/apache/maven/pull/13327#discussion_r4179222700
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/SisuDiBridgeModule.java:
##########
@@ -181,9 +201,11 @@ private <Q> Supplier<Q> getBeanSupplier(Dependency<Q> dep,
Key<Q> key) {
// Add DI bindings
list.addAll(getBindings().getOrDefault(key, Set.of()));
// Add Plexus bindings
- for (var bean : locator.get().locate(toGuiceKey(key))) {
- if (isPlexusBean(bean)) {
- list.add(new
BindingToBeanEntry<>(key).toBeanEntry(bean).prioritize(bean.getRank()));
+ if (!sisuFallbackOnly || list.isEmpty()) {
Review Comment:
Intentional, so I documented it rather than guarding it (9f2af82861).
`sisuFallbackOnly` governs which single bean is *selected* for an exact
(qualified) key. `List`/`Map` injection means "every implementation": guarding
on "the injector has any binding of the element type" would drop the plugin's
Sisu components as soon as the plugin also ships one DI-annotated
implementation, which is the bug this PR fixes, and collections would then
behave differently from the core bridge.
The scoped-proxy problem doesn't arise in practice: there is one
session/project/execution/log per mojo, so nothing injects them as a
collection. `getAllBindings` isn't reachable on the mojo injector either, since
it is a local in `loadV4Mojo` and not bound to itself.
I also checked the merge with wagon-style providers: Plexus `components.xml`
components with role hints `http`/`file` and `per-lookup` (as
wagon-http/wagon-file 3.5.3 ship them) plus a JSR-330 `@Named("sftp")` one. A
v4 mojo injecting `Map<String, Transport>` gets all three keyed by hint,
`List<Transport>` gets all three, and `per-lookup` is kept: each
`map.get("http")` returns a new instance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
