Repository: camel Updated Branches: refs/heads/master 963506415 -> 22cf585a4
CAMEL-8683: Using load balancer in onException adds duplicate outputs for each route defined Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/22cf585a Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/22cf585a Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/22cf585a Branch: refs/heads/master Commit: 22cf585a4770fcd825c505c16776874d12617dff Parents: 9635064 Author: Claus Ibsen <davscl...@apache.org> Authored: Thu Apr 23 15:48:02 2015 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Thu Apr 23 15:48:02 2015 +0200 ---------------------------------------------------------------------- .../camel/model/LoadBalanceDefinition.java | 68 ++++++-------------- .../camel/model/LoadBalancerDefinition.java | 36 ++++------- .../CustomLoadBalancerDefinition.java | 24 ++++++- .../AdviceWithOnExceptionAndInterceptTest.java | 26 ++++---- .../OnExceptionLoadBalancerDoubleIssueTest.java | 58 +++++++++++++++++ ...gOnExceptionLoadBalancerDoubleIssueTest.java | 33 ++++++++++ .../OnExceptionLoadBalancerDoubleIssueTest.xml | 58 +++++++++++++++++ 7 files changed, 221 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/camel-core/src/main/java/org/apache/camel/model/LoadBalanceDefinition.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/model/LoadBalanceDefinition.java b/camel-core/src/main/java/org/apache/camel/model/LoadBalanceDefinition.java index c2d8291..32ec236 100644 --- a/camel-core/src/main/java/org/apache/camel/model/LoadBalanceDefinition.java +++ b/camel-core/src/main/java/org/apache/camel/model/LoadBalanceDefinition.java @@ -18,11 +18,9 @@ package org.apache.camel.model; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.List; import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; -import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlElementRef; import javax.xml.bind.annotation.XmlElements; @@ -50,9 +48,6 @@ import org.apache.camel.util.CollectionStringBuffer; @XmlRootElement(name = "loadBalance") @XmlAccessorType(XmlAccessType.FIELD) public class LoadBalanceDefinition extends ProcessorDefinition<LoadBalanceDefinition> { - @XmlAttribute - @Deprecated - private String ref; @XmlElements({ @XmlElement(required = false, name = "failover", type = FailoverLoadBalancerDefinition.class), @XmlElement(required = false, name = "random", type = RandomLoadBalancerDefinition.class), @@ -88,21 +83,6 @@ public class LoadBalanceDefinition extends ProcessorDefinition<LoadBalanceDefini return true; } - public String getRef() { - return ref; - } - - /** - * To use a custom load balancer. - * This option is deprecated, use the custom load balancer type instead. - * - * @deprecated use custom load balancer - */ - @Deprecated - public void setRef(String ref) { - this.ref = ref; - } - public LoadBalancerDefinition getLoadBalancerType() { return loadBalancerType; } @@ -117,30 +97,26 @@ public class LoadBalanceDefinition extends ProcessorDefinition<LoadBalanceDefini loadBalancerType = loadbalancer; } - protected Processor createOutputsProcessor(RouteContext routeContext, - Collection<ProcessorDefinition<?>> outputs) throws Exception { - - LoadBalancer loadBalancer = LoadBalancerDefinition.getLoadBalancer(routeContext, loadBalancerType, ref); - for (ProcessorDefinition<?> processorType : outputs) { - Processor processor = createProcessor(routeContext, processorType); - loadBalancer.addProcessor(processor); - } - return loadBalancer; - } - @Override public Processor createProcessor(RouteContext routeContext) throws Exception { - LoadBalancer loadBalancer = LoadBalancerDefinition.getLoadBalancer(routeContext, loadBalancerType, ref); - for (ProcessorDefinition<?> processorType : getOutputs()) { - // output must not be another load balancer - // check for instanceof as the code below as there is compilation errors on earlier versions of JDK6 - // on Windows boxes or with IBM JDKs etc. - if (LoadBalanceDefinition.class.isInstance(processorType)) { - throw new IllegalArgumentException("Loadbalancer already configured to: " + loadBalancerType + ". Cannot set it to: " + processorType); + // the load balancer is stateful so we should only create it once in case its used from a context scoped error handler + + LoadBalancer loadBalancer = loadBalancerType.getLoadBalancer(routeContext); + if (loadBalancer == null) { + // then create it and reuse it + loadBalancer = loadBalancerType.createLoadBalancer(routeContext); + loadBalancerType.setLoadBalancer(loadBalancer); + for (ProcessorDefinition<?> processorType : getOutputs()) { + // output must not be another load balancer + // check for instanceof as the code below as there is compilation errors on earlier versions of JDK6 + // on Windows boxes or with IBM JDKs etc. + if (LoadBalanceDefinition.class.isInstance(processorType)) { + throw new IllegalArgumentException("Loadbalancer already configured to: " + loadBalancerType + ". Cannot set it to: " + processorType); + } + Processor processor = createProcessor(routeContext, processorType); + processor = wrapChannel(routeContext, processor, processorType); + loadBalancer.addProcessor(processor); } - Processor processor = createProcessor(routeContext, processorType); - processor = wrapChannel(routeContext, processor, processorType); - loadBalancer.addProcessor(processor); } return loadBalancer; } @@ -155,7 +131,9 @@ public class LoadBalanceDefinition extends ProcessorDefinition<LoadBalanceDefini * @return the builder */ public LoadBalanceDefinition loadBalance(LoadBalancer loadBalancer) { - setLoadBalancerType(new LoadBalancerDefinition(loadBalancer)); + CustomLoadBalancerDefinition def = new CustomLoadBalancerDefinition(); + def.setLoadBalancer(loadBalancer); + setLoadBalancerType(def); return this; } @@ -318,10 +296,6 @@ public class LoadBalanceDefinition extends ProcessorDefinition<LoadBalanceDefini @Override public String toString() { - if (loadBalancerType != null) { - return "LoadBalanceType[" + loadBalancerType + ", " + getOutputs() + "]"; - } else { - return "LoadBalanceType[ref:" + ref + ", " + getOutputs() + "]"; - } + return "LoadBalanceType[" + loadBalancerType + ", " + getOutputs() + "]"; } } http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/camel-core/src/main/java/org/apache/camel/model/LoadBalancerDefinition.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/model/LoadBalancerDefinition.java b/camel-core/src/main/java/org/apache/camel/model/LoadBalancerDefinition.java index c5a99fd..7ef9cb3 100644 --- a/camel-core/src/main/java/org/apache/camel/model/LoadBalancerDefinition.java +++ b/camel-core/src/main/java/org/apache/camel/model/LoadBalancerDefinition.java @@ -50,20 +50,6 @@ public class LoadBalancerDefinition extends IdentifiedType { this.loadBalancerTypeName = loadBalancerTypeName; } - public static LoadBalancer getLoadBalancer(RouteContext routeContext, LoadBalancerDefinition type, String ref) { - if (type == null) { - ObjectHelper.notNull(ref, "ref or loadBalancer"); - LoadBalancer loadBalancer = routeContext.mandatoryLookup(ref, LoadBalancer.class); - if (loadBalancer instanceof LoadBalancerDefinition) { - type = (LoadBalancerDefinition) loadBalancer; - } else { - return loadBalancer; - } - } - return type.getLoadBalancer(routeContext); - } - - /** * Sets a named property on the data format instance using introspection */ @@ -82,26 +68,30 @@ public class LoadBalancerDefinition extends IdentifiedType { } public LoadBalancer getLoadBalancer(RouteContext routeContext) { - if (loadBalancer == null) { - loadBalancer = createLoadBalancer(routeContext); - ObjectHelper.notNull(loadBalancer, "loadBalancer"); - configureLoadBalancer(loadBalancer); - } return loadBalancer; } + public void setLoadBalancer(LoadBalancer loadBalancer) { + this.loadBalancer = loadBalancer; + } + /** - * Factory method to create the load balancer instance + * Factory method to create the load balancer from the loadBalancerTypeName */ protected LoadBalancer createLoadBalancer(RouteContext routeContext) { + ObjectHelper.notEmpty(loadBalancerTypeName, "loadBalancerTypeName", this); + + LoadBalancer answer = null; if (loadBalancerTypeName != null) { - Class<?> type = routeContext.getCamelContext().getClassResolver().resolveClass(loadBalancerTypeName); + Class<?> type = routeContext.getCamelContext().getClassResolver().resolveClass(loadBalancerTypeName, LoadBalancer.class); if (type == null) { throw new IllegalArgumentException("Cannot find class: " + loadBalancerTypeName + " in the classpath"); } - return (LoadBalancer) ObjectHelper.newInstance(type); + answer = (LoadBalancer) routeContext.getCamelContext().getInjector().newInstance(type); + configureLoadBalancer(answer); } - return null; + + return answer; } @Override http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/camel-core/src/main/java/org/apache/camel/model/loadbalancer/CustomLoadBalancerDefinition.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/model/loadbalancer/CustomLoadBalancerDefinition.java b/camel-core/src/main/java/org/apache/camel/model/loadbalancer/CustomLoadBalancerDefinition.java index 905407f..4a0dd08 100644 --- a/camel-core/src/main/java/org/apache/camel/model/loadbalancer/CustomLoadBalancerDefinition.java +++ b/camel-core/src/main/java/org/apache/camel/model/loadbalancer/CustomLoadBalancerDefinition.java @@ -20,6 +20,7 @@ import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.bind.annotation.XmlTransient; import org.apache.camel.model.LoadBalancerDefinition; import org.apache.camel.processor.loadbalancer.LoadBalancer; @@ -36,6 +37,8 @@ import org.apache.camel.util.ObjectHelper; @XmlAccessorType(XmlAccessType.FIELD) public class CustomLoadBalancerDefinition extends LoadBalancerDefinition { + @XmlTransient + private LoadBalancer loadBalancer; @XmlAttribute(required = true) private String ref; @@ -53,15 +56,34 @@ public class CustomLoadBalancerDefinition extends LoadBalancerDefinition { this.ref = ref; } + public LoadBalancer getLoadBalancer() { + return loadBalancer; + } + + /** + * The custom load balancer to use. + */ + public void setLoadBalancer(LoadBalancer loadBalancer) { + this.loadBalancer = loadBalancer; + } + @Override protected LoadBalancer createLoadBalancer(RouteContext routeContext) { + if (loadBalancer != null) { + return loadBalancer; + } + ObjectHelper.notEmpty(ref, "ref", this); return CamelContextHelper.mandatoryLookup(routeContext.getCamelContext(), ref, LoadBalancer.class); } @Override public String toString() { - return "CustomLoadBalancer[" + ref + "]"; + if (loadBalancer != null) { + return "CustomLoadBalancer[" + loadBalancer + "]"; + } else { + return "CustomLoadBalancer[" + ref + "]"; + } } } http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionAndInterceptTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionAndInterceptTest.java b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionAndInterceptTest.java index 8bc6953..3813db2 100644 --- a/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionAndInterceptTest.java +++ b/camel-core/src/test/java/org/apache/camel/issues/AdviceWithOnExceptionAndInterceptTest.java @@ -30,17 +30,9 @@ import org.apache.camel.model.RouteDefinition; */ public class AdviceWithOnExceptionAndInterceptTest extends ContextTestSupport { - public RouteBuilder createRouteBuilder() { - return new RouteBuilder() { - @Override - public void configure() { - from("direct:a") - .loadBalance().failover(IOException.class) - .to("mock:a") - .to("mock:b") - .end(); - } - }; + @Override + public boolean isUseRouteBuilder() { + return false; } class AdviceWithRouteBuilder extends RouteBuilder { @@ -65,8 +57,20 @@ public class AdviceWithOnExceptionAndInterceptTest extends ContextTestSupport { } public void testFailover() throws Exception { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:a") + .loadBalance().failover(IOException.class) + .to("mock:a") + .to("mock:b") + .end(); + } + }); + RouteDefinition routeDefinition = context.getRouteDefinitions().get(0); routeDefinition.adviceWith(context, new AdviceWithRouteBuilder()); + context.start(); getMockEndpoint("mock:a").expectedMessageCount(0); getMockEndpoint("mock:b").expectedBodiesReceived("Intercepted SQL!"); http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.java b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.java new file mode 100644 index 0000000..83abb16 --- /dev/null +++ b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.java @@ -0,0 +1,58 @@ +/** + * 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.onexception; + +import org.apache.camel.ContextTestSupport; +import org.apache.camel.builder.RouteBuilder; + +public class OnExceptionLoadBalancerDoubleIssueTest extends ContextTestSupport { + + public void testNotDouble() throws Exception { + // there should only be 3 processors on the load balancer + getMockEndpoint("mock:error").expectedBodiesReceived("A", "D", "G"); + getMockEndpoint("mock:error2").expectedBodiesReceived("B", "E"); + getMockEndpoint("mock:error3").expectedBodiesReceived("C", "F"); + + template.sendBody("direct:foo", "A"); + template.sendBody("direct:foo", "B"); + template.sendBody("direct:bar", "C"); + template.sendBody("direct:bar", "D"); + template.sendBody("direct:foo", "E"); + template.sendBody("direct:bar", "F"); + template.sendBody("direct:foo", "G"); + + assertMockEndpointsSatisfied(); + } + + @Override + protected RouteBuilder createRouteBuilder() throws Exception { + return new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Exception.class) + .handled(true) + .loadBalance().roundRobin().id("round").to("mock:error", "mock:error2", "mock:error3").end(); + + from("direct:foo") + .throwException(new IllegalArgumentException("Forced")); + + from("direct:bar") + .throwException(new IllegalArgumentException("Also Forced")); + } + }; + } +} http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/components/camel-spring/src/test/java/org/apache/camel/spring/processor/onexception/SpringOnExceptionLoadBalancerDoubleIssueTest.java ---------------------------------------------------------------------- diff --git a/components/camel-spring/src/test/java/org/apache/camel/spring/processor/onexception/SpringOnExceptionLoadBalancerDoubleIssueTest.java b/components/camel-spring/src/test/java/org/apache/camel/spring/processor/onexception/SpringOnExceptionLoadBalancerDoubleIssueTest.java new file mode 100644 index 0000000..14fa582 --- /dev/null +++ b/components/camel-spring/src/test/java/org/apache/camel/spring/processor/onexception/SpringOnExceptionLoadBalancerDoubleIssueTest.java @@ -0,0 +1,33 @@ +/** + * 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.spring.processor.onexception; + +import org.apache.camel.CamelContext; +import org.apache.camel.processor.onexception.OnExceptionLoadBalancerDoubleIssueTest; + +import static org.apache.camel.spring.processor.SpringTestHelper.createSpringCamelContext; + +/** + * @version + */ +public class SpringOnExceptionLoadBalancerDoubleIssueTest extends OnExceptionLoadBalancerDoubleIssueTest { + + protected CamelContext createCamelContext() throws Exception { + return createSpringCamelContext(this, "org/apache/camel/spring/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.xml"); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/camel/blob/22cf585a/components/camel-spring/src/test/resources/org/apache/camel/spring/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.xml ---------------------------------------------------------------------- diff --git a/components/camel-spring/src/test/resources/org/apache/camel/spring/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.xml b/components/camel-spring/src/test/resources/org/apache/camel/spring/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.xml new file mode 100644 index 0000000..e5e42ce --- /dev/null +++ b/components/camel-spring/src/test/resources/org/apache/camel/spring/processor/onexception/OnExceptionLoadBalancerDoubleIssueTest.xml @@ -0,0 +1,58 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<beans xmlns="http://www.springframework.org/schema/beans"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd + http://camel.apache.org/schema/spring http://camel.apache.org/schema/spring/camel-spring.xsd + "> + + <bean id="forced" class="java.lang.IllegalArgumentException"> + <constructor-arg index="0" value="Forced"/> + </bean> + + <bean id="also" class="java.lang.IllegalArgumentException"> + <constructor-arg index="0" value="Also Forced"/> + </bean> + + <camelContext xmlns="http://camel.apache.org/schema/spring";> + + <onException> + <exception>java.lang.Exception</exception> + <handled> <constant>true</constant> </handled> + <loadBalance> + <roundRobin id="round"/> + <to uri="mock:error"/> + <to uri="mock:error2"/> + <to uri="mock:error3"/> + </loadBalance> + </onException> + + <route> + <from uri="direct:foo"/> + <throwException ref="forced"/> + </route> + + <route> + <from uri="direct:bar"/> + <throwException ref="also"/> + </route> + + </camelContext> + +</beans>
