Skip to content

Allow dynamic constraint validation messages#2246

Merged
arteam merged 1 commit intodropwizard:masterfrom
nickbabcock:cache-prefix
Jan 18, 2018
Merged

Allow dynamic constraint validation messages#2246
arteam merged 1 commit intodropwizard:masterfrom
nickbabcock:cache-prefix

Conversation

@nickbabcock
Copy link
Copy Markdown
Contributor

Previously, once a validation constraint was evaluated the entire string
was cached. The impetus for a cache is that reflection is used to
determine if we need to prefix an error message with additional context
(eg. "query parameter x ..."). Repeatedly calculating this prefix is
wasteful -- thus the cache was born.

The biggest issue with this cache is that while it worked with all
Hibernate Validation default message templates, it failed for message
templates that contained the invalid value (eg. "query parameter x
({validatedValue}) is greater than {value}"), as the same error would be
returned regardless of validatedValue. It would also fail for custom
constraint validators that didn't return the same message everytime.

The fix is to switch the cache from caching the entire error message to
just the error prefix (eg. "query parameter x", "return value", no
prefix).

This has no effect on performance and will allow for more
custom validation messages in Dropwizard.

Closes #2245
Ref #2090
CC'ing users who expressed interest: @imochurad @rtti @peterklijn

Previously, once a validation constraint was evaluated the entire string
was cached. The impetus for a cache is that reflection is used to
determine if we need to prefix an error message with additional context
(eg. "query parameter x ..."). Repeatedly calculating this prefix is
wasteful -- thus the cache was born.

The biggest issue with this cache is that while it worked with all
Hibernate Validation default message templates, it failed for message
templates that contained the invalid value (eg. "query parameter x
({validatedValue}) is greater than {value}"), as the same error would be
returned regardless of validatedValue. It would also fail for custom
constraint validators that didn't return the same message everytime.

The fix is to switch the cache from caching the entire error message to
just the error prefix (eg. "query parameter x", "return value", no
prefix).

This has no effect on performance and will allow custom validation
messages in Dropwizard.
@arteam arteam merged commit 2c1609b into dropwizard:master Jan 18, 2018
@arteam
Copy link
Copy Markdown
Member

arteam commented Jan 18, 2018

LGTM!

@arteam arteam added this to the 1.3.0 milestone Jan 18, 2018
@arteam arteam added the bug label Jan 18, 2018
arteam pushed a commit that referenced this pull request Jan 18, 2018
Previously, once a validation constraint was evaluated the entire string
was cached. The impetus for a cache is that reflection is used to
determine if we need to prefix an error message with additional context
(eg. "query parameter x ..."). Repeatedly calculating this prefix is
wasteful -- thus the cache was born.

The biggest issue with this cache is that while it worked with all
Hibernate Validation default message templates, it failed for message
templates that contained the invalid value (eg. "query parameter x
({validatedValue}) is greater than {value}"), as the same error would be
returned regardless of validatedValue. It would also fail for custom
constraint validators that didn't return the same message everytime.

The fix is to switch the cache from caching the entire error message to
just the error prefix (eg. "query parameter x", "return value", no
prefix).

This has no effect on performance and will allow custom validation
messages in Dropwizard.

(cherry picked from commit 2c1609b)
@arteam
Copy link
Copy Markdown
Member

arteam commented Jan 18, 2018

Applied to 1.2.x as 76ebbb0

@nickbabcock
Copy link
Copy Markdown
Contributor Author

Hey @arteam, I realize now that the PR title "Constraint validation caches error prefixes" may not give dropwizard users an apt description of what changes for them. Maybe something along the lines of "Allow dynamic constraint validation messages" should be in the release notes / new title of this PR.

@arteam
Copy link
Copy Markdown
Member

arteam commented Jan 19, 2018

Sounds good. I've changed the release notes for 1.3.0-rc4 and the name of the issue.

@arteam arteam changed the title Constraint validation caches error prefixes Allow dynamic constraint validation messages Jan 19, 2018
aaanders pushed a commit to aaanders/dropwizard that referenced this pull request Sep 20, 2018
Previously, once a validation constraint was evaluated the entire string
was cached. The impetus for a cache is that reflection is used to
determine if we need to prefix an error message with additional context
(eg. "query parameter x ..."). Repeatedly calculating this prefix is
wasteful -- thus the cache was born.

The biggest issue with this cache is that while it worked with all
Hibernate Validation default message templates, it failed for message
templates that contained the invalid value (eg. "query parameter x
({validatedValue}) is greater than {value}"), as the same error would be
returned regardless of validatedValue. It would also fail for custom
constraint validators that didn't return the same message everytime.

The fix is to switch the cache from caching the entire error message to
just the error prefix (eg. "query parameter x", "return value", no
prefix).

This has no effect on performance and will allow custom validation
messages in Dropwizard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom messages in constraint validators are incorrectly cached

2 participants