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

Past/Future annotation cannot be applied to java8 date types #328

Closed
neetkee opened this issue Jan 30, 2019 · 5 comments
Closed

Past/Future annotation cannot be applied to java8 date types #328

neetkee opened this issue Jan 30, 2019 · 5 comments
Labels
Milestone

Comments

@neetkee
Copy link

neetkee commented Jan 30, 2019

If we will put Past annotation on java 8 date type:

@Past
private LocalDate localBirthDay;

we will get the exception:

Caused by: java.lang.IllegalArgumentException: Can not set java.time.LocalDate field io.github.benas.randombeans.validation.BeanValidationAnnotatedBean.localBirthDay to java.util.Date
	at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
	at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
	at sun.reflect.UnsafeObjectFieldAccessorImpl.set(UnsafeObjectFieldAccessorImpl.java:81)
	at java.lang.reflect.Field.set(Field.java:764)
	at io.github.benas.randombeans.util.ReflectionUtils.setProperty(ReflectionUtils.java:96)
	at io.github.benas.randombeans.FieldPopulator.populateField(FieldPopulator.java:88)
	at io.github.benas.randombeans.EnhancedRandomImpl.populateField(EnhancedRandomImpl.java:158)
	at io.github.benas.randombeans.EnhancedRandomImpl.populateFields(EnhancedRandomImpl.java:149)
	at io.github.benas.randombeans.EnhancedRandomImpl.doPopulateBean(EnhancedRandomImpl.java:120)
	... 41 more

The problem is in PastAnnotationHandler, I guess. It returns DateRangeRandomizer without respecting the type. Should we check for type in this class, something like this?

 if (field.getType().equals(LocalDate.class)) {
            LocalDate now = LocalDate.now();
            return new LocalDateRangeRandomizer(now.minusYears(Constants.DEFAULT_DATE_RANGE), now.minusDays(1));
 }

Or there is a better way to do this?

@PascalSchumacher
Copy link
Collaborator

@fmbenhassine fmbenhassine added this to the 4.0.0 milestone Feb 2, 2019
@fmbenhassine
Copy link
Member

fmbenhassine commented Feb 2, 2019

It is not possible to "combine" criteria of two randomizer registries today. In the given example, these two criteria are "Custom date type" (coming from TimeRandomizerRegistry) and "In the past" (coming from BeanValidationRandomizerRegistry). It is possible to order registries with the @Priority annotation, and we end up with one randomizer picked up for the field (BTW @PascalSchumacher I tried to add TimeRandomizerRegistry but didn't help) but it is not possible to combine them.

Should we check for type in this class, something like this?

I won't do it that way because we will need to check all types supported by the annotation with multiple if/then statement.

I'm not sure (yet) if this is a design "bug" or a feature request, so I planned it for the next major version v4 to take some time and think how to correctly tackle the case.

In the meantime, it is always possible to use a custom randomizer for the field and leverage the LocalDateRangeRandomizer, something like:

import io.github.benas.randombeans.EnhancedRandomBuilder;
import io.github.benas.randombeans.FieldDefinitionBuilder;
import io.github.benas.randombeans.api.EnhancedRandom;
import io.github.benas.randombeans.api.Randomizer;
import io.github.benas.randombeans.randomizers.range.LocalDateRangeRandomizer;

import javax.validation.constraints.Past;
import java.time.LocalDate;

public class Foo {

    @Past
    private LocalDate birthDate;

    public static void main(String[] args) {
        EnhancedRandom enhancedRandom = new EnhancedRandomBuilder()
                .randomize(FieldDefinitionBuilder.field().named("birthDate").ofType(LocalDate.class).inClass(Foo.class).get(), new Randomizer<LocalDate>() {
                    LocalDateRangeRandomizer localDateRangeRandomizer = new LocalDateRangeRandomizer(LocalDate.now().minusYears(10), LocalDate.now());
                    @Override
                    public LocalDate getRandomValue() {
                        return localDateRangeRandomizer.getRandomValue();
                    }
                })
                .build();

        Foo foo = enhancedRandom.nextObject(Foo.class);
        System.out.println(foo.birthDate);
    }
}

PascalSchumacher added a commit to PascalSchumacher/easy-random that referenced this issue Feb 3, 2019
PascalSchumacher added a commit to PascalSchumacher/easy-random that referenced this issue Feb 3, 2019
@PascalSchumacher
Copy link
Collaborator

@benas See #331 for what I wanted to say about using TimeRandomizerRegistry.

PascalSchumacher added a commit to PascalSchumacher/easy-random that referenced this issue Feb 3, 2019
@fmbenhassine
Copy link
Member

@PascalSchumacher Thanks! Sorry for misunderstanding, I thought you mean using TimeRandomizerRegistry in client code (withEnhancedRandomBuilder#registerRandomizerRegistry) not in RB code.

@fmbenhassine
Copy link
Member

@neetkee A fix for this issue has been deployed in version 3.9.0-SNAPSHOT. Now it is possible to use Bean Validation annotations with the Date/Time API types. I'm closing this issue as resolved, but you can give the fix a try (See here for how to import the snapshot version) and re-open the ticket if necessary.

Many thanks to @PascalSchumacher for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants