-
Notifications
You must be signed in to change notification settings - Fork 571
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
HV-1552 Adding new MinAge Constraint #913
base: main
Are you sure you want to change the base?
Changes from 7 commits
32f6e73
0c450ec
1d34b29
a93a9e5
4e74cd4
edd9954
aafcad2
a46876f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* | ||
* Hibernate Validator, declare and validate application constraints | ||
* | ||
* License: Apache License, Version 2.0 | ||
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
*/ | ||
package org.hibernate.validator.cfg.defs; | ||
|
||
import org.hibernate.validator.cfg.ConstraintDef; | ||
import org.hibernate.validator.constraints.AgeMin; | ||
|
||
import java.time.temporal.ChronoUnit; | ||
|
||
/** | ||
* @author Hillmer Chona | ||
* @since 6.0.8 | ||
*/ | ||
public class AgeMinDef extends ConstraintDef<AgeMinDef, AgeMin> { | ||
|
||
public AgeMinDef() { | ||
super( AgeMin.class ); | ||
} | ||
|
||
public AgeMinDef value(int value) { | ||
addParameter( "value", value ); | ||
return this; | ||
} | ||
|
||
public AgeMinDef unit(ChronoUnit unit) { | ||
addParameter( "unit", unit ); | ||
return this; | ||
} | ||
|
||
public AgeMinDef inclusive(boolean inclusive) { | ||
addParameter( "inclusive", inclusive ); | ||
return this; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
/* | ||
* Hibernate Validator, declare and validate application constraints | ||
* | ||
* License: Apache License, Version 2.0 | ||
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
*/ | ||
package org.hibernate.validator.constraints; | ||
|
||
import javax.validation.Constraint; | ||
import javax.validation.Payload; | ||
import java.lang.annotation.Documented; | ||
import java.lang.annotation.Repeatable; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.Target; | ||
import java.time.temporal.ChronoUnit; | ||
|
||
|
||
import static java.lang.annotation.ElementType.ANNOTATION_TYPE; | ||
import static java.lang.annotation.ElementType.CONSTRUCTOR; | ||
import static java.lang.annotation.ElementType.FIELD; | ||
import static java.lang.annotation.ElementType.METHOD; | ||
import static java.lang.annotation.ElementType.PARAMETER; | ||
import static java.lang.annotation.ElementType.TYPE_USE; | ||
import static java.lang.annotation.RetentionPolicy.RUNTIME; | ||
|
||
/** | ||
* The annotated element must be an instant, date or time for which at least | ||
* the specified amount ({@link AgeMin#value()}) of Years/Days/Months/etc. defined | ||
* by {@link AgeMin#unit()} have passed till now. | ||
* <p> | ||
* Supported types are: | ||
* <ul> | ||
* <li>{@code java.util.Calendar}</li> | ||
* <li>{@code java.util.Date}</li> | ||
* <li>{@code java.time.chrono.HijrahDate}</li> | ||
* <li>{@code java.time.chrono.JapaneseDate}</li> | ||
* <li>{@code java.time.LocalDate}</li> | ||
* <li>{@code java.time.chrono.MinguoDate}</li> | ||
* <li>{@code java.time.chrono.ThaiBuddhistDate}</li> | ||
* <li>{@code java.time.Year}</li> | ||
* <li>{@code java.time.YearMonth}</li> | ||
* </ul> | ||
* <p> | ||
* {@code null} elements are considered valid. | ||
* | ||
* | ||
* @author Hillmer Chona | ||
* @since 6.0.8 | ||
*/ | ||
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE}) | ||
@Retention(RUNTIME) | ||
@Repeatable(AgeMin.List.class) | ||
@Documented | ||
@Constraint(validatedBy = {}) | ||
public @interface AgeMin { | ||
|
||
String message() default "{org.hibernate.validator.constraints.AgeMin.message}"; | ||
|
||
Class<?>[] groups() default {}; | ||
|
||
Class<? extends Payload>[] payload() default {}; | ||
|
||
/** | ||
* @return the age according to unit from a given instant, date or time must be greater or equal to | ||
*/ | ||
int value(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in the previous discussion, there was an idea to try out to support other units ( |
||
|
||
/** | ||
* Specifies the date period unit ( Years/Days/Months/etc. ) that will be used to compare the given instant, | ||
* date or time with the reference value. | ||
* By default, it is ({@link ChronoUnit#YEARS}). | ||
* | ||
* @return the date period unit | ||
*/ | ||
ChronoUnit unit() default ChronoUnit.YEARS; | ||
|
||
/** | ||
* Specifies whether the specified value is inclusive or exclusive. | ||
* By default, it is inclusive. | ||
* | ||
* @return {@code true} if the date period units from a given instant, date or time must be higher or equal to the specified value, | ||
* {@code false} if date period units from a given instant, date or time must be higher | ||
*/ | ||
boolean inclusive() default true; | ||
|
||
/** | ||
* Defines several {@link AgeMin} annotations on the same element. | ||
* | ||
* @see AgeMin | ||
*/ | ||
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE}) | ||
@Retention(RUNTIME) | ||
@Documented | ||
@interface List { | ||
AgeMin[] value(); | ||
} | ||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/* | ||
* Hibernate Validator, declare and validate application constraints | ||
* | ||
* License: Apache License, Version 2.0 | ||
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
*/ | ||
package org.hibernate.validator.internal.constraintvalidators.hv.age; | ||
|
||
import java.lang.annotation.Annotation; | ||
import java.lang.invoke.MethodHandles; | ||
import java.time.Clock; | ||
import java.time.Duration; | ||
import java.time.Instant; | ||
import java.time.LocalDate; | ||
import java.time.ZoneOffset; | ||
import java.time.temporal.ChronoUnit; | ||
|
||
import javax.validation.ConstraintValidatorContext; | ||
|
||
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidator; | ||
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext; | ||
import org.hibernate.validator.internal.util.logging.Log; | ||
import org.hibernate.validator.internal.util.logging.LoggerFactory; | ||
|
||
/** | ||
* Base class for all age validators that use an {@link Instant} to be compared to the age reference. | ||
* | ||
* @author Hillmer Chona | ||
* @since 6.0.8 | ||
*/ | ||
public abstract class AbstractAgeInstantBasedValidator<C extends Annotation, T> | ||
implements HibernateConstraintValidator<C, T> { | ||
|
||
private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() ); | ||
|
||
private Clock referenceClock; | ||
|
||
protected int referenceAge; | ||
|
||
protected boolean inclusive; | ||
|
||
protected ChronoUnit unit; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICS, at least part of these properties can be private now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can add public void initialize(int referenceAge, ChronoUnit unit, boolean inclusive, HibernateConstraintValidatorInitializationContext initializationContext) {
try {
this.referenceClock = Clock.offset(
initializationContext.getClockProvider().getClock(),
getEffectiveTemporalValidationTolerance( initializationContext.getTemporalValidationTolerance() )
);
}
catch (Exception e) {
throw LOG.getUnableToGetCurrentTimeFromClockProvider( e );
}
this.referenceAge = referenceAge;
this.unit = unit;
this.inclusive = inclusive;
} here. This way we will not need to repeat same logic for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to do something like you say, but no good ideas came to me, thanks. Done. |
||
public void initialize( | ||
int referenceAge, | ||
ChronoUnit unit, | ||
boolean inclusive, | ||
HibernateConstraintValidatorInitializationContext initializationContext) { | ||
try { | ||
this.referenceClock = Clock.offset( | ||
initializationContext.getClockProvider().getClock(), | ||
getEffectiveTemporalValidationTolerance( initializationContext.getTemporalValidationTolerance() ) | ||
); | ||
} | ||
catch (Exception e) { | ||
throw LOG.getUnableToGetCurrentTimeFromClockProvider( e ); | ||
} | ||
this.referenceAge = referenceAge; | ||
this.unit = unit; | ||
this.inclusive = inclusive; | ||
} | ||
|
||
@Override | ||
public boolean isValid(T value, ConstraintValidatorContext context) { | ||
// null values are valid | ||
if ( value == null ) { | ||
return true; | ||
} | ||
// As Instant does not support plus operation on ChronoUnits greater than DAYS we need to convert it to LocalDate | ||
// first, which supports such operations. | ||
|
||
long result = getInstant( value ).atZone( ZoneOffset.ofHours( 0 ) ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as mentioned in another comment, I think we should stick to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marko-bekhta for InstanteBase I don't see any problem using either LocalDate or LocalDateTime, but it is necessary convert to LocalDate before comparing Calendar, the reason is that Calendar has time, even though if you were born for example at 10:00 pm your new age starts from 00:00 AM.
For TimeBased, I got an A possible solution can be to create a method LocalDateTime getReferenceValue(T value, Clock reference), and for Year return the year that comes on value plus January 1st 00:00 AM, For LocalDate the date that comes on value plus 00:00 AM. I mean they only return a LocalDateTime, adding the values (date and/or time) to make the T value consistent . But I'm not sure about it. Can you see other solution? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Hilmerc that's a very good point on the time part of birthday and how we calculate the age. Thinking more about it, it might be that we wanted to do to much with one constraint and we might slightly moved away from your original intention for it. Seems that there are next two ways how it can move forward from this point:
Personally, I'm leaning more towards (1), as it will be a more versatile constraint. If you like this approach and would want to give it a try we'd need to add this additional attribute to a constraint and then use class AbstractAgeValidator {
....
public void initialize(
int referenceAge,
ChronoUnit unit,
boolean inclusive,
HibernateConstraintValidatorInitializationContext initializationContext) {
....
}
@Override
public boolean isValid(T value, ConstraintValidatorContext context) {
....
}
protected abstract LocalDateTime getLocalDateTime(T value);
....
}
class AbstractAgeInstantBasedValidator extends AbstractAgeValidator {
protected abstract Instant getInstant(T value);
@Override
protected LocalDateTime getLocalDateTime(T value){
return LocalDateTime.ofInstant(getInstant(value));
}
} and for types like Also, just as a note, in any of these cases, we would need to write up a better explanation of what the constraint is checking and how it will behave. Just to set the users expectations right :) But don't think about this part yet. That's something for later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marko-bekhta after some work and tests I figured out that the attribute ChronoUnit#truncateTo() can generate some problems like ChronoUnit#Unit.
I think we should eliminate the ability to specify a unit, make validators with years and, after use and feedback, we can explore the possibility of returning to our idea of using Units. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Hilmerc thanks a lot for experimenting with this approach. So it seems, we wanted to much from one constraint. Let's remove the units then and clean things up to keep this constraint just for age in years. I still think that the above idea with one abstract validator for all types would work (Just use |
||
.toLocalDate() | ||
.plus( referenceAge, unit ) | ||
.compareTo( | ||
LocalDate.now( referenceClock ) ); | ||
|
||
return isValid( result ); | ||
} | ||
|
||
/** | ||
* Returns the temporal validation tolerance to apply. | ||
*/ | ||
protected abstract Duration getEffectiveTemporalValidationTolerance(Duration absoluteTemporalValidationTolerance); | ||
|
||
/** | ||
* Returns the {@link Instant} measured from Epoch. | ||
*/ | ||
protected abstract Instant getInstant(T value); | ||
|
||
/** | ||
* Returns whether the result of the comparison between the validated value and the reference age is considered | ||
* valid. | ||
*/ | ||
protected abstract boolean isValid(long result); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/* | ||
* Hibernate Validator, declare and validate application constraints | ||
* | ||
* License: Apache License, Version 2.0 | ||
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
*/ | ||
package org.hibernate.validator.internal.constraintvalidators.hv.age; | ||
|
||
import java.lang.annotation.Annotation; | ||
import java.lang.invoke.MethodHandles; | ||
import java.time.Clock; | ||
import java.time.Duration; | ||
import java.time.temporal.ChronoUnit; | ||
import java.time.temporal.TemporalAccessor; | ||
|
||
import javax.validation.ClockProvider; | ||
import javax.validation.ConstraintValidatorContext; | ||
|
||
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidator; | ||
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext; | ||
import org.hibernate.validator.internal.util.logging.Log; | ||
import org.hibernate.validator.internal.util.logging.LoggerFactory; | ||
|
||
/** | ||
* Base class for all age validators that are based on the {@code java.time} package. | ||
* | ||
* @author Hillmer Chona | ||
* @since 6.0.8 | ||
*/ | ||
public abstract class AbstractAgeTimeBasedValidator<C extends Annotation, T extends TemporalAccessor & Comparable<? super T>> | ||
implements HibernateConstraintValidator<C, T> { | ||
|
||
private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() ); | ||
|
||
private Clock referenceClock; | ||
|
||
protected int referenceAge; | ||
|
||
protected boolean inclusive; | ||
|
||
protected ChronoUnit unit; | ||
|
||
public void initialize(int referenceAge, ChronoUnit unit, boolean inclusive, HibernateConstraintValidatorInitializationContext initializationContext) { | ||
try { | ||
this.referenceClock = Clock.offset( | ||
initializationContext.getClockProvider().getClock(), | ||
getEffectiveTemporalValidationTolerance( initializationContext.getTemporalValidationTolerance() ) | ||
); | ||
} | ||
catch (Exception e) { | ||
throw LOG.getUnableToGetCurrentTimeFromClockProvider( e ); | ||
} | ||
this.referenceAge = referenceAge; | ||
this.unit = unit; | ||
this.inclusive = inclusive; | ||
} | ||
|
||
@Override | ||
public boolean isValid(T value, ConstraintValidatorContext context) { | ||
// null values are valid | ||
if ( value == null ) { | ||
return true; | ||
} | ||
|
||
int result = value.compareTo( getReferenceValue( referenceClock, referenceAge, unit ) ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe better to follow the same pattern as in the instant validatior ? I mean do the operation once here in the validator rather than do the minus in each type's impl. Also I've noticed that you've used the minus for these |
||
|
||
return isValid( result ); | ||
} | ||
|
||
/** | ||
* Returns the temporal validation tolerance to apply. | ||
*/ | ||
protected abstract Duration getEffectiveTemporalValidationTolerance(Duration absoluteTemporalValidationTolerance); | ||
|
||
/** | ||
* Returns an object of the validated type corresponding to the time reference as provided by the | ||
* {@link ClockProvider} increased or decreased with the specified referenceAge of Years/Days/Months/etc. | ||
* defined by {@link ChronoUnit}. | ||
*/ | ||
protected abstract T getReferenceValue(Clock reference, int referenceAge, ChronoUnit unit ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thinking more about this method, it looks like we will have the same problem with add/minus operations as mentioned earlier. Let's assume we have a class Foo {
@AgeMin(value = 60, unit = ChronoUnit.HOURS)
private LocalDate date;
} it will result in |
||
|
||
/** | ||
* Returns whether the result of the comparison between the validated value and the age reference is considered | ||
* valid. | ||
*/ | ||
protected abstract boolean isValid(long result); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Hibernate Validator, declare and validate application constraints | ||
* | ||
* License: Apache License, Version 2.0 | ||
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>. | ||
*/ | ||
package org.hibernate.validator.internal.constraintvalidators.hv.age.min; | ||
|
||
import java.time.Duration; | ||
import java.time.Instant; | ||
|
||
import javax.validation.metadata.ConstraintDescriptor; | ||
|
||
import org.hibernate.validator.constraints.AgeMin; | ||
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext; | ||
import org.hibernate.validator.internal.constraintvalidators.hv.age.AbstractAgeInstantBasedValidator; | ||
|
||
|
||
/** | ||
* Base class for all {@code @AgeMin} validators that use an {@link Instant} to be compared to the age reference. | ||
* | ||
* @author Hillmer Chona | ||
* @since 6.0.8 | ||
*/ | ||
public abstract class AbstractAgeMinInstantBasedValidator<T> extends AbstractAgeInstantBasedValidator<AgeMin, T> { | ||
|
||
@Override | ||
public void initialize(ConstraintDescriptor<AgeMin> constraintDescriptor, HibernateConstraintValidatorInitializationContext initializationContext) { | ||
super.initialize( constraintDescriptor.getAnnotation().value(), constraintDescriptor.getAnnotation().unit(), | ||
constraintDescriptor.getAnnotation().inclusive(), initializationContext ); | ||
} | ||
|
||
@Override | ||
protected Duration getEffectiveTemporalValidationTolerance(Duration absoluteTemporalValidationTolerance) { | ||
return absoluteTemporalValidationTolerance; | ||
} | ||
|
||
@Override | ||
protected boolean isValid(long result) { | ||
return this.inclusive ? result <= 0 : result < 0; | ||
} | ||
} |
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.
Needs a license header comment as in the other classes
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.
done
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.
Hi @marko-bekhta I have done all your suggestions except the one for ChronoUnit attribute. I'm not sure what you mean. Is it something like this: add to
@interface AgeMin
a attribute likeChronoUnit unit();
, so users can define the value when use the annotation like this:@AgeMin( value = MINIMUM_AGE , inclusive = true, unit= ChronoUnit.YEARS)
or
@AgeMin( value = MINIMUM_AGE , inclusive = true, unit= ChronoUnit.MONTHS)
?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.
Hi @Hilmerc yes, that's exactly it! :) This will make the constraint more versatile.
I've also prepared a short plan, with items that are still needed to finish this work. I'll post it in the separate comment. I hope it'll be helpful.