Skip to content

Feat: Add id attribute (gav) support to Dependency, Exclusion, Mixin #11904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rbygrave wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feat: Add id attribute (gav) support to Dependency, Exclusion, Mixin #11904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f19909
Feat: Add id attribute (gav) support to Dependency, Exclusion, Mixin
rbygrave Apr 8, 2026
feeedab
Add more validation tests for dependency and exclusion id conflicts
rbygrave Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions api/maven-api-model/src/main/mdo/maven.mdo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,20 @@
]]>
</description>
<fields>
<field xml.attribute="true">
<name>id</name>
<version>4.2.0+</version>
<description>
<![CDATA[
Combined coordinates as {@code groupId:artifactId:version}.
When specified, it is split into the {@code groupId}, {@code artifactId},
and {@code version} fields during model normalization.
The individual child elements should not be specified when using this attribute.
@since Maven 4.2
]]>
</description>
<type>String</type>
</field>
<field>
<name>groupId</name>
<version>3.0.0+</version>
Expand Down Expand Up @@ -1440,6 +1454,20 @@
]]>
</description>
<fields>
<field xml.attribute="true">
<name>id</name>
<version>4.2.0+</version>
<description>
<![CDATA[
Combined coordinates as {@code groupId:artifactId}.
When specified, it is split into the {@code groupId} and {@code artifactId}
fields during model normalization.
The individual child elements should not be specified when using this attribute.
@since Maven 4.2
]]>
</description>
<type>String</type>
</field>
<field>
<name>groupId</name>
<version>4.0.0+</version>
Expand Down Expand Up @@ -1863,6 +1891,20 @@
<version>4.1.0+</version>
<superClass>Parent</superClass>
<fields>
<field xml.attribute="true" xml.tagName="id">
<name>gav</name>

@rbygrave rbygrave Apr 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using field name gav with attribute name id ... in order to avoid the conflict with the existing Parent.getId()

<version>4.2.0+</version>
<description>
<![CDATA[
Combined coordinates as {@code groupId:artifactId:version}.
When specified, it is split into the {@code groupId}, {@code artifactId},
and {@code version} fields during model normalization.
The individual child elements should not be specified when using this attribute.
@since Maven 4.2
]]>
</description>
<type>String</type>
</field>
<field>
<name>classifier</name>
<version>4.1.0+</version>
Expand Down
106 changes: 102 additions & 4 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
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.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.services.ModelBuilderRequest;
Expand All @@ -49,6 +51,9 @@ public class DefaultModelNormalizer implements ModelNormalizer {
public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProblemCollector problems) {
Model.Builder builder = Model.newBuilder(model);

// Expand id attributes on mixins
builder.mixins(injectList(model.getMixins(), this::expandMixinGav));

Build build = model.getBuild();
if (build != null) {
List<Plugin> plugins = build.getPlugins();
Expand Down Expand Up @@ -76,15 +81,20 @@ 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.
*/
// Expand id attributes on dependencies (and their exclusions), then deduplicate
List<Dependency> dependencies = model.getDependencies();
Map<String, Dependency> normalized = new LinkedHashMap<>(dependencies.size() * 2);
List<Dependency> expanded = injectList(dependencies, this::expandDependencyId);
if (expanded != null) {

@gnodet gnodet May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only expands id on top-level dependencies. Dependency management dependencies (model.getDependencyManagement().getDependencies()) and profile-scoped dependencies are also validated for id attribute conflicts (see DefaultModelValidator lines 536-596), but they won't be expanded here.

Should the expansion also apply to:

  • model.getDependencyManagement().getDependencies()
  • profile.getDependencies()
  • profile.getDependencyManagement().getDependencies()

Without this, a user writing <dependency id="g:a:v"/> in <dependencyManagement> will pass validation but the GAV fields will never be populated.

dependencies = expanded;
}

Map<String, Dependency> normalizedDeps = new LinkedHashMap<>(dependencies.size() * 2);
for (Dependency dependency : dependencies) {
normalized.put(dependency.getManagementKey(), dependency);
normalizedDeps.put(dependency.getManagementKey(), dependency);
}

if (dependencies.size() != normalized.size()) {
builder.dependencies(normalized.values());
if (expanded != null || dependencies.size() != normalizedDeps.size()) {
builder.dependencies(normalizedDeps.values());
}

return builder.build();
Expand Down Expand Up @@ -144,4 +154,92 @@ private <T> List<T> injectList(List<T> list, UnaryOperator<T> modifer) {
}
return newList;
}

/**
* Expands the {@code id} attribute on a dependency into its component fields.
* The id format is {@code groupId:artifactId:version}.
*/
Dependency expandDependencyId(Dependency d) {
String id = d.getId();
if (id == null || id.isEmpty()) {
// No id attribute, but still expand exclusion ids
List<Exclusion> expanded = injectList(d.getExclusions(), this::expandExclusionId);
return expanded != null ? d.withExclusions(expanded) : d;
}
String[] parts = id.split(":");

@gnodet gnodet May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normalizer silently ignores the id when the format is wrong (parts.length != 3), deferring to the validator. But if the validator and normalizer both split on :, and the user writes g:a:v:extra, the split produces 4 parts -- the normalizer skips it, and the validator also rejects it. This is fine.

However, consider that String.split(":") has edge cases: trailing empty strings are removed by default. For example, "g:a:" splits to ["g", "a"] (length 2, not 3). This means a dependency id="g:a:" would silently fail normalization AND pass the parts.length != 3 check in the validator -- resulting in a format error, which is correct. Just worth being aware of the edge case.

if (parts.length != 3) {
// Invalid format — will be caught by the validator
return d;
}
Dependency.Builder builder = Dependency.newBuilder(d);
if (isBlank(d.getGroupId())) {
builder.groupId(parts[0]);
}
if (isBlank(d.getArtifactId())) {
builder.artifactId(parts[1]);
}
if (isBlank(d.getVersion())) {
builder.version(parts[2]);
}
List<Exclusion> expanded = injectList(d.getExclusions(), this::expandExclusionId);
if (expanded != null) {
builder.exclusions(expanded);
}
return builder.build();
}

/**
* Expands the {@code id} attribute on an exclusion into its component fields.
* The id format is {@code groupId:artifactId}.
*/
Exclusion expandExclusionId(Exclusion e) {
String id = e.getId();
if (id == null || id.isEmpty()) {
return e;
}
String[] parts = id.split(":");
if (parts.length != 2) {
// Invalid format — will be caught by the validator
return e;
}
Exclusion.Builder builder = Exclusion.newBuilder(e);
if (isBlank(e.getGroupId())) {
builder.groupId(parts[0]);
}
if (isBlank(e.getArtifactId())) {
builder.artifactId(parts[1]);
}
return builder.build();
}

/**
* Expands the {@code id} (XML attribute) / {@code gav} (Java field) on a mixin
* into its component fields. The format is {@code groupId:artifactId:version}.
*/
Mixin expandMixinGav(Mixin m) {
String gav = m.getGav();
if (gav == null || gav.isEmpty()) {
return m;
}
String[] parts = gav.split(":");
if (parts.length != 3) {
// Invalid format — will be caught by the validator
return m;
}
Mixin.Builder builder = Mixin.newBuilder(m);
if (isBlank(m.getGroupId())) {
builder.groupId(parts[0]);
}
if (isBlank(m.getArtifactId())) {
builder.artifactId(parts[1]);
}
if (isBlank(m.getVersion())) {
builder.version(parts[2]);
}
return builder.build();
}

@gnodet gnodet May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this method name is misleading -- String.isBlank() in Java checks for whitespace-only strings, but this only checks null/empty. Consider renaming to isNullOrEmpty to avoid confusion, or use s == null || s.isBlank() to also handle whitespace-only values in coordinates.

Suggested change
private static boolean isNullOrEmpty(String s) {
return s == null || s.isEmpty();
}

private static boolean isBlank(String s) {
return s == null || s.isEmpty();
}
}
4 changes: 3 additions & 1 deletion impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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(),
Expand Down
Loading