Skip to content

Fix Boolean Nulls/Refactor Time Adapters/Adapter Tests #55

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

Merged
merged 24 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions validator/src/main/java/io/avaje/validation/Validator.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Iterator;
import java.time.Clock;
import java.time.Duration;
import java.util.Locale;
import java.util.Map;
import java.util.ResourceBundle;
import java.util.ServiceLoader;
import java.util.function.Supplier;

public interface Validator {

Expand All @@ -27,7 +29,7 @@ public interface Validator {
void validate(Object any, Locale locale) throws ConstraintViolationException;

static Builder builder() {
final Iterator<Bootstrap> bootstrapService = ServiceLoader.load(Bootstrap.class).iterator();
final var bootstrapService = ServiceLoader.load(Bootstrap.class).iterator();
if (bootstrapService.hasNext()) {
return bootstrapService.next().builder();
}
Expand Down Expand Up @@ -61,6 +63,10 @@ interface Builder {
/** Adds additional Locales for this validator */
Builder addLocales(Locale... locales);

Builder clockProvider(Supplier<Clock> clockSupplier);

Builder temporalTolerance(Duration temporalTolerance);

/** Add a AdapterBuilder which provides a ValidationAdapter to use for the given type. */
Builder add(Type type, AdapterBuilder builder);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.time.Clock;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;

import io.avaje.validation.adapter.ValidationAdapter;
import io.avaje.validation.adapter.ValidationContext;
import io.avaje.validation.core.adapters.BasicAdapters;
import io.avaje.validation.core.adapters.FuturePastAdapterFactory;
import io.avaje.validation.core.adapters.NumberAdapters;

/** Builds and caches the ValidationAdapter adapters for DValidator. */
Expand All @@ -23,12 +27,15 @@ final class CoreAdapterBuilder {
CoreAdapterBuilder(
DValidator context,
List<ValidationContext.AdapterFactory> userFactories,
List<ValidationContext.AnnotationFactory> userAnnotationFactories) {
List<ValidationContext.AnnotationFactory> userAnnotationFactories,
Supplier<Clock> clockSupplier,
Duration temporalTolerance) {
this.context = context;
this.factories.addAll(userFactories);
this.annotationFactories.addAll(userAnnotationFactories);
this.annotationFactories.add(BasicAdapters.FACTORY);
this.annotationFactories.add(NumberAdapters.FACTORY);
this.annotationFactories.add(new FuturePastAdapterFactory(clockSupplier, temporalTolerance));
}

/** Return the adapter from cache if exists else return null. */
Expand All @@ -52,7 +59,7 @@ <T> ValidationAdapter<T> annotationAdapter(Class<? extends Annotation> cls, Map<
<T> ValidationAdapter<T> build(Type type, Object cacheKey) {
// Ask each factory to create the validation adapter.
for (final ValidationContext.AdapterFactory factory : factories) {
final ValidationAdapter<T> result = (ValidationAdapter<T>) factory.create(type, context);
final var result = (ValidationAdapter<T>) factory.create(type, context);
if (result != null) {
return result;
}
Expand Down
39 changes: 34 additions & 5 deletions validator/src/main/java/io/avaje/validation/core/DValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.time.Clock;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.ResourceBundle;
import java.util.ServiceLoader;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;

import io.avaje.lang.Nullable;
import io.avaje.validation.Validator;
Expand All @@ -40,12 +42,17 @@ final class DValidator implements Validator, ValidationContext {
List<String> bundleNames,
List<ResourceBundle> bundles,
MessageInterpolator interpolator,
LocaleResolver localeResolver) {
LocaleResolver localeResolver,
Supplier<Clock> clockSupplier,
Duration temporalTolerance) {
this.localeResolver = localeResolver;
final var defaultResourceBundle = new DResourceBundleManager(bundleNames, bundles, localeResolver);
final var defaultResourceBundle =
new DResourceBundleManager(bundleNames, bundles, localeResolver);
this.templateLookup = new DTemplateLookup(defaultResourceBundle);
this.interpolator = interpolator;
this.builder = new CoreAdapterBuilder(this, factories, annotationFactories);
this.builder =
new CoreAdapterBuilder(
this, factories, annotationFactories, clockSupplier, temporalTolerance);
}

MessageInterpolator interpolator() {
Expand Down Expand Up @@ -138,6 +145,8 @@ static final class DBuilder implements Validator.Builder {
private final List<ResourceBundle> bundles = new ArrayList<>();
private final List<Locale> otherLocals = new ArrayList<>();
private Locale defaultLocal = Locale.getDefault();
private Supplier<Clock> clockSupplier = Clock::systemDefaultZone;
private Duration temporalTolerance = Duration.ZERO;

@Override
public Builder add(Type type, AdapterBuilder builder) {
Expand Down Expand Up @@ -201,6 +210,18 @@ public Builder addLocales(Locale... locals) {
return this;
}

@Override
public Builder clockProvider(Supplier<Clock> clockSupplier) {
this.clockSupplier = clockSupplier;
return this;
}

@Override
public Builder temporalTolerance(Duration temporalTolerance) {
this.temporalTolerance = temporalTolerance;
return this;
}

private void registerComponents() {
// first register all user defined ValidatorComponent
for (final ValidatorComponent next : ServiceLoader.load(ValidatorComponent.class)) {
Expand All @@ -220,7 +241,15 @@ public DValidator build() {
ServiceLoader.load(MessageInterpolator.class)
.findFirst()
.orElseGet(BasicMessageInterpolator::new);
return new DValidator(factories, afactories, bundleNames, bundles, interpolator, localeResolver);
return new DValidator(
factories,
afactories,
bundleNames,
bundles,
interpolator,
localeResolver,
clockSupplier,
temporalTolerance);
}

private static <T> AnnotationFactory newAnnotationAdapterFactory(Type type, ValidationAdapter<T> adapter) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.avaje.validation.core.adapters;

import java.lang.reflect.Array;
import java.util.Collection;
import java.util.List;
import java.util.Map;
Expand All @@ -15,23 +14,20 @@
public final class BasicAdapters {
private BasicAdapters() {}

public static final ValidationContext.AnnotationFactory FACTORY = (annotationType, context, attributes) ->
switch (annotationType.getSimpleName()) {
case "Email" -> new EmailAdapter(context.message(attributes), attributes);
case "Null" -> new NullableAdapter(context.message(attributes), true);
case "NotNull", "NonNull" -> new NullableAdapter(context.message(attributes), false);
case "AssertTrue" -> new AssertBooleanAdapter(context.message(attributes), Boolean.FALSE);
case "AssertFalse" -> new AssertBooleanAdapter(context.message(attributes), Boolean.TRUE);
case "NotBlank" -> new NotBlankAdapter(context.message(attributes));
case "NotEmpty" -> new NotEmptyAdapter(context.message(attributes));
case "Past" -> new FuturePastAdapter(context.message(attributes), true, false);
case "PastOrPresent" -> new FuturePastAdapter(context.message(attributes), true, true);
case "Future" -> new FuturePastAdapter(context.message(attributes), false, false);
case "FutureOrPresent" -> new FuturePastAdapter(context.message(attributes), false, true);
case "Pattern" -> new PatternAdapter(context.message(attributes), attributes);
case "Size" -> new SizeAdapter(context.message(attributes), attributes);
default -> null;
};
public static final ValidationContext.AnnotationFactory FACTORY =
(annotationType, context, attributes) ->
switch (annotationType.getSimpleName()) {
case "Email" -> new EmailAdapter(context.message(attributes), attributes);
case "Null" -> new NullableAdapter(context.message(attributes), true);
case "NotNull", "NonNull" -> new NullableAdapter(context.message(attributes), false);
case "AssertTrue" -> new AssertBooleanAdapter(context.message(attributes), Boolean.TRUE);
case "AssertFalse" -> new AssertBooleanAdapter(context.message(attributes), Boolean.FALSE);
case "NotBlank" -> new NotBlankAdapter(context.message(attributes));
case "NotEmpty" -> new NotEmptyAdapter(context.message(attributes));
case "Pattern" -> new PatternAdapter(context.message(attributes), attributes);
case "Size" -> new SizeAdapter(context.message(attributes), attributes);
default -> null;
};

private static final class PatternAdapter implements ValidationAdapter<CharSequence> {

Expand All @@ -54,7 +50,11 @@ private static final class PatternAdapter implements ValidationAdapter<CharSeque

@Override
public boolean validate(CharSequence value, ValidationRequest req, String propertyName) {
if (value == null || pattern.test(value.toString())) {
if (value == null) {
return true;
}

if (pattern.test(value.toString())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong / inverted boolean ? Kind of expecting a test to fail ? Hmm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, there is the negate() there ... not sure how the tests previously passed?

req.addViolation(message, propertyName);
return false;
}
Expand Down Expand Up @@ -99,7 +99,7 @@ public boolean validate(Object value, ValidationRequest req, String propertyName
return len > 0;
}
} else if (value.getClass().isArray()) {
final var len = Array.getLength(value);
final var len = arrayLength(value);
if (len > max || len < min) {
req.addViolation(message, propertyName);
return len > 0;
Expand Down Expand Up @@ -170,7 +170,7 @@ public boolean validate(Object value, ValidationRequest req, String propertyName
return false;
}
} else if (value.getClass().isArray()) {
final var len = Array.getLength(value);
final var len = arrayLength(value);
if (len == 0) {
req.addViolation(message, propertyName);
return false;
Expand All @@ -193,7 +193,11 @@ private static final class AssertBooleanAdapter implements ValidationAdapter<Boo

@Override
public boolean validate(Boolean type, ValidationRequest req, String propertyName) {
if (assertBool.equals(type)) {
if (!assertBool.booleanValue() && type == null) {
return true;
}

if (!assertBool.equals(type)) {
req.addViolation(message, propertyName);
return false;
}
Expand All @@ -220,4 +224,27 @@ public boolean validate(Object value, ValidationRequest req, String propertyName
return true;
}
}

private static int arrayLength(Object array) {

if (array instanceof int[] arr) {
return arr.length;
} else if (array instanceof boolean[] arr) {
return arr.length;
} else if (array instanceof byte[] arr) {
return arr.length;
} else if (array instanceof char[] arr) {
return arr.length;
} else if (array instanceof short[] arr) {
return arr.length;
} else if (array instanceof float[] arr) {
return arr.length;
} else if (array instanceof double[] arr) {
return arr.length;
} else if (array instanceof long[] arr) {
return arr.length;
} else {
return ((Object[]) array).length;
}
}
}
Loading