HV-1552 Adding new MinAge Constraint #913

HillmerCh · 2018-01-16T22:23:01Z

This PR has the changes suggested for you

marko-bekhta · 2018-01-22T20:24:21Z

Hi @Hilmerc ! Sorry for the late reply. It looks we are on the right track with this one. I've noticed that the build failed : http://ci.hibernate.org/job/hibernate-validator-PR/1042/org.hibernate.validator$hibernate-validator/console
and it looks like you haven't included a programatic definition file:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /var/lib/jenkins/workspace/hibernate-validator-PR/engine/src/test/java/org/hibernate/validator/test/internal/constraintvalidators/hv/AgeValidatorTest.java:[26,40] cannot find symbol
  symbol:   class AgeMinDef
  location: package org.hibernate.validator.cfg.defs
[INFO] 1 error

Could you push it as well (and any other changes if there are such 😉 )?

Also it's OK to keep the PRs opened and add new stuff to them. This way it's easier to track the changes and have discussions in one place.

HillmerCh · 2018-01-23T21:24:03Z

Hi @marko-bekhta , you right it was my mistake, I have pulled AgeMinDef, Could you check please?

marko-bekhta

Hi @Hilmerc thanks for updated PR! I've added a few small comments inline. Would be great if you could try to add ChronoUnit attribute. If that works out fine, then I'd say we could add support for other date/time classes for which this constraint would make sense. Having that in place we could move forward and add the @AgeMax constraint. And also look into some other things like documentation and Annotation Processor support. If you have any questions - please ask.

marko-bekhta · 2018-01-24T18:25:50Z

engine/src/main/java/org/hibernate/validator/cfg/defs/AgeMinDef.java

@@ -0,0 +1,21 @@
+package org.hibernate.validator.cfg.defs;


Needs a license header comment as in the other classes

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 like ChronoUnit 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) ?

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.

marko-bekhta · 2018-01-24T18:32:59Z

engine/src/main/java/org/hibernate/validator/constraints/AgeMin.java

+	/**
+	 * @return value the age in years from a given date must be greater or equal to
+	 */
+	int value();


I think in the previous discussion, there was an idea to try out to support other units (ChronoUnit) with the ChronoUnit#YEARS by default. Could you please try to experiment with that ? Should be simple to add something like ChronoUnit unit() default ChronoUnit.YEARS; to the constraint. If it works the programmatic definition (AgeMinDef) would need this attribute to be added as well.

marko-bekhta · 2018-01-24T18:37:54Z

...est/java/org/hibernate/validator/test/internal/constraintvalidators/hv/AgeValidatorTest.java

+@TestForIssue(jiraKey = "HV-1552" )
+public class AgeValidatorTest {
+
+	private int value = 18;


Maybe make a constant of it and use in other occurrences of this number as well. WDYT?

marko-bekhta · 2018-01-24T18:39:43Z

...est/java/org/hibernate/validator/test/internal/constraintvalidators/hv/AgeValidatorTest.java

+	/**
+	 * @return an initialized {@link ConstraintValidator} using {@code DUMMY_CONSTRAINT_VALIDATOR_INITIALIZATION_CONTEXT}
+	 */
+	private ConstraintValidator<AgeMin, LocalDate> getInitializedValidator(int value, boolean inclusive) {


Usually we place private methods at the end of the test class and start with the tests. It's easier to see what is tested this way.

marko-bekhta · 2018-01-24T18:45:51Z

...est/java/org/hibernate/validator/test/internal/constraintvalidators/hv/AgeValidatorTest.java

+ * @since 6.0.8
+ */
+@TestForIssue(jiraKey = "HV-1552" )
+public class AgeValidatorTest {


These set of tests would check if AgeMinValidator works as expected as well as the programmatic definition. Could you also add another test class similar for example to this one - https://github.com/hibernate/hibernate-validator/blob/master/engine/src/test/java/org/hibernate/validator/test/constraints/annotations/hv/ISBNConstrainedTest.java
We add such tests in that package to make sure that the constraint and corresponding validators are correctly registered and also that the default message is present.

marko-bekhta · 2018-01-30T09:22:44Z

@Hilmerc so about the plan that I've mentioned...
It looks we are somewhere between first and second steps of adding a new constraint. Here's a more detailed version of what I think we should do next:

Before we go further we need to get the constraints and their parameters right. This way it'll be easier and will require less changes in the next steps.
1.1. We need to add unit attribute to @AgeMin (ChronoUnit unit() default ChronoUnit.YEARS;)
1.2. Update existing validator's logic to use this new attribute in both initialization and validation.
1.3. Pull out initialization and common validation logic from existing validator into abstract/base one, which will be used as a base to implement validators for other date/time types. (it'll be somehow similar to what we already have for @Past/@Future validators).
1.4. Add implementation for data types we would like this constraint to support. I guess we could have the same list of types that we have for Past/Future constraints (JSR-310 types, java.util Calendar/Date ...).
1.5. Having this in place it should be easy to add @AgeMax constraint following the same steps.
1.6. We would need to add a few more tests for other supported data types.
1.7. Also as mentioned in the other comments, we will also need to add a test extending form ConstrainedTest for both new constraints.
Based on the above work we could update programmatic constraint definition (AgeMinDef) and add the corresponding AgeMaxDef for @AgeMax.
After we are done with the constraints, and are confident that all looks good, we can move forward and add Annotation Processor support. It's really easy to do. Just need to add two lines, one for a constraint, to TypeNames.HibernateValidatorTypes and register them in ConstraintHelper.
Then we should also mention new constraints in documentation. In our case it should be easy to do. Just need to add something like:

`@AgeMin`:: Checks that the annotated date/time property is at least `value` `unit`s old. `unit` determines the type of `ChronoUnit` to be used to compare age. The default is ChronoUnit.YEAR.
    Supported data types::: `CharSequence`
    Hibernate metadata impact::: None

We can come up with a better wording for documentation when we reach this step.

What do you think of such plan? Please let us know if you need more details or any help with any of these steps.

HillmerCh · 2018-01-31T11:36:08Z

@marko-bekhta I am working with the plan, for the moment no doubts

HillmerCh · 2018-02-08T17:41:21Z

@marko-bekhta I have finished the step number 1, abstract classes, and implementation for LocalDate and Calendar, before I follow next step, Could you look over the classes?
Especially I want to hear your thoughts about AgeMinValidatorForCalendar.

Thanks

marko-bekhta

Hi @Hilmerc good progress! I've added a couple of comments here and there, those license headers are easy to miss :). Looks good in general. I think we can try to improve those validator classes a bit more and pull the data manipulation in the abstract ones to prevent repetition. Let me know what you think of that.

marko-bekhta · 2018-02-08T23:23:22Z

engine/src/main/java/org/hibernate/validator/constraints/AgeMin.java

+	Class<? extends Payload>[] payload() default {};
+
+	/**
+	 * @return value the referenceAge according to unit from a given date must be greater or equal to


value and referenceAge seems to be a duplication. Maybe something like:
the amount of date period units, defined by {@link AgeMin#unit()}, have passed since the annotated elementdate till now.

marko-bekhta · 2018-02-08T23:35:51Z

engine/src/main/java/org/hibernate/validator/constraints/AgeMin.java

+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+/**
+ * The annotated element must be a date where the number of Years, Days, Months, etc. according


Let's make it more similar to other constraint descriptions.

* 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.time.LocalDate}</li> * <li> {@code add any other types that will be supported} </li> * </ul> * <p> * {@code null} elements are considered valid.

how about something like this?

Perfect. Done.

marko-bekhta · 2018-02-08T23:38:21Z

engine/src/main/java/org/hibernate/validator/constraints/AgeMin.java

+	 * By default, it is inclusive.
+	 *
+	 * @return {@code true} if the number of years from a given date must be higher or equal to the specified value,
+	 *         {@code false} if the number of years from a given date must be higher


years -> date period units ?

marko-bekhta · 2018-02-08T23:39:43Z

engine/src/main/java/org/hibernate/validator/constraints/AgeMin.java

+	 * with the reference value.
+	 * By default, it is YEARS.
+	 *
+	 * @return unit the date period unit


the date period unit should be enough

marko-bekhta · 2018-02-08T23:44:20Z

...bernate/validator/internal/constraintvalidators/hv/age/AbstractAgeInstantBasedValidator.java

+	protected boolean inclusive;
+
+	protected ChronoUnit unit;
+


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 referenceClock in all other validators.

I tried to do something like you say, but no good ideas came to me, thanks. Done.

marko-bekhta · 2018-02-08T23:50:34Z

...ate/validator/internal/constraintvalidators/hv/age/min/AbstractAgeMinTimeBasedValidator.java

+	@Override
+	public void initialize(ConstraintDescriptor<AgeMin> constraintDescriptor, HibernateConstraintValidatorInitializationContext initializationContext) {
+		try {
+			super.referenceClock  = Clock.offset(


same could be done here with the parent initialize method

marko-bekhta · 2018-02-09T00:07:26Z

...bernate/validator/internal/constraintvalidators/hv/age/AbstractAgeInstantBasedValidator.java

+			return true;
+		}
+
+		long result =  this.getCurrentAge( value ) - this.referenceAge;


Let's slightly change this isValid method so we wouldn't need to write the similar code that's right now in Calendar validator for the Date one. As instants does not support add/subtract operations on units > days we need to do something similar to what is already in AgeMinValidatorForCalendar. Maybe something like:

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 LocalDateTime // first, which supports such operations. long result = LocalDateTime.ofInstant( getInstant( value ), ZoneOffset.ofHours( 0 ) ).plus( referenceAge, unit ).compareTo( LocalDateTime.now( referenceClock ) ); return isValid( result ); }

with such change you'd need to change the corresponding isValid method to:

@Override protected boolean isValid(long result) { return this.inclusive ? result <= 0 : result < 0; }

Then for Date you would only need to convert it to instant.

@marko-bekhta the method LocalDate.ofInstant( ... ) comes on Java 9, I'm no sure about to use Java 9 features. I use next line than is compatible with Java 8

getInstant( value ).atZone( ZoneOffset.ofHours( 0 ) ).toLocalDate()

WDYT?

Right... We don't want Java 9 stuff :). I was suggesting to use LocalDateTime which should allow to use any of ChronoUnits . Otherwise we would have the ability to do something like:

class MyClass { @AgeMin( value = 24 * 60, unit = ChronoUnit.MINUTES ) private LocalDate date; //..... }

but the add operation would fail for such unit on LocalDate. Also I guess it will answers your other question about supported types. I think this constraint can have a more wider meaning. We could think of it as a constraint on how much time units have passed till now. And would allow having something like:

class Exam { @AgeMin(value = 5, unit = ChronoUnit.MINUTES, message = "You should at least spent 5 minutes on the exam, before you can submit it") @AgeMax(value = 60, unit = ChronoUnit.MINUTES, message = "You cannot submit your exam as the allowed time limit have been exceeded ") private LocalTime examTime; // .... }

or something like:

class Token { @AgeMax(value = 10, unit = ChronoUnit.MINUTES, message = "Token expired") private LocalDateTime issuedAt; // .... }

marko-bekhta · 2018-02-09T07:17:43Z

...hibernate/validator/internal/constraintvalidators/hv/age/min/AgeMinValidatorForCalendar.java

@@ -0,0 +1,32 @@
+package org.hibernate.validator.internal.constraintvalidators.hv.age.min;


license header is missing

marko-bekhta · 2018-02-09T07:17:55Z

...ibernate/validator/internal/constraintvalidators/hv/age/min/AgeMinValidatorForLocalDate.java

@@ -0,0 +1,25 @@
+package org.hibernate.validator.internal.constraintvalidators.hv.age.min;


license header is missing

marko-bekhta · 2018-02-09T07:20:22Z

...ibernate/validator/internal/constraintvalidators/hv/age/min/AgeMinValidatorForLocalDate.java

+
+	@Override
+	protected long getCurrentAge(LocalDate value) {
+		return super.unit.between( value, getReferenceValue( super.referenceClock ) );


It'll be good to pull this logic into abstract validator, so we wouldn't need to repeat it for other date/time types. Maybe something similar to what was mentioned for Instant based validator ?

HillmerCh · 2018-02-11T15:33:06Z

@marko-bekhta I think I have finished with AgeMin constraints. Could you look over one more time please, before I start with MaxAge.

I have an doubt about which date types support: As this constraint works with a dates I think that support for time types as LocalTime is not necessary.

marko-bekhta

hi @Hilmerc ! I think we are almost there. Mainly there's only one thing that bugs me - those date-time plus/minus operations which potentially could throw exceptions if the unit is "wrong" for the constrained type. Could you try using LocalDateTime as mentioned in the other comment, and see how it would work ?

marko-bekhta · 2018-02-11T17:17:41Z

...org/hibernate/validator/test/constraints/annotations/hv/age/AgeValidatorConstrainedTest.java

+import static org.hibernate.validator.testutil.ConstraintViolationAssert.violationOf;
+
+import java.time.LocalDate;
+import java.time.temporal.ChronoUnit;


seems to be unused import - http://ci.hibernate.org/job/hibernate-validator-PR/1062/console

marko-bekhta · 2018-02-11T17:22:56Z

...bernate/validator/internal/constraintvalidators/hv/age/AbstractAgeInstantBasedValidator.java

+		// 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 ) )


as mentioned in another comment, I think we should stick to LocalDateTime so there will be no errors for different units. In case we use LocalDate here UnsupportedTemporalTypeException will be thrown for minutes, hours and other time units.

@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.

long result = getInstant( value ).atZone( ZoneOffset.ofHours( 0 ) ) .toLocalDateTime().plus( referenceAge, unit ).toLocalDate() .compareTo( LocalDateTime.now( referenceClock).toLocalDate() );

For TimeBased, I got an UnsupportedTemporalTypeException yesterday with Year, as you say with LocalDate.

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?

@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:

As we already have the unit in place and it is possible to specify age for example in minutes it wouldn't make sense to truncate all that data up to days which is suitable for calculating the age of a person. Hence if we want to keep unit attribute in place and also have a correct logic for calculating an age of a person we would need to add another attribute to this constraint. Something like ChronoUnit truncateTo() default ChronoUnit.NANOS; a time unit to which we want to truncate the date-time. In such case if we want an age of a person we would use truncateTo=ChronoUnit.DAYS. And if we would like to keep the time part we would leave it as default.

Ignore the idea of having the ability to specify units for an age and keep this particular constraint as simple as an age of a person. But as it looks to be an interesting and useful case to be able to specify constraints on how much time have passed think of a different constraint for it.

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 LocalDateTime#truncatedTo(TemporalUnit) in validators. Also as we would only operate with LocalDateTime instances in the validator to prevent exceptions on plus/minus operations maybe it will actually make sense to have a single base validator for all types? We could then modify the current instant one to something like:

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 LocalDate and others we could just use methods on them to get to LocalDateTime and return directly from getLocalDateTime method.

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.

@marko-bekhta after some work and tests I figured out that the attribute ChronoUnit#truncateTo() can generate some problems like ChronoUnit#Unit.

jshell> LocalDateTime.now( ZoneOffset.UTC ).truncatedTo(ChronoUnit.YEARS); | java.time.temporal.UnsupportedTemporalTypeException thrown: Unit is too large to be used for truncation jshell> LocalDateTime.now( ZoneOffset.UTC ).truncatedTo(ChronoUnit.WEEKS); | java.time.temporal.UnsupportedTemporalTypeException thrown: Unit is too large to be used for truncation

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?

@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 LocalDate instead of LocalDateTime).

marko-bekhta · 2018-02-11T17:31:27Z

.../hibernate/validator/internal/constraintvalidators/hv/age/AbstractAgeTimeBasedValidator.java

+			return true;
+		}
+
+		int result = value.compareTo( getReferenceValue( referenceClock, referenceAge, unit ) );


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 TemporalAccessor types and for Instant there's add. Would be better to use the same one in both, both are fine just pick one :)

marko-bekhta · 2018-02-11T17:39:51Z

.../hibernate/validator/internal/constraintvalidators/hv/age/AbstractAgeTimeBasedValidator.java

+	 * {@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 );


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 LocalDate as below:

class Foo { @AgeMin(value = 60, unit = ChronoUnit.HOURS) private LocalDate date; }

it will result in UnsupportedTemporalTypeException when we try to perform either plus or minus operations in the validator. So I guess we need to work with LocalDateTime all the time?

gsmet · 2018-02-12T10:29:53Z

...bernate/validator/internal/constraintvalidators/hv/age/AbstractAgeInstantBasedValidator.java

+
+	protected boolean inclusive;
+
+	protected ChronoUnit unit;


AFAICS, at least part of these properties can be private now.

gsmet · 2018-02-12T10:31:14Z

...ate/validator/internal/constraintvalidators/hv/age/min/AbstractAgeMinTimeBasedValidator.java

+
+	@Override
+	protected boolean isValid(long result) {
+		return super.inclusive ? result <= 0 : result < 0;


You're using this and super quite randomly. See the class just above. Let's provide a protected isInclusive() getter in the parent class, it's going to be cleaner.

HillmerCh · 2018-02-22T12:22:34Z

Hi @marko-bekhta , I made some changes 'cause I think the idea to define the units is a good idea. I created an Enum AgeMin#Unit this Enum support only YEAR, MONTHS and DAYS using only these 3 units some of the problems that we got before can be fixed. I think unit as seconds, minutes are not necessary

enum Unit {
	YEARS( ChronoUnit.YEARS ), MONTHS( ChronoUnit.MONTHS ), DAYS( ChronoUnit.DAYS );

I haven't unified the specific validator class as you told me, I think it is much better compares the same data type: LocalDate.compareTo(otherLocalDate) , using specific validator we can support time types as Year. Please look over AgeMinValidatorForYear to see what I did.

marko-bekhta · 2018-02-22T12:39:38Z

@Hilmerc that's great. Thanks for experimenting more with this! I'll try to have a look later today.

marko-bekhta · 2018-03-12T21:45:21Z

Hi @HillmerCh ! Finally had a chance to spent some time on this one. I really liked the idea of limiting the units to a specified set that we can control.
I've noticed that there were quite a few formatting changes with the last commit. The checkstyle seems to be happy with those, so just in case you haven't seen there's https://github.com/hibernate/hibernate-ide-codestyles repository that could make formatting code in the IDE easier.
Now as for the changes, the idea of own enum Unit is great. If I remember correctly, the trouble with the truncateTo attribute was that we could only use units up to ChronoUnit.DAYS included. So couldn't we apply the same approach with our own enum to that attribute and limit the options to ChronoUnit.NANOS - ChronoUnit.DAYS ?
I'd say having two of such enums could be useful. First one would limit the options of which units can be used to define the age (I assume that we could ignore MILLENNIA, ERAS and FOREVER). Second one as mentioned earlier would limit the units used for truncateTo attribute.
As for the unified validator - it just seemed more practical to have the logic of adding units in one place. In case of a Year wouldn't year.adjustInto( LocalDateTime.now(clockReference) ) do the trick of getting the LocalDateTime and then working with it in the abstract validator ?

HillmerCh · 2018-04-14T13:53:54Z

Hi @marko-bekhta I was a little busy the last days, I'm gonna work with this PR and try to finish next week. Thanks for your help and patience.

HillmerCh · 2018-04-14T15:36:35Z

@marko-bekhta I'm not sure if I understand well you said.
About UNITs: You suggest to use only NANOS and DAYS, to use the unit between them? SECOND, MINUTES.
If you just mean it is use only NANOS and DAYS, it can be difficult to the developer to express the values, for example, if we want to say that the de age must be 18 years, Do I have to set the value 18*365? But some years have 366 days (February 29th).
About: year.adjustInto For this example: the age must be maximum 1 year old. The user input the value 14/04/2017 (From today it is one year ago). The method. year.adjustInto returns 2018-04-14T09:50:19.487032 as a LocalDateTime. Comparing the two LocalDateTime, the second is greater than the first, but someone who was born on 14/04/2017 is one year old until 13/04/2019. To avoid that kind of problem, I think it is much better compares same types.
But, As I said I don't know if I understand your recommendations.
Please, Can you say me if I am right o give more information.
Thanks,

Hillmer

marko-bekhta · 2018-04-17T10:58:52Z

Hi @HillmerCh no worries 😃. IIRC I was thinking that as we discarded the idea of truncateTo attribute because it was causing the very same issues with the date/time classes, where we couldn't be sure if the exception will or will not be thrown, we might want to re-consider it once more and limit the units of it. So basically have something like:

public @interface AgeMin {
    // ....
    int value();
    
    AgeUnit ageUnit() default AgeUnit.YEARS;
    
    TruncateUnit trunkateTo() default TruncateUnit.NANOS;
    
    boolean inclusive() default true;
}

enum AgeUnit {
    YEARS( ChronoUnit.YEARS ), MONTHS( ChronoUnit.MONTHS ), DAYS( ChronoUnit.DAYS ) ....;
}

enum TruncateUnit {
    DAYS( ChronoUnit.DAYS ), HOURS( ChronoUnit.HOURS ), MINUTES( ChronoUnit.MINUTES ), ..., NANOS( ChronoUnit.NANOS ),;
}

As these units will be the same between AgeMin and AgeMax we should probably have them outside the constraint.

About UNITs: You suggest to use only NANOS and DAYS, to use the unit between them? SECOND, MINUTES.

I was thinking about the TruncateUnit where we could have all the units between days and nanos included.

I hope this clarify the part about the units. And as for the other part of the question - I'll get back to you once I remind myself what I was thinking back then 😃

Hibernate-CI · 2021-03-19T08:47:58Z

Can one of the admins add this person to the trusted builders? (reply with: "add to whitelist" or "ok to test")

HillmerCh added 3 commits January 10, 2018 10:51

HV-1552 Adding new MinAge Constraint

32f6e73

HV-1552 Adding new MinAge Constraint

0c450ec

HV-1552 Adding new MinAge Constraint

1d34b29

HV-1552 Adding new MinAge Constraint

a93a9e5

marko-bekhta reviewed Jan 24, 2018

View reviewed changes

gsmet added Changes required Needs work and removed Changes required labels Feb 6, 2018

HV-1552 Adding new MinAge Constraint

4e74cd4

marko-bekhta reviewed Feb 9, 2018

View reviewed changes

HillmerCh added 2 commits February 11, 2018 10:14

HV-1552 Adding new MinAge Constraint

edd9954

HV-1552 Adding new MinAge Constraint

aafcad2

marko-bekhta reviewed Feb 11, 2018

View reviewed changes

gsmet reviewed Feb 12, 2018

View reviewed changes

HV-1552 Adding new MinAge Constraint

a46876f

Base automatically changed from master to main March 19, 2021 08:47

		@@ -0,0 +1,32 @@
		package org.hibernate.validator.internal.constraintvalidators.hv.age.min;

		@@ -0,0 +1,25 @@
		package org.hibernate.validator.internal.constraintvalidators.hv.age.min;

HV-1552 Adding new MinAge Constraint #913

Are you sure you want to change the base?

HV-1552 Adding new MinAge Constraint #913

Conversation

HillmerCh commented Jan 16, 2018

marko-bekhta commented Jan 22, 2018

HillmerCh commented Jan 23, 2018

marko-bekhta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marko-bekhta commented Jan 30, 2018

HillmerCh commented Jan 31, 2018

HillmerCh commented Feb 8, 2018

marko-bekhta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HillmerCh commented Feb 11, 2018

marko-bekhta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HillmerCh commented Feb 22, 2018

marko-bekhta commented Feb 22, 2018

marko-bekhta commented Mar 12, 2018

HillmerCh commented Apr 14, 2018

HillmerCh commented Apr 14, 2018

marko-bekhta commented Apr 17, 2018

Hibernate-CI commented Mar 19, 2021