Skip to content
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.

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

Example validation annotations #3082

Merged
merged 6 commits into from Jan 5, 2020
Merged

Example validation annotations #3082

merged 6 commits into from Jan 5, 2020

Conversation

HarryEH
Copy link
Contributor

@HarryEH HarryEH commented Dec 25, 2019

#3053

Problem:

As described in the Issue #3053, the Saying validation annotation doesn't entirely make sense.

Solution:

I have removed the @Max annotation from the Saying class and removed the @Valid annotation from the resource. To still show the annotation in use I have modified the Person class in core and the people resource to reflect that. I have also added a test to show the validation in action on the resource.

Result:

I know adding age to a person isn't a particularly sensible thing to do normally, but it is a good way to show basic use of these annotations in action. Feedback on the test would also be appreciated, without an exception mapper I'm not sure what to test for - other than what I have implemented.

Copy link

@acsbendi acsbendi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good overall, some minor things:

  • if you'd like to make the example a bit more sensible, perhaps you could use a year of birth field instead of age. @Max and @Min would work with that too.
  • the documentation also needs to be updated, the line "Lastly, the bean leverages validation to ensure the content size is no greater than 3." should be removed from here.

@HarryEH
Copy link
Contributor Author

HarryEH commented Dec 30, 2019

Seems good overall, some minor things:

  • if you'd like to make the example a bit more sensible, perhaps you could use a year of birth field instead of age. @Max and @Min would work with that too.
  • the documentation also needs to be updated, the line "Lastly, the bean leverages validation to ensure the content size is no greater than 3." should be removed from here.

Thanks!! I have made the changes suggested, and updated the documentation 😄. Also added a test for the @Max(value = 9999).

@jplock jplock merged commit 46ae469 into dropwizard:master Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants