feat(catalog): Add executorVersion for catalogs#91
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✅ Files skipped from review due to trivial changes (20)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdded an executorVersion field to catalog definitions and propagated it through model, index JSONs, and generator. A new CamelLauncherVersionResolver resolves appropriate camel-launcher versions (querying Maven/Red Hat metadata) and CamelCatalogGenerator stores the resolved value; tests and XML dependency for parsing were added. Changes
Sequence DiagramsequenceDiagram
participant Gen as CamelCatalogGenerator
participant Res as CamelLauncherVersionResolver
participant Repo as Maven/RedHat Repo
participant Def as CatalogDefinition
Gen->>Res: findLauncherVersion(camelVersion, runtime)
alt runtime == Quarkus
Res->>Repo: GET camel-quarkus BOM POM
Repo-->>Res: BOM XML
Res->>Res: parse BOM -> camel core version
end
Res->>Repo: GET org/apache/camel/camel-launcher/maven-metadata.xml
Repo-->>Res: metadata XML
Res->>Res: parse XML, filter by major.minor.patch, choose highest Red Hat build if present
Res-->>Gen: return executorVersion (string or null)
Gen->>Def: setExecutorVersion(value) and serialize index JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.java (1)
129-140: Network calls lack timeout configuration.The
URL.openStream()calls (here and at line 181) use default socket timeouts, which may cause indefinite blocking if the Maven repository is slow or unresponsive. This could impact build performance.♻️ Suggested fix using HttpURLConnection with timeouts
- try (InputStream is = new URI(pomUrl).toURL().openStream()) { + HttpURLConnection connection = (HttpURLConnection) new URI(pomUrl).toURL().openConnection(); + connection.setConnectTimeout(10000); // 10 seconds + connection.setReadTimeout(30000); // 30 seconds + try (InputStream is = connection.getInputStream()) {Apply the same pattern to the
fetchAvailableVersionsmethod at line 181.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.java` around lines 129 - 140, The network read uses URI(...).toURL().openStream() without timeouts in CamelLauncherVersionResolver; replace this with an HttpURLConnection obtained from new URL(pomUrl).openConnection(), set reasonable connect and read timeouts (e.g. connectTimeout/readTimeout), call connect(), check HTTP response codes and then use connection.getInputStream() in the try-with-resources, and finally disconnect the connection in a finally or ensure it's closed; apply the same change to the fetchAvailableVersions method so both places use HttpURLConnection with explicit timeouts and proper error-handling of non-2xx responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@catalog/index.json`:
- Around line 7-134: The Citrus catalog entry is missing the executorVersion
field; open the catalog entry for "fileName" : "citrus/4.10.0/index.json" (name
"Citrus 4.10.0") and add the executorVersion property (set to null to match
other generated entries) so the JSON object includes "executorVersion": null
alongside the existing keys.
In
`@src/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.java`:
- Around line 152-170: The current extractCamelVersionFromPom method uses a
fragile regex that assumes <groupId>, <artifactId>, <version> ordering; instead
parse the POM XML properly (use the existing XmlMapper or an XML parser) to
locate dependency elements and find a dependency whose groupId equals
"org.apache.camel" and then read its version element regardless of element
order; update extractCamelVersionFromPom to parse pomContent into a DOM or
XmlMapper object, iterate dependencies (or dependency nodes), match groupId ==
"org.apache.camel" and return the associated version text, and preserve the
existing logging/error handling in the method.
In
`@src/test/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolverTest.java`:
- Around line 50-53: Replace the fragile hard-coded build-number assertions on
launcherVersion with a regex-based check: update the assertions in
CamelLauncherVersionResolverTest (the assertions referencing
launcherVersion.startsWith(...) and launcherVersion.contains(...)) to assert
that launcherVersion matches a pattern like "4\\.14\\.2\\.redhat-\\d{5}" (same
approach as used in testHighestBuildNumberSelection()) so any valid 5-digit Red
Hat build number is accepted; keep the startsWith check for "4.14.2.redhat-" if
desired but prefer a single regex match against launcherVersion to validate the
entire version string.
---
Nitpick comments:
In
`@src/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.java`:
- Around line 129-140: The network read uses URI(...).toURL().openStream()
without timeouts in CamelLauncherVersionResolver; replace this with an
HttpURLConnection obtained from new URL(pomUrl).openConnection(), set reasonable
connect and read timeouts (e.g. connectTimeout/readTimeout), call connect(),
check HTTP response codes and then use connection.getInputStream() in the
try-with-resources, and finally disconnect the connection in a finally or ensure
it's closed; apply the same change to the fetchAvailableVersions method so both
places use HttpURLConnection with explicit timeouts and proper error-handling of
non-2xx responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f84ab6e-e8d2-4502-b43e-7fdfbd4cfe37
📒 Files selected for processing (26)
catalog/camel-main/4.10.7.redhat-00009/index-1f57a659af08f398f2020a923d6d3099.jsoncatalog/camel-main/4.14.2.redhat-00019/index-c986f5a09e5fb0a0a83d13f44b05cdfc.jsoncatalog/camel-main/4.14.5/index-aad789f467553ebb007c4de1dd0fc19.jsoncatalog/camel-main/4.18.0/index-aafd5061bbeab5d06ca7521685240305.jsoncatalog/camel-main/4.4.0.redhat-00046/index-d63c691542e5df096a8b94b9838fe015.jsoncatalog/camel-main/4.8.5.redhat-00008/index-fe183e75eb64081a586884d6ea2872dd.jsoncatalog/camel-quarkus/3.15.0.redhat-00010/index-d9a2e40faa59b11f811a597f1c6ab6c7.jsoncatalog/camel-quarkus/3.20.0.redhat-00010/index-991a69a180a5bbeb93bac6a5460a0b72.jsoncatalog/camel-quarkus/3.27.1.redhat-00004/index-a5e53639f248b7bdc28f355a4b7da22e.jsoncatalog/camel-quarkus/3.27.2/index-6cfd62b67d3d0ff5c9e16f2748aa32e9.jsoncatalog/camel-quarkus/3.32.0/index-440c26eaeb10f18d3c4faf3a1f3a426b.jsoncatalog/camel-quarkus/3.8.0.redhat-00018/index-6f327d62de4ab29abb0eb8865668566d.jsoncatalog/camel-springboot/4.10.7.redhat-00013/index-20c9c8cbe4d6cd35a2db626c291468aa.jsoncatalog/camel-springboot/4.14.2.redhat-00018/index-a5950700bff86b4b4a396443402daecb.jsoncatalog/camel-springboot/4.14.5/index-4dc4f492b1b1335fa466f8fdc908e43a.jsoncatalog/camel-springboot/4.18.0/index-18f3c5769aa0668d2a79009acb97772b.jsoncatalog/camel-springboot/4.4.0.redhat-00039/index-5535bb5222a5b4e5732bb76af7ce419e.jsoncatalog/camel-springboot/4.8.5.redhat-00008/index-b91f4df1bc547f38d300d161ca65d99c.jsoncatalog/index.jsonpom.xmlsrc/main/java/io/kaoto/camelcatalog/generator/CamelCatalogGenerator.javasrc/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.javasrc/main/java/io/kaoto/camelcatalog/model/CatalogDefinition.javasrc/main/java/io/kaoto/camelcatalog/model/CatalogLibrary.javasrc/main/java/io/kaoto/camelcatalog/model/CatalogLibraryEntry.javasrc/test/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolverTest.java
| private String extractCamelVersionFromPom(String pomContent) { | ||
| try { | ||
| // Simple regex to find org.apache.camel dependency version | ||
| // Pattern: <groupId>org.apache.camel</groupId>....<version>X.Y.Z</version> | ||
| java.util.regex.Pattern pattern = java.util.regex.Pattern.compile( | ||
| "<groupId>org\\.apache\\.camel</groupId>\\s*<artifactId>[^<]+</artifactId>\\s*<version>([^<]+)</version>", | ||
| java.util.regex.Pattern.DOTALL | ||
| ); | ||
| java.util.regex.Matcher matcher = pattern.matcher(pomContent); | ||
|
|
||
| if (matcher.find()) { | ||
| return matcher.group(1); | ||
| } | ||
| } catch (Exception e) { | ||
| LOGGER.log(Level.WARNING, "Failed to extract Camel version from POM content", e); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Fragile regex pattern may fail on valid POM structures.
The regex assumes <groupId> → <artifactId> → <version> ordering, but Maven POMs don't guarantee element order within <dependency>. Elements like <type>, <scope>, or <classifier> could appear between them, or the order could be different.
🛠️ Suggested more flexible approach
private String extractCamelVersionFromPom(String pomContent) {
try {
- // Simple regex to find org.apache.camel dependency version
- // Pattern: <groupId>org.apache.camel</groupId>....<version>X.Y.Z</version>
- java.util.regex.Pattern pattern = java.util.regex.Pattern.compile(
- "<groupId>org\\.apache\\.camel</groupId>\\s*<artifactId>[^<]+</artifactId>\\s*<version>([^<]+)</version>",
- java.util.regex.Pattern.DOTALL
- );
- java.util.regex.Matcher matcher = pattern.matcher(pomContent);
-
- if (matcher.find()) {
- return matcher.group(1);
+ // Use a more flexible pattern that finds dependency blocks containing org.apache.camel
+ java.util.regex.Pattern depPattern = java.util.regex.Pattern.compile(
+ "<dependency>([^<]*(?:<(?!/dependency>)[^<]*)*)</dependency>",
+ java.util.regex.Pattern.DOTALL
+ );
+ java.util.regex.Matcher depMatcher = depPattern.matcher(pomContent);
+
+ while (depMatcher.find()) {
+ String depBlock = depMatcher.group(1);
+ if (depBlock.contains("<groupId>org.apache.camel</groupId>")) {
+ java.util.regex.Pattern versionPattern = java.util.regex.Pattern.compile("<version>([^<]+)</version>");
+ java.util.regex.Matcher versionMatcher = versionPattern.matcher(depBlock);
+ if (versionMatcher.find()) {
+ return versionMatcher.group(1);
+ }
+ }
}
} catch (Exception e) {
LOGGER.log(Level.WARNING, "Failed to extract Camel version from POM content", e);Alternatively, consider using the XmlMapper that's already available to properly parse the POM XML.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/io/kaoto/camelcatalog/generator/CamelLauncherVersionResolver.java`
around lines 152 - 170, The current extractCamelVersionFromPom method uses a
fragile regex that assumes <groupId>, <artifactId>, <version> ordering; instead
parse the POM XML properly (use the existing XmlMapper or an XML parser) to
locate dependency elements and find a dependency whose groupId equals
"org.apache.camel" and then read its version element regardless of element
order; update extractCamelVersionFromPom to parse pomContent into a DOM or
XmlMapper object, iterate dependencies (or dependency nodes), match groupId ==
"org.apache.camel" and return the associated version text, and preserve the
existing logging/error handling in the method.
5371958 to
a5c7c4f
Compare
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "name" : "Camel Main 4.18.0", | |||
There was a problem hiding this comment.
there was a patch release 4.18.1, maybe a good time to bump a version in our catalog too?
(it can be done also in a separate PR)
There was a problem hiding this comment.
Ah, sure, I'll do it in a separate PR to avoid the noise
djelinek
left a comment
There was a problem hiding this comment.
LGTM, thank you for this Ricardo 🙏 👍🏻
Context
Add executor version to the catalog
Summary by CodeRabbit
New Features
Tests