gnodet-bot commented on code in PR #13366:
URL: https://github.com/apache/maven/pull/13366#discussion_r4228181685
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultMojoExecution.java:
##########
@@ -38,110 +38,163 @@
import org.apache.maven.api.plugin.descriptor.lifecycle.Lifecycle;
import org.apache.maven.api.xml.XmlNode;
import org.apache.maven.impl.DefaultNode;
-import org.codehaus.plexus.util.xml.Xpp3Dom;
import org.eclipse.aether.graph.DependencyNode;
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Immutable snapshot of a mojo execution, captured at the point when
execution begins
+ * (after configuration merging, descriptor resolution, and lifecycle phase
assignment are complete).
+ * All state is copied at construction time; no reference to the mutable legacy
+ * {@link org.apache.maven.plugin.MojoExecution} is retained after
construction.
+ */
public class DefaultMojoExecution implements MojoExecution {
- private final InternalMavenSession session;
- private final org.apache.maven.plugin.MojoExecution delegate;
+
+ private final Plugin plugin;
+ private final Optional<PluginExecution> model;
+ private final MojoDescriptor descriptor;
+ private final Optional<String> executionId;
+ private final String goal;
+ private final Optional<String> lifecyclePhase;
+ private final XmlNode configuration;
public DefaultMojoExecution(InternalMavenSession session,
org.apache.maven.plugin.MojoExecution delegate) {
- this.session = session;
- this.delegate = delegate;
+ requireNonNull(session, "session");
+ requireNonNull(delegate, "delegate");
+ this.descriptor = requireNonNull(delegate.getMojoDescriptor(),
"delegate.mojoDescriptor")
+ .getMojoDescriptorV4();
+ this.executionId = Optional.ofNullable(delegate.getExecutionId());
+ this.goal = requireNonNull(delegate.getGoal(), "delegate.goal");
+ this.lifecyclePhase =
Optional.ofNullable(delegate.getLifecyclePhase());
+ this.configuration = delegate.getConfiguration() != null
+ ? delegate.getConfiguration().getDom()
+ : null;
+ this.plugin = buildPlugin(session, delegate);
+ this.model = buildModel(delegate);
}
- public org.apache.maven.plugin.MojoExecution getDelegate() {
- return delegate;
- }
+ private static Plugin buildPlugin(InternalMavenSession session,
org.apache.maven.plugin.MojoExecution delegate) {
+ org.apache.maven.plugin.descriptor.MojoDescriptor legacyDescriptor =
delegate.getMojoDescriptor();
+ org.apache.maven.plugin.descriptor.PluginDescriptor
legacyPluginDescriptor =
+ legacyDescriptor.getPluginDescriptor();
+ PluginDescriptor pluginDescriptorV4 =
legacyPluginDescriptor.getPluginDescriptorV4();
+
+ ClassLoader classLoader = legacyDescriptor.getRealm();
+
+ org.apache.maven.artifact.Artifact legacyArtifact =
legacyPluginDescriptor.getPluginArtifact();
+ org.eclipse.aether.artifact.Artifact resolverArtifact =
RepositoryUtils.toArtifact(legacyArtifact);
+ Artifact artifact = resolverArtifact != null ?
session.getArtifact(resolverArtifact) : null;
+
+ DependencyNode resolverNode =
legacyPluginDescriptor.getDependencyNode();
+ Map<String, Dependency> dependenciesMap = resolverNode != null
+ ? Collections.unmodifiableMap(new DefaultNode(session,
resolverNode, false)
+ .stream()
+ .filter(Objects::nonNull)
+ .map(Node::getDependency)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toMap(
+ d -> d.getGroupId() + ":" +
d.getArtifactId(),
+ d -> d,
+ (a, b) -> a))) // first-wins on
duplicate groupId:artifactId
+ // (transitive duplicates are normal)
+ : Collections.emptyMap();
+
+ org.apache.maven.api.model.Plugin modelPlugin =
+ delegate.getPlugin() != null ?
delegate.getPlugin().getDelegate() : null;
+
+ // Eagerly capture lifecycle mappings so the Plugin snapshot is truly
immutable.
+ // getLifecycleMappings() reads from the plugin JAR; it may fail if
the artifact
+ // is not yet resolved (e.g. CLI-invoked goals) — fall back to an
empty list.
+ List<Lifecycle> lifecycles;
+ try {
+ lifecycles = Collections.unmodifiableList(new ArrayList<>(
+ legacyPluginDescriptor.getLifecycleMappings().values()));
+ } catch (Exception e) {
+ lifecycles = Collections.emptyList();
Review Comment:
⚠️ **Silent exception swallowing — behavioral change from prior code**
The old `getLifecycles()` threw `RuntimeException("Unable to load plugin
lifecycles", e)` on failure. This `catch` block replaces that with
`Collections.emptyList()` and discards the exception entirely. If
`getLifecycleMappings()` fails for a reason other than "no lifecycle metadata"
(e.g., corrupt JAR, I/O error, malformed XML), the failure vanishes silently
and the plugin appears to have no lifecycles — a diagnosable error becomes
invisible.
The comment says "may fail if the artifact is not yet resolved (e.g.
CLI-invoked goals)" but `DefaultMojoExecution` is constructed inside
`executeMojo()` — at that point the artifact IS resolved. A failure here is
almost certainly a genuine problem worth surfacing.
At minimum, log at debug level:
```suggestion
// Eagerly capture lifecycle mappings so the Plugin snapshot is
truly immutable.
// getLifecycleMappings() reads from the plugin JAR; plugins that do
not bundle
// custom lifecycle metadata will throw here — fall back to an empty
list.
List<Lifecycle> lifecycles;
try {
lifecycles = Collections.unmodifiableList(new ArrayList<>(
legacyPluginDescriptor.getLifecycleMappings().values()));
} catch (Exception e) {
org.slf4j.LoggerFactory.getLogger(DefaultMojoExecution.class)
.debug("Unable to load lifecycle mappings for {}: {}",
legacyPluginDescriptor.getId(), e.getMessage());
lifecycles = Collections.emptyList();
}
```
--
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]
