This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch feature/CAMEL-25750-body-type-hint in repository https://gitbox.apache.org/repos/asf/camel.git
commit 668b71589bb4f1fe6d0cbd82f79a7174881befa4 Author: Claus Ibsen <[email protected]> AuthorDate: Tue Oct 20 14:43:57 2026 +0200 CAMEL-25750: camel-support - a hint on the body says when other messages reached the step with another kind of body When a step fails on the body and the message history is on, the hint of BodyFacts ends with what the BodyTypeRegistry knows about the step: "; other messages reached this step with data (LinkedHashMap), so the body here depends on the path a message takes". The step is right for those messages, and a path to it, such as a branch that does not unmarshal, is what differs. BodyTypeRegistry.getBodyTypes(routeId, nodeId, kind) gives the types of one kind. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../apache/camel/catalog/docs/message-history.adoc | 4 + .../org/apache/camel/spi/BodyTypeRegistry.java | 11 +++ .../camel/impl/engine/DefaultBodyTypeRegistry.java | 64 ++++++++++++---- .../docs/modules/eips/pages/message-history.adoc | 4 + .../impl/engine/DefaultBodyTypeRegistryTest.java | 4 + .../camel/support/BodyFactsOtherBodiesTest.java | 85 ++++++++++++++++++++++ .../java/org/apache/camel/support/BodyFacts.java | 58 ++++++++++++++- 7 files changed, 215 insertions(+), 15 deletions(-) diff --git a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/message-history.adoc b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/message-history.adoc index 174e4ad313fe..d6052494a2dd 100644 --- a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/message-history.adoc +++ b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/message-history.adoc @@ -464,6 +464,10 @@ The step after the choice has been reached with a `byte[]` by the high priority others. Both are a payload, so the step is dynamic but needs no attention. Had the branch unmarshalled the body instead, the line would say `body: LinkedHashMap | String (data and payload)`. +When a step fails on the body, such as reading a field of text, its error says so too: `...; other messages reached this +step with data (LinkedHashMap), so the body here depends on the path a message takes`. The step is right for some +messages, and a path to it, such as a branch that does not unmarshal, is what differs. + The body types of a route are forgotten when the route is removed, such as when it is reloaded in dev mode. In Java the registry is a context plugin: diff --git a/core/camel-api/src/main/java/org/apache/camel/spi/BodyTypeRegistry.java b/core/camel-api/src/main/java/org/apache/camel/spi/BodyTypeRegistry.java index d220447b24d0..662588861cfe 100644 --- a/core/camel-api/src/main/java/org/apache/camel/spi/BodyTypeRegistry.java +++ b/core/camel-api/src/main/java/org/apache/camel/spi/BodyTypeRegistry.java @@ -100,6 +100,17 @@ public interface BodyTypeRegistry { */ List<Kind> getBodyKinds(String routeId, String nodeId); + /** + * The body types of one kind the node has been reached with, in the order they were first seen. + * + * @param routeId the id of the route the node belongs to + * @param nodeId the id of the node + * @param kind the kind + * @return the body types of the kind, or an empty list when no exchange has reached the node with a body of + * the kind (or only with types beyond the first few) + */ + List<String> getBodyTypes(String routeId, String nodeId, Kind kind); + /** * The body types of every node of the route that an exchange has reached. * diff --git a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistry.java b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistry.java index 7c7c65a1dbb3..b72af0d3287d 100644 --- a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistry.java +++ b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistry.java @@ -16,7 +16,9 @@ */ package org.apache.camel.impl.engine; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -56,7 +58,7 @@ public class DefaultBodyTypeRegistry implements BodyTypeRegistry { @Override public List<String> getBodyTypes(String routeId, String nodeId) { NodeTypes types = node(routeId, nodeId); - return types != null ? List.of(types.types) : List.of(); + return types != null ? types.types(null) : List.of(); } @Override @@ -65,6 +67,12 @@ public class DefaultBodyTypeRegistry implements BodyTypeRegistry { return types != null ? List.of(types.kinds) : List.of(); } + @Override + public List<String> getBodyTypes(String routeId, String nodeId, Kind kind) { + NodeTypes types = node(routeId, nodeId); + return types != null && kind != null ? types.types(kind) : List.of(); + } + @Override public Map<String, List<String>> getBodyTypes(String routeId) { Map<String, NodeTypes> nodes = routes.get(routeId); @@ -72,7 +80,7 @@ public class DefaultBodyTypeRegistry implements BodyTypeRegistry { return Map.of(); } Map<String, List<String>> answer = new LinkedHashMap<>(); - nodes.forEach((id, types) -> answer.put(id, List.of(types.types))); + nodes.forEach((id, types) -> answer.put(id, types.types(null))); return answer; } @@ -91,40 +99,68 @@ public class DefaultBodyTypeRegistry implements BodyTypeRegistry { return nodes != null ? nodes.get(nodeId) : null; } + /** + * A body type and its kind. + */ + private record TypeKind(String type, Kind kind) { + } + /** * The distinct types and kinds of one node, as small arrays that are replaced when a new one is seen: reading them * (on every exchange) takes no lock. */ private static final class NodeTypes { - private volatile String[] types = new String[0]; + private volatile TypeKind[] types = new TypeKind[0]; private volatile Kind[] kinds = new Kind[0]; void add(String type, Kind kind) { - String[] known = types; + TypeKind[] known = types; // a node keeps every kind, also of a type beyond the first few: a new kind is what needs attention - if (contains(known, type) - || known.length >= MAX_TYPES_PER_NODE && (kind == null || contains(kinds, kind))) { + if (containsType(known, type) + || known.length >= MAX_TYPES_PER_NODE && (kind == null || containsKind(kinds, kind))) { return; } synchronized (this) { - if (kind != null && !contains(kinds, kind)) { + if (kind != null && !containsKind(kinds, kind)) { Kind[] next = Arrays.copyOf(kinds, kinds.length + 1); next[kinds.length] = kind; kinds = next; } - String[] current = types; - if (!contains(current, type) && current.length < MAX_TYPES_PER_NODE) { - String[] next = Arrays.copyOf(current, current.length + 1); - next[current.length] = type; + TypeKind[] current = types; + if (!containsType(current, type) && current.length < MAX_TYPES_PER_NODE) { + TypeKind[] next = Arrays.copyOf(current, current.length + 1); + next[current.length] = new TypeKind(type, kind); types = next; } } } - private static <T> boolean contains(T[] array, T value) { - for (T t : array) { - if (t.equals(value)) { + /** + * The types, of the kind only when one is given. + */ + List<String> types(Kind kind) { + List<String> answer = new ArrayList<>(); + for (TypeKind t : types) { + if (kind == null || kind == t.kind()) { + answer.add(t.type()); + } + } + return Collections.unmodifiableList(answer); + } + + private static boolean containsType(TypeKind[] types, String type) { + for (TypeKind t : types) { + if (t.type().equals(type)) { + return true; + } + } + return false; + } + + private static boolean containsKind(Kind[] kinds, Kind kind) { + for (Kind k : kinds) { + if (k == kind) { return true; } } diff --git a/core/camel-core-engine/src/main/docs/modules/eips/pages/message-history.adoc b/core/camel-core-engine/src/main/docs/modules/eips/pages/message-history.adoc index 174e4ad313fe..d6052494a2dd 100644 --- a/core/camel-core-engine/src/main/docs/modules/eips/pages/message-history.adoc +++ b/core/camel-core-engine/src/main/docs/modules/eips/pages/message-history.adoc @@ -464,6 +464,10 @@ The step after the choice has been reached with a `byte[]` by the high priority others. Both are a payload, so the step is dynamic but needs no attention. Had the branch unmarshalled the body instead, the line would say `body: LinkedHashMap | String (data and payload)`. +When a step fails on the body, such as reading a field of text, its error says so too: `...; other messages reached this +step with data (LinkedHashMap), so the body here depends on the path a message takes`. The step is right for some +messages, and a path to it, such as a branch that does not unmarshal, is what differs. + The body types of a route are forgotten when the route is removed, such as when it is reloaded in dev mode. In Java the registry is a context plugin: diff --git a/core/camel-core/src/test/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistryTest.java b/core/camel-core/src/test/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistryTest.java index dc9097d0b1ec..aabcecede98a 100644 --- a/core/camel-core/src/test/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistryTest.java +++ b/core/camel-core/src/test/java/org/apache/camel/impl/engine/DefaultBodyTypeRegistryTest.java @@ -43,6 +43,10 @@ public class DefaultBodyTypeRegistryTest { registry.addBodyType("orders", "to1", "java.util.LinkedHashMap", DATA); assertEquals(List.of(PAYLOAD, DATA), registry.getBodyKinds("orders", "to1")); + // the types of one kind + assertEquals(List.of("java.lang.String", "byte[]"), registry.getBodyTypes("orders", "to1", PAYLOAD)); + assertEquals(List.of("java.util.LinkedHashMap"), registry.getBodyTypes("orders", "to1", DATA)); + assertTrue(registry.getBodyTypes("orders", "to1", NULL).isEmpty()); // a node of another route with the same id is another node assertTrue(registry.getBodyTypes("invoices", "to1").isEmpty()); diff --git a/core/camel-core/src/test/java/org/apache/camel/support/BodyFactsOtherBodiesTest.java b/core/camel-core/src/test/java/org/apache/camel/support/BodyFactsOtherBodiesTest.java new file mode 100644 index 000000000000..35c12bde0e4f --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/support/BodyFactsOtherBodiesTest.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.support; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A step that failed on the body says when other messages reached it with a body of another kind (CAMEL-25750): the + * step is right for those, and the path this message took is what differs. + */ +public class BodyFactsOtherBodiesTest extends ContextTestSupport { + + @Test + public void testTheHintSaysOtherMessagesReachedTheStepWithData() { + // the first message takes the branch that makes data, so its quantity is read + Exchange parsed = template.request("direct:start", e -> { + e.getMessage().setHeader("parsed", true); + e.getMessage().setBody("{\"qty\": 2}"); + }); + assertNull(parsed.getException()); + + // the second does not, and the same read fails on the text + Exchange text = template.request("direct:start", e -> e.getMessage().setBody("{\"qty\": 2}")); + assertNotNull(text.getException()); + String message = text.getException().getMessage(); + assertTrue(message.contains("unmarshal it first (unmarshal: json) to read its fields; other messages reached this" + + " step with data (LinkedHashMap), so the body here depends on the path a message" + + " takes"), + message); + } + + @Test + public void testNothingMoreWhenEveryMessageBroughtTheSameKind() { + template.request("direct:start", e -> e.getMessage().setBody("{\"qty\": 2}")); + Exchange text = template.request("direct:start", e -> e.getMessage().setBody("{\"qty\": 3}")); + + assertNotNull(text.getException()); + String message = text.getException().getMessage(); + assertTrue(message.contains("unmarshal it first (unmarshal: json) to read its fields"), message); + assertFalse(message.contains("other messages reached this step"), message); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + context.setMessageHistory(true); + + from("direct:start").routeId("orders") + .choice() + .when(header("parsed")) + .process(e -> e.getMessage().setBody(new LinkedHashMap<>(Map.of("qty", 2)))) + .end() + .setHeader("qty", simple("${body.qty}")).id("readQty") + .to("mock:result"); + } + }; + } +} diff --git a/core/camel-support/src/main/java/org/apache/camel/support/BodyFacts.java b/core/camel-support/src/main/java/org/apache/camel/support/BodyFacts.java index 2d5bf7f5cad1..90c065dfa2ec 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/BodyFacts.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/BodyFacts.java @@ -26,9 +26,11 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.StringJoiner; import java.util.stream.BaseStream; import java.util.stream.Collectors; +import org.apache.camel.CamelContext; import org.apache.camel.Exchange; import org.apache.camel.ExchangePropertyKey; import org.apache.camel.MessageHistory; @@ -112,6 +114,11 @@ import org.apache.camel.util.URISupport; * </table> * When the Content-Type, the file name or the text names no data format, the hint lists the usual ones (json, * jacksonXml, csv). A hint is null when the body is not why the step failed. + * <p/> + * While the message history is on, the hint also says when other messages have reached the failing step with a body of + * another kind (as the {@link BodyTypeRegistry} keeps them): "; other messages reached this step with data + * (LinkedHashMap), so the body here depends on the path a message takes". Then the step is right for some messages, and + * a path to it, such as a branch that does not unmarshal, is what differs. * * @since 4.24 */ @@ -173,11 +180,12 @@ public final class BodyFacts { */ public static String hint(Exchange exchange, Need need, String target) { Object body = exchange.getMessage().getBody(); - return switch (need) { + String answer = switch (need) { case DATA -> dataHint(exchange, body); case BYTES -> bytesHint(body, target); case A_BODY -> isEmpty(body) ? readTheDataFirst() : null; }; + return answer != null ? answer + otherBodies(exchange, body) : null; } /** @@ -333,6 +341,54 @@ public final class BodyFacts { return ", unchanged since from: " + URISupport.sanitizeUri(exchange.getFromEndpoint().getEndpointUri()); } + /** + * When other messages have reached the failing step with a body of another kind, as the {@link BodyTypeRegistry} + * knows while the message history is on: the body depends on the path a message takes, such as a branch that does + * not unmarshal. Empty otherwise (CAMEL-25750). + */ + private static String otherBodies(Exchange exchange, Object body) { + List<MessageHistory> history = exchange.getProperty(ExchangePropertyKey.MESSAGE_HISTORY, List.class); + CamelContext context = exchange.getContext(); + if (history == null || history.isEmpty() || context == null) { + return ""; + } + // the step that failed is the last one the message reached + MessageHistory step = history.get(history.size() - 1); + String routeId = step.getRouteId(); + String nodeId = step.getNode() != null ? step.getNode().getId() : null; + BodyTypeRegistry registry = context.getCamelContextExtension().getContextPlugin(BodyTypeRegistry.class); + if (registry == null || routeId == null || nodeId == null) { + return ""; + } + BodyTypeRegistry.Kind kind = kindOf(body); + StringJoiner others = new StringJoiner(" and "); + for (BodyTypeRegistry.Kind other : registry.getBodyKinds(routeId, nodeId)) { + if (other != kind) { + others.add(kindWords(other, registry.getBodyTypes(routeId, nodeId, other))); + } + } + if (others.length() == 0) { + return ""; + } + return "; other messages reached this step with " + others + + ", so the body here depends on the path a message takes"; + } + + /** A kind in words, with the simple names of its types: data (LinkedHashMap), or no body. */ + private static String kindWords(BodyTypeRegistry.Kind kind, List<String> types) { + String words = switch (kind) { + case NULL -> "no body"; + case PAYLOAD -> "a payload"; + case DATA -> "data"; + case ITERATOR -> "an iterator"; + }; + if (kind == BodyTypeRegistry.Kind.NULL || types.isEmpty()) { + return words; + } + return words + " (" + types.stream().map(t -> StringHelper.afterLast(t, ".", t)).collect(Collectors.joining(", ")) + + ")"; + } + /** A step as people find it: unmarshal at orders.yaml:9, or the step and its label when there is no location. */ private static String step(NamedNode node) { String location = LoggerHelper.getLineNumberLoggerName(node);
