Fix #12531: filter NO_REPOSITORY sentinel from mapped exceptions in ArtifactResolverResult

[gnodet]

Summary

Fixes #12531 — Plugins that resolve dependencies (cyclonedx-maven-plugin, camel-spring-boot-generator) throw IllegalArgumentException: Unsupported repository type: class org.eclipse.aether.resolution.ArtifactResult$NoRepository when building the dependency tree under Maven 4.

Root cause

ArtifactResult.NO_REPOSITORY is a sentinel used in the resolver's mapped exceptions for errors with no associated repository. When DefaultArtifactResolver.toResult() converts these to the Maven API, the NO_REPOSITORY key was mapped to a null key in the Map<Repository, List<Exception>> exposed through ResultItem.getExceptions(). This null key could cause NullPointerException in downstream code iterating over the exceptions map.

Changes

• Filter out NO_REPOSITORY entries from the mapped exceptions map in toResult(), ensuring no null keys leak into the public API
• Preserve all exceptions (including NO_REPOSITORY ones) in a separate flat list (allExceptions) so that isMissing() still correctly considers all exceptions
• Use merge() when building the exceptions map for robustness against duplicate keys
• Add 3 unit tests covering:
  • NO_REPOSITORY entries don't produce null keys in the exceptions map
  • isMissing() correctly considers NO_REPOSITORY exceptions (including non-ArtifactNotFoundException types)
  • Resolved artifacts with NO_REPOSITORY exceptions are handled properly

This builds on commit a2aed72 which added NO_REPOSITORY handling to AbstractSession.getRepository().

Test plan

• New unit tests: DefaultArtifactResolverTest (3 tests)
• Module tests: mvn verify in impl/maven-impl (515 tests pass)
• Full reactor build: mvn clean install -DskipTests passes
• Verify with affected plugins (cyclonedx-maven-plugin, camel-spring-boot-generator) against this build

🤖 Generated with Claude Code

[gnodet]

Code Review

Clean, well-scoped fix for issue #12531. The PR correctly filters the NO_REPOSITORY sentinel from the public API's exceptions map while preserving all exceptions in an internal list for isMissing() semantics. The use of merge() when building the map is a robustness improvement over the old Collectors.toMap() (gracefully merges exception lists instead of throwing on duplicate keys). Three new unit tests adequately cover the fix scenarios.

Two minor observations (non-blocking):

• The allExceptions ArrayList could be wrapped with List.copyOf() for defensive immutability, consistent with record semantics.
• Pre-existing: isMissing() returns true on an empty allExceptions list due to allMatch() on empty stream (vacuous truth) — consider guarding with !allExceptions.isEmpty() if the intent requires at least one ArtifactNotFoundException.

LGTM.

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

Claude Code on behalf of gnodet

[gnodet]

Thanks for the review! Both observations addressed in 27ee62b:

1. List.copyOf(allExceptions) — wrapped for defensive immutability, consistent with record semantics.
2. Vacuous truth guard on isMissing() — added !allExceptions.isEmpty() check so an empty exception list doesn't falsely report the artifact as missing. Added a dedicated test for this edge case.

[gnodet]

Code Review (Re-review)

Both observations from the previous review have been properly addressed in commit 27ee62b:

1. List.copyOf() for allExceptions — ✅ The allExceptions ArrayList is now wrapped with List.copyOf() for defensive immutability before being passed to the record constructor.

2. Vacuous truth guard in isMissing() — ✅ The method now checks !allExceptions.isEmpty() before the allMatch() call, preventing an empty exception list from falsely reporting the artifact as missing. A dedicated test (isMissingReturnsFalseWhenNoExceptions) was added to cover this edge case.

One minor note (non-blocking):

• The mappedExceptions HashMap is not wrapped for immutability (e.g., Map.copyOf()), while allExceptions on the same line is wrapped with List.copyOf(). This is a minor inconsistency — the pre-existing code also used a mutable map, so not a regression.

LGTM.

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

Claude Code on behalf of gnodet

[gnodet]

Addressed the additional observation in 3446694: wrapped mappedExceptions with Map.copyOf() for consistency with the List.copyOf(allExceptions) wrapping. Updated test assertions to use keySet().stream().noneMatch() instead of containsKey(null) since immutable maps throw NPE on null key lookups.