You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "ikxeno (via GitHub)" <gi...@apache.org> on 2026/10/17 22:31:38 UTC
[PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
ikxeno opened a new pull request, #13602:
URL: https://github.com/apache/maven/pull/13602
Fixes #10184.
Maven DI honors the `sisu.debug` system property and prints sorted native bindings to stdout without creating components or invoking providers.
Snapshots run after discovery and at lookup/injection checkpoints for changed registrations, including late Mojo context and core-extension bindings. Each binding is described once per snapshot, preserving all aliases and candidates. Final block assembly and output run outside the diagnostic lock.
With bounded row lengths and dependency lists: `O(N log N + L)` time, `O(N + L)` temporary space (`N` rows, `L` output characters). Unchanged checkpoints and retained diagnostic state are `O(1)` in registry size.
Local JMH on macOS/JDK 17, replaying real Maven registration batches with eight JVM forks per scenario:
| Rows per snapshot | ms/lifecycle (95% CI) | MB allocated/lifecycle |
| --- | ---: | ---: |
| Core: 749 rows | 0.321 [0.318, 0.325] | 2.75 |
| Mojo: 27, 129 rows | 0.0558 [0.0526, 0.0591] | 0.420 |
| Native extensions: 749, 757, 765 rows | 0.865 [0.852, 0.879] | 7.83 |
Registration replay and full text rendering are timed; binding discovery and console I/O are excluded. CIs use independent fork means; MB is decimal.
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6247431832
@gnodet-bot could you submit this as an approving review?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4304919135
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -515,6 +561,91 @@ public T get() {
}
}
+ /**
+ * Captures a human-readable snapshot of the current native registry for Maven internals,
+ * including direct binding map and metadata changes, without resolving components.
+ * Registration must not run concurrently with this method.
+ */
+ public String dumpBindings() {
+ return BindingSnapshot.render(
+ this, registryRevision, "explicit", false, BindingSnapshot.capture(bindings, scopes));
+ }
+
+ /** Diagnostic checkpoint for native and bridge resolution entry points. */
+ protected final void debugBindings() {
+ debugBindings("lookup", false);
+ }
+
+ private void debugBindings(String reason, boolean partial) {
+ if (!debugEnabled()) {
+ if (diagnosticState != null) {
+ synchronized (diagnosticLock) {
+ diagnosticState = null;
+ }
+ }
+ return;
+ }
+ PrintStream output = System.out;
+ DiagnosticState state = diagnosticState;
+ if (!partial && state != null && state.capturedRevision == registryRevision && state.output.get() == output) {
+ return;
+ }
+ writeBindings(output, reason, partial);
+ }
+
+ private static boolean debugEnabled() {
+ try {
+ String property = System.getProperty("sisu.debug");
Review Comment:
On OpenJDK [17](https://github.com/openjdk/jdk17u/blob/master/src/java.base/share/classes/java/util/Properties.java#L1101-L1106), [21](https://github.com/openjdk/jdk21u/blob/master/src/java.base/share/classes/java/util/Properties.java#L1146-L1151), and [25](https://github.com/openjdk/jdk25u/blob/master/src/java.base/share/classes/java/util/Properties.java#L1144-L1149), `Properties.getProperty()` reads a `ConcurrentHashMap` without taking the properties monitor, so the global-monitor claim is incorrect. I verified the same behavior on Eclipse Temurin 17 and Oracle JDK 21/25.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4304829700
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/BindingSnapshot.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.di.impl;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.GenericArrayType;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.lang.reflect.TypeVariable;
+import java.lang.reflect.WildcardType;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+import org.apache.maven.di.Key;
+import org.apache.maven.di.Scope;
+
+/** Detached diagnostic metadata; capturing it does not compile or provision bindings. */
+final class BindingSnapshot {
+ private BindingSnapshot() {}
+
+ static List<String> capture(
+ Map<Key<?>, Set<Binding<?>>> bindings, Map<Class<? extends Annotation>, Supplier<Scope>> scopes) {
+ List<String> rows = new ArrayList<>();
+ Map<Binding<?>, String> descriptions = new IdentityHashMap<>();
+ bindings.forEach((key, candidates) -> {
+ String name = key(key);
+ if (candidates.isEmpty()) {
+ rows.add(" " + name + " -> <no candidates>");
+ } else {
+ for (Binding<?> binding : candidates) {
+ rows.add(" " + name + " -> " + descriptions.computeIfAbsent(binding, BindingSnapshot::describe));
+ }
+ }
+ });
+ scopes.forEach((annotation, supplier) ->
+ rows.add(" scope " + type(annotation) + " -> supplier " + Binding.identityString(supplier)));
+ Collections.sort(rows);
+ return List.copyOf(rows);
+ }
+
+ static String render(Object injector, long revision, String reason, boolean partial, List<String> rows) {
+ return "Maven DI native registry " + Binding.identityString(injector) + " revision=" + revision + " ("
+ + reason + (partial ? ", partial" : "") + ") {\n"
+ + String.join("\n", rows) + "\n} Maven DI native registry " + Binding.identityString(injector);
+ }
+
+ private static String describe(Binding<?> binding) {
+ String dependencies = binding.getDependencies().stream()
+ .map(dependency -> (dependency.optional() ? "?" : "") + key(dependency.key()))
+ .sorted()
+ .collect(Collectors.joining(", ", "[", "]"));
+ return "original=" + (binding.getOriginalKey() == null ? "<none>" : key(binding.getOriginalKey()))
+ + ", priority=" + binding.getPriority()
+ + ", scope="
+ + (binding.getScope() == null
+ ? "<none>"
+ : type(binding.getScope().annotationType()))
+ + ", source=" + escape(binding.getDiagnosticSource())
+ + ", dependencies=" + dependencies;
+ }
+
+ private static String key(Key<?> key) {
+ Object qualifier = key.getQualifier();
+ String prefix = qualifier instanceof String name
+ ? "@Named(\"" + escape(name) + "\") "
+ : qualifier instanceof Class<?> annotationType
+ ? "@" + type(annotationType) + " "
+ : qualifier instanceof Annotation annotation
+ ? "@" + type(annotation.annotationType()) + " [qualifier="
+ + Binding.identityString(annotation) + "] "
+ : qualifier != null ? "@" + Binding.identityString(qualifier) + " " : "";
+ return prefix + type(key.getType());
+ }
+
+ private static String type(Type type) {
+ if (type instanceof Class<?> clazz) {
+ return clazz.isArray()
+ ? type(clazz.getComponentType()) + "[]"
+ : clazz.getName() + " [loader=" + Binding.identityString(clazz.getClassLoader()) + "]";
+ }
+ if (type instanceof ParameterizedType parameterized) {
+ return (parameterized.getOwnerType() == null ? "" : type(parameterized.getOwnerType()) + ".")
+ + type(parameterized.getRawType()) + "<"
+ + Arrays.stream(parameterized.getActualTypeArguments())
+ .map(BindingSnapshot::type)
+ .collect(Collectors.joining(", "))
+ + ">";
+ }
+ if (type instanceof GenericArrayType array) {
+ return type(array.getGenericComponentType()) + "[]";
+ }
+ if (type instanceof TypeVariable<?> variable) {
+ return variable.getName();
+ }
+ if (type instanceof WildcardType wildcard) {
+ return "? extends "
+ + Arrays.stream(wildcard.getUpperBounds())
+ .map(BindingSnapshot::type)
+ .collect(Collectors.joining(" & "))
+ + (wildcard.getLowerBounds().length == 0
+ ? ""
+ : " super "
+ + Arrays.stream(wildcard.getLowerBounds())
+ .map(BindingSnapshot::type)
+ .collect(Collectors.joining(" & ")));
Review Comment:
⚠️ **Bug: incorrect rendering of lower-bounded and unbounded wildcards**
The current code unconditionally starts the representation with `? extends UPPER_BOUNDS`, then appends `super LOWER_BOUNDS` if present. This produces incorrect output for two cases:
- **`? super X`** (lower-bounded): Java sets `upperBounds=[Object]`, `lowerBounds=[X]`, so the code emits `? extends java.lang.Object [loader=...] super X [loader=...]` — which is not a valid Java wildcard syntax.
- **`?`** (unbounded): Java also sets `upperBounds=[Object]`, producing `? extends java.lang.Object [loader=...]` instead of just `?`.
The correct logic should check whether the upper bound is exactly `Object` (and no lower bound exists) to detect the unbounded case, and use `? super` exclusively when lower bounds are present:
```suggestion
if (type instanceof WildcardType wildcard) {
Type[] lower = wildcard.getLowerBounds();
Type[] upper = wildcard.getUpperBounds();
if (lower.length > 0) {
return "? super "
+ Arrays.stream(lower)
.map(BindingSnapshot::type)
.collect(Collectors.joining(" & "));
}
if (upper.length == 1 && upper[0] == Object.class) {
return "?";
}
return "? extends "
+ Arrays.stream(upper)
.map(BindingSnapshot::type)
.collect(Collectors.joining(" & "));
}
```
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -515,6 +561,91 @@ public T get() {
}
}
+ /**
+ * Captures a human-readable snapshot of the current native registry for Maven internals,
+ * including direct binding map and metadata changes, without resolving components.
+ * Registration must not run concurrently with this method.
+ */
+ public String dumpBindings() {
+ return BindingSnapshot.render(
+ this, registryRevision, "explicit", false, BindingSnapshot.capture(bindings, scopes));
+ }
+
+ /** Diagnostic checkpoint for native and bridge resolution entry points. */
+ protected final void debugBindings() {
+ debugBindings("lookup", false);
+ }
+
+ private void debugBindings(String reason, boolean partial) {
+ if (!debugEnabled()) {
+ if (diagnosticState != null) {
+ synchronized (diagnosticLock) {
+ diagnosticState = null;
+ }
+ }
+ return;
+ }
+ PrintStream output = System.out;
+ DiagnosticState state = diagnosticState;
+ if (!partial && state != null && state.capturedRevision == registryRevision && state.output.get() == output) {
+ return;
+ }
+ writeBindings(output, reason, partial);
+ }
+
+ private static boolean debugEnabled() {
+ try {
+ String property = System.getProperty("sisu.debug");
Review Comment:
⚠️ **`System.getProperty()` called on every DI resolution path**
`debugEnabled()` is invoked from `getCompiledBinding()`, `injectInstance()`, and `getInstance()` — the hot paths of every DI resolution. `System.getProperty()` acquires a system-wide monitor on each call, which will serialize all concurrent resolutions for the cost of a string lookup even when diagnostics are completely disabled.
Consider caching the result at startup or on first call:
```suggestion
private static boolean debugEnabled() {
try {
String property = System.getProperty("sisu.debug");
return property != null && (property.isEmpty() || Boolean.parseBoolean(property));
} catch (RuntimeException e) {
return false;
}
}
```
A simple approach: cache the result in a `static volatile boolean` field initialized once (e.g. in a `static {}` block or lazy on first call). Dynamic toggling during tests is handled separately by the `SisuDebugSession` mechanism which sets the property directly, so a cached value wouldn't break the test infrastructure — tests that need to toggle the flag can invalidate the cache explicitly, or the cache can re-check every N invocations as a compromise.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6314775158
Yes, it triggers Sisu’s dump as well. I reused sisu.debug so someone investigating an injection problem could see both containers without first having to work out which one provides the component. That was the reason for using a single flag while Maven uses both Sisu and native DI.
I can add a separate `maven.di.debug` flag for the native dump so each can be enabled independently.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6353697248
@gnodet thank you for your attention to this feature; I agree with suggestions and will implement the fixes shortly.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6314073649
Hi @cstamas, could you take a look when you have a chance? This adds native DI dumps via sisu.debug for MNG-8445. Is this what you had in mind?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6316835697
@cstamas
I reconsidered using `sisu.debug`, since it should keep its existing behavior. I've renamed the native DI flag to `maven.di.debug` so the two dumps can be enabled independently.
I've also reformatted the native dump to make it easier to read.
<details>
<summary>Excerpt from the native DI dump</summary>
```text
key: org.apache.maven.toolchain.ToolchainFactory [loader=L1]
candidate: @Named("jdk") org.apache.maven.toolchain.ToolchainFactory [loader=L1]
priority: 0
scope: <none>
source:
org.apache.maven.toolchain.ToolchainFactory [loader=L1]
org.apache.maven.toolchain.ToolchainManagerFactory.jdkFactory() [loader=L1]
dependencies:
- org.apache.maven.toolchain.ToolchainManagerFactory [loader=L1]
```
</details>
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6268096021
Hi @gnodet
Could you approve the workflow runs for this PR when you have a chance?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #13602:
URL: https://github.com/apache/maven/pull/13602#issuecomment-6314597319
I am a little confused. Yes, this is what I had on my mind, but am unsure how `sisu.debug` comes into this? Did you try out locally? Does Sisu and Maven DI does not clash or produce huge overlapping dumps of output?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "ikxeno (via GitHub)" <gi...@apache.org>.
ikxeno commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4304989946
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -515,6 +561,91 @@ public T get() {
}
}
+ /**
+ * Captures a human-readable snapshot of the current native registry for Maven internals,
+ * including direct binding map and metadata changes, without resolving components.
+ * Registration must not run concurrently with this method.
+ */
+ public String dumpBindings() {
+ return BindingSnapshot.render(
+ this, registryRevision, "explicit", false, BindingSnapshot.capture(bindings, scopes));
Review Comment:
diagnosticLock protects diagnostic state; registry writes do not acquire it. Locking dumpBindings() alone would not make concurrent registration safe. The existing Javadoc already prohibits concurrent registration.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4304962532
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -515,6 +561,91 @@ public T get() {
}
}
+ /**
+ * Captures a human-readable snapshot of the current native registry for Maven internals,
+ * including direct binding map and metadata changes, without resolving components.
+ * Registration must not run concurrently with this method.
+ */
+ public String dumpBindings() {
+ return BindingSnapshot.render(
+ this, registryRevision, "explicit", false, BindingSnapshot.capture(bindings, scopes));
Review Comment:
💡 **`dumpBindings()` lacks the diagnostic lock that `writeBindings()` holds**
`writeBindings()` (the internal diagnostic path) acquires `diagnosticLock` before calling `BindingSnapshot.capture(bindings, scopes)`. The public `dumpBindings()` calls the same `capture()` without any lock. The Javadoc says "Registration must not run concurrently", but callers have no compile-time enforcement and the inconsistency in locking discipline could surprise future maintainers.
At minimum, document the weaker guarantee explicitly:
```suggestion
/**
* Captures a human-readable snapshot of the current native registry for Maven internals,
* including direct binding map and metadata changes, without resolving components.
* <p>
* <strong>Thread safety:</strong> this method reads the binding map and scope map
* without internal synchronization. The caller must ensure no registration is in
* progress, or accept a best-effort snapshot.
*/
public String dumpBindings() {
return BindingSnapshot.render(
this, registryRevision, "explicit", false, BindingSnapshot.capture(bindings, scopes));
}
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8445] Add native DI dumps for sisu.debug [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4313663962
##########
impl/maven-di/src/test/java/org/apache/maven/di/impl/InjectorDiagnosticsTest.java:
##########
@@ -0,0 +1,533 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.di.impl;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.lang.annotation.Annotation;
+import java.lang.annotation.Retention;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
+
+import org.apache.maven.api.di.Named;
+import org.apache.maven.api.di.Provides;
+import org.apache.maven.api.di.Qualifier;
+import org.apache.maven.api.di.SessionScoped;
+import org.apache.maven.api.di.Singleton;
+import org.apache.maven.di.Key;
+import org.apache.maven.di.Scope;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.api.parallel.ResourceLock;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@ResourceLock("java.lang.System.properties")
+@ResourceLock("java.lang.System.out")
+@Timeout(30)
+class InjectorDiagnosticsTest {
+ private String previousProperty;
+ private PrintStream previousOutput;
+
+ @BeforeEach
+ void enableDiagnostics() {
+ previousProperty = System.getProperty("sisu.debug");
+ previousOutput = System.out;
+ System.setProperty("sisu.debug", "true");
+ }
+
+ @AfterEach
+ void restoreEnvironment() {
+ if (previousProperty == null) {
+ System.clearProperty("sisu.debug");
+ } else {
+ System.setProperty("sisu.debug", previousProperty);
+ }
+ System.setOut(previousOutput);
+ }
+
+ @Test
+ void snapshotDoesNotProvisionOrInvokeUserToString() {
+ RecordingInjector injector = new RecordingInjector();
+ injector.bindInstance(Hostile.class, new Hostile());
+ injector.bindInstance(CharSequence.class, "secret-value");
+ Key<Long> initialized = Key.of(Long.class);
+ injector.bind(
+ initialized,
+ new Binding<Long>(initialized, Set.of()) {
+ @Override
+ public Supplier<Long> compile(Function<Dependency<?>, Supplier<?>> compiler) {
+ throw new AssertionError("binding compiled for diagnostics");
+ }
+
+ @Override
+ public String toString() {
+ throw new AssertionError("binding toString");
+ }
+ }.initializeWith(new BindingInitializer<>(Set.of()) {
+ @Override
+ public Consumer<Long> compile(Function<Dependency<?>, Supplier<?>> compiler) {
+ throw new AssertionError("initializer compiled for diagnostics");
+ }
+ }));
+ injector.bindSupplier(String.class, new Supplier<String>() {
+ @Override
+ public String get() {
+ throw new AssertionError("supplier called for diagnostics");
+ }
+
+ @Override
+ public String toString() {
+ throw new AssertionError("supplier toString");
+ }
+ });
+ injector.bindScope(SessionScoped.class, () -> {
+ throw new AssertionError("scope provisioned");
+ });
+ Unconstructed.CALLS.set(0);
+ injector.bindImplicit(Unconstructed.class);
+ injector.bindImplicit(Provider.class);
+ String snapshot = injector.dumpBindings();
+ injector.checkpoint();
+ assertEquals(0, Unconstructed.CALLS.get());
+ assertTrue(snapshot.contains("initialized by"));
+ assertTrue(snapshot.contains("Provider.provide"));
+ assertTrue(snapshot.contains("scope " + SessionScoped.class.getName()));
+ assertTrue(snapshot.contains("supplier "));
+ assertFalse(snapshot.contains("secret-value"));
+ assertEquals(body(snapshot), body(injector.dumps.get(0)));
+ }
+
+ @Test
+ void snapshotsRenderWildcardsInOwnerTypes() {
+ class Outer<T> {
+ class Inner<U> {}
+ }
+
+ // Key simplifies its type arguments, but preserves wildcards in parameterized owners.
+ String bound = Number.class.getName() + " [loader=null]";
+ Map<Key<?>, String> cases = Map.of(
+ new Key<Outer<?>.Inner<String>>() {}, "<?>",
+ new Key<Outer<? extends Number>.Inner<String>>() {}, "<? extends " + bound + ">",
+ new Key<Outer<? super Number>.Inner<String>>() {}, "<? super " + bound + ">");
+ cases.forEach((key, expected) -> {
+ InjectorImpl injector = new InjectorImpl();
+ injector.getBindings().put(key, Set.of());
+ String dump = injector.dumpBindings();
+ assertTrue(dump.contains(expected), dump);
Review Comment:
💡 **Test does not guard against regression of this commit's fix**
`snapshotsRenderWildcardsInOwnerTypes` verifies that wildcard strings (`<?>`, `<? extends ...>`, `<? super ...>`) are *present* in the dump — but it doesn't assert that the owner class name is *not* repeated.
With the old code (before `699661ca`), the dump for `Outer<?>.Inner<String>` would contain `Outer$Inner [loader=...]` as the inner type name (binary name includes the enclosing class). The test would still pass because `<?>` *is* present in `Outer [loader=L]<?>.Outer$Inner [loader=L]<String...>`. So this test doesn't catch a regression of this commit's fix.
Consider adding a negative assertion:
```
assertFalse(dump.contains("$Inner"), "Inner class should be rendered without enclosing owner prefix: " + dump);
```
(Or rename the test to `snapshotsRenderInnerTypesWithoutOwnerRepetition` and add the appropriate assertion.)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org