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 7c0fca7a09cb7cc2acf70715edd597a8adb334c1 Author: Luigi De Masi <[email protected]> AuthorDate: Thu Oct 8 09:19:32 2026 +0200 CAMEL-25382: Validate predicates registered during replacement Check reentrant validator registrations against the pending replacement as well as the published definitions. This closes the gap for predicates created after the replacement has copied its existing validators, without exposing pending definitions through public getters or changing Simple. Test compatible and incompatible replacements, rollback and subsequent registration. Clarify published-snapshot reads and nested validation scopes as requested in review. Co-authored-by: Codex <[email protected]> Signed-off-by: Luigi De Masi <[email protected]> --- .../camel/language/semantic/SemanticLanguage.java | 1 + .../apache/camel/semantic/SemanticEvaluations.java | 33 +++++++++---- .../camel/semantic/SemanticContractTest.java | 54 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/language/semantic/SemanticLanguage.java b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/language/semantic/SemanticLanguage.java index 3ddc4d7620c4..25bbbbda764e 100644 --- a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/language/semantic/SemanticLanguage.java +++ b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/language/semantic/SemanticLanguage.java @@ -321,6 +321,7 @@ public class SemanticLanguage extends LanguageSupport { private <T> T validationOnly(Map<String, SemanticEvaluation> snapshot, Supplier<T> action) { Validation previous = validation.get(); + // An explicit snapshot overrides the scope; nested calls without one inherit the outer validation scope. validation.set(snapshot != null || previous == null ? new Validation(snapshot) : previous); try { return action.get(); diff --git a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluations.java b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluations.java index 30f65e8a1730..b343b6ee1851 100644 --- a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluations.java +++ b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluations.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.WeakHashMap; import java.util.function.Consumer; @@ -36,6 +37,8 @@ public final class SemanticEvaluations { private final Map<String, Resource> resources = new HashMap<>(); private volatile Map<String, SemanticEvaluation> evaluations = Map.of(); private final Map<Consumer<Map<String, SemanticEvaluation>>, List<String>> validators = new WeakHashMap<>(); + // Accessed only under this registry's monitor, including reentrant validator registration. + private PendingReplacement pendingReplacement; private SemanticEvaluations(CamelContext context) { this.context = context; @@ -51,11 +54,16 @@ public final class SemanticEvaluations { /** * Validate current declarations and weakly track a callback owned by its initialized expression. The callback * receives the complete immutable snapshot so nested references can be validated against the same replacement. + * Registrations made during replacement validation must also accept the pending replacement. */ public synchronized void setValidator(List<String> names, Consumer<Map<String, SemanticEvaluation>> validator) { // A replacement may have occurred between the expression's initial compilation and registration. get(names); + // Reentrant registration first checks published definitions; the replacement is not visible until it succeeds. validator.accept(evaluations); + if (pendingReplacement != null && !Collections.disjoint(names, pendingReplacement.changedNames)) { + validator.accept(pendingReplacement.snapshot); + } validators.put(validator, List.copyOf(names)); } @@ -97,15 +105,21 @@ public final class SemanticEvaluations { } }); Map<String, SemanticEvaluation> snapshot = Map.copyOf(replacement); - if (context.isStarted() && !definitions.isEmpty()) { - ((SemanticLanguage) context.resolveLanguage("semantic")).validateDeclarations(definitions, snapshot); - } - // Expert callbacks may initialize expressions and register additional validators reentrantly. - new LinkedHashMap<>(validators).forEach((validator, names) -> { - if (!Collections.disjoint(names, definitions.keySet())) { - validator.accept(snapshot); + PendingReplacement previous = pendingReplacement; + pendingReplacement = new PendingReplacement(snapshot, Set.copyOf(definitions.keySet())); + try { + if (context.isStarted() && !definitions.isEmpty()) { + ((SemanticLanguage) context.resolveLanguage("semantic")).validateDeclarations(definitions, snapshot); } - }); + // Expert callbacks may initialize expressions and register additional validators reentrantly. + new LinkedHashMap<>(validators).forEach((validator, names) -> { + if (!Collections.disjoint(names, definitions.keySet())) { + validator.accept(snapshot); + } + }); + } finally { + pendingReplacement = previous; + } if (definitions.isEmpty()) { sources.remove(source); } else { @@ -174,4 +188,7 @@ public final class SemanticEvaluations { return Collections.unmodifiableMap(selected); } + private record PendingReplacement(Map<String, SemanticEvaluation> snapshot, Set<String> changedNames) { + } + } diff --git a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticContractTest.java b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticContractTest.java index 7825fc0cffce..bc5eb7133641 100644 --- a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticContractTest.java +++ b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticContractTest.java @@ -25,6 +25,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import org.apache.camel.Predicate; import org.apache.camel.builder.RouteBuilder; import org.apache.camel.dsl.yaml.common.YamlDeserializationContext; import org.apache.camel.impl.DefaultCamelContext; @@ -239,6 +240,59 @@ class SemanticContractTest { } } + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void latePredicatesValidatePendingReplacement(boolean compatible) throws Exception { + try (var context = new DefaultCamelContext()) { + var registry = SemanticEvaluations.get(context); + var original = new SemanticEvaluation("detect", "content", null, Map.of()); + var candidate = new SemanticEvaluation("detect", "content", "${header.candidate}", Map.of()); + var expert = new ContentExpert() { + int candidateValidations; + boolean attempted; + Predicate latePredicate; + + @Override + public void validate(SemanticEvaluation evaluation) { + // First validate the declaration; then enter the already-copied expression validators. + if (evaluation == candidate && ++candidateValidations == 2) { + attempted = true; + assertThat(registry.get("target")).isSameAs(original); + latePredicate = context.resolveLanguage("semantic").createPredicate("ref:target"); + } + } + }; + context.getRegistry().bind("content", expert); + registry.replace("test", Map.of("outer", original, "target", original)); + context.start(); + var language = context.resolveLanguage("semantic"); + var outer = language.createExpression("ref:outer"); + var target = compatible + ? new SemanticEvaluation("detect", "content", null, Map.of("threshold", 0.9)) + : new SemanticEvaluation("rank", "content", null, Map.of()); + Map<String, SemanticEvaluation> replacement = Map.of("outer", candidate, "target", target); + var exchange = new DefaultExchange(context); + exchange.getMessage().setBody("published state"); + exchange.getMessage().setHeader("candidate", "replacement state"); + if (compatible) { + registry.replace("test", replacement); + assertThat(registry.get("target")).isSameAs(target); + assertThat(expert.latePredicate.matches(exchange)).isFalse(); + } else { + assertThatThrownBy(() -> registry.replace("test", replacement)) + .hasMessageContaining("Semantic predicate requires a boolean evaluation: target"); + assertThat(registry.get("outer")).isSameAs(original); + assertThat(registry.get("target")).isSameAs(original); + assertThat(expert.latePredicate).isNull(); + } + assertThat(expert.attempted).isTrue(); + assertThat(outer.evaluate(exchange, Boolean.class)).isTrue(); + // Both success and failure must clear the pending replacement before later registrations. + registry.replace("test", Map.of("outer", original, "target", original)); + assertThat(language.createPredicate("ref:target").matches(exchange)).isTrue(); + } + } + @ParameterizedTest @ValueSource(strings = { "${semantic('inner')} ready",
