-
Notifications
You must be signed in to change notification settings - Fork 155
#1164 Added Mapping Conflict Check to Spring WebMvc Configuration #1165
base: master
Are you sure you want to change the base?
Conversation
/** | ||
* Fix for https://github.com/stormpath/stormpath-sdk-java/issues/1164 | ||
* | ||
* @since 1.2.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change this to 1.3.0?
* | ||
* @since 1.2.3 | ||
*/ | ||
private <T extends AbstractController> void assertUniqueMethodMapping(T c) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat with IntelliJ's default settings (e.g. space after for
, space after if
, before brace, etc). Thanks!
throw new IllegalStateException("Mapping conflict. Stormpath cannot map '" + c.getUri() + "'. " + | ||
"There is already '" + handlerMethod.getBean() + | ||
"' bean method\n" + handlerMethod + " mapped."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this to read: `Mapping confict: Stormpath cannot map '" + c.getUri() + "'. " + handlerMethod.getBean() + "#" + handlerMethod + " is already mapped to this URI."
@mraible The changes you've requested are done. Thanks. |
@kanderson450 Cool - thanks! LGTM 👍 Will merge after CI passes. |
This PR fixes #1164 by adding a method that checks whether any
Mappings
defined by the User conflict with those that Stormpath wants to create. If a conflict is found, anIllegalStateException
is thrown which adheres to the standard Spring behavior.