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.git
commit 856a1bbaf176629286010b2d933b90ad1e57cb28 Author: Luigi De Masi <[email protected]> AuthorDate: Wed Sep 30 11:14:33 2026 +0200 CAMEL-25138: Preserve XML source positions and decouple MCP guard Parse routes from their original resource bytes while skipping the validated semantic declaration subtree. Accept dotted XML filenames through the component loader and retain custom loader precedence. Discover the optional semantic registry at runtime before generic MCP exports, preserving rejection of lossy conversions without a production dependency on camel-semantic. Cover original locations and diagnostics, dotted filenames, and empty registries; document the XML filename support. Co-authored-by: Codex <[email protected]> Signed-off-by: Luigi De Masi <[email protected]> --- .../camel/catalog/docs/semantic-language.adoc | 2 + .../src/main/docs/semantic-language.adoc | 2 + .../apache/camel/semantic/SemanticXmlLoader.java | 2 +- .../semantic/SemanticXmlRoutesBuilderLoader.java | 44 +++++++++---- .../semantic/SemanticXmlAutoDiscoveryTest.java | 76 +++++++++++++++++++++- dsl/camel-jbang/camel-jbang-mcp/pom.xml | 10 +-- .../jbang/core/commands/mcp/TransformTools.java | 12 ++-- .../core/commands/mcp/TransformToolsTest.java | 19 ++++++ 8 files changed, 141 insertions(+), 26 deletions(-) diff --git a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/semantic-language.adoc b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/semantic-language.adoc index 4f37730d403b..41c3797b928b 100644 --- a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/semantic-language.adoc +++ b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/semantic-language.adoc @@ -148,6 +148,8 @@ automatically at startup: use ordinary `*.xml` files without registering a loade the Camel core model. XML documents without semantic declarations are handled by the standard XML loader, including its bean and route configuration support. XML loaders registered by the application before route loading retain precedence over automatic discovery. +Filenames may contain dots, such as `my.tickets.xml`. Routes retain their original resource +locations and line numbers for debugging and error messages. Both layouts are supported. To keep declarations alongside routes, use `tickets.xml`: diff --git a/components/camel-ai/camel-semantic/src/main/docs/semantic-language.adoc b/components/camel-ai/camel-semantic/src/main/docs/semantic-language.adoc index 4f37730d403b..41c3797b928b 100644 --- a/components/camel-ai/camel-semantic/src/main/docs/semantic-language.adoc +++ b/components/camel-ai/camel-semantic/src/main/docs/semantic-language.adoc @@ -148,6 +148,8 @@ automatically at startup: use ordinary `*.xml` files without registering a loade the Camel core model. XML documents without semantic declarations are handled by the standard XML loader, including its bean and route configuration support. XML loaders registered by the application before route loading retain precedence over automatic discovery. +Filenames may contain dots, such as `my.tickets.xml`. Routes retain their original resource +locations and line numbers for debugging and error messages. Both layouts are supported. To keep declarations alongside routes, use `tickets.xml`: diff --git a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlLoader.java b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlLoader.java index dd2d06e2e723..e5d829e9e62b 100644 --- a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlLoader.java +++ b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlLoader.java @@ -47,7 +47,7 @@ final class SemanticXmlLoader extends RoutesBuilderLoaderSupport { @Override public boolean isSupportedExtension(String extension) { // DefaultCamelContext may build before application beans are bound. Give later custom loaders precedence too. - return ("xml".equals(extension) || "camel.xml".equals(extension)) + return ("xml".equals(extension) || extension.endsWith(".xml")) && getCamelContext().getRegistry().findByType(RoutesBuilderLoader.class).stream() .noneMatch(loader -> loader != this && loader.isSupportedExtension(extension)); } diff --git a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlRoutesBuilderLoader.java b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlRoutesBuilderLoader.java index 7be5c330fa00..fcbc0e9986b7 100644 --- a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlRoutesBuilderLoader.java +++ b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticXmlRoutesBuilderLoader.java @@ -16,18 +16,14 @@ */ package org.apache.camel.semantic; +import java.io.IOException; import java.io.InputStream; -import java.io.StringReader; -import java.io.StringWriter; import java.util.ArrayList; import java.util.List; import java.util.Set; import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.transform.TransformerFactory; -import javax.xml.transform.dom.DOMSource; -import javax.xml.transform.stream.StreamResult; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -40,6 +36,7 @@ import org.apache.camel.spi.Resource; import org.apache.camel.spi.annotations.RoutesLoader; import org.apache.camel.support.RoutesBuilderLoaderSupport; import org.apache.camel.xml.in.ModelParser; +import org.apache.camel.xml.io.XmlPullParserException; /** Parses standalone semantic declarations or declarations alongside XML routes. */ @RoutesLoader("semantic.xml") @@ -104,19 +101,14 @@ public class SemanticXmlRoutesBuilderLoader extends RoutesBuilderLoaderSupport { } found = true; declarations(child, questions); - root.removeChild(child); } else if (!"route".equals(child.getLocalName())) { throw new IllegalArgumentException("Unexpected element in routes: " + child.getTagName()); } } - TransformerFactory transformer = TransformerFactory.newInstance(); - transformer.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - transformer.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - transformer.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); - StringWriter xml = new StringWriter(); - transformer.newTransformer().transform(new DOMSource(root), new StreamResult(xml)); - routes = new ModelParser(new StringReader(xml.toString()), namespace).parseRoutesDefinition() - .orElseThrow(() -> new IllegalArgumentException("Expected XML routes")); + try (InputStream stream = resource.getInputStream()) { + routes = new SemanticModelParser(resource, stream, namespace).parseRoutesDefinition() + .orElseThrow(() -> new IllegalArgumentException("Expected XML routes")); + } } else { throw new IllegalArgumentException("Expected semantic or routes root element"); } @@ -125,6 +117,30 @@ public class SemanticXmlRoutesBuilderLoader extends RoutesBuilderLoaderSupport { return routes; } + private static final class SemanticModelParser extends ModelParser { + private SemanticModelParser( + Resource resource, InputStream stream, String namespace) + throws IOException, + XmlPullParserException { + super(stream, namespace); + this.resource = resource; + } + + @Override + protected boolean handleUnexpectedElement(String namespace, String name) throws XmlPullParserException { + if ("semantic".equals(name) && parser.getDepth() == 2) { + // The DOM pass already validated this block. Keep the original route bytes and source positions. + try { + parser.skipSubTree(); + } catch (IOException e) { + throw new XmlPullParserException("Cannot read semantic declaration", parser, e); + } + return true; + } + return super.handleUnexpectedElement(namespace, name); + } + } + private static void declarations(Element semantic, SemanticQuestionsBuilder questions) { attributes(semantic, Set.of()); for (Element element : children(semantic)) { diff --git a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticXmlAutoDiscoveryTest.java b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticXmlAutoDiscoveryTest.java index ee784482f1fb..8a3a3f5fcb5c 100644 --- a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticXmlAutoDiscoveryTest.java +++ b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticXmlAutoDiscoveryTest.java @@ -32,6 +32,7 @@ import org.apache.camel.support.PluginHelper; import org.apache.camel.support.ResourceHelper; import org.apache.camel.support.SimpleRegistry; import org.apache.camel.support.service.ServiceSupport; +import org.apache.camel.xml.io.XmlPullParserLocationException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; @@ -39,13 +40,34 @@ import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class SemanticXmlAutoDiscoveryTest { + private static final String LOCATED_ROUTES = """ + <?xml version="1.0"?> + <routes xmlns="http://camel.apache.org/schema/xml-io";> + <semantic xmlns="http://camel.apache.org/schema/semantic";> + <question name="urgent" type="boolean"> + <instructions>Urgent?</instructions> + </question> + </semantic> + <route id="located"> + <from uri="direct:located"/> + <filter> + <simple>${body} != null</simple> + <log message="Hello"/> + </filter> + </route> + </routes> + """; + @TempDir Path directory; @ParameterizedTest - @CsvSource({ "false,routes.xml", "true,routes.xml", "false,routes.camel.xml", "true,routes.camel.xml" }) + @CsvSource({ + "false,routes.xml", "true,routes.xml", "false,routes.camel.xml", "true,routes.camel.xml", + "false,my.tickets.xml", "true,my.tickets.xml", "false,my.tickets.semantic.xml", "true,my.tickets.semantic.xml" }) void mainLoadsOrdinaryXmlWithoutLoaderRegistration(boolean standalone, String filename) throws Exception { String declarations = """ <semantic xmlns="http://camel.apache.org/schema/semantic";> @@ -63,7 +85,7 @@ class SemanticXmlAutoDiscoveryTest { Path routes = directory.resolve(filename); String files; if (standalone) { - Path questions = directory.resolve("questions.xml"); + Path questions = directory.resolve("my.questions.xml"); Files.writeString(questions, declarations); Files.writeString(routes, "<routes>" + route + "</routes>"); // The consumer is deliberately listed first. @@ -86,6 +108,56 @@ class SemanticXmlAutoDiscoveryTest { } } + @ParameterizedTest + @ValueSource(strings = { "tickets.xml", "tickets.semantic.xml", "my.tickets.xml", "my.tickets.semantic.xml" }) + void declarationsPreserveOriginalRouteSourceLocations(String filename) throws Exception { + try (var context = new DefaultCamelContext()) { + context.setSourceLocationEnabled(true); + PluginHelper.getRoutesLoader(context).loadRoutes(ResourceHelper.fromString(filename, LOCATED_ROUTES)); + var route = context.getRouteDefinitions().get(0); + assertThat(route.getLocation()).isEqualTo(filename); + assertThat(route.getLineNumber()).isEqualTo(8); + assertThat(route.getInput().getLocation()).isEqualTo(filename); + assertThat(route.getInput().getLineNumber()).isEqualTo(9); + var filter = route.getOutputs().get(0); + assertThat(filter.getLocation()).isEqualTo(filename); + assertThat(filter.getLineNumber()).isEqualTo(10); + assertThat(filter.getOutputs().get(0).getLocation()).isEqualTo(filename); + assertThat(filter.getOutputs().get(0).getLineNumber()).isEqualTo(12); + } + } + + @Test + void nestedSemanticElementIsRejectedWithOriginalSourceLocation() throws Exception { + try (var context = new DefaultCamelContext()) { + context.setSourceLocationEnabled(true); + String xml = LOCATED_ROUTES.replace("<log message=\"Hello\"/>", "<semantic/>"); + assertThatThrownBy(() -> PluginHelper.getRoutesLoader(context) + .loadRoutes(ResourceHelper.fromString("invalid.tickets.xml", xml))) + .isInstanceOfSatisfying(XmlPullParserLocationException.class, error -> { + assertThat(error.getResource().getLocation()).isEqualTo("invalid.tickets.xml"); + assertThat(error.getLineNumber()).isEqualTo(12); + assertThat(error).hasMessageContaining("invalid.tickets.xml, line 12") + .hasMessageContaining("<semantic/>"); + }); + assertThat(SemanticQuestions.get(context).isEmpty()).isTrue(); + } + } + + @Test + void applicationLoaderForDottedExtensionTakesPrecedence() throws Exception { + XmlRoutesBuilderLoader custom = new XmlRoutesBuilderLoader() { + @Override + public boolean isSupportedExtension(String extension) { + return "tickets.xml".equals(extension); + } + }; + try (var context = new DefaultCamelContext()) { + context.getRegistry().bind("ticketsLoader", custom); + assertThat(PluginHelper.getRoutesLoader(context).getRoutesLoader("tickets.xml")).isSameAs(custom); + } + } + @Test void ordinaryXmlRetainsBeansRouteConfigurationsAndDelegateLifecycle() throws Exception { AtomicReference<RoutesBuilderLoader> delegate = new AtomicReference<>(); diff --git a/dsl/camel-jbang/camel-jbang-mcp/pom.xml b/dsl/camel-jbang/camel-jbang-mcp/pom.xml index 227616abc2bb..40786dbc0ab9 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/pom.xml +++ b/dsl/camel-jbang/camel-jbang-mcp/pom.xml @@ -116,11 +116,6 @@ <artifactId>camel-java-joor-dsl</artifactId> </dependency> - <dependency> - <groupId>org.apache.camel</groupId> - <artifactId>camel-semantic</artifactId> - </dependency> - <!-- Swagger/OpenAPI parser for contract-first OpenAPI tools --> <dependency> <groupId>io.swagger.core.v3</groupId> @@ -152,6 +147,11 @@ </dependency> <!-- test dependencies --> + <dependency> + <groupId>org.apache.camel</groupId> + <artifactId>camel-semantic</artifactId> + <scope>test</scope> + </dependency> <dependency> <groupId>org.apache.camel</groupId> <artifactId>camel-xml-io-dsl</artifactId> diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformTools.java b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformTools.java index 8ab79c7e752c..d068f2403245 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformTools.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformTools.java @@ -34,7 +34,6 @@ import org.apache.camel.model.ExpressionNode; import org.apache.camel.model.ProcessorDefinitionHelper; import org.apache.camel.model.RouteDefinition; import org.apache.camel.model.RoutesDefinition; -import org.apache.camel.semantic.SemanticQuestions; import org.apache.camel.spi.Resource; import org.apache.camel.support.PluginHelper; import org.apache.camel.support.ResourceHelper; @@ -170,9 +169,14 @@ public class TransformTools { } } - private static void requireSeparateDeclarations(DefaultCamelContext context) { - SemanticQuestions questions = context.getCamelContextExtension().getContextPlugin(SemanticQuestions.class); - if (questions != null && !questions.isEmpty()) { + private static void requireSeparateDeclarations(DefaultCamelContext context) throws ReflectiveOperationException { + // Semantic declarations are optional and live outside the model exported by this converter. + Class<?> type = context.getClassResolver().resolveClass("org.apache.camel.semantic.SemanticQuestions"); + if (type == null) { + return; + } + Object questions = context.getCamelContextExtension().getContextPlugin(type); + if (questions != null && !(boolean) type.getMethod("isEmpty").invoke(questions)) { throw new IllegalArgumentException( "Semantic declarations cannot be exported by the generic route converter. " + "Keep them in a separate declaration resource and convert only the routes."); diff --git a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformToolsTest.java b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformToolsTest.java index ee2b90c181be..a5dc8ed53fb3 100644 --- a/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformToolsTest.java +++ b/dsl/camel-jbang/camel-jbang-mcp/src/test/java/org/apache/camel/dsl/jbang/core/commands/mcp/TransformToolsTest.java @@ -69,6 +69,25 @@ class TransformToolsTest { .isInstanceOf(ToolCallException.class).hasMessageContaining("Keep them in a separate declaration resource"); } + @ParameterizedTest + @ValueSource(strings = { "xml", "yaml" }) + void emptySemanticRegistryDoesNotPreventConversion(String target) { + String route = """ + import org.apache.camel.builder.RouteBuilder; + import static org.apache.camel.semantic.SemanticQuestionsBuilder.semanticQuestions; + public class EmptySemanticRoute extends RouteBuilder { + public void configure() { + semanticQuestions(this).register(); + from("direct:input").log("Hello"); + } + } + """; + var result = createTools().camel_transform_route(route, "java", target); + + assertThat(result.supported).isTrue(); + assertThat(result.result).contains("direct:input").doesNotContain("semantic"); + } + @ParameterizedTest @CsvSource({ "yaml,xml", "java,xml", "java,yaml" }) void semanticConversionReportsMissingNumericProperty(String source, String target) {
