You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2021/01/08 10:38:19 UTC
[camel] branch master updated: CAMEL-16005: Fix route template
duplicate id clash for auto assinged ids. Thanks to Marco Collovati for
reporting and the unit test.
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/master by this push:
new 61f8109 CAMEL-16005: Fix route template duplicate id clash for auto assinged ids. Thanks to Marco Collovati for reporting and the unit test.
61f8109 is described below
commit 61f8109005e8a94ff085d229cac12780120d6ea8
Author: Claus Ibsen <cl...@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 7424b51..a8d7bf6 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 {