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

Add validation annotations to CleanupConfig #302

Closed
sleberknight opened this issue Oct 24, 2023 · 1 comment · Fixed by #303
Closed

Add validation annotations to CleanupConfig #302

sleberknight opened this issue Oct 24, 2023 · 1 comment · Fixed by #303
Assignees
Labels
enhancement A request for change or improvement to an existing feature
Milestone

Comments

@sleberknight
Copy link
Member

Add Jakarta Bean Validation annotations to the CleanupConfig class to permit validation, for example if an application uses external configuration to specify the configuration to use when cleaning up application errors.

This does not require adding a new dependency, since this library already depends (transitively) on jakarta.validation:jakarta.validation-api. However, it does now make it an explicit dependency, so we should probably make it explicit in the POM.

@sleberknight sleberknight added the enhancement A request for change or improvement to an existing feature label Oct 24, 2023
@sleberknight sleberknight added this to the 2.1.0 milestone Oct 24, 2023
@sleberknight sleberknight self-assigned this Oct 24, 2023
@sleberknight
Copy link
Member Author

sleberknight commented Oct 24, 2023

Since we're using Dropwizard's Duration class (io.dropwizard.util.Duration) in this configuration class, it makes sense to not only use standard validation Jakarta Bean Validation annotations like @NotNull but also to use Dropwizard's own custom validation annotations, in this case to use @MinDuration on the Duration fields. Since we mainly want to ensure that the duration isn't zero (or even negative), we can make the minimum as 1 minute for the fields. That probably isn't very useful in real life for most applications, but it ensures there is at least a valid minimum. I don't think we should set any maximum durations, since maybe someone wants to save them for 1 year, 2 years, etc. or even effectively forever by using a very large number of days.

So, we'll also want to add dropwizard-validation as an explicit dependency. Currently it is a transitive dependency via dropwizard-core.

sleberknight added a commit that referenced this issue Oct 25, 2023
* Update the version in the POM to 2.1.0-SNAPSHOT
* Add dropwizard-validation and jakarta.validation-api as explicit
  dependencies, since we're using both standard Jakarta annotations
  and a Dropwizard one
* Make all references types @NotNull in CleanupConfig
* Make all Duration types @MinDuration of 1 minute
* Add a bunch of tests, and enhance the existing test of default values

Closes #302
chrisrohr pushed a commit that referenced this issue Oct 26, 2023
* Update the version in the POM to 2.1.0-SNAPSHOT
* Add dropwizard-validation and jakarta.validation-api as explicit
  dependencies, since we're using both standard Jakarta annotations
  and a Dropwizard one
* Make all references types @NotNull in CleanupConfig
* Make all Duration types @MinDuration of 1 minute
* Add a bunch of tests, and enhance the existing test of default values

Closes #302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A request for change or improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant