diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java new file mode 100644 index 000000000000..d85c745f18cc --- /dev/null +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java @@ -0,0 +1,200 @@ +/* + * 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.cling.invoker.mvnup.goals; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Element; +import org.apache.maven.api.cli.mvnup.UpgradeOptions; +import org.apache.maven.api.di.Named; +import org.apache.maven.api.di.Priority; +import org.apache.maven.api.di.Singleton; +import org.apache.maven.cling.invoker.mvnup.UpgradeContext; + +/** + * Strategy for removing duplicate XML elements in POM files. + * + *

Maven 4's stricter POM parser rejects duplicate XML elements (such as + * {@code }, {@code }, {@code }, etc.) that + * Maven 3 silently accepted using last-wins semantics. This strategy scans + * each POM element's children and removes duplicates, keeping only the last + * occurrence of each element name. + * + *

This strategy only targets "scalar" elements — elements that should appear + * at most once within their parent according to the Maven POM schema. Elements + * inside list containers (e.g., {@code } inside {@code }, + * {@code } inside {@code }) are not affected by this strategy + * since duplicate dependencies and plugins are handled by + * {@link DeduplicateDependenciesStrategy}. + * + * @see #12530 + */ +@Named +@Singleton +@Priority(21) +public class DuplicateElementStrategy extends AbstractUpgradeStrategy { + + /** + * Parent element names whose children are expected to repeat (list containers). + * These are skipped when checking for duplicate children since their child + * elements are naturally repeated (e.g., multiple {@code } in + * {@code }). + */ + static final Set LIST_CONTAINER_ELEMENTS = Set.of( + "dependencies", + "plugins", + "modules", + "subprojects", + "profiles", + "repositories", + "pluginRepositories", + "extensions", + "exclusions", + "executions", + "resources", + "testResources", + "notifiers", + "contributors", + "developers", + "licenses", + "mailingLists", + "goals", + "otherArchives", + "includes", + "excludes", + "filters", + "roles", + "reports"); + + @Override + public boolean isApplicable(UpgradeContext context) { + UpgradeOptions options = getOptions(context); + return isOptionEnabled(options, options.model(), true); + } + + @Override + public String getDescription() { + return "Removing duplicate XML elements"; + } + + @Override + protected UpgradeResult doApply(UpgradeContext context, Map pomMap) { + Set processedPoms = new HashSet<>(); + Set modifiedPoms = new HashSet<>(); + Set errorPoms = new HashSet<>(); + + for (Map.Entry entry : pomMap.entrySet()) { + Path pomPath = entry.getKey(); + Document pomDocument = entry.getValue(); + processedPoms.add(pomPath); + + context.info(pomPath + " (checking for duplicate XML elements)"); + context.indent(); + + try { + boolean hasIssues = removeDuplicateElements(pomDocument.root(), context); + + if (hasIssues) { + context.success("Duplicate XML elements removed"); + modifiedPoms.add(pomPath); + } else { + context.success("No duplicate XML elements found"); + } + } catch (Exception e) { + context.failure("Failed to remove duplicate XML elements: " + e.getMessage()); + errorPoms.add(pomPath); + } finally { + context.unindent(); + } + } + + return new UpgradeResult(processedPoms, modifiedPoms, errorPoms); + } + + /** + * Recursively scans an element's children for duplicates and removes them. + * Uses last-wins semantics (consistent with Maven 3's behavior). + * + * @param element the element to scan + * @param context the upgrade context for logging + * @return true if any duplicates were removed + */ + private boolean removeDuplicateElements(Element element, UpgradeContext context) { + boolean removed = false; + + // Skip list container elements — their children naturally repeat + if (LIST_CONTAINER_ELEMENTS.contains(element.name())) { + // Still recurse into each child to check for duplicates within them + for (Element child : element.childElements().toList()) { + removed |= removeDuplicateElements(child, context); + } + return removed; + } + + // Collect children, tracking last occurrence of each name + List children = element.childElements().toList(); + Map lastSeen = new HashMap<>(); + List duplicates = new ArrayList<>(); + + for (Element child : children) { + String name = child.name(); + Element previous = lastSeen.put(name, child); + if (previous != null) { + // Previous occurrence is the duplicate (last-wins) + duplicates.add(previous); + } + } + + // Remove duplicates + for (Element duplicate : duplicates) { + String elementPath = buildElementPath(element); + context.detail("Removed duplicate <" + duplicate.name() + "> element in " + elementPath); + DomUtils.removeElement(duplicate); + removed = true; + } + + // Recurse into surviving children + List survivingChildren = element.childElements().toList(); + for (Element child : survivingChildren) { + removed |= removeDuplicateElements(child, context); + } + + return removed; + } + + /** + * Builds a human-readable path for the element for logging purposes. + */ + private String buildElementPath(Element element) { + List segments = new ArrayList<>(); + Element current = element; + while (current != null) { + segments.add(0, current.name()); + current = current.parent() instanceof Element p ? p : null; + } + return String.join("/", segments); + } +} diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginMigration.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginMigration.java new file mode 100644 index 000000000000..8cb365eb4c34 --- /dev/null +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginMigration.java @@ -0,0 +1,40 @@ +/* + * 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.cling.invoker.mvnup.goals; + +/** + * Plugin migration configuration for Maven 4 compatibility. + * This record holds information about plugins that need to be replaced + * by a different artifact (different groupId and/or artifactId) to work + * properly with Maven 4. + * + * @param oldGroupId the Maven groupId of the old plugin to migrate from + * @param oldArtifactId the Maven artifactId of the old plugin to migrate from + * @param newGroupId the Maven groupId of the new plugin to migrate to + * @param newArtifactId the Maven artifactId of the new plugin to migrate to + * @param minVersion the minimum version of the new plugin required for Maven 4 compatibility + * @param reason the reason why this plugin needs to be migrated + */ +public record PluginMigration( + String oldGroupId, + String oldArtifactId, + String newGroupId, + String newArtifactId, + String minVersion, + String reason) {} diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgrade.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgrade.java index d2aedf760e78..e6895cfd5766 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgrade.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgrade.java @@ -25,7 +25,19 @@ * * @param groupId the Maven groupId of the plugin * @param artifactId the Maven artifactId of the plugin - * @param minVersion the minimum version required for Maven 4 compatibility + * @param minVersion the minimum version required for Maven 4 compatibility (for 3.x users) + * @param latestPreRelease the latest available 4.x pre-release version, or {@code null} if + * the plugin has no 4.x pre-release line. Used to upgrade old 4.x alpha/beta/RC versions + * to the latest pre-release rather than downgrading to a 3.x version. * @param reason the reason why this plugin needs to be upgraded */ -public record PluginUpgrade(String groupId, String artifactId, String minVersion, String reason) {} +public record PluginUpgrade( + String groupId, String artifactId, String minVersion, String latestPreRelease, String reason) { + + /** + * Convenience constructor for plugins without a 4.x pre-release line. + */ + public PluginUpgrade(String groupId, String artifactId, String minVersion, String reason) { + this(groupId, artifactId, minVersion, null, reason); + } +} diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java index 46bcb1e8b512..5ae5f910c144 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java @@ -68,7 +68,11 @@ public class PluginUpgradeStrategy extends AbstractUpgradeStrategy { private static final List PLUGIN_UPGRADES = List.of( new PluginUpgrade( - DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-compiler-plugin", "3.2", MAVEN_4_COMPATIBILITY_REASON), + DEFAULT_MAVEN_PLUGIN_GROUP_ID, + "maven-compiler-plugin", + "3.11.0", + "4.0.0-beta-4", + "Versions before 3.11 cannot find ErrorProne plug-in under Maven 4 classloading"), new PluginUpgrade("org.codehaus.mojo", "exec-maven-plugin", "3.5.0", MAVEN_4_COMPATIBILITY_REASON), new PluginUpgrade( DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-enforcer-plugin", "3.5.0", MAVEN_4_COMPATIBILITY_REASON), @@ -98,7 +102,32 @@ public class PluginUpgradeStrategy extends AbstractUpgradeStrategy { DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-resources-plugin", "3.3.1", - "Beta/RC versions compiled against different Maven 4 API signatures"), + "4.0.0-beta-1", + "Pre-release versions compiled against different Maven 4 API signatures"), + new PluginUpgrade( + DEFAULT_MAVEN_PLUGIN_GROUP_ID, + "maven-jar-plugin", + "3.5.0", + "4.0.0-beta-1", + "Pre-release versions compiled against different Maven 4 API signatures"), + new PluginUpgrade( + DEFAULT_MAVEN_PLUGIN_GROUP_ID, + "maven-install-plugin", + "3.1.4", + "4.0.0-beta-2", + "Pre-release versions compiled against different Maven 4 API signatures"), + new PluginUpgrade( + DEFAULT_MAVEN_PLUGIN_GROUP_ID, + "maven-deploy-plugin", + "3.1.4", + "4.0.0-beta-2", + "Pre-release versions compiled against different Maven 4 API signatures"), + new PluginUpgrade( + DEFAULT_MAVEN_PLUGIN_GROUP_ID, + "maven-clean-plugin", + "3.5.0", + "4.0.0-beta-2", + "Pre-release versions compiled against different Maven 4 API signatures"), new PluginUpgrade( "org.codehaus.mojo", "jaxb2-maven-plugin", @@ -123,6 +152,18 @@ public class PluginUpgradeStrategy extends AbstractUpgradeStrategy { "1.4", "Versions before 1.4 use a removed DependencyGraphBuilder API incompatible with Maven 4")); + /** + * Plugin migrations: old groupId:artifactId → new groupId:artifactId with minimum version. + * Used for plugins that have been replaced by a different artifact. + */ + static final List PLUGIN_MIGRATIONS = List.of(new PluginMigration( + "org.scala-tools", + "maven-scala-plugin", + "net.alchim31.maven", + "scala-maven-plugin", + "4.9.5", + "Ancient plugin (unmaintained since 2011) calls add() on immutable lists returned by Maven 4 API")); + @Inject public PluginUpgradeStrategy() {} @@ -257,53 +298,14 @@ private boolean upgradePluginsInDocument(Document pomDocument, UpgradeContext co * Returns the map of plugins that need to be upgraded for Maven 4 compatibility. */ private Map getPluginUpgradesMap() { - Map upgrades = new HashMap<>(); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-compiler-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-compiler-plugin", "3.2")); - upgrades.put( - "org.codehaus.mojo:exec-maven-plugin", - new PluginUpgradeInfo("org.codehaus.mojo", "exec-maven-plugin", "3.5.0")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-enforcer-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-enforcer-plugin", "3.5.0")); - upgrades.put( - "org.codehaus.mojo:flatten-maven-plugin", - new PluginUpgradeInfo("org.codehaus.mojo", "flatten-maven-plugin", "1.2.7")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-shade-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-shade-plugin", "3.5.0")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-remote-resources-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-remote-resources-plugin", "3.0.0")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-surefire-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-surefire-plugin", "3.5.2")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-failsafe-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-failsafe-plugin", "3.5.2")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-surefire-report-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-surefire-report-plugin", "3.5.2")); - upgrades.put( - "net.alchim31.maven:scala-maven-plugin", - new PluginUpgradeInfo("net.alchim31.maven", "scala-maven-plugin", "4.9.5")); - upgrades.put( - DEFAULT_MAVEN_PLUGIN_GROUP_ID + ":maven-resources-plugin", - new PluginUpgradeInfo(DEFAULT_MAVEN_PLUGIN_GROUP_ID, "maven-resources-plugin", "3.3.1")); - upgrades.put( - "org.codehaus.mojo:jaxb2-maven-plugin", - new PluginUpgradeInfo("org.codehaus.mojo", "jaxb2-maven-plugin", "3.2.0")); - upgrades.put( - "io.quarkus:quarkus-maven-plugin", - new PluginUpgradeInfo("io.quarkus", "quarkus-maven-plugin", "3.26.0")); - upgrades.put( - "io.quarkus.platform:quarkus-maven-plugin", - new PluginUpgradeInfo("io.quarkus.platform", "quarkus-maven-plugin", "3.26.0")); - upgrades.put( - "org.codehaus.gmavenplus:gmavenplus-plugin", - new PluginUpgradeInfo("org.codehaus.gmavenplus", "gmavenplus-plugin", "4.2.0")); - return upgrades; + return PLUGIN_UPGRADES.stream() + .collect(Collectors.toMap( + upgrade -> upgrade.groupId() + ":" + upgrade.artifactId(), + upgrade -> new PluginUpgradeInfo( + upgrade.groupId(), + upgrade.artifactId(), + upgrade.minVersion(), + upgrade.latestPreRelease()))); } /** @@ -316,6 +318,8 @@ private boolean upgradePluginsInSection( String sectionName, UpgradeContext context) { + Map pluginMigrations = getPluginMigrationsMap(); + return pluginsElement .childElements(PLUGIN) .map(pluginElement -> { @@ -329,11 +333,19 @@ private boolean upgradePluginsInSection( } if (groupId != null && artifactId != null) { + // Check for plugin migration first (groupId/artifactId change) String pluginKey = groupId + ":" + artifactId; - PluginUpgradeInfo upgrade = pluginUpgrades.get(pluginKey); + PluginMigration migration = pluginMigrations.get(pluginKey); - if (upgrade != null) { - upgraded = upgradePluginVersion(pluginElement, upgrade, pomDocument, sectionName, context); + if (migration != null) { + upgraded = migratePlugin(pluginElement, migration, pomDocument, sectionName, context); + } else { + PluginUpgradeInfo upgrade = pluginUpgrades.get(pluginKey); + + if (upgrade != null) { + upgraded = + upgradePluginVersion(pluginElement, upgrade, pomDocument, sectionName, context); + } } } @@ -401,7 +413,23 @@ && isPropertyUsedByQuarkusBom(pomDocument, propertyName)) { // Update property value if it's below minimum version return upgradePropertyVersion(pomDocument, propertyName, upgrade, sectionName, context); } else { - // Direct version comparison and upgrade + // Check for Maven 4 pre-release versions (alpha/beta/rc) that should be + // upgraded to the latest available pre-release rather than downgraded to 3.x. + if (isMaven4PreRelease(currentVersion) && upgrade.latestPreRelease != null) { + if (isVersionBelow(currentVersion, upgrade.latestPreRelease)) { + Editor editor = new Editor(pomDocument); + editor.setTextContent(versionElement, upgrade.latestPreRelease); + context.detail("Upgraded " + upgrade.groupId + ":" + upgrade.artifactId + " from pre-release " + + currentVersion + " to " + upgrade.latestPreRelease + " in " + sectionName); + return true; + } else { + context.debug("Plugin " + upgrade.groupId + ":" + upgrade.artifactId + " version " + currentVersion + + " is already >= " + upgrade.latestPreRelease); + } + return false; + } + + // Direct version comparison and upgrade (for 3.x versions) if (isVersionBelow(currentVersion, upgrade.minVersion)) { Editor editor = new Editor(pomDocument); editor.setTextContent(versionElement, upgrade.minVersion); @@ -435,7 +463,19 @@ private boolean upgradePropertyVersion( propertiesElement.childElement(propertyName).orElse(null); if (propertyElement != null) { String currentVersion = propertyElement.textContentTrimmed(); - if (isVersionBelow(currentVersion, upgrade.minVersion)) { + // For 4.x pre-release versions, upgrade to latest pre-release (not 3.x) + if (isMaven4PreRelease(currentVersion) && upgrade.latestPreRelease != null) { + if (isVersionBelow(currentVersion, upgrade.latestPreRelease)) { + editor.setTextContent(propertyElement, upgrade.latestPreRelease); + context.detail("Upgraded property " + propertyName + " (for " + upgrade.groupId + ":" + + upgrade.artifactId + ") from pre-release " + currentVersion + " to " + + upgrade.latestPreRelease + " in " + sectionName); + return true; + } else { + context.debug("Property " + propertyName + " version " + currentVersion + " is already >= " + + upgrade.latestPreRelease); + } + } else if (isVersionBelow(currentVersion, upgrade.minVersion)) { editor.setTextContent(propertyElement, upgrade.minVersion); context.detail( "Upgraded property " + propertyName + " (for " + upgrade.groupId + ":" + upgrade.artifactId @@ -455,6 +495,63 @@ private boolean upgradePropertyVersion( return false; } + /** + * Migrates a plugin from one groupId:artifactId to another, updating the groupId, + * artifactId, and version elements. Used for plugins that have been replaced by a + * different artifact (e.g., org.scala-tools:maven-scala-plugin → net.alchim31.maven:scala-maven-plugin). + */ + private boolean migratePlugin( + Element pluginElement, + PluginMigration migration, + Document pomDocument, + String sectionName, + UpgradeContext context) { + Editor editor = new Editor(pomDocument); + + // Update groupId + Element groupIdElement = pluginElement.childElement(GROUP_ID).orElse(null); + if (groupIdElement != null) { + editor.setTextContent(groupIdElement, migration.newGroupId()); + } + + // Update artifactId + Element artifactIdElement = pluginElement.childElement(ARTIFACT_ID).orElse(null); + if (artifactIdElement != null) { + editor.setTextContent(artifactIdElement, migration.newArtifactId()); + } + + // Set or update version + Element versionElement = pluginElement.childElement(VERSION).orElse(null); + if (versionElement != null) { + editor.setTextContent(versionElement, migration.minVersion()); + } else { + DomUtils.insertContentElement(pluginElement, VERSION, migration.minVersion()); + } + + context.detail("Migrated " + migration.oldGroupId() + ":" + migration.oldArtifactId() + " to " + + migration.newGroupId() + ":" + migration.newArtifactId() + ":" + migration.minVersion() + " in " + + sectionName + " — " + migration.reason()); + return true; + } + + /** + * Returns the map of plugin migrations keyed by old groupId:artifactId. + */ + private static final Map PLUGIN_MIGRATIONS_MAP = PLUGIN_MIGRATIONS.stream() + .collect(Collectors.toMap( + migration -> migration.oldGroupId() + ":" + migration.oldArtifactId(), migration -> migration)); + + private Map getPluginMigrationsMap() { + return PLUGIN_MIGRATIONS_MAP; + } + + /** + * Gets the list of plugin migrations. + */ + public static List getPluginMigrations() { + return PLUGIN_MIGRATIONS; + } + /** * Upgrades plugin dependencies (e.g., extra-enforcer-rules inside maven-enforcer-plugin). */ @@ -491,8 +588,25 @@ private Map getPluginDependencyUpgradesMap() { return PLUGIN_DEPENDENCY_UPGRADES.stream() .collect(Collectors.toMap( upgrade -> upgrade.groupId() + ":" + upgrade.artifactId(), - upgrade -> - new PluginUpgradeInfo(upgrade.groupId(), upgrade.artifactId(), upgrade.minVersion()))); + upgrade -> new PluginUpgradeInfo( + upgrade.groupId(), + upgrade.artifactId(), + upgrade.minVersion(), + upgrade.latestPreRelease()))); + } + + /** + * Checks if a version string is a Maven 4 pre-release version. + * These versions use API methods that were renamed or removed before the GA release, + * causing NoSuchMethodError at runtime. They need to be upgraded regardless of the + * numeric version comparison (since 4.0.0-beta-1 > 3.x in Maven version semantics). + */ + static boolean isMaven4PreRelease(String version) { + if (version == null) { + return false; + } + // Match patterns like: 4.0.0-beta-1, 4.0.0-alpha-1, 4.0.0-SNAPSHOT, 4.0.0-beta1 + return version.startsWith("4.0.0-"); } /** @@ -1171,9 +1285,12 @@ public static class PluginUpgradeInfo { /** The Maven artifactId of the plugin */ final String artifactId; - /** The minimum version required for Maven 4 compatibility */ + /** The minimum version required for Maven 4 compatibility (for 3.x users) */ final String minVersion; + /** The latest available 4.x pre-release version, or null if none exists */ + final String latestPreRelease; + /** * Creates a new plugin upgrade information holder. * @@ -1183,11 +1300,18 @@ public static class PluginUpgradeInfo { * the Maven artifactId of the plugin * @param minVersion * the minimum version required for Maven 4 compatibility + * @param latestPreRelease + * the latest 4.x pre-release version, or null */ - PluginUpgradeInfo(String groupId, String artifactId, String minVersion) { + PluginUpgradeInfo(String groupId, String artifactId, String minVersion, String latestPreRelease) { this.groupId = groupId; this.artifactId = artifactId; this.minVersion = minVersion; + this.latestPreRelease = latestPreRelease; + } + + PluginUpgradeInfo(String groupId, String artifactId, String minVersion) { + this(groupId, artifactId, minVersion, null); } } } diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java new file mode 100644 index 000000000000..602cc0626958 --- /dev/null +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java @@ -0,0 +1,509 @@ +/* + * 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.cling.invoker.mvnup.goals; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Map; + +import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Element; +import org.apache.maven.cling.invoker.mvnup.UpgradeContext; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for {@link DuplicateElementStrategy}. + */ +@DisplayName("DuplicateElementStrategy") +class DuplicateElementStrategyTest { + + private DuplicateElementStrategy strategy; + + @BeforeEach + void setUp() { + strategy = new DuplicateElementStrategy(); + } + + private UpgradeContext createMockContext() { + return TestUtils.createMockContext(); + } + + @Nested + @DisplayName("Applicability") + class ApplicabilityTests { + + @Test + @DisplayName("should be applicable with default options") + void shouldBeApplicableWithDefaults() { + UpgradeContext context = createMockContext(); + assertTrue(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should be applicable when --model is true") + void shouldBeApplicableWithModelTrue() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithFixModel(true)); + assertTrue(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should not be applicable when --model is false") + void shouldNotBeApplicableWithModelFalse() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithFixModel(false)); + assertFalse(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should be applicable when --all is set") + void shouldBeApplicableWithAll() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithAll(true)); + assertTrue(strategy.isApplicable(context)); + } + } + + @Nested + @DisplayName("Duplicate Element Removal") + class DuplicateElementRemovalTests { + + @Test + @DisplayName("should remove duplicate artifactId keeping last occurrence") + void shouldRemoveDuplicateArtifactId() { + String pomXml = """ + + + 4.0.0 + test + first-name + second-name + 1.0.0 + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("second-name"), "Should keep last artifactId"); + assertFalse(xml.contains("first-name"), "Should remove first artifactId"); + } + + @Test + @DisplayName("should remove duplicate properties blocks keeping last") + void shouldRemoveDuplicateProperties() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + old-value + + + new-value + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("new-value"), "Should keep last properties block"); + assertFalse(xml.contains("old-value"), "Should remove first properties block"); + } + + @Test + @DisplayName("should remove duplicate version element keeping last") + void shouldRemoveDuplicateVersion() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + 2.0.0 + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("2.0.0"), "Should keep last version"); + assertFalse(xml.contains("1.0.0"), "Should remove first version"); + } + + @Test + @DisplayName("should remove duplicate groupId element keeping last") + void shouldRemoveDuplicateGroupId() { + String pomXml = """ + + + 4.0.0 + com.old + com.new + test + 1.0.0 + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("com.new"), "Should keep last groupId"); + assertFalse(xml.contains("com.old"), "Should remove first groupId"); + } + + @Test + @DisplayName("should remove multiple different duplicate elements in same POM") + void shouldRemoveMultipleDifferentDuplicates() { + String pomXml = """ + + + 4.0.0 + com.old + com.new + old-name + new-name + 1.0.0 + 2.0.0 + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("com.new"), "Should keep last groupId"); + assertFalse(xml.contains("com.old"), "Should remove first groupId"); + assertTrue(xml.contains("new-name"), "Should keep last artifactId"); + assertFalse(xml.contains("old-name"), "Should remove first artifactId"); + assertTrue(xml.contains("2.0.0"), "Should keep last version"); + assertFalse(xml.contains("1.0.0"), "Should remove first version"); + } + + @Test + @DisplayName("should not modify POM with no duplicates") + void shouldNotModifyWhenNoDuplicates() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + } + } + + @Nested + @DisplayName("Nested Duplicate Elements") + class NestedDuplicateElementTests { + + @Test + @DisplayName("should remove duplicate groupId inside a dependency element") + void shouldRemoveDuplicateGroupIdInDependency() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + old-group + new-group + some-lib + 1.0 + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("new-group"), "Should keep last groupId in dependency"); + assertFalse(xml.contains("old-group"), "Should remove first groupId in dependency"); + } + + @Test + @DisplayName("should remove duplicate version inside parent element") + void shouldRemoveDuplicateVersionInParent() { + String pomXml = """ + + + 4.0.0 + + com.example + parent + 1.0.0 + 2.0.0 + + child + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(1, result.modifiedCount(), "Should have modified 1 POM"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("2.0.0"), "Should keep last version in parent"); + assertFalse(xml.contains("1.0.0"), "Should remove first version in parent"); + } + } + + @Nested + @DisplayName("List Container Skipping") + class ListContainerSkippingTests { + + @Test + @DisplayName("should not remove multiple dependency elements in dependencies") + void shouldNotRemoveDependenciesInList() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + commons-io + commons-io + 2.15.0 + + + commons-lang + commons-lang3 + 3.14.0 + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + Element deps = document.root().childElement("dependencies").orElse(null); + assertNotNull(deps, "dependencies element should still exist"); + assertEquals(2, deps.childElements("dependency").count(), "Should still have 2 dependency elements"); + } + + @Test + @DisplayName("should not remove multiple plugin elements in plugins") + void shouldNotRemovePluginsInList() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.13.0 + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + Element plugins = document.root() + .childElement("build") + .orElseThrow() + .childElement("plugins") + .orElse(null); + assertNotNull(plugins, "plugins element should still exist"); + assertEquals(2, plugins.childElements("plugin").count(), "Should still have 2 plugin elements"); + } + + @Test + @DisplayName("should not remove multiple module elements in modules") + void shouldNotRemoveModulesInList() { + String pomXml = """ + + + 4.0.0 + test + test-parent + 1.0.0 + pom + + module-a + module-b + module-c + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should not have modified any POM"); + + Element modules = document.root().childElement("modules").orElse(null); + assertNotNull(modules, "modules element should still exist"); + assertEquals(3, modules.childElements("module").count(), "Should still have 3 module elements"); + } + } + + @Nested + @DisplayName("Strategy Description") + class StrategyDescriptionTests { + + @Test + @DisplayName("should return non-null description") + void shouldReturnNonNullDescription() { + assertNotNull(strategy.getDescription(), "Description should not be null"); + } + + @Test + @DisplayName("should return non-empty description") + void shouldReturnNonEmptyDescription() { + assertFalse(strategy.getDescription().isEmpty(), "Description should not be empty"); + } + } + + @Nested + @DisplayName("Error Handling") + class ErrorHandlingTests { + + @Test + @DisplayName("should return success with 0 modifications for POM with no duplicates") + void shouldReturnSuccessWithZeroModifications() { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + A simple project + + UTF-8 + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Strategy should succeed"); + assertEquals(0, result.modifiedCount(), "Should report 0 modifications"); + assertEquals(0, result.errorCount(), "Should report 0 errors"); + } + } +} diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java index 5d80d47dd913..ba583fbd6d2f 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java @@ -1666,7 +1666,7 @@ void shouldFormatPluginManagementWithProperIndentation() throws Exception { String result = DomUtils.toXml(document); // Check that the plugin version was upgraded - assertTrue(result.contains("3.2"), "Plugin version should be upgraded to 3.2"); + assertTrue(result.contains("3.11.0"), "Plugin version should be upgraded to 3.11.0"); // Verify that the XML formatting is correct - no malformed closing tags assertFalse(result.contains(""), "Should not have malformed closing tags"); @@ -1728,4 +1728,253 @@ void shouldFormatPluginManagementWithProperIndentationWhenAdded() throws Excepti } } } + + @Nested + @DisplayName("Maven 4 Pre-release Version Detection") + class Maven4PreReleaseTests { + + @Test + @DisplayName("should detect 4.0.0 pre-release versions") + void shouldDetectPreReleaseVersions() { + assertTrue(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0-beta-1")); + assertTrue(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0-beta-2")); + assertTrue(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0-alpha-1")); + assertTrue(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0-SNAPSHOT")); + assertTrue(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0-rc-1")); + } + + @Test + @DisplayName("should not detect non-pre-release versions") + void shouldNotDetectNonPreReleaseVersions() { + assertFalse(PluginUpgradeStrategy.isMaven4PreRelease("4.0.0")); + assertFalse(PluginUpgradeStrategy.isMaven4PreRelease("3.5.0")); + assertFalse(PluginUpgradeStrategy.isMaven4PreRelease("3.14.0")); + assertFalse(PluginUpgradeStrategy.isMaven4PreRelease(null)); + } + + @Test + @DisplayName("should upgrade beta-1 to latest pre-release, not downgrade to 3.x") + void shouldUpgradeBetaToLatestPreRelease() throws Exception { + Document doc = PomBuilder.create() + .plugin("org.apache.maven.plugins", "maven-compiler-plugin", "4.0.0-beta-1") + .buildDocument(); + strategy.doApply(createMockContext(), Map.of(Paths.get("pom.xml"), doc)); + String xml = DomUtils.toXml(doc); + assertTrue( + xml.contains("4.0.0-beta-4"), + "Should upgrade to latest pre-release, not downgrade to 3.11.0"); + } + + @Test + @DisplayName("should not downgrade when already at latest pre-release") + void shouldNotDowngradeWhenAtLatestPreRelease() throws Exception { + Document doc = PomBuilder.create() + .plugin("org.apache.maven.plugins", "maven-compiler-plugin", "4.0.0-beta-4") + .buildDocument(); + strategy.doApply(createMockContext(), Map.of(Paths.get("pom.xml"), doc)); + assertTrue( + DomUtils.toXml(doc).contains("4.0.0-beta-4"), + "Should keep latest pre-release unchanged"); + } + + @Test + @DisplayName("should upgrade pre-release property to latest pre-release") + void shouldUpgradePreReleaseProperty() throws Exception { + Document doc = PomBuilder.create() + .property("compiler.version", "4.0.0-beta-1") + .plugin("org.apache.maven.plugins", "maven-compiler-plugin", "${compiler.version}") + .buildDocument(); + strategy.doApply(createMockContext(), Map.of(Paths.get("pom.xml"), doc)); + assertTrue( + DomUtils.toXml(doc).contains(">4.0.0-beta-4 + + 4.0.0 + test + test + 1.0.0 + + + + org.scala-tools + maven-scala-plugin + 2.15.2 + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Plugin migration should succeed"); + assertTrue(result.modifiedCount() > 0, "Should have migrated maven-scala-plugin"); + + Editor editor = new Editor(document); + Element root = editor.root(); + Element pluginElement = root.path("build", "plugins", "plugin").orElse(null); + assertNotNull(pluginElement, "Plugin element should exist"); + + String groupId = pluginElement + .childElement("groupId") + .map(Element::textContentTrimmed) + .orElse(null); + String artifactId = pluginElement + .childElement("artifactId") + .map(Element::textContentTrimmed) + .orElse(null); + String version = pluginElement + .childElement("version") + .map(Element::textContentTrimmed) + .orElse(null); + + assertEquals("net.alchim31.maven", groupId, "groupId should be migrated to net.alchim31.maven"); + assertEquals("scala-maven-plugin", artifactId, "artifactId should be migrated to scala-maven-plugin"); + assertEquals("4.9.5", version, "version should be set to 4.9.5"); + } + + @Test + @DisplayName("should migrate org.scala-tools:maven-scala-plugin in pluginManagement") + void shouldMigrateScalaPluginInPluginManagement() throws Exception { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + + org.scala-tools + maven-scala-plugin + 2.15.2 + + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Plugin migration should succeed"); + assertTrue(result.modifiedCount() > 0, "Should have migrated maven-scala-plugin in pluginManagement"); + + Editor editor = new Editor(document); + Element root = editor.root(); + Element pluginElement = + root.path("build", "pluginManagement", "plugins", "plugin").orElse(null); + assertNotNull(pluginElement, "Plugin element should exist in pluginManagement"); + + String groupId = pluginElement + .childElement("groupId") + .map(Element::textContentTrimmed) + .orElse(null); + String artifactId = pluginElement + .childElement("artifactId") + .map(Element::textContentTrimmed) + .orElse(null); + String version = pluginElement + .childElement("version") + .map(Element::textContentTrimmed) + .orElse(null); + + assertEquals("net.alchim31.maven", groupId, "groupId should be migrated to net.alchim31.maven"); + assertEquals("scala-maven-plugin", artifactId, "artifactId should be migrated to scala-maven-plugin"); + assertEquals("4.9.5", version, "version should be set to 4.9.5"); + } + + @Test + @DisplayName("should not modify POM without migratable plugins") + void shouldNotModifyPomWithoutMigratablePlugins() throws Exception { + String pomXml = """ + + + 4.0.0 + test + test + 1.0.0 + + + + net.alchim31.maven + scala-maven-plugin + 4.9.5 + + + + + """; + + Document document = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + // Verify the plugin was not changed + Editor editor = new Editor(document); + Element root = editor.root(); + String groupId = root.path("build", "plugins", "plugin", "groupId") + .map(Element::textContentTrimmed) + .orElse(null); + String artifactId = root.path("build", "plugins", "plugin", "artifactId") + .map(Element::textContentTrimmed) + .orElse(null); + String version = root.path("build", "plugins", "plugin", "version") + .map(Element::textContentTrimmed) + .orElse(null); + + assertEquals("net.alchim31.maven", groupId, "groupId should remain net.alchim31.maven"); + assertEquals("scala-maven-plugin", artifactId, "artifactId should remain scala-maven-plugin"); + assertEquals("4.9.5", version, "version should remain 4.9.5"); + } + + @Test + @DisplayName("should have predefined plugin migrations") + void shouldHavePredefinedPluginMigrations() { + List migrations = PluginUpgradeStrategy.getPluginMigrations(); + + assertFalse(migrations.isEmpty(), "Should have predefined plugin migrations"); + + boolean hasScalaPlugin = migrations.stream() + .anyMatch(m -> + "org.scala-tools".equals(m.oldGroupId()) && "maven-scala-plugin".equals(m.oldArtifactId())); + + assertTrue(hasScalaPlugin, "Should include org.scala-tools:maven-scala-plugin migration"); + + // Verify migration target coordinates + PluginMigration scalaMigration = migrations.stream() + .filter(m -> + "org.scala-tools".equals(m.oldGroupId()) && "maven-scala-plugin".equals(m.oldArtifactId())) + .findFirst() + .orElse(null); + assertNotNull(scalaMigration, "Scala plugin migration should exist"); + assertEquals("net.alchim31.maven", scalaMigration.newGroupId()); + assertEquals("scala-maven-plugin", scalaMigration.newArtifactId()); + assertEquals("4.9.5", scalaMigration.minVersion()); + } + } }