diff --git a/api/maven-api-model/src/main/mdo/maven.mdo b/api/maven-api-model/src/main/mdo/maven.mdo index eb5618d9af02..cd5d679e586e 100644 --- a/api/maven-api-model/src/main/mdo/maven.mdo +++ b/api/maven-api-model/src/main/mdo/maven.mdo @@ -1205,6 +1205,36 @@ ]]> + + id + 4.2.0+ + + + + String + groupId 3.0.0+ @@ -1441,6 +1471,20 @@ ]]> + + id + 4.2.0+ + + + + String + groupId 4.0.0+ @@ -1864,6 +1908,20 @@ 4.1.0+ Parent + + gav + 4.2.0+ + + + + String + classifier 4.1.0+ diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategy.java new file mode 100644 index 000000000000..0182712c2ec9 --- /dev/null +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategy.java @@ -0,0 +1,302 @@ +/* + * 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.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; + +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.ARTIFACT_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.BUILD; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.CLASSIFIER; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCIES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCY; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCY_MANAGEMENT; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.EXCLUSION; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.EXCLUSIONS; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.GROUP_ID; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGINS; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.TYPE; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.VERSION; +import static eu.maveniverse.domtrip.maven.MavenPomElements.ModelVersions.MODEL_VERSION_4_2_0; + +/** + * Strategy for collapsing dependency coordinate child elements into compact {@code id} attributes. + * Only applies to POMs at model version 4.2.0 or higher. + * + *

Transforms verbose dependency declarations: + *

{@code
+ * 
+ *   org.example
+ *   lib
+ *   1.0
+ * 
+ * }
+ * into compact form: + *
{@code
+ * 
+ * }
+ * + *

Supported formats (trailing {@code :} means version is managed): + *

    + *
  • {@code g:a} — version managed
  • + *
  • {@code g:a:v} — default type "jar"
  • + *
  • {@code g:a:type:} — non-default type, version managed
  • + *
  • {@code g:a:type:v} — non-default type
  • + *
  • {@code g:a:type:classifier:} — with classifier, version managed
  • + *
  • {@code g:a:type:classifier:v} — with classifier
  • + *
+ * + *

Also collapses exclusions from {@code }/{@code } into {@code id="g:a"}. + */ +@Named +@Singleton +@Priority(25) +public class DependencyIdStrategy extends AbstractUpgradeStrategy { + + private static final String DEFAULT_TYPE = "jar"; + + @Override + public boolean isApplicable(UpgradeContext context) { + UpgradeOptions options = getOptions(context); + + if (options.all().orElse(false)) { + return true; + } + + String modelVersion = options.modelVersion().orElse(null); + return MODEL_VERSION_4_2_0.equals(modelVersion); + } + + @Override + public String getDescription() { + return "Collapsing dependency coordinates into id attributes"; + } + + @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); + + String currentVersion = ModelVersionUtils.detectModelVersion(pomDocument); + context.info(pomPath + " (current: " + currentVersion + ")"); + context.indent(); + + try { + if (!MODEL_VERSION_4_2_0.equals(currentVersion)) { + context.success("Skipping (model version " + currentVersion + " is not 4.2.0)"); + continue; + } + + boolean hasChanges = collapseDependencies(context, pomDocument); + + if (hasChanges) { + modifiedPoms.add(pomPath); + context.success("Dependency coordinates collapsed into id attributes"); + } else { + context.success("No dependencies to collapse"); + } + } catch (Exception e) { + context.failure("Failed to collapse dependency coordinates: " + e.getMessage()); + errorPoms.add(pomPath); + } finally { + context.unindent(); + } + } + + return new UpgradeResult(processedPoms, modifiedPoms, errorPoms); + } + + private boolean collapseDependencies(UpgradeContext context, Document pomDocument) { + Element root = pomDocument.root(); + boolean hasChanges = false; + + // Process + hasChanges |= processDependenciesSection( + context, root.childElement(DEPENDENCIES).orElse(null)); + + // Process + hasChanges |= root.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) + .map(deps -> processDependenciesSection(context, deps)) + .orElse(false); + + // Process dependencies and dependencyManagement + hasChanges |= root.childElement(PROFILES).stream() + .flatMap(profiles -> profiles.childElements(PROFILE)) + .map(profile -> { + boolean profileChanges = false; + profileChanges |= profile.childElement(DEPENDENCIES) + .map(deps -> processDependenciesSection(context, deps)) + .orElse(false); + profileChanges |= profile.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) + .map(deps -> processDependenciesSection(context, deps)) + .orElse(false); + return profileChanges; + }) + .reduce(false, Boolean::logicalOr); + + // Process + hasChanges |= root.childElement(BUILD).flatMap(build -> build.childElement(PLUGINS)).stream() + .flatMap(plugins -> plugins.childElements(PLUGIN)) + .map(plugin -> plugin.childElement(DEPENDENCIES) + .map(deps -> processDependenciesSection(context, deps)) + .orElse(false)) + .reduce(false, Boolean::logicalOr); + + return hasChanges; + } + + private boolean processDependenciesSection(UpgradeContext context, Element dependenciesElement) { + if (dependenciesElement == null) { + return false; + } + + List dependencyElements = + dependenciesElement.childElements(DEPENDENCY).toList(); + boolean hasChanges = false; + + for (Element dependency : dependencyElements) { + hasChanges |= collapseDependency(context, dependency); + hasChanges |= collapseExclusions(context, dependency); + } + + return hasChanges; + } + + private boolean collapseDependency(UpgradeContext context, Element dependency) { + if (dependency.attribute("id") != null) { + return false; + } + + String groupId = dependency.childText(GROUP_ID); + String artifactId = dependency.childText(ARTIFACT_ID); + String version = dependency.childText(VERSION); + + if (groupId == null || artifactId == null) { + return false; + } + + String type = dependency.childText(TYPE); + String classifier = dependency.childText(CLASSIFIER); + + String id = buildIdValue(groupId, artifactId, version, type, classifier); + dependency.attribute("id", id); + + removeChildElement(dependency, GROUP_ID); + removeChildElement(dependency, ARTIFACT_ID); + if (version != null) { + removeChildElement(dependency, VERSION); + } + + if (classifier != null) { + removeChildElement(dependency, CLASSIFIER); + removeChildElement(dependency, TYPE); + } else if (type != null) { + removeChildElement(dependency, TYPE); + } + + context.detail("Collapsed: " + id); + return true; + } + + private boolean collapseExclusions(UpgradeContext context, Element dependency) { + Element exclusionsElement = dependency.childElement(EXCLUSIONS).orElse(null); + if (exclusionsElement == null) { + return false; + } + + List exclusionElements = + exclusionsElement.childElements(EXCLUSION).toList(); + boolean hasChanges = false; + + for (Element exclusion : exclusionElements) { + hasChanges |= collapseExclusion(context, exclusion); + } + + return hasChanges; + } + + private boolean collapseExclusion(UpgradeContext context, Element exclusion) { + if (exclusion.attribute("id") != null) { + return false; + } + + String groupId = exclusion.childText(GROUP_ID); + String artifactId = exclusion.childText(ARTIFACT_ID); + + if (groupId == null || artifactId == null) { + return false; + } + + String id = groupId + ":" + artifactId; + exclusion.attribute("id", id); + + removeChildElement(exclusion, GROUP_ID); + removeChildElement(exclusion, ARTIFACT_ID); + + context.detail("Collapsed exclusion: " + id); + return true; + } + + static String buildIdValue(String groupId, String artifactId, String version, String type, String classifier) { + if (classifier != null && !classifier.isEmpty()) { + String effectiveType = (type != null && !type.isEmpty()) ? type : DEFAULT_TYPE; + if (version != null) { + return groupId + ":" + artifactId + ":" + effectiveType + ":" + classifier + ":" + version; + } else { + return groupId + ":" + artifactId + ":" + effectiveType + ":" + classifier + ":"; + } + } else if (type != null && !type.isEmpty() && !DEFAULT_TYPE.equals(type)) { + if (version != null) { + return groupId + ":" + artifactId + ":" + type + ":" + version; + } else { + return groupId + ":" + artifactId + ":" + type + ":"; + } + } else if (version != null) { + return groupId + ":" + artifactId + ":" + version; + } else { + return groupId + ":" + artifactId; + } + } + + private static void removeChildElement(Element parent, String childName) { + parent.childElement(childName).ifPresent(DomUtils::removeElement); + } +} diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategyTest.java new file mode 100644 index 000000000000..3be91fb16434 --- /dev/null +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DependencyIdStrategyTest.java @@ -0,0 +1,967 @@ +/* + * 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.HashMap; +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@DisplayName("DependencyIdStrategy") +class DependencyIdStrategyTest { + + private DependencyIdStrategy strategy; + + @BeforeEach + void setUp() { + strategy = new DependencyIdStrategy(); + } + + @Nested + @DisplayName("Applicability") + class ApplicabilityTests { + + @Test + @DisplayName("should not be applicable by default") + void shouldNotBeApplicableByDefault() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createDefaultOptions()); + assertFalse(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should be applicable with --model-version 4.2.0") + void shouldBeApplicableWithModelVersion420() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + assertTrue(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should not be applicable with --model-version 4.1.0") + void shouldNotBeApplicableWithModelVersion410() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.1.0")); + assertFalse(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should be applicable with --all") + void shouldBeApplicableWithAll() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithAll(true)); + assertTrue(strategy.isApplicable(context)); + } + + @Test + @DisplayName("should not be applicable with --all=false") + void shouldNotBeApplicableWithAllFalse() { + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithAll(false)); + assertFalse(strategy.isApplicable(context)); + } + } + + @Nested + @DisplayName("Dependency Collapsing") + class DependencyCollapsingTests { + + @Test + @DisplayName("should collapse g:a:v dependency") + void shouldCollapseGav() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + Map pomMap = Map.of(Paths.get("pom.xml"), doc); + + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(pomMap)); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "groupId")); + assertNull(DomUtils.findChildElement(dependency, "artifactId")); + assertNull(DomUtils.findChildElement(dependency, "version")); + } + + @Test + @DisplayName("should collapse g:a:type:v dependency with non-default type") + void shouldCollapseGavWithType() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + pom + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:pom:1.0", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "type")); + } + + @Test + @DisplayName("should collapse g:a:type:classifier:v dependency") + void shouldCollapseGavWithClassifier() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + jar + sources + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:jar:sources:1.0", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "type")); + assertNull(DomUtils.findChildElement(dependency, "classifier")); + } + + @Test + @DisplayName("should use default type jar in 5-part format when type absent but classifier present") + void shouldUseDefaultTypeWhenClassifierPresent() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + sources + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:jar:sources:1.0", dependency.attribute("id")); + } + + @Test + @DisplayName("should not include default type jar in 3-part format") + void shouldNotIncludeDefaultTypeInId() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + jar + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "type")); + } + + @Test + @DisplayName("should preserve scope when collapsing") + void shouldPreserveScopeWhenCollapsing() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + test + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + assertNotNull(DomUtils.findChildElement(dependency, "scope")); + assertEquals("test", dependency.childText("scope")); + } + + @Test + @DisplayName("should preserve optional when collapsing") + void shouldPreserveOptionalWhenCollapsing() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + true + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + assertNotNull(DomUtils.findChildElement(dependency, "optional")); + } + + @Test + @DisplayName("should collapse dependency without version to g:a") + void shouldCollapseDependencyWithoutVersion() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "groupId")); + assertNull(DomUtils.findChildElement(dependency, "artifactId")); + } + + @Test + @DisplayName("should collapse managed-version dependency with non-default type to g:a:type:") + void shouldCollapseManagedWithType() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + pom + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:pom:", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "groupId")); + assertNull(DomUtils.findChildElement(dependency, "artifactId")); + assertNull(DomUtils.findChildElement(dependency, "type")); + } + + @Test + @DisplayName("should collapse managed-version dependency with classifier to g:a:type:classifier:") + void shouldCollapseManagedWithClassifier() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + jar + sources + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:jar:sources:", dependency.attribute("id")); + assertNull(DomUtils.findChildElement(dependency, "groupId")); + assertNull(DomUtils.findChildElement(dependency, "artifactId")); + assertNull(DomUtils.findChildElement(dependency, "type")); + assertNull(DomUtils.findChildElement(dependency, "classifier")); + } + + @Test + @DisplayName("should skip dependency that already has id attribute") + void shouldSkipDependencyWithExistingId() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + assertEquals( + "org.example:lib:1.0", + doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow() + .attribute("id")); + } + + @Test + @DisplayName("should collapse multiple dependencies") + void shouldCollapseMultipleDependencies() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib-a + 1.0 + + + org.example + lib-b + 2.0 + test + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + var deps = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElements("dependency") + .toList(); + + assertEquals(2, deps.size()); + assertEquals("org.example:lib-a:1.0", deps.get(0).attribute("id")); + assertEquals("org.example:lib-b:2.0", deps.get(1).attribute("id")); + } + } + + @Nested + @DisplayName("Exclusion Collapsing") + class ExclusionCollapsingTests { + + @Test + @DisplayName("should collapse exclusion into id attribute") + void shouldCollapseExclusion() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + + + org.unwanted + bad + + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + + Element exclusion = dependency + .childElement("exclusions") + .orElseThrow() + .childElement("exclusion") + .orElseThrow(); + + assertEquals("org.unwanted:bad", exclusion.attribute("id")); + assertNull(DomUtils.findChildElement(exclusion, "groupId")); + assertNull(DomUtils.findChildElement(exclusion, "artifactId")); + } + + @Test + @DisplayName("should collapse multiple exclusions") + void shouldCollapseMultipleExclusions() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + + + org.a + bad-a + + + org.b + bad-b + + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + var exclusions = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow() + .childElement("exclusions") + .orElseThrow() + .childElements("exclusion") + .toList(); + + assertEquals(2, exclusions.size()); + assertEquals("org.a:bad-a", exclusions.get(0).attribute("id")); + assertEquals("org.b:bad-b", exclusions.get(1).attribute("id")); + } + } + + @Nested + @DisplayName("Section Processing") + class SectionProcessingTests { + + @Test + @DisplayName("should process dependencyManagement dependencies") + void shouldProcessDependencyManagement() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + + org.example + lib + 1.0 + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencyManagement") + .orElseThrow() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + } + + @Test + @DisplayName("should process profile dependencies") + void shouldProcessProfileDependencies() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + test-profile + + + org.example + lib + 1.0 + + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("profiles") + .orElseThrow() + .childElement("profile") + .orElseThrow() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + } + + @Test + @DisplayName("should process profile dependencyManagement") + void shouldProcessProfileDependencyManagement() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + test-profile + + + + org.example + lib + 1.0 + + + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("profiles") + .orElseThrow() + .childElement("profile") + .orElseThrow() + .childElement("dependencyManagement") + .orElseThrow() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + } + + @Test + @DisplayName("should process plugin dependencies") + void shouldProcessPluginDependencies() { + String pomXml = """ + + + 4.2.0 + com.example + test + 1.0 + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.11.0 + + + org.example + lib + 1.0 + + + + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("build") + .orElseThrow() + .childElement("plugins") + .orElseThrow() + .childElement("plugin") + .orElseThrow() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertEquals("org.example:lib:1.0", dependency.attribute("id")); + } + } + + @Nested + @DisplayName("Model Version Filtering") + class ModelVersionFilteringTests { + + @Test + @DisplayName("should skip non-4.2.0 POMs") + void shouldSkipNon420Poms() { + String pomXml = """ + + + 4.1.0 + com.example + test + 1.0 + + + org.example + lib + 1.0 + + + + """; + + Document doc = Document.of(pomXml); + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + UpgradeResult result = strategy.doApply(context, new HashMap<>(Map.of(Paths.get("pom.xml"), doc))); + + Element dependency = doc.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow(); + + assertNull(dependency.attribute("id")); + assertNotNull(DomUtils.findChildElement(dependency, "groupId")); + assertEquals(0, result.modifiedCount()); + } + + @Test + @DisplayName("should process only 4.2.0 POMs in mixed map") + void shouldProcessOnly420PomsInMixedMap() { + String pom410Xml = """ + + + 4.1.0 + com.example + old-module + 1.0 + + + org.example + lib + 1.0 + + + + """; + + String pom420Xml = """ + + + 4.2.0 + com.example + new-module + 1.0 + + + org.example + lib + 1.0 + + + + """; + + Document doc410 = Document.of(pom410Xml); + Document doc420 = Document.of(pom420Xml); + Map pomMap = new HashMap<>(); + pomMap.put(Paths.get("old", "pom.xml"), doc410); + pomMap.put(Paths.get("new", "pom.xml"), doc420); + + UpgradeContext context = TestUtils.createMockContext(TestUtils.createOptionsWithModelVersion("4.2.0")); + UpgradeResult result = strategy.doApply(context, pomMap); + + // 4.1.0 POM should be untouched + assertNull(doc410.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow() + .attribute("id")); + + // 4.2.0 POM should be collapsed + assertEquals( + "org.example:lib:1.0", + doc420.root() + .childElement("dependencies") + .orElseThrow() + .childElement("dependency") + .orElseThrow() + .attribute("id")); + + assertEquals(1, result.modifiedCount()); + } + } + + @Nested + @DisplayName("buildIdValue") + class BuildIdValueTests { + + @Test + void twoPartFormat() { + assertEquals("g:a", DependencyIdStrategy.buildIdValue("g", "a", null, null, null)); + } + + @Test + void threePartFormat() { + assertEquals("g:a:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", null, null)); + } + + @Test + void threePartFormatWithDefaultType() { + assertEquals("g:a:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", "jar", null)); + } + + @Test + void fourPartFormat() { + assertEquals("g:a:pom:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", "pom", null)); + } + + @Test + void fivePartFormat() { + assertEquals("g:a:jar:sources:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", "jar", "sources")); + } + + @Test + void fivePartFormatWithNonDefaultType() { + assertEquals( + "g:a:test-jar:tests:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", "test-jar", "tests")); + } + + @Test + void fivePartFormatClassifierNoType() { + assertEquals("g:a:jar:sources:1.0", DependencyIdStrategy.buildIdValue("g", "a", "1.0", null, "sources")); + } + + @Test + void fourPartTrailingColon() { + assertEquals("g:a:pom:", DependencyIdStrategy.buildIdValue("g", "a", null, "pom", null)); + } + + @Test + void fivePartTrailingColon() { + assertEquals("g:a:jar:sources:", DependencyIdStrategy.buildIdValue("g", "a", null, "jar", "sources")); + } + + @Test + void fivePartTrailingColonClassifierNoType() { + assertEquals("g:a:jar:sources:", DependencyIdStrategy.buildIdValue("g", "a", null, null, "sources")); + } + } +} diff --git a/impl/maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformerTest.java b/impl/maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformerTest.java index 81870abd0f3b..b5610e314830 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformerTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformerTest.java @@ -29,6 +29,7 @@ import org.apache.maven.api.Constants; import org.apache.maven.api.services.Sources; +import org.apache.maven.impl.model.DefaultModelNormalizer; import org.apache.maven.model.Model; import org.apache.maven.model.v4.MavenStaxReader; import org.apache.maven.project.MavenProject; @@ -114,6 +115,44 @@ void transformJarConsumerPom() throws Exception { assertFalse(diff.hasDifferences(), "XML files should be identical: " + diff.toString()); } + @Test + void transformJarConsumerPomWithDependencyId() throws Exception { + RepositorySystemSession systemSessionMock = Mockito.mock(RepositorySystemSession.class); + SessionData sessionDataMock = Mockito.mock(SessionData.class); + when(systemSessionMock.getData()).thenReturn(sessionDataMock); + + Path beforePomFile = Paths.get("src/test/resources/projects/transform/jar-with-id/before.pom") + .toAbsolutePath(); + Path afterPomFile = Paths.get("src/test/resources/projects/transform/jar-with-id/after.pom") + .toAbsolutePath(); + Path tempFile = Files.createTempFile("", ".pom"); + Files.delete(tempFile); + DefaultModelNormalizer normalizer = new DefaultModelNormalizer(); + try (InputStream expected = Files.newInputStream(beforePomFile)) { + org.apache.maven.api.model.Model rawModel = new MavenStaxReader().read(expected); + // Normalize: expand id attributes and clear them + rawModel = normalizer.mergeDuplicates(rawModel, null, null); + Model model = new Model(rawModel); + MavenProject project = new MavenProject(model); + project.setOriginalModel(model); + ConsumerPomArtifactTransformer t = new ConsumerPomArtifactTransformer((s, p, f) -> { + try (InputStream is = f.openStream()) { + org.apache.maven.api.model.Model m = new MavenStaxReader().read(is); + m = normalizer.mergeDuplicates(m, null, null); + return DefaultConsumerPomBuilder.transformNonPom(m, project); + } + }); + + t.transform(project, systemSessionMock, Sources.buildSource(beforePomFile), tempFile); + } + Diff diff = DiffBuilder.compare(afterPomFile.toFile()) + .withTest(tempFile.toFile()) + .ignoreComments() + .ignoreWhitespace() + .build(); + assertFalse(diff.hasDifferences(), "XML files should be identical: " + diff.toString()); + } + @Test void injectTransformedArtifactsWithoutPomShouldNotInjectAnyArtifacts() throws IOException { MavenProject emptyProject = new MavenProject(); diff --git a/impl/maven-core/src/test/resources/projects/transform/jar-with-id/after.pom b/impl/maven-core/src/test/resources/projects/transform/jar-with-id/after.pom new file mode 100644 index 000000000000..6c3ef9035a3e --- /dev/null +++ b/impl/maven-core/src/test/resources/projects/transform/jar-with-id/after.pom @@ -0,0 +1,39 @@ + + + 4.0.0 + io.github.test + jar-with-id + 1.0.0 + jar-with-id + + + org.slf4j + slf4j-api + 2.0.17 + + + org.example + lib-b + 1.0 + pom + import + + + org.example + lib-c + 1.0 + sources + + + org.example + lib-d + 1.0 + + + org.unwanted + bad-lib + + + + + diff --git a/impl/maven-core/src/test/resources/projects/transform/jar-with-id/before.pom b/impl/maven-core/src/test/resources/projects/transform/jar-with-id/before.pom new file mode 100644 index 000000000000..26ba0dfaedd5 --- /dev/null +++ b/impl/maven-core/src/test/resources/projects/transform/jar-with-id/before.pom @@ -0,0 +1,20 @@ + + + 4.2.0 + io.github.test + jar-with-id + 1.0.0 + jar-with-id + + + + import + + + + + + + + + diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java index f36918f770aa..6011a2879235 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java @@ -29,8 +29,12 @@ import org.apache.maven.api.di.Singleton; import org.apache.maven.api.model.Build; import org.apache.maven.api.model.Dependency; +import org.apache.maven.api.model.DependencyManagement; +import org.apache.maven.api.model.Exclusion; +import org.apache.maven.api.model.Mixin; import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Plugin; +import org.apache.maven.api.model.Profile; import org.apache.maven.api.services.ModelBuilderRequest; import org.apache.maven.api.services.ModelProblemCollector; import org.apache.maven.api.services.model.ModelNormalizer; @@ -49,6 +53,8 @@ public class DefaultModelNormalizer implements ModelNormalizer { public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model.Builder builder = Model.newBuilder(model); + builder.mixins(injectList(model.getMixins(), this::expandMixinGav)); + Build build = model.getBuild(); if (build != null) { List plugins = build.getPlugins(); @@ -76,15 +82,21 @@ public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProb * the way 2.x works. When we're in strict mode, the removal of duplicates just saves other merging steps from * aftereffects and bogus error messages. */ - List dependencies = model.getDependencies(); - Map normalized = new LinkedHashMap<>(dependencies.size() * 2); + builder.dependencies(expandAndDeduplicateDependencies(model.getDependencies())); - for (Dependency dependency : dependencies) { - normalized.put(dependency.getManagementKey(), dependency); + DependencyManagement mgmt = model.getDependencyManagement(); + if (mgmt != null) { + List expandedMgmt = expandAndDeduplicateDependencies(mgmt.getDependencies()); + if (expandedMgmt != null) { + builder.dependencyManagement(DependencyManagement.newBuilder(mgmt) + .dependencies(expandedMgmt) + .build()); + } } - if (dependencies.size() != normalized.size()) { - builder.dependencies(normalized.values()); + List profiles = injectList(model.getProfiles(), this::expandProfileDependencyIds); + if (profiles != null) { + builder.profiles(profiles); } return builder.build(); @@ -144,4 +156,190 @@ private List injectList(List list, UnaryOperator modifer) { } return newList; } + + private List expandAndDeduplicateDependencies(List dependencies) { + List expanded = injectList(dependencies, this::expandDependencyId); + if (expanded != null) { + dependencies = expanded; + } + Map normalized = new LinkedHashMap<>(dependencies.size() * 2); + for (Dependency dependency : dependencies) { + normalized.put(dependency.getManagementKey(), dependency); + } + if (expanded != null || dependencies.size() != normalized.size()) { + return new ArrayList<>(normalized.values()); + } + return null; + } + + private Profile expandProfileDependencyIds(Profile profile) { + Profile.Builder pb = null; + + List deps = expandAndDeduplicateDependencies(profile.getDependencies()); + if (deps != null) { + pb = Profile.newBuilder(profile); + pb.dependencies(deps); + } + + DependencyManagement mgmt = profile.getDependencyManagement(); + if (mgmt != null) { + List mgmtDeps = expandAndDeduplicateDependencies(mgmt.getDependencies()); + if (mgmtDeps != null) { + if (pb == null) { + pb = Profile.newBuilder(profile); + } + pb.dependencyManagement(DependencyManagement.newBuilder(mgmt) + .dependencies(mgmtDeps) + .build()); + } + } + + return pb != null ? pb.build() : profile; + } + + /** + * Expands the {@code id} attribute on a dependency into its component fields. + * Supported formats (trailing {@code :} means version is managed, + * {@code @scope} sets the scope, trailing {@code ?} marks as optional): + *

    + *
  • {@code groupId:artifactId} (version managed)
  • + *
  • {@code groupId:artifactId:} (version managed, explicit trailing colon)
  • + *
  • {@code groupId:artifactId:version}
  • + *
  • {@code groupId:artifactId:type:} (version managed, with type)
  • + *
  • {@code groupId:artifactId:type:version}
  • + *
  • {@code groupId:artifactId:type:classifier:} (version managed, with type and classifier)
  • + *
  • {@code groupId:artifactId:type:classifier:version}
  • + *
+ * All formats may be suffixed with {@code @scope} and/or {@code ?}. + */ + Dependency expandDependencyId(Dependency d) { + String id = d.getId(); + if (id == null || id.isEmpty()) { + List expanded = injectList(d.getExclusions(), this::expandExclusionId); + return expanded != null ? d.withExclusions(expanded) : d; + } + + String remaining = id; + boolean optional = false; + if (remaining.endsWith("?")) { + optional = true; + remaining = remaining.substring(0, remaining.length() - 1); + } + + String scope = null; + int atIndex = remaining.lastIndexOf('@'); + if (atIndex >= 0) { + scope = remaining.substring(atIndex + 1); + remaining = remaining.substring(0, atIndex); + } + + String[] parts = remaining.split(":", -1); + if (parts.length < 2 || parts.length > 5) { + return d; + } + Dependency.Builder builder = Dependency.newBuilder(d, true); + builder.id(null); + if (!parts[0].isEmpty() && isNullOrEmpty(d.getGroupId())) { + builder.groupId(parts[0]); + } + if (!parts[1].isEmpty() && isNullOrEmpty(d.getArtifactId())) { + builder.artifactId(parts[1]); + } + switch (parts.length) { + case 2: + // g:a (version managed) + break; + case 3: + // g:a:v or g:a: (managed) + if (!parts[2].isEmpty() && isNullOrEmpty(d.getVersion())) { + builder.version(parts[2]); + } + break; + case 4: + // g:a:type:v or g:a:type: (managed) + if (!parts[2].isEmpty() && isNullOrEmptyOrDefault(d.getType())) { + builder.type(parts[2]); + } + if (!parts[3].isEmpty() && isNullOrEmpty(d.getVersion())) { + builder.version(parts[3]); + } + break; + case 5: + // g:a:type:classifier:v or g:a:type:classifier: (managed) + if (!parts[2].isEmpty() && isNullOrEmptyOrDefault(d.getType())) { + builder.type(parts[2]); + } + if (!parts[3].isEmpty() && isNullOrEmpty(d.getClassifier())) { + builder.classifier(parts[3]); + } + if (!parts[4].isEmpty() && isNullOrEmpty(d.getVersion())) { + builder.version(parts[4]); + } + break; + default: + break; + } + if (scope != null && isNullOrEmpty(d.getScope())) { + builder.scope(scope); + } + if (optional && isNullOrEmpty(d.getOptional())) { + builder.optional("true"); + } + List expanded = injectList(d.getExclusions(), this::expandExclusionId); + if (expanded != null) { + builder.exclusions(expanded); + } + return builder.build(); + } + + Exclusion expandExclusionId(Exclusion e) { + String id = e.getId(); + if (id == null || id.isEmpty()) { + return e; + } + String[] parts = id.split(":"); + if (parts.length != 2) { + return e; + } + Exclusion.Builder builder = Exclusion.newBuilder(e, true); + builder.id(null); + if (isNullOrEmpty(e.getGroupId())) { + builder.groupId(parts[0]); + } + if (isNullOrEmpty(e.getArtifactId())) { + builder.artifactId(parts[1]); + } + return builder.build(); + } + + Mixin expandMixinGav(Mixin m) { + String gav = m.getGav(); + if (gav == null || gav.isEmpty()) { + return m; + } + String[] parts = gav.split(":"); + if (parts.length != 3) { + return m; + } + Mixin.Builder builder = Mixin.newBuilder(m, true); + builder.gav(null); + if (isNullOrEmpty(m.getGroupId())) { + builder.groupId(parts[0]); + } + if (isNullOrEmpty(m.getArtifactId())) { + builder.artifactId(parts[1]); + } + if (isNullOrEmpty(m.getVersion())) { + builder.version(parts[2]); + } + return builder.build(); + } + + private static boolean isNullOrEmpty(String s) { + return s == null || s.isEmpty(); + } + + private static boolean isNullOrEmptyOrDefault(String type) { + return type == null || type.isEmpty() || "jar".equals(type); + } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java index bec34fe0fa76..03791aaee4c1 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java @@ -237,7 +237,8 @@ private static boolean objectsEqual(Object obj1, Object obj2) { */ private static boolean dependenciesEqual( org.apache.maven.api.model.Dependency dep1, org.apache.maven.api.model.Dependency dep2) { - return Objects.equals(dep1.getGroupId(), dep2.getGroupId()) + return Objects.equals(dep1.getId(), dep2.getId()) + && Objects.equals(dep1.getGroupId(), dep2.getGroupId()) && Objects.equals(dep1.getArtifactId(), dep2.getArtifactId()) && Objects.equals(dep1.getVersion(), dep2.getVersion()) && Objects.equals(dep1.getType(), dep2.getType()) @@ -286,6 +287,7 @@ private static int computeHashCode(Object obj) { */ private static int dependencyHashCode(org.apache.maven.api.model.Dependency dep) { return Objects.hash( + dep.getId(), dep.getGroupId(), dep.getArtifactId(), dep.getVersion(), diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index 8cc8cb3459b3..9d479de55fef 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -57,6 +57,7 @@ import org.apache.maven.api.model.Exclusion; import org.apache.maven.api.model.InputLocation; import org.apache.maven.api.model.InputLocationTracker; +import org.apache.maven.api.model.Mixin; import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Parent; import org.apache.maven.api.model.Plugin; @@ -372,6 +373,54 @@ && equals(parent.getArtifactId(), model.getArtifactId())) { // Validate each mixin for (Parent mixin : model.getMixins()) { + // Validate id/gav attribute format and conflict with child elements + if (mixin instanceof Mixin m + && m.getGav() != null + && !m.getGav().isEmpty()) { + String gav = m.getGav(); + String[] parts = gav.split(":"); + if (parts.length != 3) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + "mixins.mixin.id", + null, + "must have the format 'groupId:artifactId:version' but is '" + gav + "'.", + mixin); + } + if (mixin.getGroupId() != null && !mixin.getGroupId().isEmpty()) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + "mixins.mixin.groupId", + null, + "must not be specified when the 'id' attribute is used.", + mixin); + } + if (mixin.getArtifactId() != null && !mixin.getArtifactId().isEmpty()) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + "mixins.mixin.artifactId", + null, + "must not be specified when the 'id' attribute is used.", + mixin); + } + if (mixin.getVersion() != null && !mixin.getVersion().isEmpty()) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + "mixins.mixin.version", + null, + "must not be specified when the 'id' attribute is used.", + mixin); + } + } + if (mixin.getRelativePath() != null && !mixin.getRelativePath().isEmpty() && (mixin.getGroupId() != null && !mixin.getGroupId().isEmpty() @@ -1141,7 +1190,22 @@ private void validate20RawDependencies( Map index = new HashMap<>(); for (Dependency dependency : dependencies) { - String key = dependency.getManagementKey(); + // When 'id' attribute is set, extract the GAV portion (stripping @scope and ?) + // for deduplication, since normalization has not yet run + String key; + if (dependency.getId() != null && !dependency.getId().isEmpty()) { + key = stripScopeAndOptional(dependency.getId()); + } else { + key = dependency.getManagementKey(); + } + + // Validate id attribute format and conflict with child elements + validateDependencyIdAttribute(problems, dependency, prefix + prefix2); + + // Validate id attribute on exclusions + for (Exclusion exclusion : dependency.getExclusions()) { + validateExclusionIdAttribute(problems, exclusion, prefix + prefix2); + } if ("import".equals(dependency.getScope())) { if (!"pom".equals(dependency.getType())) { @@ -1258,6 +1322,165 @@ private void validate20RawDependencies( } } + private void validateDependencyIdAttribute(ModelProblemCollector problems, Dependency dependency, String prefix) { + String id = dependency.getId(); + if (id == null || id.isEmpty()) { + return; + } + + String remaining = id; + boolean hasOptionalMarker = false; + if (remaining.endsWith("?")) { + hasOptionalMarker = true; + remaining = remaining.substring(0, remaining.length() - 1); + } + + boolean hasScopeMarker = false; + int atIndex = remaining.lastIndexOf('@'); + if (atIndex >= 0) { + hasScopeMarker = true; + remaining = remaining.substring(0, atIndex); + } + + int colonCount = remaining.length() - remaining.replace(":", "").length(); + if (colonCount < 1 || colonCount > 4) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "id", + null, + "has invalid format '" + id + "', must be 'groupId:artifactId[:]', " + + "'groupId:artifactId:version', " + + "'groupId:artifactId:type:[version]', or " + + "'groupId:artifactId:type:classifier:[version]'. " + + "An optional '@scope' suffix and/or trailing '?' may be appended.", + dependency); + return; + } + if (!isNullOrEmpty(dependency.getGroupId())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "groupId", + null, + "must not be specified when 'id' attribute is used.", + dependency); + } + if (!isNullOrEmpty(dependency.getArtifactId())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "artifactId", + null, + "must not be specified when 'id' attribute is used.", + dependency); + } + if (!isNullOrEmpty(dependency.getVersion())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "version", + null, + "must not be specified when 'id' attribute is used.", + dependency); + } + if (colonCount >= 3 && !isNullOrEmpty(dependency.getType()) && !"jar".equals(dependency.getType())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "type", + null, + "must not be specified when 'id' attribute includes type.", + dependency); + } + if (colonCount >= 4 && !isNullOrEmpty(dependency.getClassifier())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "classifier", + null, + "must not be specified when 'id' attribute includes classifier.", + dependency); + } + if (hasScopeMarker && !isNullOrEmpty(dependency.getScope())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "scope", + null, + "must not be specified when 'id' attribute contains '@scope'.", + dependency); + } + if (hasOptionalMarker && !isNullOrEmpty(dependency.getOptional())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "optional", + null, + "must not be specified when 'id' attribute contains '?'.", + dependency); + } + } + + private static String stripScopeAndOptional(String id) { + String remaining = id; + if (remaining.endsWith("?")) { + remaining = remaining.substring(0, remaining.length() - 1); + } + int atIndex = remaining.lastIndexOf('@'); + if (atIndex >= 0) { + remaining = remaining.substring(0, atIndex); + } + return remaining; + } + + private void validateExclusionIdAttribute(ModelProblemCollector problems, Exclusion exclusion, String prefix) { + String id = exclusion.getId(); + if (id == null || id.isEmpty()) { + return; + } + int colonCount = id.length() - id.replace(":", "").length(); + if (colonCount != 1) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "exclusions.exclusion.id", + null, + "has invalid format '" + id + "', must be 'groupId:artifactId'.", + exclusion); + return; + } + if (!isNullOrEmpty(exclusion.getGroupId())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "exclusions.exclusion.groupId", + null, + "must not be specified when 'id' attribute is used.", + exclusion); + } + if (!isNullOrEmpty(exclusion.getArtifactId())) { + addViolation( + problems, + Severity.ERROR, + Version.V42, + prefix + "exclusions.exclusion.artifactId", + null, + "must not be specified when 'id' attribute is used.", + exclusion); + } + } + private void validate20RawDependenciesSelfReferencing( ModelProblemCollector problems, Model model, List dependencies, String prefix) { // We only check for groupId/artifactId/version/classifier cause if there is another @@ -2509,4 +2732,8 @@ static SourceHint resourceDirectory(Resource resource) { return resource::getDirectory; } } + + private static boolean isNullOrEmpty(String s) { + return s == null || s.isEmpty(); + } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelNormalizerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelNormalizerTest.java new file mode 100644 index 000000000000..26132ebcb82f --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelNormalizerTest.java @@ -0,0 +1,577 @@ +/* + * 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.impl.model; + +import java.util.List; + +import org.apache.maven.api.model.Dependency; +import org.apache.maven.api.model.DependencyManagement; +import org.apache.maven.api.model.Exclusion; +import org.apache.maven.api.model.Mixin; +import org.apache.maven.api.model.Model; +import org.apache.maven.api.model.Profile; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class DefaultModelNormalizerTest { + + private final DefaultModelNormalizer normalizer = new DefaultModelNormalizer(); + + @Test + void testExpandDependencyId2parts() { + Dependency dep = Dependency.newBuilder(false).id("org.example:lib").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib", result.getArtifactId()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId2partsTrailingColon() { + Dependency dep = Dependency.newBuilder(false).id("org.example:lib:").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib", result.getArtifactId()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId3parts() { + Dependency dep = + Dependency.newBuilder(false).id("org.slf4j:slf4j-api:2.0.17").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.slf4j", result.getGroupId()); + assertEquals("slf4j-api", result.getArtifactId()); + assertEquals("2.0.17", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId4parts() { + Dependency dep = + Dependency.newBuilder(false).id("org.example:lib-b:pom:1.0").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib-b", result.getArtifactId()); + assertEquals("pom", result.getType()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId4partsTrailingColon() { + Dependency dep = + Dependency.newBuilder(false).id("org.example:lib-b:pom:").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib-b", result.getArtifactId()); + assertEquals("pom", result.getType()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId5parts() { + Dependency dep = Dependency.newBuilder(false) + .id("org.example:lib-c:jar:sources:1.0") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib-c", result.getArtifactId()); + assertEquals("jar", result.getType()); + assertEquals("sources", result.getClassifier()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId5partsTrailingColon() { + Dependency dep = Dependency.newBuilder(false) + .id("org.example:lib-c:jar:sources:") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib-c", result.getArtifactId()); + assertEquals("jar", result.getType()); + assertEquals("sources", result.getClassifier()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdDoesNotOverrideExistingFields() { + Dependency dep = Dependency.newBuilder(false) + .id("org.slf4j:slf4j-api:2.0.17") + .groupId("org.override") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.override", result.getGroupId()); + assertEquals("slf4j-api", result.getArtifactId()); + assertEquals("2.0.17", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdInvalidFormat() { + Dependency dep = Dependency.newBuilder(false).id("invalid-no-colons").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertNull(result.getArtifactId()); + } + + @Test + void testExpandDependencyIdNull() { + Dependency dep = Dependency.newBuilder() + .groupId("org.example") + .artifactId("my-lib") + .version("1.0") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertEquals("1.0", result.getVersion()); + } + + @Test + void testExpandDependencyIdAlsoExpandsExclusions() { + Exclusion exc = Exclusion.newBuilder(false).id("com.example:unwanted").build(); + Dependency dep = Dependency.newBuilder(false) + .id("org.slf4j:slf4j-api:2.0.17") + .exclusions(List.of(exc)) + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.slf4j", result.getGroupId()); + assertEquals("slf4j-api", result.getArtifactId()); + assertEquals("2.0.17", result.getVersion()); + assertNull(result.getId()); + assertEquals(1, result.getExclusions().size()); + assertEquals("com.example", result.getExclusions().get(0).getGroupId()); + assertEquals("unwanted", result.getExclusions().get(0).getArtifactId()); + assertNull(result.getExclusions().get(0).getId()); + } + + // ===== @scope and ? (optional) tests ===== + + @Test + void testExpandDependencyIdWithScope() { + Dependency dep = Dependency.newBuilder(false) + .id("org.junit.jupiter:junit-jupiter-api:5.0@test") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.junit.jupiter", result.getGroupId()); + assertEquals("junit-jupiter-api", result.getArtifactId()); + assertEquals("5.0", result.getVersion()); + assertEquals("test", result.getScope()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdWithOptional() { + Dependency dep = + Dependency.newBuilder(false).id("commons-io:commons-io:2.11.0?").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("commons-io", result.getGroupId()); + assertEquals("commons-io", result.getArtifactId()); + assertEquals("2.11.0", result.getVersion()); + assertEquals("true", result.getOptional()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdWithScopeAndOptional() { + Dependency dep = Dependency.newBuilder(false) + .id("org.apache.maven:maven-core:3.9.0@provided?") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.apache.maven", result.getGroupId()); + assertEquals("maven-core", result.getArtifactId()); + assertEquals("3.9.0", result.getVersion()); + assertEquals("provided", result.getScope()); + assertEquals("true", result.getOptional()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdWithImportScope() { + Dependency dep = Dependency.newBuilder(false) + .id("org.junit:junit-bom:5.12.0@import") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.junit", result.getGroupId()); + assertEquals("junit-bom", result.getArtifactId()); + assertEquals("5.12.0", result.getVersion()); + assertEquals("import", result.getScope()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdScopeDoesNotOverrideExisting() { + Dependency dep = Dependency.newBuilder(false) + .id("org.junit.jupiter:junit-jupiter-api:5.0@test") + .scope("compile") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("compile", result.getScope()); + } + + @Test + void testExpandDependencyIdOptionalDoesNotOverrideExisting() { + Dependency dep = Dependency.newBuilder(false) + .id("commons-io:commons-io:2.11.0?") + .optional("false") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("false", result.getOptional()); + } + + @Test + void testExpandDependencyIdPlainGavNoScopeOrOptional() { + Dependency dep = + Dependency.newBuilder(false).id("org.slf4j:slf4j-api:2.0.17").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.slf4j", result.getGroupId()); + assertEquals("slf4j-api", result.getArtifactId()); + assertEquals("2.0.17", result.getVersion()); + assertNull(result.getScope()); + assertNull(result.getOptional()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdManagedWithScope() { + Dependency dep = Dependency.newBuilder(false) + .id("org.junit.jupiter:junit-jupiter-api@test") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.junit.jupiter", result.getGroupId()); + assertEquals("junit-jupiter-api", result.getArtifactId()); + assertNull(result.getVersion()); + assertEquals("test", result.getScope()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyId4partsWithScope() { + Dependency dep = Dependency.newBuilder(false) + .id("org.example:lib:pom:1.0@import") + .build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib", result.getArtifactId()); + assertEquals("pom", result.getType()); + assertEquals("1.0", result.getVersion()); + assertEquals("import", result.getScope()); + assertNull(result.getId()); + } + + // ===== Empty segment (inference) tests ===== + + @Test + void testExpandDependencyIdEmptyGroupId() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib:1.0").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyGroupIdManaged() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyGroupIdWithScope() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib@test").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertEquals("test", result.getScope()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyVersion() { + Dependency dep = Dependency.newBuilder(false).id("org.example:my-lib:").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyGroupIdAndVersion() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib:").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyTypeSkipped() { + Dependency dep = + Dependency.newBuilder(false).id("org.example:my-lib::1.0").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertNull(result.getType()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyClassifierSkipped() { + Dependency dep = + Dependency.newBuilder(false).id("org.example:my-lib:jar::1.0").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertEquals("org.example", result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertEquals("jar", result.getType()); + assertNull(result.getClassifier()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdAllEmptyExceptArtifactId() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib::").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertNull(result.getType()); + assertNull(result.getVersion()); + assertNull(result.getId()); + } + + @Test + void testExpandDependencyIdEmptyGroupIdWithTypeAndVersion() { + Dependency dep = Dependency.newBuilder(false).id(":my-lib:pom:1.0").build(); + + Dependency result = normalizer.expandDependencyId(dep); + + assertNull(result.getGroupId()); + assertEquals("my-lib", result.getArtifactId()); + assertEquals("pom", result.getType()); + assertEquals("1.0", result.getVersion()); + assertNull(result.getId()); + } + + // ===== Exclusion tests ===== + + @Test + void testExpandExclusionId() { + Exclusion exc = + Exclusion.newBuilder(false).id("com.example:unwanted-lib").build(); + + Exclusion result = normalizer.expandExclusionId(exc); + + assertEquals("com.example", result.getGroupId()); + assertEquals("unwanted-lib", result.getArtifactId()); + assertNull(result.getId()); + } + + @Test + void testExpandExclusionIdWildcard() { + Exclusion exc = Exclusion.newBuilder(false).id("*:*").build(); + + Exclusion result = normalizer.expandExclusionId(exc); + + assertEquals("*", result.getGroupId()); + assertEquals("*", result.getArtifactId()); + assertNull(result.getId()); + } + + @Test + void testExpandExclusionIdNull() { + Exclusion exc = + Exclusion.newBuilder().groupId("org.example").artifactId("lib").build(); + + Exclusion result = normalizer.expandExclusionId(exc); + + assertEquals("org.example", result.getGroupId()); + assertEquals("lib", result.getArtifactId()); + } + + // ===== Mixin tests ===== + + @Test + void testExpandMixinGav() { + Mixin mixin = Mixin.newBuilder(false) + .gav("com.example.mixins:java-mixin:1.0.0") + .build(); + + Mixin result = normalizer.expandMixinGav(mixin); + + assertEquals("com.example.mixins", result.getGroupId()); + assertEquals("java-mixin", result.getArtifactId()); + assertEquals("1.0.0", result.getVersion()); + assertNull(result.getGav()); + } + + @Test + void testExpandMixinGavNull() { + Mixin mixin = Mixin.newBuilder() + .groupId("com.example") + .artifactId("my-mixin") + .version("2.0") + .build(); + + Mixin result = normalizer.expandMixinGav(mixin); + + assertEquals("com.example", result.getGroupId()); + assertEquals("my-mixin", result.getArtifactId()); + assertEquals("2.0", result.getVersion()); + } + + // ===== Integration tests ===== + + @Test + void testMergeDuplicatesExpandsDependencyManagement() { + Dependency dep = + Dependency.newBuilder(false).id("org.slf4j:slf4j-api:2.0.17").build(); + Model model = Model.newBuilder() + .dependencyManagement(DependencyManagement.newBuilder() + .dependencies(List.of(dep)) + .build()) + .build(); + + Model result = normalizer.mergeDuplicates(model, null, null); + + assertEquals(1, result.getDependencyManagement().getDependencies().size()); + Dependency expanded = result.getDependencyManagement().getDependencies().get(0); + assertEquals("org.slf4j", expanded.getGroupId()); + assertEquals("slf4j-api", expanded.getArtifactId()); + assertEquals("2.0.17", expanded.getVersion()); + assertNull(expanded.getId()); + } + + @Test + void testMergeDuplicatesExpandsProfileDependencies() { + Dependency dep = + Dependency.newBuilder(false).id("org.slf4j:slf4j-api:2.0.17").build(); + Profile profile = Profile.newBuilder() + .id("test-profile") + .dependencies(List.of(dep)) + .build(); + Model model = Model.newBuilder().profiles(List.of(profile)).build(); + + Model result = normalizer.mergeDuplicates(model, null, null); + + assertEquals(1, result.getProfiles().size()); + assertEquals(1, result.getProfiles().get(0).getDependencies().size()); + Dependency expanded = result.getProfiles().get(0).getDependencies().get(0); + assertEquals("org.slf4j", expanded.getGroupId()); + assertEquals("slf4j-api", expanded.getArtifactId()); + assertEquals("2.0.17", expanded.getVersion()); + assertNull(expanded.getId()); + } + + @Test + void testMergeDuplicatesExpandsProfileDependencyManagement() { + Dependency dep = + Dependency.newBuilder(false).id("org.slf4j:slf4j-api:2.0.17").build(); + Profile profile = Profile.newBuilder() + .id("test-profile") + .dependencyManagement(DependencyManagement.newBuilder() + .dependencies(List.of(dep)) + .build()) + .build(); + Model model = Model.newBuilder().profiles(List.of(profile)).build(); + + Model result = normalizer.mergeDuplicates(model, null, null); + + assertEquals(1, result.getProfiles().size()); + DependencyManagement mgmt = result.getProfiles().get(0).getDependencyManagement(); + assertEquals(1, mgmt.getDependencies().size()); + Dependency expanded = mgmt.getDependencies().get(0); + assertEquals("org.slf4j", expanded.getGroupId()); + assertEquals("slf4j-api", expanded.getArtifactId()); + assertEquals("2.0.17", expanded.getVersion()); + assertNull(expanded.getId()); + } +} diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java index 591191e8920f..911d1f236c2e 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java @@ -1022,4 +1022,157 @@ void profileActivationConditionWithBasedirExpression() throws Exception { "raw-model/profile-activation-condition-with-basedir.xml", ModelValidator.VALIDATION_LEVEL_STRICT); assertViolations(result, 0, 0, 0); } + + // ===== dependency id attribute tests ===== + + @Test + void testDependencyId2partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-2parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyId3partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-3parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyId4partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-4parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyId5partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-5parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyIdTrailingColon3partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-trailing-colon-3parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyIdTrailingColon4partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-trailing-colon-4parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyIdTrailingColon5partsValid() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-trailing-colon-5parts-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyIdBadFormat() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-bad-format.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "invalid format"); + } + + @Test + void testDependencyIdConflictGroupId() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-conflict-groupId.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testDependencyIdConflictArtifactId() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-conflict-artifactId.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testDependencyIdConflictVersion() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-conflict-version.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testDependencyIdConflictType() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-conflict-type.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute includes type"); + } + + @Test + void testDependencyIdConflictMultiple() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-conflict-multiple.xml"); + assertViolations(result, 0, 2, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testValidDependencyIdWithScope() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-with-scope-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testValidDependencyIdWithOptional() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-with-optional-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testValidDependencyIdWithScopeAndOptional() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-with-scope-optional-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testDependencyIdScopeConflict() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-scope-conflict.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute contains '@scope'"); + } + + @Test + void testDependencyIdOptionalConflict() throws Exception { + SimpleProblemCollector result = validateFile("dependency-id-optional-conflict.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute contains '?'"); + } + + // ===== exclusion id attribute tests ===== + + @Test + void testExclusionIdValid() throws Exception { + SimpleProblemCollector result = validateFile("exclusion-id-valid.xml"); + assertViolations(result, 0, 0, 0); + } + + @Test + void testExclusionIdBadFormat() throws Exception { + SimpleProblemCollector result = validateFile("exclusion-id-bad-format.xml"); + assertViolations(result, 0, 1, 0); + } + + @Test + void testExclusionIdConflictGroupId() throws Exception { + SimpleProblemCollector result = validateFile("exclusion-id-conflict-groupId.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testExclusionIdConflictArtifactId() throws Exception { + SimpleProblemCollector result = validateFile("exclusion-id-conflict-artifactId.xml"); + assertViolations(result, 0, 1, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } + + @Test + void testExclusionIdConflictMultiple() throws Exception { + SimpleProblemCollector result = validateFile("exclusion-id-conflict-multiple.xml"); + assertViolations(result, 0, 2, 0); + assertContains(result.getErrors().get(0), "must not be specified when 'id' attribute is used"); + } } diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-2parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-2parts-valid.xml new file mode 100644 index 000000000000..3702648630b7 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-2parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-3parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-3parts-valid.xml new file mode 100644 index 000000000000..448265d0dbe9 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-3parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-4parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-4parts-valid.xml new file mode 100644 index 000000000000..05baf6afb331 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-4parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-5parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-5parts-valid.xml new file mode 100644 index 000000000000..133c0480d96a --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-5parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-bad-format.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-bad-format.xml new file mode 100644 index 000000000000..e53959b8b05d --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-bad-format.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-artifactId.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-artifactId.xml new file mode 100644 index 000000000000..8820417e71f4 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-artifactId.xml @@ -0,0 +1,12 @@ + + 4.2.0 + aid + gid + 0.1 + + + + slf4j-api + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-groupId.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-groupId.xml new file mode 100644 index 000000000000..171ded535502 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-groupId.xml @@ -0,0 +1,13 @@ + + 4.2.0 + test + test + 1.0 + + + org.other + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-multiple.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-multiple.xml new file mode 100644 index 000000000000..9729ef96404e --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-multiple.xml @@ -0,0 +1,13 @@ + + 4.2.0 + aid + gid + 0.1 + + + + org.slf4j + slf4j-api + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-type.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-type.xml new file mode 100644 index 000000000000..f1fb01b3dfd8 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-type.xml @@ -0,0 +1,13 @@ + + 4.2.0 + test + test + 1.0 + + + pom + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-version.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-version.xml new file mode 100644 index 000000000000..a303683e10b9 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-conflict-version.xml @@ -0,0 +1,12 @@ + + 4.2.0 + aid + gid + 0.1 + + + + 2.0.17 + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-optional-conflict.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-optional-conflict.xml new file mode 100644 index 000000000000..b1bd0158b6a3 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-optional-conflict.xml @@ -0,0 +1,12 @@ + + 4.2.0 + aid + gid + 0.1 + + + + true + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-scope-conflict.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-scope-conflict.xml new file mode 100644 index 000000000000..3fbea6c1213a --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-scope-conflict.xml @@ -0,0 +1,12 @@ + + 4.2.0 + aid + gid + 0.1 + + + + test + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-3parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-3parts-valid.xml new file mode 100644 index 000000000000..a76ebe1741ac --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-3parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-4parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-4parts-valid.xml new file mode 100644 index 000000000000..731c4d5c431d --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-4parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-5parts-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-5parts-valid.xml new file mode 100644 index 000000000000..e169e92fa980 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-trailing-colon-5parts-valid.xml @@ -0,0 +1,11 @@ + + 4.2.0 + test + test + 1.0 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-valid.xml new file mode 100644 index 000000000000..9618e8a7ff41 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-valid.xml @@ -0,0 +1,13 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + test + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-optional-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-optional-valid.xml new file mode 100644 index 000000000000..85cae9f857c4 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-optional-valid.xml @@ -0,0 +1,10 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-optional-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-optional-valid.xml new file mode 100644 index 000000000000..bd48b58a4635 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-optional-valid.xml @@ -0,0 +1,10 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-valid.xml new file mode 100644 index 000000000000..0a2479544407 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/dependency-id-with-scope-valid.xml @@ -0,0 +1,10 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-bad-format.xml b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-bad-format.xml new file mode 100644 index 000000000000..045977602564 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-bad-format.xml @@ -0,0 +1,15 @@ + + 4.2.0 + test + test + 1.0 + + + + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-artifactId.xml b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-artifactId.xml new file mode 100644 index 000000000000..5bfcedaba71d --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-artifactId.xml @@ -0,0 +1,16 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + + * + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-groupId.xml b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-groupId.xml new file mode 100644 index 000000000000..1d88746adffe --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-groupId.xml @@ -0,0 +1,16 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + + * + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-multiple.xml b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-multiple.xml new file mode 100644 index 000000000000..6c55c1652606 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-conflict-multiple.xml @@ -0,0 +1,17 @@ + + 4.2.0 + aid + gid + 0.1 + + + + + + * + * + + + + + diff --git a/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-valid.xml b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-valid.xml new file mode 100644 index 000000000000..3cd9825f2209 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/exclusion-id-valid.xml @@ -0,0 +1,15 @@ + + 4.2.0 + test + test + 1.0 + + + + + + + + diff --git a/impl/maven-support/src/test/java/org/apache/maven/model/v4/MavenStaxReaderTest.java b/impl/maven-support/src/test/java/org/apache/maven/model/v4/MavenStaxReaderTest.java index 8f5b4c355fb3..4be211767e7d 100644 --- a/impl/maven-support/src/test/java/org/apache/maven/model/v4/MavenStaxReaderTest.java +++ b/impl/maven-support/src/test/java/org/apache/maven/model/v4/MavenStaxReaderTest.java @@ -281,6 +281,74 @@ void testLocationReportingForListElements() throws Exception { assertEquals(5, module3Location.getColumnNumber(), "Module 3 should start at column 5"); } + @Test + void testDependencyIdAttribute() throws Exception { + String xml = "\n" + + " 4.2.0\n" + + " \n" + + " \n" + + " \n" + + ""; + + Model model = fromXml(xml); + assertEquals(1, model.getDependencies().size()); + Dependency dep = model.getDependencies().get(0); + assertEquals("org.slf4j:slf4j-api:2.0.17", dep.getId()); + } + + @Test + void testDependencyIdAttributeWithScope() throws Exception { + String xml = "\n" + + " 4.2.0\n" + + " \n" + + " \n" + + " test\n" + + " \n" + + " \n" + + ""; + + Model model = fromXml(xml); + assertEquals(1, model.getDependencies().size()); + Dependency dep = model.getDependencies().get(0); + assertEquals("org.junit.jupiter:junit-jupiter-api:5.14.1", dep.getId()); + assertEquals("test", dep.getScope()); + } + + @Test + void testExclusionIdAttribute() throws Exception { + String xml = "\n" + + " 4.2.0\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + Model model = fromXml(xml); + assertEquals(1, model.getDependencies().size()); + Dependency dep = model.getDependencies().get(0); + assertEquals("org.postgresql:postgresql:42.7.3", dep.getId()); + assertEquals(1, dep.getExclusions().size()); + assertEquals("*:*", dep.getExclusions().get(0).getId()); + } + + @Test + void testMixinIdAttribute() throws Exception { + String xml = "\n" + + " 4.2.0\n" + + " \n" + + " \n" + + " \n" + + ""; + + Model model = fromXml(xml); + assertEquals(1, model.getMixins().size()); + assertEquals("com.example:java-mixin:1.0.0", model.getMixins().get(0).getGav()); + } + private Model fromXml(String xml) throws XMLStreamException { MavenStaxReader reader = new MavenStaxReader(); return reader.read(new StringReader(xml), true, null); diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11904DependencyIdAttributeTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11904DependencyIdAttributeTest.java new file mode 100644 index 000000000000..41a5ba5a2ff1 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11904DependencyIdAttributeTest.java @@ -0,0 +1,91 @@ +/* + * 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.it; + +import java.io.Reader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Stream; + +import org.apache.maven.api.model.Model; +import org.apache.maven.model.v4.MavenStaxReader; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Integration test for GH-11904. + * Verifies that dependency {@code id} attributes are expanded into individual + * GAV elements in the consumer POM and that no {@code id} attribute remains. + */ +class MavenITgh11904DependencyIdAttributeTest extends AbstractMavenIntegrationTestCase { + + MavenITgh11904DependencyIdAttributeTest() { + super("[4.0.0-rc-3-SNAPSHOT,)"); + } + + @Test + void testConsumerPomDoesNotContainIdAttributes() throws Exception { + Path basedir = extractResources("/gh-11904-dependency-id-attribute"); + + Verifier verifier = newVerifier(basedir); + verifier.addCliArguments("install"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Verify consumer POM for the child module + Path consumerPomPath = verifier.getArtifactPath( + "org.apache.maven.its.gh-11904", "child", "1.0.0-SNAPSHOT", "pom"); + assertTrue(Files.exists(consumerPomPath), "consumer POM should exist at " + consumerPomPath); + + // Verify raw XML does not contain any id= attribute on dependency or exclusion elements + List consumerPomLines; + try (Stream lines = Files.lines(consumerPomPath)) { + consumerPomLines = lines.toList(); + } + assertTrue( + consumerPomLines.stream() + .noneMatch(line -> line.contains(" elements"); + assertTrue( + consumerPomLines.stream() + .noneMatch(line -> line.contains(" elements"); + + // Parse and verify the consumer POM model + Model consumerModel; + try (Reader r = Files.newBufferedReader(consumerPomPath)) { + consumerModel = new MavenStaxReader().read(r); + } + + // Verify dependency was expanded correctly + assertNotNull(consumerModel.getDependencies(), "Consumer POM should have dependencies"); + assertEquals(1, consumerModel.getDependencies().size(), "Consumer POM should have one dependency"); + + var dep = consumerModel.getDependencies().get(0); + assertNull(dep.getId(), "Dependency id attribute should be null in consumer POM"); + assertEquals("org.apache.maven.its.gh-11904", dep.getGroupId()); + assertEquals("lib-a", dep.getArtifactId()); + assertEquals("1.0.0-SNAPSHOT", dep.getVersion()); + } +} diff --git a/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/.mvn/.gitkeep b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/.mvn/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/pom.xml b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/pom.xml new file mode 100644 index 000000000000..834fd8b271a8 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/pom.xml @@ -0,0 +1,20 @@ + + + 4.2.0 + + + org.apache.maven.its.gh-11904 + parent + 1.0.0-SNAPSHOT + + + child + + + + + + + diff --git a/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/src/main/java/Test.java b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/src/main/java/Test.java new file mode 100644 index 000000000000..ef22a69f1931 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/child/src/main/java/Test.java @@ -0,0 +1 @@ +class Test {} diff --git a/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/lib-a/pom.xml b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/lib-a/pom.xml new file mode 100644 index 000000000000..26ab3ed3cd06 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/lib-a/pom.xml @@ -0,0 +1,15 @@ + + + 4.2.0 + + + org.apache.maven.its.gh-11904 + parent + 1.0.0-SNAPSHOT + + + lib-a + + diff --git a/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/pom.xml b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/pom.xml new file mode 100644 index 000000000000..185a83958493 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11904-dependency-id-attribute/pom.xml @@ -0,0 +1,21 @@ + + + 4.2.0 + + org.apache.maven.its.gh-11904 + parent + 1.0.0-SNAPSHOT + pom + + + lib-a + child + + + + 17 + + +