This is an automated email from the ASF dual-hosted git repository. luigidemasi pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-kit.git
commit 9c5224afae649fea5e934c75b39152401dd7da42 Author: Jinal Sarvaiya <[email protected]> AuthorDate: Wed Nov 11 14:21:54 2026 +0000 address review findings --- .../org/apache/camel/kit/jbang/KitInitCommand.java | 14 +++ .../camel/kit/generator/ManagedBlockMerger.java | 6 +- .../kit/generator/SkillResourceInstaller.java | 8 +- .../org/apache/camel/kit/service/InitRequest.java | 22 +++++ .../org/apache/camel/kit/service/InitService.java | 22 +++-- .../camel-brainstorm/guides/version-selection.md | 5 +- .../main/resources/skills/camel-validate/SKILL.md | 66 +++++++++------ .../apache/camel/kit/graph/ProjectDiscovery.java | 99 ++++++++++++++++++++-- .../camel/kit/graph/parser/ConfigParser.java | 60 ++++++++++--- 9 files changed, 241 insertions(+), 61 deletions(-) diff --git a/kit/camel-jbang-plugin-kit/src/main/java/org/apache/camel/kit/jbang/KitInitCommand.java b/kit/camel-jbang-plugin-kit/src/main/java/org/apache/camel/kit/jbang/KitInitCommand.java index dacbef1d..ffeb2214 100644 --- a/kit/camel-jbang-plugin-kit/src/main/java/org/apache/camel/kit/jbang/KitInitCommand.java +++ b/kit/camel-jbang-plugin-kit/src/main/java/org/apache/camel/kit/jbang/KitInitCommand.java @@ -60,6 +60,17 @@ public class KitInitCommand extends CamelCommand { description = "Source platform for migration graph analysis: mulesoft, camel, biztalk, auto (default: auto)") String sourcePlatform; + @Option(names = {"--dry-run"}, description = "Preview changes without writing files") + boolean dryRun; + + @Option(names = {"--replace-wrapper"}, + description = "Replace Maven wrapper even if it exists (default: preserve existing wrapper)") + boolean replaceWrapper; + + @Option(names = {"--record-baseline"}, + description = "Run existing project tests during init to record baseline (executes project build code; default: off)") + boolean recordBaseline; + private final CamelKitMain camelKitMain; public KitInitCommand(CamelJBangMain main) { @@ -86,6 +97,9 @@ public class KitInitCommand extends CamelCommand { initCommand.properties = this.properties; initCommand.withCitrusMcp = this.withCitrusMcp; initCommand.sourcePlatform = this.sourcePlatform; + initCommand.dryRun = this.dryRun; + initCommand.replaceWrapper = this.replaceWrapper; + initCommand.recordBaseline = this.recordBaseline; return initCommand.call(); } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ManagedBlockMerger.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ManagedBlockMerger.java index 23d1e597..1ca5b3ff 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ManagedBlockMerger.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/ManagedBlockMerger.java @@ -32,7 +32,7 @@ public class ManagedBlockMerger { if (!Files.exists(targetFile)) { // File doesn't exist, create it with the managed block - Files.writeString(targetFile, fullManagedBlock + "\n"); + AtomicFileWriter.write(targetFile, fullManagedBlock + "\n"); return; } @@ -46,11 +46,11 @@ public class ManagedBlockMerger { if (matcher.find()) { // Replace existing managed block String updated = matcher.replaceFirst(Matcher.quoteReplacement(fullManagedBlock)); - Files.writeString(targetFile, updated); + AtomicFileWriter.write(targetFile, updated); } else { // Append managed block to existing file String updated = existingContent.trim() + "\n\n" + fullManagedBlock + "\n"; - Files.writeString(targetFile, updated); + AtomicFileWriter.write(targetFile, updated); } } } diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java index b0747d55..efe68ade 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/SkillResourceInstaller.java @@ -144,12 +144,14 @@ class SkillResourceInstaller { throws Exception { Files.createDirectories(destination.getParent()); - // Always update SKILL.md and guide files (in guides/ subdirectory) - // Preserve user files (anything outside guides/ that isn't SKILL.md) + // Always update shipped resources: SKILL.md, guide files, and shared/ resources + // Preserve user files (custom skills the user created) String fileName = destination.getFileName().toString(); boolean isSkillManifest = fileName.equals("SKILL.md"); boolean isGuideFile = destination.getParent().getFileName().toString().equals("guides"); - boolean shouldUpdate = isSkillManifest || isGuideFile; + boolean isSharedResource = destination.toString().contains("/shared/") + || destination.toString().contains("\\shared\\"); + boolean shouldUpdate = isSkillManifest || isGuideFile || isSharedResource; if (Files.exists(destination) && !shouldUpdate) { // Skip existing user files to preserve customizations diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitRequest.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitRequest.java index 24b2c978..bafd9bee 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitRequest.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitRequest.java @@ -31,6 +31,28 @@ public record InitRequest( boolean replaceWrapper, boolean recordBaseline) { + /** + * Creates a request with default flags (no dry-run, no wrapper replacement, no baseline recording). Used by tests + * that predate the new flags. + */ + public InitRequest( + String projectName, + String agentName, + Path targetDir, + String citrusVersion, + boolean noFetch, + boolean withCitrusMcp, + String sourcePlatform, + String commandPrefix, + String defaultCitrusVersion, + DistributionConfig distribution, + Printer printer, + InitProgress progress, + InitReporter reporter) { + this(projectName, agentName, targetDir, citrusVersion, noFetch, withCitrusMcp, sourcePlatform, + commandPrefix, defaultCitrusVersion, distribution, printer, progress, reporter, false, false, false); + } + /** * Creates a request that does not run the project's tests to record a baseline. */ diff --git a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java index ff4e31d1..aad1c69f 100644 --- a/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java +++ b/kit/camel-kit-core/src/main/java/org/apache/camel/kit/service/InitService.java @@ -191,7 +191,11 @@ public class InitService { } else { config.setProperty("citrus.mcp.enabled", "false"); } - writeVersionConfig(config, request.distribution(), "main"); + // For greenfield projects only: seed distribution defaults + // Existing projects get versions from discovery in updateConfigWithRuntime + if (!isExistingProject) { + writeVersionConfig(config, request.distribution(), "main"); + } if (request.sourcePlatform() != null && !"auto".equals(request.sourcePlatform())) { config.setProperty("project.sourcePlatform", request.sourcePlatform()); } @@ -453,7 +457,7 @@ public class InitService { List<String> contractFiles = findContractFiles(projectRoot); if (!contractFiles.isEmpty()) { config.setProperty("project.contractFilesCount", String.valueOf(contractFiles.size())); - writeBaselineLines(dir.resolve("baseline-contracts.txt"), contractFiles); + writeBaselineLines(dir.resolve("baseline-contracts.txt"), contractFiles, projectRoot); } // Gaps the graph could not fill (e.g. routes defined in unsupported layouts): workflow must inspect source @@ -878,12 +882,13 @@ public class InitService { */ private void writeBaselineStatus(Path file, String status, String reason) { try { + org.apache.camel.kit.util.ConfinedPaths.requirePath(file.getParent().getParent(), file); String json = String.format(Locale.ROOT, "{\n \"status\": \"%s\",\n \"reason\": \"%s\",\n \"timestamp\": \"%s\"\n}", status, reason.replace("\"", "\\\""), java.time.Instant.now().toString()); - Files.writeString(file, json); + AtomicFileWriter.write(file, json); } catch (Exception e) { // Best-effort - ignore write failures } @@ -913,7 +918,8 @@ public class InitService { if (!testFiles.isEmpty()) { Path baselineFile = camelKitDir.resolve("baseline-tests.txt"); - Files.write(baselineFile, testFiles.stream().sorted().toList()); + org.apache.camel.kit.util.ConfinedPaths.requirePath(projectRoot, baselineFile); + AtomicFileWriter.write(baselineFile, String.join("\n", testFiles.stream().sorted().toList())); } } catch (Exception e) { // Non-fatal - baseline recording is best-effort @@ -931,6 +937,7 @@ public class InitService { private void writeBaselineRoutes(Path camelKitDir, List<String> routeFiles, Path projectRoot) { try { Path baselineFile = camelKitDir.resolve("baseline-routes.txt"); + org.apache.camel.kit.util.ConfinedPaths.requirePath(projectRoot, baselineFile); List<String> relativePaths = routeFiles.stream() .map(r -> { Path routePath = Path.of(r); @@ -945,7 +952,7 @@ public class InitService { }) .sorted() .toList(); - Files.write(baselineFile, relativePaths); + AtomicFileWriter.write(baselineFile, String.join("\n", relativePaths)); } catch (Exception e) { // Non-fatal - baseline recording is best-effort } @@ -981,9 +988,10 @@ public class InitService { } } - private void writeBaselineLines(Path file, List<String> lines) { + private void writeBaselineLines(Path file, List<String> lines, Path projectRoot) { try { - Files.write(file, lines); + org.apache.camel.kit.util.ConfinedPaths.requirePath(projectRoot, file); + AtomicFileWriter.write(file, String.join("\n", lines)); } catch (Exception e) { // Non-fatal - baseline recording is best-effort } diff --git a/kit/camel-kit-core/src/main/resources/skills/camel-brainstorm/guides/version-selection.md b/kit/camel-kit-core/src/main/resources/skills/camel-brainstorm/guides/version-selection.md index 2830b5c5..4fd2675e 100644 --- a/kit/camel-kit-core/src/main/resources/skills/camel-brainstorm/guides/version-selection.md +++ b/kit/camel-kit-core/src/main/resources/skills/camel-brainstorm/guides/version-selection.md @@ -40,7 +40,10 @@ if project.type == "existing": - project.camelVersion = {detected version} - project.platformBomVersion = {detected platform BOM} - SKIP the user selection questions below — proceed to Step 3. + SKIP Steps 1-4 entirely — existing projects use their detected versions AS-IS. + The detected versions are already stored in config.properties by init. + DO NOT validate against mapping tables (Step 3) — existing projects may use + versions not in the shipped tables, and that's acceptable. ``` **For existing projects:** diff --git a/kit/camel-kit-core/src/main/resources/skills/camel-validate/SKILL.md b/kit/camel-kit-core/src/main/resources/skills/camel-validate/SKILL.md index 512f40cc..f1d20d46 100644 --- a/kit/camel-kit-core/src/main/resources/skills/camel-validate/SKILL.md +++ b/kit/camel-kit-core/src/main/resources/skills/camel-validate/SKILL.md @@ -98,22 +98,19 @@ fi ``` **If baseline exists, use it:** -- `status: "passed"` → Baseline tests all passed +- `status: "passed"` → Baseline tests all passed before execute - `status: "failed"` → Baseline had pre-existing failures (NOT a regression) -- `status: "not_run"` → Tests couldn't run during init (establish baseline now) -- `status: "timeout"` → Tests timed out (re-run to establish baseline) +- `status: "not_run"` → Tests were not run before execute (CANNOT verify preservation - see Unavailable Evidence) +- `status: "timeout"` → Tests timed out during init (CANNOT verify preservation - see Unavailable Evidence) -**If baseline doesn't exist or status is "not_run", run tests now:** +**CRITICAL: Baseline must be captured BEFORE execute modifies code.** +If `status` is "not_run" or "timeout", you CANNOT establish baseline now (code is already modified). +Running tests now would capture post-change results as "baseline", hiding regressions. -```bash -# Run existing tests (those that existed before this pipeline) -./mvnw test -B - -# Record results -BASELINE_TEST_COUNT=$(grep -E "Tests run:" target/surefire-reports/*.txt | wc -l) -BASELINE_FAILURES=$(grep -E "Failures:" target/surefire-reports/*.txt) -BASELINE_ERRORS=$(grep -E "Errors:" target/surefire-reports/*.txt) -``` +**If baseline is unavailable (not_run/timeout/missing):** +- Report in **Unavailable Evidence**: "Cannot verify baseline preservation - tests were not run before execute" +- Mark "Baseline Preserved" as **UNVERIFIED** +- DO NOT run tests and call that the baseline **Record baseline state (if not already recorded):** ```json @@ -253,19 +250,36 @@ done < .camel-kit/baseline-tests.txt ### Regression Analysis -If current failures > baseline failures: - - New failures detected: - - TestName: testExistingRoute() - Baseline: PASS - Current: FAIL - Reason: [error message] - - VERDICT: REGRESSION - existing functionality broken by new additions - -If current failures == baseline failures: - - VERDICT: BASELINE PRESERVED - no regressions introduced +**CRITICAL: Compare per-test results, not just totals.** + +Parse baseline-test-results.json (if available) to get per-test outcomes: +```json +{ + "tests": { + "com.example.ExistingTest#testA": {"status": "passed"}, + "com.example.ExistingTest#testB": {"status": "failed", "reason": "..."} + } +} +``` + +Parse current test results (from Surefire XML reports or output). + +**For each test:** +- If baseline=PASS and current=FAIL → **REGRESSION** +- If baseline=FAIL and current=FAIL → Pre-existing failure (not a regression) +- If baseline=PASS and current=PASS → OK +- If baseline=FAIL and current=PASS → Fixed (note as improvement) +- If test is NEW (not in baseline) → Validate separately + +**If baseline-test-results.json is unavailable:** +- Totals comparison is NOT sufficient (can hide regressions where one test improves, another regresses) +- Report in **Unavailable Evidence**: "Cannot verify per-test preservation - baseline-test-results.json missing" +- Mark "Baseline Preserved" as **UNVERIFIED** + +**Verdict:** +- **BASELINE PRESERVED**: No tests went from PASS→FAIL +- **REGRESSION**: At least one test went from PASS→FAIL (list them) +- **UNVERIFIED**: Cannot verify (see Unavailable Evidence) ``` ### Step 5: Report Structure for Existing Projects diff --git a/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/ProjectDiscovery.java b/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/ProjectDiscovery.java index cf1481c1..624d18c5 100644 --- a/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/ProjectDiscovery.java +++ b/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/ProjectDiscovery.java @@ -23,8 +23,14 @@ public final class ProjectDiscovery { * @return discovery result with project metadata */ public static DiscoveryResult discover(ProjectGraph graph) { - String runtime = RuntimeDetector.detect(graph); - String camelVersion = detectCamelVersion(graph); + // Detect all runtimes present (for conflict detection) + Set<String> detectedRuntimes = detectAllRuntimes(graph); + String runtime = detectedRuntimes.isEmpty() ? "main" : detectedRuntimes.iterator().next(); + + // Detect all Camel versions present (for conflict detection) + Set<String> allCamelVersions = detectAllCamelVersions(graph); + String camelVersion = allCamelVersions.isEmpty() ? null : allCamelVersions.iterator().next(); + String platformBomVersion = detectPlatformBomVersion(graph, runtime); Set<DslType> dslTypes = detectDslTypes(graph); List<String> routes = detectRoutes(graph); @@ -34,7 +40,8 @@ public final class ProjectDiscovery { int beansCount = detectBeansCount(graph); int configFilesCount = detectConfigFilesCount(graph); - return new DiscoveryResult( + // Build result with conflict information + DiscoveryResult result = new DiscoveryResult( runtime, camelVersion, platformBomVersion, @@ -44,12 +51,65 @@ public final class ProjectDiscovery { hasTests, modules, beansCount, - configFilesCount); + configFilesCount, + detectedRuntimes, + allCamelVersions); + + return result; } - private static String detectCamelVersion(ProjectGraph graph) { + /** + * Detect all runtimes present in the project (for conflict detection). + */ + private static Set<String> detectAllRuntimes(ProjectGraph graph) { + Set<String> runtimes = new HashSet<>(); + for (GraphNode artifact : graph.findByType(NodeType.MAVEN_ARTIFACT)) { + String artifactId = artifact.properties().getOrDefault("artifactId", ""); + if (artifactId.startsWith("camel-spring-boot")) { + runtimes.add("spring-boot"); + } + if (artifactId.startsWith("camel-quarkus")) { + runtimes.add("quarkus"); + } + if ("camel-blueprint".equals(artifactId)) { + runtimes.add("karaf"); + } + } + return runtimes; + } + + /** + * Detect all distinct Camel versions in the project (for conflict detection). + */ + private static Set<String> detectAllCamelVersions(ProjectGraph graph) { + Set<String> versions = new HashSet<>(); + // Check for camel.version property GraphNode versionProp = graph.getNode("maven-property:camel.version"); + if (versionProp != null) { + String value = versionProp.properties().get("value"); + if (value != null && !value.isEmpty() && !value.contains("${")) { + versions.add(value); + } + } + + // Check all Camel artifacts + for (GraphNode node : graph.findByType(NodeType.MAVEN_ARTIFACT)) { + if ("org.apache.camel".equals(node.properties().get("groupId"))) { + String version = node.properties().get("version"); + if (version != null && !"managed".equals(version) && !"unknown".equals(version) + && !version.contains("${")) { + versions.add(version); + } + } + } + + return versions; + } + + private static String detectCamelVersion(ProjectGraph graph) { + // Check for camel.version property (highest priority) + GraphNode versionProp = graph.getNode("maven-property:camel.version"); if (versionProp != null) { String value = versionProp.properties().get("value"); if (value != null && !value.isEmpty() && !value.contains("${")) { @@ -77,7 +137,7 @@ public final class ProjectDiscovery { } } - // Search all Camel artifacts + // Search all Camel artifacts (fallback) for (GraphNode node : graph.findByType(NodeType.MAVEN_ARTIFACT)) { if ("org.apache.camel".equals(node.properties().get("groupId"))) { String version = node.properties().get("version"); @@ -191,8 +251,10 @@ public final class ProjectDiscovery { } /** - * @param routes route IDs (one entry per route) - * @param routeFiles unique route source files, sorted + * @param routes route IDs (one entry per route) + * @param routeFiles unique route source files, sorted + * @param detectedRuntimes all runtimes found (for conflict detection) + * @param allCamelVersions all Camel versions found (for conflict detection) */ public record DiscoveryResult( String runtime, @@ -204,7 +266,9 @@ public final class ProjectDiscovery { boolean hasTests, List<String> modules, int beansCount, - int configFilesCount) { + int configFilesCount, + Set<String> detectedRuntimes, + Set<String> allCamelVersions) { public boolean isUnsupportedVersion() { if (camelVersion == null) { @@ -240,6 +304,23 @@ public final class ProjectDiscovery { */ public List<String> coverageGaps() { List<String> gaps = new ArrayList<>(); + + // Check for conflicting runtimes + if (detectedRuntimes != null && detectedRuntimes.size() > 1) { + gaps.add("Multiple conflicting runtimes detected: " + + String.join(", ", detectedRuntimes) + + ". A project cannot use multiple runtimes simultaneously. " + + "Inspect pom.xml dependencies and resolve the conflict before proceeding."); + } + + // Check for conflicting Camel versions + if (allCamelVersions != null && allCamelVersions.size() > 1) { + gaps.add("Multiple conflicting Camel versions detected: " + + String.join(", ", allCamelVersions) + + ". Inspect pom.xml dependencies and camel.version property. " + + "All org.apache.camel artifacts must use the same version."); + } + if (routes.isEmpty()) { gaps.add("No routes were found in the project graph. Inspect the existing route sources before " + "changing them."); diff --git a/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/parser/ConfigParser.java b/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/parser/ConfigParser.java index d20e36b7..8ea5e7d8 100644 --- a/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/parser/ConfigParser.java +++ b/kit/camel-kit-graph/src/main/java/org/apache/camel/kit/graph/parser/ConfigParser.java @@ -61,25 +61,61 @@ public class ConfigParser implements GraphParser { */ private void parseYaml(Path file, Path projectRoot, ProjectGraph graph) { String relativePath = projectRoot.relativize(file).toString(); - String profile = extractProfile(file.getFileName().toString()); - Map<String, String> flattened = new LinkedHashMap<>(); + String filenameProfile = extractProfile(file.getFileName().toString()); + int docIndex = 0; try (var documents = yamlMapper.readerFor(JsonNode.class).readValues(file.toFile())) { while (documents.hasNextValue()) { - flatten("", (JsonNode) documents.nextValue(), flattened); + JsonNode document = (JsonNode) documents.nextValue(); + Map<String, String> flattened = new LinkedHashMap<>(); + + // Check for document-level profile (spring.config.activate.on-profile) + String documentProfile = extractDocumentProfile(document); + + flatten("", document, flattened); + + // Determine effective profile (document > filename) + String effectiveProfile = documentProfile != null ? documentProfile : filenameProfile; + + // Make docIndex effectively final for lambda + final int currentDocIndex = docIndex; + flattened.forEach((key, value) -> { + Map<String, String> nodeProps = new HashMap<>(); + nodeProps.put("key", key); + nodeProps.put("value", value); + nodeProps.put("file", relativePath); + nodeProps.put("documentIndex", String.valueOf(currentDocIndex)); + if (effectiveProfile != null) { + nodeProps.put("profile", effectiveProfile); + } + // Use document-scoped node ID to preserve per-document values + String nodeId = "config:" + key + ":doc" + currentDocIndex + + (effectiveProfile != null ? ":profile=" + effectiveProfile : ""); + graph.addNode(new GraphNode(nodeId, NodeType.CONFIG_PROPERTY, nodeProps)); + }); + docIndex++; } } catch (IOException e) { throw new RuntimeException("Failed to parse YAML configuration: " + file, e); } - flattened.forEach((key, value) -> { - Map<String, String> nodeProps = new HashMap<>(); - nodeProps.put("key", key); - nodeProps.put("value", value); - nodeProps.put("file", relativePath); - if (profile != null) { - nodeProps.put("profile", profile); + } + + /** + * Extract profile from document content (spring.config.activate.on-profile). + */ + private String extractDocumentProfile(JsonNode document) { + if (document.has("spring")) { + JsonNode spring = document.get("spring"); + if (spring.has("config")) { + JsonNode config = spring.get("config"); + if (config.has("activate")) { + JsonNode activate = config.get("activate"); + if (activate.has("on-profile")) { + return activate.get("on-profile").asText(); + } + } } - graph.addNode(new GraphNode("config:" + key, NodeType.CONFIG_PROPERTY, nodeProps)); - }); + } + return null; } private static void flatten(String prefix, JsonNode node, Map<String, String> out) {
