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 6a54949082794d582759af5f10d7b67c1a4fa67d Author: Luigi De Masi <[email protected]> AuthorDate: Wed Oct 7 17:13:52 2026 +0200 CAMEL-25382: Reduce semantic registry and declaration overhead Return an existing evaluation registry before acquiring the private creation lock, retaining the second check for concurrent creation and avoiding public CamelContext monitors. Copy nested parameter maps recursively in one pass without an intermediate map. Preserve blank-key and string-key validation and deep immutability. Cover cached lookup during another context's blocked registry lookup, invalid nested keys, immutable containers and Java builder diagnostics. Co-authored-by: Codex <[email protected]> Signed-off-by: Luigi De Masi <[email protected]> --- .../apache/camel/semantic/SemanticEvaluation.java | 7 +++-- .../apache/camel/semantic/SemanticEvaluations.java | 16 +++++++---- .../camel/semantic/SemanticCapabilitiesTest.java | 20 ++++++++++++++ .../semantic/SemanticEvaluationBuilderTest.java | 10 +++++++ .../camel/semantic/SemanticInitializationTest.java | 32 ++++++++++++++++++++++ 5 files changed, 77 insertions(+), 8 deletions(-) diff --git a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluation.java b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluation.java index 3125cfee27c3..3ef2b236e3ce 100644 --- a/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluation.java +++ b/components/camel-ai/camel-semantic/src/main/java/org/apache/camel/semantic/SemanticEvaluation.java @@ -69,9 +69,12 @@ public final class SemanticEvaluation { if (!(key instanceof String name)) { throw new IllegalArgumentException("Parameter maps require string keys"); } - copy.put(name, entry); + if (name.isBlank()) { + throw new IllegalArgumentException("Parameter names must not be blank"); + } + copy.put(name, immutableValue(entry)); }); - return immutableMap(copy); + return Collections.unmodifiableMap(copy); } if (value instanceof List<?> list) { List<Object> copy = new ArrayList<>(); 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 a7fb706f27ea..80816e246840 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 @@ -57,14 +57,18 @@ public final class SemanticEvaluations { } public static SemanticEvaluations get(CamelContext context) { - synchronized (CREATION_LOCK) { - SemanticEvaluations answer = context.getCamelContextExtension().getContextPlugin(SemanticEvaluations.class); - if (answer == null) { - answer = new SemanticEvaluations(context); - context.getCamelContextExtension().addContextPlugin(SemanticEvaluations.class, answer); + var extension = context.getCamelContextExtension(); + SemanticEvaluations answer = extension.getContextPlugin(SemanticEvaluations.class); + if (answer == null) { + synchronized (CREATION_LOCK) { + answer = extension.getContextPlugin(SemanticEvaluations.class); + if (answer == null) { + answer = new SemanticEvaluations(context); + extension.addContextPlugin(SemanticEvaluations.class, answer); + } } - return answer; } + return answer; } /** diff --git a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticCapabilitiesTest.java b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticCapabilitiesTest.java index 8cfba855aca3..7cf60c8d3a16 100644 --- a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticCapabilitiesTest.java +++ b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticCapabilitiesTest.java @@ -18,6 +18,7 @@ package org.apache.camel.semantic; import java.math.BigDecimal; import java.util.ArrayList; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -79,6 +80,25 @@ class SemanticCapabilitiesTest { assertThat(evaluation.getParameters()).doesNotContainKey("threshold"); assertThat(evaluation.getParameters().get("policy")).isEqualTo(Map.of("labels", List.of("privacy"))); assertThatThrownBy(() -> evaluation.getParameters().clear()).isInstanceOf(UnsupportedOperationException.class); + var frozenPolicy = (Map<?, ?>) evaluation.getParameters().get("policy"); + var frozenLabels = (List<?>) frozenPolicy.get("labels"); + assertThatThrownBy(frozenPolicy::clear).isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(frozenLabels::clear).isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void nestedParameterMapsRejectInvalidKeys() { + for (Object key : Arrays.asList("", " \t", null, 1)) { + Map<Object, Object> invalid = new LinkedHashMap<>(); + invalid.put(key, true); + for (Object value : List.of(invalid, List.of(invalid))) { + assertThatThrownBy(() -> new SemanticEvaluation("custom", null, null, Map.of("policy", value))) + .isExactlyInstanceOf(IllegalArgumentException.class) + .hasMessage(key instanceof String + ? "Parameter names must not be blank" + : "Parameter maps require string keys"); + } + } } @Test diff --git a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticEvaluationBuilderTest.java b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticEvaluationBuilderTest.java index 5be99c852b98..7712886a4bdf 100644 --- a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticEvaluationBuilderTest.java +++ b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticEvaluationBuilderTest.java @@ -116,6 +116,16 @@ class SemanticEvaluationBuilderTest { } } + @Test + void nonStringNestedKeysFailWithValidationError() throws Exception { + try (var context = new DefaultCamelContext()) { + var builder = new SemanticEvaluationBuilder().operation("custom").parameter("policy", Map.of(1, true)); + assertThatThrownBy(() -> builder.build(context)) + .isExactlyInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameter maps require string keys"); + } + } + @Test void nullPolicyIsRejectedWithContractContext() throws Exception { try (var context = new DefaultCamelContext()) { diff --git a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticInitializationTest.java b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticInitializationTest.java index 6599143d05f5..12ccaabcd399 100644 --- a/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticInitializationTest.java +++ b/components/camel-ai/camel-semantic/src/test/java/org/apache/camel/semantic/SemanticInitializationTest.java @@ -76,6 +76,38 @@ class SemanticInitializationTest { } } + @Test + void cachedRegistryLookupDoesNotWaitForAnotherContext() throws Exception { + ExecutorService callers = Executors.newFixedThreadPool(2); + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + try (var creating = new DefaultCamelContext(); var cached = new DefaultCamelContext()) { + var expected = SemanticEvaluations.get(cached); + creating.getCamelContextExtension().lazyAddContextPlugin(SemanticEvaluations.class, () -> { + entered.countDown(); + try { + assertThat(release.await(30, TimeUnit.SECONDS)).isTrue(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + return null; + }); + var creation = callers.submit(() -> SemanticEvaluations.get(creating)); + try { + assertThat(entered.await(10, TimeUnit.SECONDS)).isTrue(); + var lookup = callers.submit(() -> SemanticEvaluations.get(cached)); + assertThat(lookup.get(10, TimeUnit.SECONDS)).isSameAs(expected); + } finally { + release.countDown(); + assertThat(creation.get(10, TimeUnit.SECONDS)).isNotNull(); + } + } finally { + release.countDown(); + callers.shutdownNow(); + } + } + @Test void concurrentLanguagesCannotOverwriteTheAdapterOwner() throws Exception { ExecutorService callers = Executors.newFixedThreadPool(2);
