Avoid IllegalStateException on duplicate profile ids in DefaultModelBuilder

[mvanhorn]

Summary

Projects with duplicate profile ids build again instead of dying with java.lang.IllegalStateException: Duplicate key default (attempted merging values ...). Maven 3 accepted these POMs; Maven 4 rc-1/rc-2 crash during dependency collection.

Why this matters

In #10209, @gnodet confirmed the exception is misleading: the activation save/restore map in model building is keyed by profile id, so the real condition is two <profile> entries sharing an id (e.g. two <id>default</id>), surfacing as an opaque JDK collector error instead of a model diagnostic. @cstamas linked the regression to the MNG-8391 change. The issue is labeled bug/priority:major with no prior PRs.

Changes

• getProfileActivations / injectProfileActivations in DefaultModelBuilder no longer key the activation save/restore by profile id - activations round-trip positionally with model.getProfiles(), so duplicate ids cannot collide in a map. This restores Maven 3's leniency in the model-building path while DefaultModelValidator remains the single owner of the duplicate-id diagnostic ("must be unique but found duplicate profile with id ..."), so no raw IllegalStateException escapes to the user.

Testing

mvn -pl impl/maven-impl -am test -Dtest=DefaultModelBuilderTest - 13 tests, 0 failures, including a new test with duplicate-profile-ids.xml (two profiles sharing an id, each with its own activation) verifying model building succeeds and both activations survive the save/restore round-trip.

Fixes #10209

[elharo]

Does this fix #10209? if so indicate that in the PR descriptiion so it will autoclose when merged

[mvanhorn]

Yes, this fixes #10209 (the MNG-8418 duplicate-profile-id regression @gnodet diagnosed there). The description has a Fixes #10209 line at the bottom, so it'll auto-close on merge.

[gnodet]

Review Summary

This PR correctly fixes issue #10209 (MNG-8418) — a regression where Collectors.toMap() in getProfileActivations threw IllegalStateException when a POM contained profiles with duplicate IDs.

The fix replaces the id-keyed Map<String, Activation> with a positionally-indexed List<Activation>, which avoids the toMap collision while preserving the save/restore semantics. The approach is sound:

• Positional safety: getProfileActivations and injectProfileActivations run back-to-back on the same model with no intervening profile-list mutations, so the positional correspondence is guaranteed.
• Backwards compatibility: DefaultModelValidator continues to report duplicate profile IDs as a validation diagnostic. This PR removes the crash while preserving the warning — restoring Maven 3's behavior.
• Test quality: The test directly exercises duplicate-ID profiles and verifies both activations survive the round-trip.

Minor suggestion

The new getProfileActivations includes all profiles via .map(Profile::getActivation), producing null entries for profiles without activations. This is functionally safe because injectProfileActivations guards access with if (activation != null), but the invariant is subtle. A brief comment (e.g., // null entries for profiles without activation; only accessed by index when activation is non-null) would make the contract explicit for future maintainers.

---

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

Claude Code on behalf of Guillaume Nodet

[mvanhorn]

The initial-build failure traced to Spotless formatting drift in the PR files; fixed in b33e52a. Verified locally on JDK 17: DefaultModelBuilderTest 13/13 passing.

[gnodet]

Re-review — New Commit

The new commit (b33e52a) is a Spotless formatting fix only — it collapses multi-line statements onto single lines and adjusts line wrapping to satisfy the project's code formatter. No logic, test coverage, or behavior changed.

The core fix (positional list-based activation tracking instead of map-based keyed by profile id) remains correct and well-tested. The test testDuplicateProfileIdsRetainActivations properly validates that two profiles sharing the id "default" retain their distinct activations. ✅

---

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

Claude Code on behalf of Guillaume Nodet