This is an automated email from the ASF dual-hosted git repository.
Croway pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new f86ebea1ad33 CAMEL-24187: camel-core - CamelContext.getEndpoint()
never caches a uri containing a % character
f86ebea1ad33 is described below
commit f86ebea1ad331d046ad9fe91a08cbdf8c495e21a
Author: croway <[email protected]>
AuthorDate: Thu Jul 16 16:39:56 2026 +0200
CAMEL-24187: camel-core - CamelContext.getEndpoint() never caches a uri
containing a % character
AbstractCamelContext.doGetEndpoint() normalizes the uri once for its cache
lookup, but on a cache miss handed that already-normalized uri to
addEndpointToRegistry(), which normalized it a second time before storing
the registry key. EndpointHelper.normalizeEndpointUri() is not idempotent
for uris containing a literal '%' (it re-encodes it to '%25' on every
pass), so the stored key never matched the lookup key computed on a later
call, and a brand-new Endpoint was created and started on every single
getEndpoint() call for such a uri instead of the cached singleton being
reused.
Add a normalized-aware overload of getEndpointKey()/addEndpointToRegistry()
so only the doGetEndpoint() call site (whose uri is provably already
normalized) skips the redundant second normalization pass. The public
hasEndpoint(String), addEndpoint(String, Endpoint) and
removeEndpoints(String) APIs still receive raw, caller-supplied uris and
keep normalizing exactly as before.
Discovered while investigating CAMEL-24171 (camel-plc4x S7 tag addresses
like RAW(%DB1.DBX0.0:BOOL) contain a '%', unlike Modbus' RAW(coil:1),
which is why only the S7 route reloaded its DefaultPlcDriverManager on
every poll).
Co-authored-by: Claude Sonnet 5 <[email protected]>
---
.../Plc4XPollEnrichDoStartReproducerTest.java | 161 +++++++++++++++++++++
.../camel/impl/engine/AbstractCamelContext.java | 38 ++++-
.../camel/impl/DefaultEndpointRegistryTest.java | 26 ++++
3 files changed, 220 insertions(+), 5 deletions(-)
diff --git
a/components/camel-plc4x/src/test/java/org/apache/camel/component/plc4x/Plc4XPollEnrichDoStartReproducerTest.java
b/components/camel-plc4x/src/test/java/org/apache/camel/component/plc4x/Plc4XPollEnrichDoStartReproducerTest.java
new file mode 100644
index 000000000000..af99dfdc6275
--- /dev/null
+++
b/components/camel-plc4x/src/test/java/org/apache/camel/component/plc4x/Plc4XPollEnrichDoStartReproducerTest.java
@@ -0,0 +1,161 @@
+/*
+ * 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.component.plc4x;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.Endpoint;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.apache.camel.util.PropertiesHelper;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.sameInstance;
+
+/**
+ * Reproducer for CAMEL-24171: the reporter observed that, on a route shaped
like
+ *
+ * <pre>
+ * from(timer("s7").period(5000))
+ *
.pollEnrich(plc4x("s7://localhost:102").autoReconnect(true).tags(Map.of("var1",
"RAW(%DB1.DBX0.0:BOOL)")), 5000)
+ * </pre>
+ *
+ * {@code Plc4XEndpoint#doStart()} runs again on every poll cycle (reloading
{@code DefaultPlcDriverManager}), while a
+ * structurally identical Modbus route only runs it once.
+ * <p>
+ * {@link #directGetEndpointCachingProbe()} isolates the actual root cause: it
is not specific to S7, pollEnrich, or
+ * camel-plc4x at all. Any endpoint URI whose query contains a {@code %}
character (as S7 tag addresses like
+ * {@code RAW(%DB1.DBX0.0:BOOL)} do, unlike Modbus' {@code RAW(coil:1)}) makes
{@code CamelContext#getEndpoint(String)}
+ * return a brand-new {@link Endpoint} instance on every call instead of the
cached singleton, even though
+ * {@code getEndpointUri()} prints byte-for-byte identical text both times.
The generic, camel-core-level root cause
+ * (double-normalization in {@code AbstractCamelContext#getEndpointKey}) is
reproduced without any plc4x involvement in
+ * {@code
org.apache.camel.impl.DefaultEndpointRegistryTest#testGetEndpointIsCachedForUriContainingPercentCharacter}
in
+ * camel-core. {@link
#doStartRunsOnlyOnceEvenThoughBothRoutesArePolledManyTimes()} then shows the
real-world
+ * consequence through the exact reported route shape: with a S7-style {@code
%}-containing tag,
+ * {@code Plc4XEndpoint#doStart()} (which recreates {@code
DefaultPlcDriverManager}) fires again on every single poll,
+ * while the Modbus-style route only runs it once.
+ */
+public class Plc4XPollEnrichDoStartReproducerTest extends CamelTestSupport {
+
+ final AtomicInteger modbusStartCount = new AtomicInteger();
+ final AtomicInteger s7StartCount = new AtomicInteger();
+
+ @Override
+ protected CamelContext createCamelContext() throws Exception {
+ CamelContext ctx = super.createCamelContext();
+ ctx.addComponent("plc4x", new CountingPlc4XComponent());
+ return ctx;
+ }
+
+ @Test
+ public void directGetEndpointCachingProbe() throws Exception {
+ String uri =
"plc4x:mock:s7-probe?autoReconnect=true&tag.var1=RAW(%DB1.DBX0.0:BOOL)";
+
+ Endpoint first = context.getEndpoint(uri);
+ Endpoint second = context.getEndpoint(uri);
+
+ assertThat("getEndpointUri() must be identical between the two lookups
(it is: normalization is stable)",
+ second.getEndpointUri(), is(first.getEndpointUri()));
+ assertThat("calling getEndpoint(uri) twice with the identical uri
string must return the same cached instance",
+ second, sameInstance(first));
+ }
+
+ @Test
+ public void doStartRunsOnlyOnceEvenThoughBothRoutesArePolledManyTimes()
throws Exception {
+ MockEndpoint modbusResult = getMockEndpoint("mock:modbusResult");
+ MockEndpoint s7Result = getMockEndpoint("mock:s7Result");
+ modbusResult.expectedMinimumMessageCount(10);
+ s7Result.expectedMinimumMessageCount(10);
+
+ MockEndpoint.assertIsSatisfied(context, 5, TimeUnit.SECONDS);
+
+ assertThat("Plc4XEndpoint#doStart() must only run once per endpoint
for the modbus-shaped route",
+ modbusStartCount.get(), is(1));
+ assertThat("Plc4XEndpoint#doStart() must only run once per endpoint
for the s7-shaped route",
+ s7StartCount.get(), is(1));
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ from("timer:modbus?period=20")
+
.pollEnrich("plc4x:mock:modbus-like?autoReconnect=true&tag.coil-1=RAW(coil:1)",
5000)
+ .routeId("modbus")
+ .to("mock:modbusResult");
+
+ from("timer:s7?period=20")
+
.pollEnrich("plc4x:mock:s7-like?autoReconnect=true&tag.var1=RAW(%DB1.DBX0.0:BOOL)",
5000)
+ .routeId("s7")
+ .to("mock:s7Result");
+ }
+ };
+ }
+
+ /**
+ * Duplicates {@link Plc4XComponent#createEndpoint} but hands back an
endpoint whose doStart() is countable, without
+ * touching production code.
+ */
+ private class CountingPlc4XComponent extends Plc4XComponent {
+
+ @Override
+ protected Endpoint createEndpoint(String uri, String remaining,
Map<String, Object> parameters) throws Exception {
+ AtomicInteger counter = uri.contains("s7-like") ? s7StartCount :
modbusStartCount;
+ Plc4XEndpoint endpoint = new Plc4XEndpoint(uri, this) {
+ @Override
+ protected void doStart() throws Exception {
+ counter.incrementAndGet();
+ super.doStart();
+ }
+ };
+
+ Map<String, String> tags =
getAndRemoveOrResolveReferenceParameter(parameters, "tags", Map.class);
+ Map<String, Object> map =
PropertiesHelper.extractProperties(parameters, "tag.");
+ if (map != null) {
+ if (tags == null) {
+ tags = new LinkedHashMap<>();
+ }
+ for (Map.Entry<String, Object> me : map.entrySet()) {
+ tags.put(me.getKey(), me.getValue().toString());
+ }
+ }
+ if (tags != null) {
+ endpoint.setTags(tags);
+ }
+
+ String trigger =
getAndRemoveOrResolveReferenceParameter(parameters, "trigger", String.class);
+ if (trigger != null) {
+ endpoint.setTrigger(trigger);
+ }
+ Integer period =
getAndRemoveOrResolveReferenceParameter(parameters, "period", Integer.class);
+ if (period != null) {
+ endpoint.setPeriod(period);
+ }
+ setProperties(endpoint, parameters);
+ return endpoint;
+ }
+ }
+
+}
diff --git
a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/AbstractCamelContext.java
b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/AbstractCamelContext.java
index c07f7b7bafef..f9b61322b617 100644
---
a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/AbstractCamelContext.java
+++
b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/AbstractCamelContext.java
@@ -873,8 +873,8 @@ public abstract class AbstractCamelContext extends
BaseService
if (answer != null) {
if (!prototype) {
addService(answer);
- // register in registry
- answer = addEndpointToRegistry(uri, answer);
+ // register in registry (uri is already normalized
above, so avoid normalizing it again)
+ answer = addEndpointToRegistry(uri, answer, true);
} else {
addPrototypeService(answer);
// if there is endpoint strategies, then use the
endpoints they return
@@ -925,6 +925,21 @@ public abstract class AbstractCamelContext extends
BaseService
* @return the added endpoint
*/
protected Endpoint addEndpointToRegistry(String uri, Endpoint endpoint) {
+ return addEndpointToRegistry(uri, endpoint, false);
+ }
+
+ /**
+ * Strategy to add the given endpoint to the internal endpoint registry
+ *
+ * @param uri uri of the endpoint
+ * @param endpoint the endpoint to add
+ * @param normalized whether the uri is already normalized (for example
because it was normalized earlier to do the
+ * cache lookup that preceded this call).
Re-normalizing an already normalized uri is not
+ * idempotent for uris containing a literal {@code %}
character, so callers must not normalize
+ * twice or the stored key will never match a later
lookup key (CAMEL-24171).
+ * @return the added endpoint
+ */
+ protected Endpoint addEndpointToRegistry(String uri, Endpoint endpoint,
boolean normalized) {
StringHelper.notEmpty(uri, "uri");
ObjectHelper.notNull(endpoint, "endpoint");
@@ -933,7 +948,7 @@ public abstract class AbstractCamelContext extends
BaseService
for (EndpointStrategy strategy : getEndpointStrategies()) {
endpoint = strategy.registerEndpoint(uri, endpoint);
}
- endpoints.put(getEndpointKey(uri, endpoint), endpoint);
+ endpoints.put(getEndpointKey(uri, endpoint, normalized), endpoint);
return endpoint;
}
@@ -955,11 +970,24 @@ public abstract class AbstractCamelContext extends
BaseService
* @return the key
*/
protected NormalizedUri getEndpointKey(String uri, Endpoint endpoint) {
+ return getEndpointKey(uri, endpoint, false);
+ }
+
+ /**
+ * Gets the endpoint key to use for lookup or whe adding endpoints to the
{@link DefaultEndpointRegistry}
+ *
+ * @param uri the endpoint uri
+ * @param endpoint the endpoint
+ * @param normalized whether the uri is already normalized; see
+ * {@link #addEndpointToRegistry(String, Endpoint,
boolean)}
+ * @return the key
+ */
+ protected NormalizedUri getEndpointKey(String uri, Endpoint endpoint,
boolean normalized) {
if (endpoint != null && !endpoint.isSingleton()) {
int counter = endpointKeyCounter.incrementAndGet();
- return NormalizedUri.newNormalizedUri(uri + ":" + counter, false);
+ return NormalizedUri.newNormalizedUri(uri + ":" + counter,
normalized);
} else {
- return NormalizedUri.newNormalizedUri(uri, false);
+ return NormalizedUri.newNormalizedUri(uri, normalized);
}
}
diff --git
a/core/camel-core/src/test/java/org/apache/camel/impl/DefaultEndpointRegistryTest.java
b/core/camel-core/src/test/java/org/apache/camel/impl/DefaultEndpointRegistryTest.java
index ccbb1604e32f..7511dd42efa3 100644
---
a/core/camel-core/src/test/java/org/apache/camel/impl/DefaultEndpointRegistryTest.java
+++
b/core/camel-core/src/test/java/org/apache/camel/impl/DefaultEndpointRegistryTest.java
@@ -102,6 +102,32 @@ public class DefaultEndpointRegistryTest {
ctx.stop();
}
+ // Testing the issue https://issues.apache.org/jira/browse/CAMEL-24171
+ @Test
+ public void testGetEndpointIsCachedForUriContainingPercentCharacter() {
+ DefaultCamelContext ctx = new DefaultCamelContext();
+ ctx.start();
+
+ // A % anywhere in the query string makes
EndpointHelper.normalizeEndpointUri() non-idempotent:
+ // running it twice turns %41 into %2541 instead of leaving it alone.
doGetEndpoint() normalizes
+ // the uri once for the cache lookup
(NormalizedUri.newNormalizedUri(uri, true) - "already normalized").
+ // But on a cache miss it hands that *already normalized* uri to
addEndpointToRegistry(), which calls
+ // getEndpointKey(uri, endpoint) ->
NormalizedUri.newNormalizedUri(uri, false) - "not normalized yet,
+ // please normalize" - normalizing it a second time before storing it.
The stored key (double
+ // normalized) then never matches the lookup key computed on a later
call (single normalized), so a
+ // brand-new Endpoint (with its own doStart()) gets created on every
single call instead of the
+ // cached singleton being reused.
+ String uri = "controlbus:route?routeId=RAW(%41)&action=status";
+
+ Endpoint first = ctx.getEndpoint(uri);
+ Endpoint second = ctx.getEndpoint(uri);
+
+ Assertions.assertSame(first, second,
+ "getEndpoint(uri) must return the cached singleton instance on
a second call with the identical uri");
+ Assertions.assertEquals(1, ctx.getEndpoints().size(),
+ "the endpoint registry must not accumulate a new entry on
every call for the same uri");
+ }
+
@Test
public void testMigration() {
DefaultCamelContext ctx = new DefaultCamelContext();
