davsclaus commented on code in PR #11906:
URL: https://github.com/apache/camel/pull/11906#discussion_r1382909044
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterControlChannelProcessor.java:
##########
@@ -156,20 +157,22 @@ DynamicRouterControlMessage handleControlMessage(final
Exchange exchange) {
public boolean process(final Exchange exchange, final AsyncCallback
callback) {
LOG.debug("Received control channel message");
DynamicRouterControlMessage controlMessage =
handleControlMessage(exchange);
- final DynamicRouterProcessor processor =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
- .orElseThrow(() -> new IllegalArgumentException(
- "Control channel message is invalid: wrong channel, or
no processors present."));
- switch (controlMessage.getMessageType()) {
- case SUBSCRIBE:
- processor.addFilter(controlMessage);
- exchange.getIn().setBody(controlMessage.getId(), String.class);
- break;
- case UNSUBSCRIBE:
- processor.removeFilter(controlMessage.getId());
- break;
- default:
- // Cannot get here due to enum
- break;
+ try (DynamicRouterMulticastProcessor processor
+ =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Control channel message is invalid: wrong
channel, or no processors present."))) {
+ switch (controlMessage.getMessageType()) {
+ case SUBSCRIBE -> {
+ processor.addFilter(controlMessage);
+ exchange.getIn().setBody(controlMessage.getId(),
String.class);
+ }
+ case UNSUBSCRIBE ->
processor.removeFilter(controlMessage.getId());
+ default -> {
+ // Cannot get here due to enum
+ }
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
Review Comment:
This is wrong, you should do
exchange.setException(e);
##########
components/camel-dynamic-router/pom.xml:
##########
@@ -69,4 +69,23 @@
<scope>test</scope>
</dependency>
</dependencies>
+
+ <build>
Review Comment:
Is these settings really needed, we dont have this kind in the other
compomnents
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterControlChannelProcessor.java:
##########
@@ -156,20 +157,22 @@ DynamicRouterControlMessage handleControlMessage(final
Exchange exchange) {
public boolean process(final Exchange exchange, final AsyncCallback
callback) {
LOG.debug("Received control channel message");
DynamicRouterControlMessage controlMessage =
handleControlMessage(exchange);
- final DynamicRouterProcessor processor =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
- .orElseThrow(() -> new IllegalArgumentException(
- "Control channel message is invalid: wrong channel, or
no processors present."));
- switch (controlMessage.getMessageType()) {
- case SUBSCRIBE:
- processor.addFilter(controlMessage);
- exchange.getIn().setBody(controlMessage.getId(), String.class);
- break;
- case UNSUBSCRIBE:
- processor.removeFilter(controlMessage.getId());
- break;
- default:
- // Cannot get here due to enum
- break;
+ try (DynamicRouterMulticastProcessor processor
+ =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Control channel message is invalid: wrong
channel, or no processors present."))) {
Review Comment:
This is wrong you should set exchange on exchange and not throw it. And then
call callback done true and return true to exit
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterMulticastProcessor.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.dynamicrouter;
+
+import java.util.*;
+import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+import org.apache.camel.*;
+import org.apache.camel.processor.FilterProcessor;
+import org.apache.camel.processor.MulticastProcessor;
+import org.apache.camel.processor.ProcessorExchangePair;
+import org.apache.camel.spi.ProducerCache;
+import org.apache.camel.support.*;
+import org.apache.camel.support.service.ServiceHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.camel.component.dynamicrouter.DynamicRouterConstants.MODE_FIRST_MATCH;
+
+public class DynamicRouterMulticastProcessor extends MulticastProcessor {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(DynamicRouterMulticastProcessor.class);
+
+ /**
+ * Template for a logging endpoint, showing all, and multiline.
+ */
+ private static final String LOG_ENDPOINT =
"log:%s.%s?level=%s&showAll=true&multiline=true";
+
+ private boolean ignoreInvalidEndpoints;
+
+ private final ProducerCache producerCache;
+
+ /**
+ * {@link FilterProcessor}s, mapped by subscription ID, to determine if
the incoming exchange should be routed based
+ * on the content.
+ */
+ private final TreeMap<String, PrioritizedFilter> filterMap;
+
+ /**
+ * Indicates the behavior of the Dynamic Router when routing participants
are selected to receive an incoming
+ * exchange. If the mode is "firstMatch", then the exchange is routed only
to the first participant that has a
+ * matching predicate. If the mode is "allMatch", then the exchange is
routed to all participants that have a
+ * matching predicate.
+ */
+ private final String recipientMode;
+
+ /**
+ * The {@link FilterProcessor} factory.
+ */
+ private final Supplier<PrioritizedFilter.PrioritizedFilterFactory>
filterProcessorFactorySupplier;
+
+ /**
+ * Flag to log a warning if a message is dropped due to no matching
filters.
+ */
+ private final boolean warnDroppedMessage;
+
+ public DynamicRouterMulticastProcessor(String id, CamelContext
camelContext, Route route, String recipientMode,
+ final boolean warnDroppedMessage,
+ final
Supplier<PrioritizedFilter.PrioritizedFilterFactory>
filterProcessorFactorySupplier,
+ ProducerCache producerCache,
AggregationStrategy aggregationStrategy,
+ boolean parallelProcessing,
ExecutorService executorService,
+ boolean shutdownExecutorService,
+ boolean streaming, boolean
stopOnException,
+ long timeout, Processor onPrepare,
boolean shareUnitOfWork,
+ boolean parallelAggregate) {
+ super(camelContext, route, new ArrayList<>(), aggregationStrategy,
parallelProcessing, executorService,
+ shutdownExecutorService,
+ streaming, stopOnException, timeout, onPrepare,
+ shareUnitOfWork, parallelAggregate);
+ setId(id);
+ this.producerCache = producerCache;
+ this.filterMap = new TreeMap<>();
+ this.recipientMode = recipientMode;
+ this.filterProcessorFactorySupplier = filterProcessorFactorySupplier;
+ this.warnDroppedMessage = warnDroppedMessage;
+ }
+
+ public boolean isIgnoreInvalidEndpoints() {
+ return ignoreInvalidEndpoints;
+ }
+
+ public void setIgnoreInvalidEndpoints(boolean ignoreInvalidEndpoints) {
+ this.ignoreInvalidEndpoints = ignoreInvalidEndpoints;
+ }
+
+ protected List<Processor> createEndpointProcessors(Exchange exchange) {
+ List<String> recipientList = matchFilters(exchange).stream()
+ .map(PrioritizedFilter::getEndpoint)
+ .distinct()
+ .map(String::trim)
+ .collect(Collectors.toList());
+ if (recipientList.isEmpty()) {
+ // No matching filters, so we will use the default filter that
will create a
+ // notification that there were no routing participants that
matched the
+ // exchange, which results in a "dropped" message.
+ Message exchangeIn = exchange.getIn();
+ Object originalBody = exchangeIn.getBody();
+ exchangeIn.setHeader("originalBody", originalBody);
+ String endpoint = String.format(LOG_ENDPOINT,
this.getClass().getCanonicalName(), getId(),
+ warnDroppedMessage ? "WARN" : "DEBUG");
+ recipientList.add(endpoint);
+ String error = String.format(
Review Comment:
Hmmm why is this a plain string message returned and not an exception or
something, its a bit weird
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterEndpoint.java:
##########
@@ -140,30 +149,87 @@ protected void doInit() throws Exception {
super.doInit();
Review Comment:
Is all this code in the constructor? If so it should be moved to doInit
where you do all kind of initialization
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterControlChannelProcessor.java:
##########
@@ -156,20 +157,22 @@ DynamicRouterControlMessage handleControlMessage(final
Exchange exchange) {
public boolean process(final Exchange exchange, final AsyncCallback
callback) {
LOG.debug("Received control channel message");
DynamicRouterControlMessage controlMessage =
handleControlMessage(exchange);
- final DynamicRouterProcessor processor =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
- .orElseThrow(() -> new IllegalArgumentException(
- "Control channel message is invalid: wrong channel, or
no processors present."));
- switch (controlMessage.getMessageType()) {
- case SUBSCRIBE:
- processor.addFilter(controlMessage);
- exchange.getIn().setBody(controlMessage.getId(), String.class);
- break;
- case UNSUBSCRIBE:
- processor.removeFilter(controlMessage.getId());
- break;
- default:
- // Cannot get here due to enum
- break;
+ try (DynamicRouterMulticastProcessor processor
+ =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Control channel message is invalid: wrong
channel, or no processors present."))) {
+ switch (controlMessage.getMessageType()) {
+ case SUBSCRIBE -> {
+ processor.addFilter(controlMessage);
+ exchange.getIn().setBody(controlMessage.getId(),
String.class);
+ }
+ case UNSUBSCRIBE ->
processor.removeFilter(controlMessage.getId());
+ default -> {
+ // Cannot get here due to enum
+ }
+ }
+ } catch (IOException e) {
Review Comment:
Should catch Exception not only IO
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/DynamicRouterControlChannelProcessor.java:
##########
@@ -156,20 +157,22 @@ DynamicRouterControlMessage handleControlMessage(final
Exchange exchange) {
public boolean process(final Exchange exchange, final AsyncCallback
callback) {
LOG.debug("Received control channel message");
DynamicRouterControlMessage controlMessage =
handleControlMessage(exchange);
- final DynamicRouterProcessor processor =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
- .orElseThrow(() -> new IllegalArgumentException(
- "Control channel message is invalid: wrong channel, or
no processors present."));
- switch (controlMessage.getMessageType()) {
- case SUBSCRIBE:
- processor.addFilter(controlMessage);
- exchange.getIn().setBody(controlMessage.getId(), String.class);
- break;
- case UNSUBSCRIBE:
- processor.removeFilter(controlMessage.getId());
- break;
- default:
- // Cannot get here due to enum
- break;
+ try (DynamicRouterMulticastProcessor processor
+ =
Optional.ofNullable(component.getRoutingProcessor(controlMessage.getChannel()))
+ .orElseThrow(() -> new IllegalArgumentException(
+ "Control channel message is invalid: wrong
channel, or no processors present."))) {
+ switch (controlMessage.getMessageType()) {
+ case SUBSCRIBE -> {
+ processor.addFilter(controlMessage);
+ exchange.getIn().setBody(controlMessage.getId(),
String.class);
Review Comment:
Should maybe be changed from getIn to getMessage
--
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]
