gnodet-bot commented on code in PR #13881:
URL: https://github.com/apache/maven/pull/13881#discussion_r4502543876
##########
its/core-it-suite/src/test/resources/mng-7836-alternative-pom-syntax/maven-hocon-extension/src/main/java/org/apache/maven/hocon/HoconModelReader.java:
##########
@@ -41,6 +41,10 @@
@Priority(1)
public class HoconModelReader implements ModelParser {
+ public String id() {
Review Comment:
๐งน **Missing `@Override` on `id()`** โ `HoconModelReader` implements
`ModelParser.id()` but doesn't annotate it. Same issue in
`DumbModelParser1`โ`4` (all IT fixtures except `PropertiesSettingsParser`).
```suggestion
@Override
public String id() {
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java:
##########
@@ -75,19 +76,30 @@
public class DefaultModelProcessor implements ModelProcessor {
private final ModelXmlFactory modelXmlFactory;
- private final Map<String, ModelParser> modelParsers;
+ private final List<ModelParser> modelParsers;
@Inject
- public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable
Map<String, ModelParser> modelParsers) {
+ public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable
List<ModelParser> modelParsers) {
this.modelXmlFactory = modelXmlFactory;
- this.modelParsers = modelParsers != null ? modelParsers : Map.of();
+ this.modelParsers = deduplicate(modelParsers);
+ }
+
+ private static List<ModelParser> deduplicate(@Nullable List<ModelParser>
parsers) {
+ if (parsers == null || parsers.isEmpty()) {
+ return List.of();
+ }
+ Map<String, ModelParser> seen = new LinkedHashMap<>();
+ for (ModelParser parser : parsers) {
+ seen.putIfAbsent(parser.id(), parser);
Review Comment:
๐ **Silent deduplication โ still no WARN log here.**
`DefaultSettingsBuilder.deduplicate()` logs a `WARN` when a duplicate parser id
is detected (you added that explicitly there).
`DefaultModelProcessor.deduplicate()` uses `putIfAbsent` with no log at all โ a
`ModelParser` extension registering the same `id()` as another silently
disappears with zero user-visible indication.
Add the same `LOGGER.warn(...)` pattern used in `DefaultSettingsBuilder`:
```suggestion
if (seen.putIfAbsent(parser.id(), parser) != null) {
LOGGER.warn(
"Duplicate model parser id '{}': {} ignored in
favour of {}",
parser.id(),
parser.getClass().getName(),
seen.get(parser.id()).getClass().getName());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
