From 2942aadcc3c56e8cfe040c45dfdfcfc25fbb38e5 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 22 May 2024 15:45:13 +0200 Subject: [PATCH] [MNG-8123] Fix Lifecycle in v3 (#1524) --- .../maven/api/services/LifecycleRegistry.java | 4 +- .../internal/impl/standalone/ApiRunner.java | 4 - .../EmptyLifecycleBindingsInjector.java | 9 -- .../impl/DefaultLifecycleRegistry.java | 150 ++++++++++++++---- .../impl/DefaultPackagingRegistry.java | 2 +- .../maven/lifecycle/DefaultLifecycles.java | 8 +- .../org/apache/maven/lifecycle/Lifecycle.java | 7 +- .../lifecycle/internal/ExecutionPlanItem.java | 2 +- .../lifecycle/DefaultLifecyclesTest.java | 3 +- .../lifecycle/MavenExecutionPlanTest.java | 2 - .../EmptyLifecycleBindingsInjector.java | 9 -- 11 files changed, 132 insertions(+), 68 deletions(-) diff --git a/api/maven-api-core/src/main/java/org/apache/maven/api/services/LifecycleRegistry.java b/api/maven-api-core/src/main/java/org/apache/maven/api/services/LifecycleRegistry.java index 2718582a24..05970e76fc 100644 --- a/api/maven-api-core/src/main/java/org/apache/maven/api/services/LifecycleRegistry.java +++ b/api/maven-api-core/src/main/java/org/apache/maven/api/services/LifecycleRegistry.java @@ -18,16 +18,14 @@ */ package org.apache.maven.api.services; -import java.util.List; import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.apache.maven.api.Lifecycle; public interface LifecycleRegistry extends ExtensibleEnumRegistry, Iterable { + default Stream stream() { return StreamSupport.stream(spliterator(), false); } - - List computePhases(Lifecycle lifecycle); } diff --git a/maven-api-impl/src/test/java/org/apache/maven/internal/impl/standalone/ApiRunner.java b/maven-api-impl/src/test/java/org/apache/maven/internal/impl/standalone/ApiRunner.java index 2106a03467..b6e2bbc815 100644 --- a/maven-api-impl/src/test/java/org/apache/maven/internal/impl/standalone/ApiRunner.java +++ b/maven-api-impl/src/test/java/org/apache/maven/internal/impl/standalone/ApiRunner.java @@ -311,10 +311,6 @@ public class ApiRunner { @Provides static LifecycleRegistry newLifecycleRegistry() { return new LifecycleRegistry() { - @Override - public List computePhases(Lifecycle lifecycle) { - return List.of(); - } @Override public Iterator iterator() { diff --git a/maven-compat/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java b/maven-compat/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java index 1029f58690..513bb0236b 100644 --- a/maven-compat/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java +++ b/maven-compat/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java @@ -52,10 +52,6 @@ public class EmptyLifecycleBindingsInjector extends DefaultLifecycleBindingsInje private static PackagingRegistry packagingRegistry; private static final LifecycleRegistry emptyLifecycleRegistry = new LifecycleRegistry() { - @Override - public List computePhases(Lifecycle lifecycle) { - return List.of(); - } @Override public Iterator iterator() { @@ -136,11 +132,6 @@ public class EmptyLifecycleBindingsInjector extends DefaultLifecycleBindingsInje return getDelegate().lookup(id); } - @Override - public List computePhases(Lifecycle lifecycle) { - return getDelegate().computePhases(lifecycle); - } - @Override public Iterator iterator() { return getDelegate().iterator(); diff --git a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java index 4370d449e3..acd8fd74c2 100644 --- a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java +++ b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java @@ -20,6 +20,7 @@ package org.apache.maven.internal.impl; import javax.inject.Inject; import javax.inject.Named; +import javax.inject.Provider; import javax.inject.Singleton; import java.util.ArrayList; @@ -27,15 +28,24 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.maven.api.Lifecycle; +import org.apache.maven.api.model.Plugin; import org.apache.maven.api.services.LifecycleRegistry; +import org.apache.maven.api.services.LookupException; +import org.apache.maven.api.spi.ExtensibleEnumProvider; import org.apache.maven.api.spi.LifecycleProvider; +import org.apache.maven.lifecycle.mapping.LifecyclePhase; +import org.codehaus.plexus.PlexusContainer; +import org.codehaus.plexus.component.repository.exception.ComponentLookupException; import static java.util.Arrays.asList; import static java.util.Collections.singleton; @@ -47,23 +57,19 @@ import static org.apache.maven.internal.impl.Lifecycles.plugin; */ @Named @Singleton -public class DefaultLifecycleRegistry - extends ExtensibleEnumRegistries.DefaultExtensibleEnumRegistry - implements LifecycleRegistry { +public class DefaultLifecycleRegistry implements LifecycleRegistry { + + private final List providers; public DefaultLifecycleRegistry() { - super(Collections.emptyList()); + this(Collections.emptyList()); } @Inject - public DefaultLifecycleRegistry( - List providers, Map lifecycles) { - super( - concat(providers, new LifecycleWrapperProvider(lifecycles)), - new CleanLifecycle(), - new DefaultLifecycle(), - new SiteLifecycle(), - new WrapperLifecycle()); + public DefaultLifecycleRegistry(List providers) { + List p = new ArrayList<>(providers); + p.add(() -> List.of(new CleanLifecycle(), new DefaultLifecycle(), new SiteLifecycle(), new WrapperLifecycle())); + this.providers = p; // validate lifecycle for (Lifecycle lifecycle : this) { Set set = new HashSet<>(); @@ -78,37 +84,44 @@ public class DefaultLifecycleRegistry @Override public Iterator iterator() { - return values.values().iterator(); + return stream().toList().iterator(); } @Override public Stream stream() { - return values.values().stream(); - } - - static List concat(List l, T t) { - List nl = new ArrayList<>(l.size() + 1); - nl.addAll(l); - nl.add(t); - return nl; + return providers.stream().map(ExtensibleEnumProvider::provides).flatMap(Collection::stream); } @Override - public List computePhases(Lifecycle lifecycle) { - return lifecycle.phases().stream().map(Lifecycle.Phase::name).toList(); + public Optional lookup(String id) { + return stream().filter(lf -> Objects.equals(id, lf.id())).findAny(); } - static class LifecycleWrapperProvider implements LifecycleProvider { - private final Map lifecycles; + @Named + @Singleton + public static class LifecycleWrapperProvider implements LifecycleProvider { + private final PlexusContainer container; @Inject - LifecycleWrapperProvider(Map lifecycles) { - this.lifecycles = lifecycles; + public LifecycleWrapperProvider(PlexusContainer container) { + this.container = container; } @Override public Collection provides() { - return lifecycles.values().stream().map(this::wrap).collect(Collectors.toList()); + try { + Map all = + container.lookupMap(org.apache.maven.lifecycle.Lifecycle.class); + return all.keySet().stream() + .filter(id -> !Lifecycle.CLEAN.equals(id) + && !Lifecycle.DEFAULT.equals(id) + && !Lifecycle.SITE.equals(id) + && !Lifecycle.WRAPPER.equals(id)) + .map(id -> wrap(all.get(id))) + .collect(Collectors.toList()); + } catch (ComponentLookupException e) { + throw new LookupException(e); + } } private Lifecycle wrap(org.apache.maven.lifecycle.Lifecycle lifecycle) { @@ -120,13 +133,90 @@ public class DefaultLifecycleRegistry @Override public Collection phases() { - // TODO: implement - throw new UnsupportedOperationException(); + return lifecycle.getPhases().stream() + .map(name -> (Phase) new Phase() { + @Override + public String name() { + return name; + } + + @Override + public List plugins() { + Map lfPhases = lifecycle.getDefaultLifecyclePhases(); + LifecyclePhase phase = lfPhases != null ? lfPhases.get(name) : null; + if (phase != null) { + Map plugins = new LinkedHashMap<>(); + DefaultPackagingRegistry.parseLifecyclePhaseDefinitions(plugins, name, phase); + return plugins.values().stream().toList(); + } + return List.of(); + } + }) + .toList(); } }; } } + static class WrappedLifecycle extends org.apache.maven.lifecycle.Lifecycle { + WrappedLifecycle(Lifecycle lifecycle) { + super(lifecycle); + } + } + + abstract static class BaseLifecycleProvider implements Provider { + @Inject + private PlexusContainer lookup; + + private final String name; + + BaseLifecycleProvider(String name) { + this.name = name; + } + + @Override + public org.apache.maven.lifecycle.Lifecycle get() { + try { + LifecycleRegistry registry = lookup.lookup(LifecycleRegistry.class); + return new WrappedLifecycle(registry.require(name)); + } catch (ComponentLookupException e) { + throw new LookupException(e); + } + } + } + + @Singleton + @Named(Lifecycle.CLEAN) + static class CleanLifecycleProvider extends BaseLifecycleProvider { + CleanLifecycleProvider() { + super(Lifecycle.CLEAN); + } + } + + @Singleton + @Named(Lifecycle.DEFAULT) + static class DefaultLifecycleProvider extends BaseLifecycleProvider { + DefaultLifecycleProvider() { + super(Lifecycle.DEFAULT); + } + } + + @Singleton + @Named(Lifecycle.SITE) + static class SiteLifecycleProvider extends BaseLifecycleProvider { + SiteLifecycleProvider() { + super(Lifecycle.SITE); + } + } + + @Singleton + @Named(Lifecycle.WRAPPER) + static class WrapperLifecycleProvider extends BaseLifecycleProvider { + WrapperLifecycleProvider() { + super(Lifecycle.WRAPPER); + } + } + static class CleanLifecycle implements Lifecycle { private static final String MAVEN_CLEAN_PLUGIN_VERSION = "3.2.0"; diff --git a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultPackagingRegistry.java b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultPackagingRegistry.java index 6039991438..afe42e73a9 100644 --- a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultPackagingRegistry.java +++ b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultPackagingRegistry.java @@ -104,7 +104,7 @@ public class DefaultPackagingRegistry return lfs; } - private void parseLifecyclePhaseDefinitions(Map plugins, String phase, LifecyclePhase goals) { + static void parseLifecyclePhaseDefinitions(Map plugins, String phase, LifecyclePhase goals) { InputSource inputSource = new InputSource(DefaultLifecyclePluginAnalyzer.DEFAULTLIFECYCLEBINDINGS_MODELID, null); InputLocation location = new InputLocation(-1, -1, inputSource, 0); diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java index 3987985cbe..91e1adbc64 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java @@ -155,11 +155,9 @@ public class DefaultLifecycles { // Lifecycles cannot be cached as extensions might add custom lifecycles later in the execution. try { - Map lifecycles = new HashMap<>(lookup.lookupMap(Lifecycle.class)); - for (org.apache.maven.api.Lifecycle lf : registry) { - lifecycles.put(lf.id(), new Lifecycle(registry, lf)); - } - return lifecycles; + return registry != null + ? registry.stream().collect(Collectors.toMap(lf -> lf.id(), lf -> new Lifecycle(lf))) + : Map.of(); } catch (LookupException e) { throw new IllegalStateException("Unable to lookup lifecycles from the plexus container", e); } diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/Lifecycle.java b/maven-core/src/main/java/org/apache/maven/lifecycle/Lifecycle.java index 089e412ad5..a88307459c 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/Lifecycle.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/Lifecycle.java @@ -35,11 +35,12 @@ public class Lifecycle { this.defaultPhases = defaultPhases; } - public Lifecycle( - org.apache.maven.api.services.LifecycleRegistry registry, org.apache.maven.api.Lifecycle lifecycle) { + public Lifecycle(org.apache.maven.api.Lifecycle lifecycle) { this.lifecycle = lifecycle; this.id = lifecycle.id(); - this.phases = registry.computePhases(lifecycle); + this.phases = lifecycle.phases().stream() + .map(org.apache.maven.api.Lifecycle.Phase::name) + .toList(); this.defaultPhases = getDefaultPhases(lifecycle); } diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ExecutionPlanItem.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ExecutionPlanItem.java index 9f25fc1143..fc64733e6e 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ExecutionPlanItem.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ExecutionPlanItem.java @@ -68,6 +68,6 @@ public class ExecutionPlanItem { @Override public String toString() { - return "ExecutionPlanItem{" + ", mojoExecution=" + mojoExecution + '}' + super.toString(); + return "ExecutionPlanItem{" + mojoExecution.toString() + "}@" + Integer.toHexString(hashCode()); } } diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/DefaultLifecyclesTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/DefaultLifecyclesTest.java index 99b8793eae..ba12bcade0 100644 --- a/maven-core/src/test/java/org/apache/maven/lifecycle/DefaultLifecyclesTest.java +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/DefaultLifecyclesTest.java @@ -96,7 +96,8 @@ class DefaultLifecyclesTest { when(mockedPlexusContainer.lookupMap(Lifecycle.class)).thenReturn(lifeCycles); DefaultLifecycles dl = new DefaultLifecycles( - new DefaultLifecycleRegistry(Collections.emptyList(), Collections.emptyMap()), + new DefaultLifecycleRegistry( + List.of(new DefaultLifecycleRegistry.LifecycleWrapperProvider(mockedPlexusContainer))), new DefaultLookup(mockedPlexusContainer)); assertThat(dl.getLifeCycles().get(0).getId(), is("clean")); diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/MavenExecutionPlanTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/MavenExecutionPlanTest.java index c24ec44f01..2ca490772e 100644 --- a/maven-core/src/test/java/org/apache/maven/lifecycle/MavenExecutionPlanTest.java +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/MavenExecutionPlanTest.java @@ -38,8 +38,6 @@ class MavenExecutionPlanTest { MavenExecutionPlan plan = LifecycleExecutionPlanCalculatorStub.getProjectAExecutionPlan(); ExecutionPlanItem expected = plan.findLastInPhase("package"); - ExecutionPlanItem beerPhase = plan.findLastInPhase("BEER"); // Beer comes straight after package in stub - assertEquals(expected, beerPhase); assertNotNull(expected); } diff --git a/maven-core/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java b/maven-core/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java index 4976d22a17..747f2b8c3f 100644 --- a/maven-core/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java +++ b/maven-core/src/test/java/org/apache/maven/project/EmptyLifecycleBindingsInjector.java @@ -51,10 +51,6 @@ public class EmptyLifecycleBindingsInjector extends DefaultLifecycleBindingsInje private static PackagingRegistry packagingRegistry; private static final LifecycleRegistry emptyLifecycleRegistry = new LifecycleRegistry() { - @Override - public List computePhases(Lifecycle lifecycle) { - return List.of(); - } @Override public Iterator iterator() { @@ -135,11 +131,6 @@ public class EmptyLifecycleBindingsInjector extends DefaultLifecycleBindingsInje return getDelegate().lookup(id); } - @Override - public List computePhases(Lifecycle lifecycle) { - return getDelegate().computePhases(lifecycle); - } - @Override public Iterator iterator() { return getDelegate().iterator();