Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-2873: Preserve mapping order in the router (#2877)

Browse Source 
* GH-2873: Preserve mapping order in the router

Fixes https://github.com/spring-projects/spring-integration/issues/2873

Sometime it is important to map to most specific exception instead of
its super class.

* Use `LinkedHashMap` for mapping keys in the
`ErrorMessageExceptionTypeRouter`, as well as in its
`AbstractMappingMessageRouter` superclass.
Since we don't do that internal map modification, there is no reason to
worry about concurrent access: we just replace an internal instance
atomically with a new `LinkedHashMap` every time we modify a mapping
for router

**Cherry-pick to 5.1.x**

* * Fix `RouterSpec.RouterMappingProvider` to `LinkedHashMap` as well

* * Fix `RouterTests` for proper mapping order
* Polishing for `AbstractMappingMessageRouter` hierarchy, so we don't
use `protected channelMappings` field access any more
This commit is contained in:
Artem Bilan
2019-04-01 15:29:39 -04:00
committed by Gary Russell
parent 3fc278688c
commit 23a73fab87
7 changed files with 51 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
spring-integration-core/src/main/java/org/springframework/integration/dsl/RouterSpec.java
View File

@@ -16,7 +16,7 @@
package org.springframework.integration.dsl;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import org.springframework.core.convert.ConversionService;
@@ -188,7 +188,7 @@ public final class RouterSpec<K, R extends AbstractMappingMessageRouter>
private final MappingMessageRouterManagement router;
private final Map<Object, NamedComponent> mapping = new HashMap<>();
private final Map<Object, NamedComponent> mapping = new LinkedHashMap<>();
RouterMappingProvider(MappingMessageRouterManagement router) {
this.router = router;

50
spring-integration-core/src/main/java/org/springframework/integration/router/AbstractMappingMessageRouter.java
View File

@@ -20,14 +20,12 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.springframework.integration.support.management.MappingMessageRouterManagement;
import org.springframework.jmx.export.annotation.ManagedAttribute;
@@ -48,32 +46,35 @@ import org.springframework.util.StringUtils;
* @author Gunnar Hillert
* @author Gary Russell
* @author Artem Bilan
*
* @since 2.1
*/
public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter implements MappingMessageRouterManagement {
public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
implements MappingMessageRouterManagement {
private static final int DEFAULT_DYNAMIC_CHANNEL_LIMIT = 100;
private int dynamicChannelLimit = DEFAULT_DYNAMIC_CHANNEL_LIMIT;
@SuppressWarnings("serial")
private final Map<String, MessageChannel> dynamicChannels = Collections.<String, MessageChannel>synchronizedMap(
new LinkedHashMap<String, MessageChannel>(DEFAULT_DYNAMIC_CHANNEL_LIMIT, 0.75f, true) {
private final Map<String, MessageChannel> dynamicChannels =
Collections.synchronizedMap(
new LinkedHashMap<String, MessageChannel>(DEFAULT_DYNAMIC_CHANNEL_LIMIT, 0.75f, true) {
@Override
protected boolean removeEldestEntry(Entry<String, MessageChannel> eldest) {
return this.size() > AbstractMappingMessageRouter.this.dynamicChannelLimit;
}
@Override
protected boolean removeEldestEntry(Entry<String, MessageChannel> eldest) {
return this.size() > AbstractMappingMessageRouter.this.dynamicChannelLimit;
}
});
});
protected volatile Map<String, String> channelMappings = new ConcurrentHashMap<String, String>();
private String prefix;
private volatile String prefix;
private String suffix;
private volatile String suffix;
private boolean resolutionRequired = true;
private volatile boolean resolutionRequired = true;
private volatile Map<String, String> channelMappings = new LinkedHashMap<>();
/**
@@ -86,7 +87,7 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@ManagedAttribute
public void setChannelMappings(Map<String, String> channelMappings) {
Assert.notNull(channelMappings, "'channelMappings' must not be null");
Map<String, String> newChannelMappings = new ConcurrentHashMap<String, String>(channelMappings);
Map<String, String> newChannelMappings = new LinkedHashMap<>(channelMappings);
doSetChannelMappings(newChannelMappings);
}
@@ -135,7 +136,7 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@Override
@ManagedAttribute
public Map<String, String> getChannelMappings() {
return new HashMap<String, String>(this.channelMappings);
return new LinkedHashMap<>(this.channelMappings);
}
/**
@@ -146,7 +147,7 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@Override
@ManagedOperation
public void setChannelMapping(String key, String channelName) {
Map<String, String> newChannelMappings = new ConcurrentHashMap<String, String>(this.channelMappings);
Map<String, String> newChannelMappings = new LinkedHashMap<>(this.channelMappings);
newChannelMappings.put(key, channelName);
this.channelMappings = newChannelMappings;
}
@@ -158,7 +159,7 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@Override
@ManagedOperation
public void removeChannelMapping(String key) {
Map<String, String> newChannelMappings = new ConcurrentHashMap<String, String>(this.channelMappings);
Map<String, String> newChannelMappings = new LinkedHashMap<>(this.channelMappings);
newChannelMappings.remove(key);
this.channelMappings = newChannelMappings;
}
@@ -181,8 +182,8 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@Override
protected Collection<MessageChannel> determineTargetChannels(Message<?> message) {
Collection<MessageChannel> channels = new ArrayList<MessageChannel>();
Collection<Object> channelKeys = this.getChannelKeys(message);
Collection<MessageChannel> channels = new ArrayList<>();
Collection<Object> channelKeys = getChannelKeys(message);
addToCollection(channels, channelKeys, message);
return channels;
}
@@ -201,12 +202,12 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
@ManagedOperation
public void replaceChannelMappings(Properties channelMappings) {
Assert.notNull(channelMappings, "'channelMappings' must not be null");
Map<String, String> newChannelMappings = new ConcurrentHashMap<String, String>();
Map<String, String> newChannelMappings = new LinkedHashMap<>();
Set<String> keys = channelMappings.stringPropertyNames();
for (String key : keys) {
newChannelMappings.put(key.trim(), channelMappings.getProperty(key).trim());
}
this.doSetChannelMappings(newChannelMappings);
doSetChannelMappings(newChannelMappings);
}
private void doSetChannelMappings(Map<String, String> newChannelMappings) {
@@ -227,9 +228,6 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
throw new MessagingException(message, "failed to resolve channel name '" + channelName + "'", e);
}
}
if (channel == null && this.resolutionRequired) {
throw new MessagingException(message, "failed to resolve channel name '" + channelName + "'");
}
return channel;
}
@@ -258,7 +256,7 @@ public abstract class AbstractMappingMessageRouter extends AbstractMessageRouter
MessageChannel channel = resolveChannelForName(channelName, message);
if (channel != null) {
channels.add(channel);
if (!mapped && !(this.dynamicChannels.get(channelName) != null)) {
if (!mapped && this.dynamicChannels.get(channelName) == null) {
this.dynamicChannels.put(channelName, channel);
}
}

14
spring-integration-core/src/main/java/org/springframework/integration/router/ErrorMessageExceptionTypeRouter.java
View File

@@ -17,11 +17,11 @@
package org.springframework.integration.router;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.springframework.jmx.export.annotation.ManagedAttribute;
import org.springframework.jmx.export.annotation.ManagedOperation;
@@ -46,7 +46,7 @@ import org.springframework.util.ClassUtils;
*/
public class ErrorMessageExceptionTypeRouter extends AbstractMappingMessageRouter {
private volatile Map<String, Class<?>> classNameMappings = new ConcurrentHashMap<>();
private volatile Map<String, Class<?>> classNameMappings = new LinkedHashMap<>();
private volatile boolean initialized;
@@ -60,7 +60,7 @@ public class ErrorMessageExceptionTypeRouter extends AbstractMappingMessageRoute
}
private void populateClassNameMapping(Set<String> classNames) {
Map<String, Class<?>> newClassNameMappings = new ConcurrentHashMap<>();
Map<String, Class<?>> newClassNameMappings = new LinkedHashMap<>();
for (String className : classNames) {
newClassNameMappings.put(className, resolveClassFromName(className));
}
@@ -82,7 +82,7 @@ public class ErrorMessageExceptionTypeRouter extends AbstractMappingMessageRoute
public void setChannelMapping(String key, String channelName) {
super.setChannelMapping(key, channelName);
if (this.initialized) {
Map<String, Class<?>> newClassNameMappings = new ConcurrentHashMap<>(this.classNameMappings);
Map<String, Class<?>> newClassNameMappings = new LinkedHashMap<>(this.classNameMappings);
newClassNameMappings.put(key, resolveClassFromName(key));
this.classNameMappings = newClassNameMappings;
}
@@ -92,7 +92,7 @@ public class ErrorMessageExceptionTypeRouter extends AbstractMappingMessageRoute
@ManagedOperation
public void removeChannelMapping(String key) {
super.removeChannelMapping(key);
Map<String, Class<?>> newClassNameMappings = new ConcurrentHashMap<>(this.classNameMappings);
Map<String, Class<?>> newClassNameMappings = new LinkedHashMap<>(this.classNameMappings);
newClassNameMappings.remove(key);
this.classNameMappings = newClassNameMappings;
}
@@ -101,13 +101,13 @@ public class ErrorMessageExceptionTypeRouter extends AbstractMappingMessageRoute
@ManagedOperation
public void replaceChannelMappings(Properties channelMappings) {
super.replaceChannelMappings(channelMappings);
populateClassNameMapping(this.channelMappings.keySet());
populateClassNameMapping(getChannelMappings().keySet());
}
@Override
protected void onInit() {
super.onInit();
populateClassNameMapping(this.channelMappings.keySet());
populateClassNameMapping(getChannelMappings().keySet());
this.initialized = true;
}

19
spring-integration-core/src/main/java/org/springframework/integration/router/PayloadTypeRouter.java
View File

@@ -16,8 +16,8 @@
package org.springframework.integration.router;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import org.springframework.messaging.Message;
@@ -25,12 +25,12 @@ import org.springframework.util.CollectionUtils;
/**
* A Message Router that resolves the {@link org.springframework.messaging.MessageChannel}
* based on the
* {@link Message Message's} payload type.
* based on the {@link Message Message's} payload type.
*
* @author Mark Fisher
* @author Oleg Zhurakousky
* @author Gary Russell
* @author Artem Bilan
*/
public class PayloadTypeRouter extends AbstractMappingMessageRouter {
@@ -47,7 +47,7 @@ public class PayloadTypeRouter extends AbstractMappingMessageRouter {
*/
@Override
protected List<Object> getChannelKeys(Message<?> message) {
if (CollectionUtils.isEmpty(this.channelMappings)) {
if (CollectionUtils.isEmpty(getChannelMappings())) {
return null;
}
Class<?> type = message.getPayload().getClass();
@@ -55,15 +55,14 @@ public class PayloadTypeRouter extends AbstractMappingMessageRouter {
if (isArray) {
type = type.getComponentType();
}
String closestMatch = this.findClosestMatch(type, isArray);
return (closestMatch != null) ? Collections.<Object>singletonList(closestMatch) : null;
String closestMatch = findClosestMatch(type, isArray);
return (closestMatch != null) ? Collections.singletonList(closestMatch) : null;
}
private String findClosestMatch(Class<?> type, boolean isArray) {
int minTypeDiffWeight = Integer.MAX_VALUE;
List<String> matches = new ArrayList<String>();
for (String candidate : this.channelMappings.keySet()) {
List<String> matches = new LinkedList<>();
for (String candidate : getChannelMappings().keySet()) {
if (isArray) {
if (!candidate.endsWith(ARRAY_SUFFIX)) {
continue;
@@ -118,7 +117,7 @@ public class PayloadTypeRouter extends AbstractMappingMessageRouter {
// exhausted hierarchy and found no match
return Integer.MAX_VALUE;
}
return this.determineTypeDifferenceWeight(candidate, type.getSuperclass(), level + 2);
return determineTypeDifferenceWeight(candidate, type.getSuperclass(), level + 2);
}
}

2
spring-integration-core/src/test/java/org/springframework/integration/dsl/routers/RouterTests.java
View File

@@ -743,8 +743,8 @@ public class RouterTests {
public IntegrationFlow exceptionTypeRouteFlow() {
return f -> f
.routeByException(r -> r
.channelMapping(IllegalArgumentException.class, "illegalArgumentChannel")
.channelMapping(RuntimeException.class, "runtimeExceptionChannel")
.channelMapping(IllegalArgumentException.class, "illegalArgumentChannel")
.subFlowMapping(MessageHandlingException.class, sf ->
sf.channel("messageHandlingExceptionChannel"))
.defaultOutputChannel("exceptionRouterDefaultChannel"));

6
spring-integration-core/src/test/java/org/springframework/integration/router/ErrorMessageExceptionTypeRouterTests.java
View File

@@ -79,8 +79,8 @@ public class ErrorMessageExceptionTypeRouterTests {
ErrorMessageExceptionTypeRouter router = new ErrorMessageExceptionTypeRouter();
router.setBeanFactory(this.context);
router.setApplicationContext(this.context);
router.setChannelMapping(IllegalArgumentException.class.getName(), "illegalArgumentChannel");
router.setChannelMapping(RuntimeException.class.getName(), "runtimeExceptionChannel");
router.setChannelMapping(IllegalArgumentException.class.getName(), "illegalArgumentChannel");
router.setChannelMapping(MessageHandlingException.class.getName(), "messageHandlingExceptionChannel");
router.setDefaultOutputChannel(this.defaultChannel);
router.afterPropertiesSet();
@@ -104,7 +104,7 @@ public class ErrorMessageExceptionTypeRouterTests {
router.setBeanFactory(this.context);
router.setApplicationContext(this.context);
router.setChannelMapping(RuntimeException.class.getName(), "runtimeExceptionChannel");
router.setChannelMapping(MessageHandlingException.class.getName(), "runtimeExceptionChannel");
router.setChannelMapping(MessageHandlingException.class.getName(), "messageHandlingExceptionChannel");
router.setDefaultOutputChannel(this.defaultChannel);
router.afterPropertiesSet();
@@ -192,8 +192,8 @@ public class ErrorMessageExceptionTypeRouterTests {
ErrorMessageExceptionTypeRouter router = new ErrorMessageExceptionTypeRouter();
router.setBeanFactory(this.context);
router.setApplicationContext(this.context);
router.setChannelMapping(IllegalArgumentException.class.getName(), "illegalArgumentChannel");
router.setChannelMapping(RuntimeException.class.getName(), "runtimeExceptionChannel");
router.setChannelMapping(IllegalArgumentException.class.getName(), "illegalArgumentChannel");
router.setChannelMapping(MessageHandlingException.class.getName(), "messageHandlingExceptionChannel");
router.setDefaultOutputChannel(this.defaultChannel);
router.afterPropertiesSet();

11
src/reference/asciidoc/router.adoc
View File

@@ -750,13 +750,12 @@ See <<xml-xpath-routing>>.
Spring Integration also provides a special type-based router called `ErrorMessageExceptionTypeRouter` for routing error messages (defined as messages whose `payload` is a `Throwable` instance).
`ErrorMessageExceptionTypeRouter` is similar to the `PayloadTypeRouter`.
In fact, they are almost identical.
The only difference is that, while `PayloadTypeRouter` navigates the instance hierarchy of a payload instance (for example, `payload.getClass().getSuperclass()`) to find the most specific type and channel mappings,
the `ErrorMessageExceptionTypeRouter` navigates the hierarchy of 'exception causes' (for example, `payload.getCause()`)
to find the most specific `Throwable` type or channel mappings and uses `mappingClass.isInstance(cause)` to match the
`cause` to the class or any super class.
The only difference is that, while `PayloadTypeRouter` navigates the instance hierarchy of a payload instance (for example, `payload.getClass().getSuperclass()`) to find the most specific type and channel mappings, the `ErrorMessageExceptionTypeRouter` navigates the hierarchy of 'exception causes' (for example, `payload.getCause()`) to find the most specific `Throwable` type or channel mappings and uses `mappingClass.isInstance(cause)` to match the `cause` to the class or any super class.
NOTE: Since version 4.3 the `ErrorMessageExceptionTypeRouter` loads all mapping classes during the initialization
phase to fail-fast for a `ClassNotFoundException`.
IMPORTANT: The channel mapping order in this case matters.
So, if there is a requirement to get mapping for an `IllegalArgumentException`, but not a `RuntimeException`, the last one must be configured on router first.
NOTE: Since version 4.3 the `ErrorMessageExceptionTypeRouter` loads all mapping classes during the initialization phase to fail-fast for a `ClassNotFoundException`.
The following example shows a sample configuration for `ErrorMessageExceptionTypeRouter`: