gnodet commented on code in PR #25198: URL: https://github.com/apache/camel/pull/25198#discussion_r3675971876
########## core/camel-core/src/test/java/org/apache/camel/processor/DynamicEndpointMessagePlaceholderTest.java: ########## @@ -0,0 +1,113 @@ +/* + * 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.processor; + +import java.util.Properties; + +import org.apache.camel.CamelContext; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.builder.AggregationStrategies; +import org.apache.camel.builder.RouteBuilder; +import org.junit.jupiter.api.Test; + +/** + * The dynamic-uri EIPs {@code toD} and {@code enrich} resolve property placeholders ({@code {{...}}}) once, at build + * time, on the endpoint uri written in the route (via ToDynamicReifier/EnrichReifier). A {@code {{...}}} token that + * only appears in the per-message evaluated recipient (e.g. from a header) is therefore treated as a literal endpoint + * uri and not re-expanded. See CAMEL-24282. + * <p/> + * {@code recipientList} / {@code routingSlip} compute their recipients entirely from a runtime expression (no static + * template resolved at build time), so they continue to resolve {@code {{...}}} in those recipients - this is relied + * upon e.g. by routes carrying a {@code {{port}}}-style placeholder in the recipient header, and must be preserved. + */ +class DynamicEndpointMessagePlaceholderTest extends ContextTestSupport { + + @Override + protected CamelContext createCamelContext() throws Exception { + CamelContext context = super.createCamelContext(); + Properties prop = new Properties(); + prop.setProperty("secretTarget", "resolved"); + context.getPropertiesComponent().setInitialProperties(prop); + return context; + } + + @Test + void toDPlaceholderInHeaderNotResolved() throws Exception { + getMockEndpoint("mock:resolved").expectedMessageCount(0); + getMockEndpoint("mock:done").expectedMessageCount(1); + + template.sendBodyAndHeader("direct:tod", "Hello", "target", "mock:{{secretTarget}}"); + + assertMockEndpointsSatisfied(); + } + + @Test + void enrichPlaceholderInHeaderNotResolved() throws Exception { + getMockEndpoint("mock:resolved").expectedMessageCount(0); + getMockEndpoint("mock:done").expectedMessageCount(1); + + template.sendBodyAndHeader("direct:en", "Hello", "target", "mock:{{secretTarget}}"); + + assertMockEndpointsSatisfied(); + } + + @Test + void recipientListPlaceholderInHeaderStillResolved() throws Exception { + // recipientList has no build-time template, so {{...}} in the recipient header is still resolved + getMockEndpoint("mock:resolved").expectedMessageCount(1); + getMockEndpoint("mock:done").expectedMessageCount(1); + + template.sendBodyAndHeader("direct:rl", "Hello", "target", "mock:{{secretTarget}}"); + + assertMockEndpointsSatisfied(); + } + + @Test + void routingSlipPlaceholderInHeaderStillResolved() throws Exception { + getMockEndpoint("mock:resolved").expectedMessageCount(1); + getMockEndpoint("mock:done").expectedMessageCount(1); + + template.sendBodyAndHeader("direct:rs", "Hello", "target", "mock:{{secretTarget}}"); + + assertMockEndpointsSatisfied(); + } + + @Test + void placeholderInStaticTemplateIsResolved() throws Exception { + // control: a placeholder written in the route template itself is still resolved at build time + getMockEndpoint("mock:resolved").expectedMessageCount(1); + + template.sendBody("direct:staticTemplate", "Hello"); + + assertMockEndpointsSatisfied(); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("direct:tod").toD("${header.target}").to("mock:done"); + from("direct:en").enrich().simple("${header.target}") + .aggregationStrategy(AggregationStrategies.useOriginal()).to("mock:done"); + from("direct:rl").recipientList(header("target")).to("mock:done"); + from("direct:rs").routingSlip(header("target")).to("mock:done"); Review Comment: The PR description claims tests for "each EIP (toD, recipientList, routingSlip, enrich, pollEnrich)" but there is no `pollEnrich` test here, and no `dynamicRouter` test either. Consider adding: - A `pollEnrichPlaceholderInHeaderStillResolved()` test to document `pollEnrich`'s current behavior and catch regressions if it is later changed. - A `dynamicRouterPlaceholderInHeaderStillResolved()` test (lower priority since `DynamicRouter` extends `RoutingSlip`, but it would make the test suite match the upgrade guide's claims). ########## docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc: ########## @@ -13,6 +13,33 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation] page for details. == Upgrading Camel 4.21 to 4.22 +=== camel-core + +==== Property placeholders in toD and enrich dynamic endpoint URIs + +`toD` and `enrich` no longer resolve Camel property placeholders (`{{...}}`) on the _per-message +evaluated_ recipient. Property placeholders are resolved once, at build time, on the endpoint URI +written in the route (the static template); a `{{...}}` token that only appears at runtime in the +value produced by the `toD` / `enrich` expression (for example coming from a message header or +body) is now treated as a literal part of the endpoint URI instead of being expanded. + +This only affects routes that produced a `{{...}}` token from message content and relied on it +being expanded, such as a `toD` whose recipient came from a header that contained a `{{...}}` +placeholder. Placeholders written directly in the route continue to work unchanged, for example: + +[source,java] +---- +.toD("{{myEndpoint}}/${header.id}") +.toD("mock:{{name}}") +---- + +`recipientList`, `routingSlip`, `dynamicRouter` and `pollEnrich` are unchanged: their recipients +are computed entirely from a runtime expression (there is no static template resolved at build Review Comment: This statement is factually incorrect for `pollEnrich`. `PollEnricher.doInit()` (line 570-575) resolves property placeholders on the static `uri` field at build time via `EndpointHelper.resolveEndpointUriPropertyPlaceholders(camelContext, uri)` — identical to `SendDynamicProcessor.doInit()`. The claim "no static template resolved at build time" is only accurate for `recipientList`, `routingSlip`, and `dynamicRouter`. Consider either: - Removing `pollEnrich` from this group and noting it separately (with the reason for not changing it), or - Applying the same fix to `PollEnricher` and moving `pollEnrich` to the "changed" group above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
