gnodet-bot commented on code in PR #26595:
URL: https://github.com/apache/camel/pull/26595#discussion_r4046605979
##########
dsl/camel-kamelet-main/src/main/known-third-party-libraries.properties:
##########
@@ -145,10 +146,10 @@ com.fasterxml.jackson.dataformat.yaml =
com.fasterxml.jackson.dataformat:jackson
com.fasterxml.jackson.dataformat.csv =
com.fasterxml.jackson.dataformat:jackson-dataformat-csv:${jackson2-version}
com.google.gson = com.google.code.gson:gson:${gson-version}
com.alibaba.fastjson = com.alibaba:fastjson:${fastjson-version}
-org.eclipse.yasson = org.eclipse:yasson:${yasson-version}
+org.eclipse.yasson = org.eclipse.yasson:yasson:${yasson-version}
Review Comment:
⚠️ **Wrong group ID for Yasson.** `org.eclipse.yasson:yasson` does **not**
exist on Maven Central — `numFound: 0` from the search API. The correct
published coordinates are `org.eclipse:yasson` (latest 3.0.4), exactly what was
there before.
Confirmed:
- `org.eclipse:yasson:3.0.4` → 200 OK, real JAR on Central
- `org.eclipse.yasson:yasson:3.0.5` → 200 OK, also real JAR — **wait, this
one does exist** (it's a new groupId that appeared in 3.0.5). But
`org.eclipse:yasson:3.0.5` is a relocation POM with
`<packaging>pom</packaging>` pointing to `org.eclipse.yasson:yasson`, meaning
the old coord is now pom-only and the real artifact moved.
So the mapping is actually correct for 3.0.5+, **but it breaks for 3.0.4 and
earlier** since `org.eclipse.yasson:yasson` didn't exist before 3.0.5. The
`${yasson-version}` in camel parent is currently `3.0.5`, so at this exact
version it works. However:
1. The comment in the PR body says the old coord was a relocation — that's
only true from 3.0.5 onward. For 3.0.4 (and camel-4.x branches still on 3.0.4),
this mapping would resolve to a groupId that doesn't exist for that version.
2. No comment in the properties file explains this version-dependency.
Consider adding a comment like:
```suggestion
# org.eclipse.yasson:yasson is the real groupId since 3.0.5;
org.eclipse:yasson is a relocation pom from that version on.
org.eclipse.yasson = org.eclipse.yasson:yasson:${yasson-version}
```
##########
dsl/camel-kamelet-main/src/test/java/org/apache/camel/main/download/KnownDependenciesResolverTest.java:
##########
@@ -66,7 +66,7 @@ void theShippedMappingResolvesThirdPartyClassesByPackage() {
assertGav(resolver, "org.postgresql.ds.PGConnectionPoolDataSource",
"org.postgresql", "postgresql");
assertGav(resolver, "org.h2.jdbcx.JdbcDataSource", "com.h2database",
"h2");
assertGav(resolver, "com.zaxxer.hikari.HikariConfig", "com.zaxxer",
"HikariCP");
- assertGav(resolver,
"org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory",
"org.apache.activemq",
+ assertGav(resolver,
"org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory",
"org.apache.artemis",
"artemis-jakarta-client-all");
Review Comment:
⚠️ **Missing test assertions for the yasson and univocity groupId changes.**
This PR changes two more groupIds in the properties file (`org.eclipse.yasson`
and `com.sonofab1rd`) but the test only covers the Artemis change. If those
mappings are silently wrong, the test won't catch it.
Add assertions after line 70:
```suggestion
"artemis-jakarta-client-all");
assertGav(resolver, "org.eclipse.yasson.internal.Unmarshaller",
"org.eclipse.yasson", "yasson");
assertGav(resolver, "com.univocity.parsers.csv.CsvParser",
"com.sonofab1rd", "univocity-parsers");
```
(Use any real class from those JARs — these are illustrative but the class
names are plausible. Pick actual class names from the JARs if these don't
compile.)
--
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]
