This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch camel-3.7.x in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/camel-3.7.x by this push: new d2f2f7d CAMEL-16005: Fix route template duplicate id clash for auto assinged ids. Thanks to Marco Collovati for reporting and the unit test. d2f2f7d is described below commit d2f2f7d0a4e6e5cb912fe057597994aaf2af3828 Author: Claus Ibsen <claus.ib...@gmail.com> AuthorDate: Fri Jan 8 11:17:57 2021 +0100 CAMEL-16005: Fix route template duplicate id clash for auto assinged ids. Thanks to Marco Collovati for reporting and the unit test. --- .../main/java/org/apache/camel/spi/IdAware.java | 9 +++ .../impl/engine/DefaultExecutorServiceManager.java | 3 +- .../org/apache/camel/impl/DefaultCamelContext.java | 4 ++ .../org/apache/camel/model/ChoiceDefinition.java | 8 +-- .../camel/model/OptionalIdentifiedDefinition.java | 12 +++- .../camel/model/ProcessorDefinitionHelper.java | 29 +++++++++ .../core/xml/AbstractCamelContextFactoryBean.java | 2 +- .../builder/RouteTemplateDuplicateIdIssueTest.java | 68 ++++++++++++++++++++++ .../camel/main/DefaultConfigurationConfigurer.java | 2 +- .../java/org/apache/camel/xml/in/ModelParser.java | 9 +-- 10 files changed, 132 insertions(+), 14 deletions(-) diff --git a/core/camel-api/src/main/java/org/apache/camel/spi/IdAware.java b/core/camel-api/src/main/java/org/apache/camel/spi/IdAware.java index 0d5c7a9..568a23b 100644 --- a/core/camel-api/src/main/java/org/apache/camel/spi/IdAware.java +++ b/core/camel-api/src/main/java/org/apache/camel/spi/IdAware.java @@ -32,4 +32,13 @@ public interface IdAware extends HasId { */ void setId(String id); + /** + * Sets the id which has been auto generated + * + * @param id the auto generated id + */ + default void setGeneratedId(String id) { + setId(id); + } + } diff --git a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultExecutorServiceManager.java b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultExecutorServiceManager.java index 33f23fe..791a030 100644 --- a/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultExecutorServiceManager.java +++ b/core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultExecutorServiceManager.java @@ -56,7 +56,8 @@ public class DefaultExecutorServiceManager extends BaseExecutorServiceManager { NodeIdFactory factory = getCamelContext().adapt(ExtendedCamelContext.class).getNodeIdFactory(); if (node.getId() == null) { String id = factory.createId(node); - ((IdAware) source).setId(id); + // we auto generated an id to be assigned + ((IdAware) source).setGeneratedId(id); } } return source; diff --git a/core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultCamelContext.java b/core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultCamelContext.java index 589df12..43e4941 100644 --- a/core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultCamelContext.java +++ b/core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultCamelContext.java @@ -43,6 +43,7 @@ import org.apache.camel.model.Model; import org.apache.camel.model.ModelCamelContext; import org.apache.camel.model.ModelLifecycleStrategy; import org.apache.camel.model.ProcessorDefinition; +import org.apache.camel.model.ProcessorDefinitionHelper; import org.apache.camel.model.Resilience4jConfigurationDefinition; import org.apache.camel.model.RouteDefinition; import org.apache.camel.model.RouteDefinitionHelper; @@ -588,6 +589,9 @@ public class DefaultCamelContext extends SimpleCamelContext implements ModelCame Properties prop = new Properties(); prop.putAll(routeDefinition.getTemplateParameters()); pc.setLocalProperties(prop); + + // need to reset auto assigned ids, so there is no clash when creating routes + ProcessorDefinitionHelper.resetAllAutoAssignedNodeIds(routeDefinition); } // must ensure route is prepared, before we can start it diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java b/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java index de865d3..bf3d5ea 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/ChoiceDefinition.java @@ -199,16 +199,16 @@ public class ChoiceDefinition extends ProcessorDefinition<ChoiceDefinition> impl } @Override - public void setId(String value) { + public void setId(String id) { // when setting id, we should set it on the fine grained element, if // possible if (otherwise != null) { - otherwise.setId(value); + otherwise.setId(id); } else if (!getWhenClauses().isEmpty()) { int size = getWhenClauses().size(); - getWhenClauses().get(size - 1).setId(value); + getWhenClauses().get(size - 1).setId(id); } else { - super.setId(value); + super.setId(id); } } diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/OptionalIdentifiedDefinition.java b/core/camel-core-model/src/main/java/org/apache/camel/model/OptionalIdentifiedDefinition.java index f300031..a680f06 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/OptionalIdentifiedDefinition.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/OptionalIdentifiedDefinition.java @@ -49,11 +49,17 @@ public abstract class OptionalIdentifiedDefinition<T extends OptionalIdentifiedD */ @XmlAttribute @Metadata(description = "The id of this node") - public void setId(String value) { - this.id = value; + public void setId(String id) { + this.id = id; customId = true; } + @Override + public void setGeneratedId(String id) { + this.id = id; + customId = null; + } + public DescriptionDefinition getDescription() { return description; } @@ -142,7 +148,7 @@ public abstract class OptionalIdentifiedDefinition<T extends OptionalIdentifiedD */ public String idOrCreate(NodeIdFactory factory) { if (id == null) { - id = factory.createId(this); + setGeneratedId(factory.createId(this)); } return id; } diff --git a/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinitionHelper.java b/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinitionHelper.java index cb20243..7925053 100644 --- a/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinitionHelper.java +++ b/core/camel-core-model/src/main/java/org/apache/camel/model/ProcessorDefinitionHelper.java @@ -222,6 +222,35 @@ public final class ProcessorDefinitionHelper { return set; } + /** + * Resets (nulls) all the auto assigned ids on the node and the nested children (outputs) + */ + public static void resetAllAutoAssignedNodeIds(ProcessorDefinition<?> node) { + if (node == null) { + return; + } + + // skip abstract + if (node.isAbstract()) { + return; + } + + if (node.getId() != null) { + if (!node.hasCustomIdAssigned()) { + node.setId(null); + } + } + + // traverse outputs and recursive children as well + List<ProcessorDefinition<?>> children = node.getOutputs(); + if (children != null && !children.isEmpty()) { + for (ProcessorDefinition<?> child : children) { + // traverse children also + resetAllAutoAssignedNodeIds(child); + } + } + } + private static <T> void doFindType(List<ProcessorDefinition<?>> outputs, Class<T> type, List<T> found, int maxDeep) { // do we have any top level abstracts, then we should max deep one more diff --git a/core/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java b/core/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java index fd38f3c..66228f8 100644 --- a/core/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java +++ b/core/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java @@ -360,7 +360,7 @@ public abstract class AbstractCamelContextFactoryBean<T extends ModelCamelContex ServiceRegistry service = entry.getValue(); if (service.getId() == null) { - service.setId(getContext().getUuidGenerator().generateUuid()); + service.setGeneratedId(getContext().getUuidGenerator().generateUuid()); } LOG.info("Using ServiceRegistry with id: {} and implementation: {}", service.getId(), service); diff --git a/core/camel-core/src/test/java/org/apache/camel/builder/RouteTemplateDuplicateIdIssueTest.java b/core/camel-core/src/test/java/org/apache/camel/builder/RouteTemplateDuplicateIdIssueTest.java new file mode 100644 index 0000000..345273c --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/builder/RouteTemplateDuplicateIdIssueTest.java @@ -0,0 +1,68 @@ +/* + * 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.builder; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.Processor; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class RouteTemplateDuplicateIdIssueTest extends ContextTestSupport { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + @Test + void shouldNotFailDueToDuplicatedNodeId() throws Exception { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + routeTemplate("myTemplate") + .templateParameter("input") + .from("direct:{{input}}") + .recipientList(constant("mock:a,mock:b")).parallelProcessing() + .to("mock:result"); + } + }); + + Map one = new HashMap(); + one.put("input", "a"); + context.addRouteFromTemplate("testRouteId1", "myTemplate", one); + + Map two = new HashMap(); + two.put("input", "b"); + context.addRouteFromTemplate("testRouteId2", "myTemplate", two); + + assertDoesNotThrow(() -> context.start(), "Route creation should not fail"); + + // should generate unique id per template for the runtime processors + Processor p1 = context.getProcessor("recipientList1"); + Processor p2 = context.getProcessor("recipientList2"); + assertNotNull(p1); + assertNotNull(p2); + assertNotSame(p1, p2); + + context.stop(); + } + +} diff --git a/core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java b/core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java index 7b37952..51c200f 100644 --- a/core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java +++ b/core/camel-main/src/main/java/org/apache/camel/main/DefaultConfigurationConfigurer.java @@ -379,7 +379,7 @@ public final class DefaultConfigurationConfigurer { ServiceRegistry service = entry.getValue(); if (service.getId() == null) { - service.setId(camelContext.getUuidGenerator().generateUuid()); + service.setGeneratedId(camelContext.getUuidGenerator().generateUuid()); } LOG.info("Using ServiceRegistry with id: {} and implementation: {}", service.getId(), service); diff --git a/core/camel-xml-io/src/generated/java/org/apache/camel/xml/in/ModelParser.java b/core/camel-xml-io/src/generated/java/org/apache/camel/xml/in/ModelParser.java index 77fce70..f173d8b 100644 --- a/core/camel-xml-io/src/generated/java/org/apache/camel/xml/in/ModelParser.java +++ b/core/camel-xml-io/src/generated/java/org/apache/camel/xml/in/ModelParser.java @@ -159,11 +159,12 @@ public class ModelParser extends BaseParser { } protected <T extends OptionalIdentifiedDefinition> ElementHandler<T> optionalIdentifiedDefinitionElementHandler() { return (def, key) -> { - if ("description".equals(key)) { - def.setDescription(doParseDescriptionDefinition()); - return true; + switch (key) { + case "description": def.setDescription(doParseDescriptionDefinition()); break; + case "generatedId": def.setGeneratedId(doParseText()); break; + default: return false; } - return false; + return true; }; } protected DescriptionDefinition doParseDescriptionDefinition() throws IOException, XmlPullParserException {