Skip to content

Exception match order in ErrorMessageExceptionTypeRouter #2873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
yamadasnn opened this issue Apr 1, 2019 · 1 comment · Fixed by #2877
Closed

Exception match order in ErrorMessageExceptionTypeRouter #2873

yamadasnn opened this issue Apr 1, 2019 · 1 comment · Fixed by #2877
Assignees
@artembilan
Labels
in: core type: enhancement
Milestone
5.2.M1

Comments

@yamadasnn
Copy link

ErrorMessageExceptionTypeRouter uses ConcurrentHashMap internally, and this does not guarantee the order.
Therefore, it is not possible to flexibly route Exception with hierarchy.

link : https://github.com/spring-projects/spring-integration/blob/v5.1.3.RELEASE/spring-integration-core/src/main/java/org/springframework/integration/router/ErrorMessageExceptionTypeRouter.java#L49

For example, when routing NullPointerException and RuntimeException, NullPointerException may be routed to the channel of RuntimeException.
This is the routing that are not intended.

<int:exception-type-router>
  <int:mapping exception-type="java.lang.NullPointerException" channel="channelA" />
  <int:mapping exception-type="java.lang.RuntimeException" channel="channelB" />
</int:exception-type-router>

The following measures are required to solve this problem.

  • Hold mapping information in the form of guaranteeing access order
// example
Map<String,Object> classNameMappings = Collections.synchronizedMap(new LinkedHashMap<String,Object>());
Map<String,Object> classNameMappings =new LinkedHashMap<String,Object>();
  • Determine the mapping in the defined order
@artembilan
Copy link
Member

The ErrorMessageExceptionTypeRouter doesn't modify an internal classNameMappings at all.
We don only an atomic full replacement.
So, it is really safe just use LinkedHashMap internally as is.

@artembilan artembilan self-assigned this Apr 1, 2019
@artembilan artembilan added this to the 5.2.M1 milestone Apr 1, 2019
artembilan added a commit to artembilan/spring-integration that referenced this issue Apr 1, 2019
@artembilan
spring-projectsGH-2873: Preserve mapping order in the router
2b08511 
Fixes spring-projects#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**
@artembilan artembilan mentioned this issue Apr 1, 2019
Merged
artembilan added a commit to artembilan/spring-integration that referenced this issue Apr 1, 2019
@artembilan
spring-projectsGH-2873: Preserve mapping order in the router
5130d51 
Fixes spring-projects#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**
garyrussell pushed a commit that referenced this issue Apr 1, 2019
@artembilan @garyrussell
GH-2873: Preserve mapping order in the router (#2877)
23a73fa 
* GH-2873: Preserve mapping order in the router

Fixes #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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@artembilan artembilan

Labels
in: core type: enhancement
Projects
None yet
Milestone
5.2.M1
Development

Successfully merging a pull request may close this issue.

GH-2873: Preserve mapping order in the router artembilan/spring-integration
2 participants
@artembilan @yamadasnn