Skip to content

Introduce validation control - #12548

Merged
cstamas merged 1 commit into
apache:maven-4.0.xfrom
cstamas:maven-4.0.x-validation-control
Jul 27, 2026
Merged

cstamas merged 1 commit into
apache:maven-4.0.xfrom
cstamas:maven-4.0.x-validation-control

Conversation

@cstamas

@cstamas cstamas commented Jul 27, 2026

Copy link
Copy Markdown
Member

Enables validation level choice in Maven Resolver validator.

This change should go to every Maven version having MavenValidator (3.10, 4.0 and 4.1).

Related: apache/maven-resolver#2007

Backport of: 5703b7d

Enables validation level choice in Maven Resolver validator.

This change should go to every Maven version having `MavenValidator` (3.10, 4.0 and 4.1).

Related: apache/maven-resolver#2007

Backport of: 5703b7d
@cstamas cstamas self-assigned this Jul 27, 2026
@cstamas cstamas added the enhancement New feature or request label Jul 27, 2026
@cstamas
cstamas marked this pull request as ready for review July 27, 2026 18:33

@gnodet gnodet left a comment

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.

Clean backport of commit 5703b7d (PR #12520) to maven-4.0.x. The design and implementation are sound, but there is a case-sensitivity issue in the documented config values.

Case-sensitive enum mismatch (medium): The Javadoc documents lowercase values (default, mild, off) and @Config(defaultValue = "default") also uses lowercase, but ConfigUtils.getEnum in resolver 2.0.21 uses Enum.valueOf which is case-sensitive. Users following the docs with -Dmaven.resolver.validation=default will get IllegalArgumentException because the enum constant is DEFAULT. The runtime default (when the property is unset) works correctly since it uses ValidationLevel.DEFAULT directly.

Consider either updating the Javadoc to document uppercase values, or handling case conversion before the enum lookup. This same issue exists in the original merge on master (PR #12520) and in the 3.10.x backport (PR #12549).

Minor: No unit tests exist for MavenValidator or MavenValidatorFactory — this is a pre-existing gap, not introduced by this PR.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet


/**
* Resolver validation control.
* Can be <code>default</code> (full validation), <code>mild</code> (only uninterpolated placeholders) or

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 Javadoc documents lowercase values (default, mild, off) and @Config(defaultValue = "default") uses lowercase too. However, ConfigUtils.getEnum in resolver 2.0.21 calls Enum.valueOf(enumClass, (String) value) which is case-sensitive. A user setting -Dmaven.resolver.validation=default (following this doc) will hit IllegalArgumentException because the enum constant is DEFAULT.

Consider either:

  1. Updating the Javadoc/defaultValue to use uppercase (DEFAULT, MILD, OFF), or
  2. Adding a toUpperCase(Locale.ROOT) call before the enum lookup in MavenValidatorFactory.

gnodet added a commit to gnodet/maven that referenced this pull request Jul 27, 2026
@cstamas

cstamas commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Damn, I mixed helper methods assuming it is fixing the case 😞
https://github.com/apache/maven-resolver/blob/838477a48a6cbcc26a51c56fff7f964440f2aaa3/maven-resolver-util/src/main/java/org/eclipse/aether/util/ConfigUtils.java#L347-L371

Will have another round to fix Javadoc (3.10, 4.0, 4.1)

@cstamas
cstamas merged commit 84a9a29 into apache:maven-4.0.x Jul 27, 2026
22 checks passed
@cstamas
cstamas deleted the maven-4.0.x-validation-control branch July 27, 2026 19:30
@github-actions github-actions Bot added this to the 4.0.0-rc-6 milestone Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants