Refactor CoreAdapterBuilder cache use (#276)

* Refactor CoreAdapterBuilder cache use

We can merge the 2 build() methods because the cacheKey is always just the type.

* The key for adapterCache can be Type rather than Object

* Refactor CoreAdapterBuilder to put ValidationAdapters into the cache

Previously, it always created new adapters and didn't actually use the adapterCache.

---------

Co-authored-by: Josiah Noel <32279667+SentryMan@users.noreply.github.com>

Author: rbygrave

validator/src/main/java/io/avaje/validation/core/CoreAdapterBuilder.java

@@ -31,7 +31,7 @@ final class CoreAdapterBuilder {
   private final DValidator context;
   private final List<AdapterFactory> factories = new ArrayList<>();
   private final List<AnnotationFactory> annotationFactories = new ArrayList<>();
-  private final Map<Object, ValidationAdapter<?>> adapterCache = new ConcurrentHashMap<>();
+  private final Map<Type, ValidationAdapter<?>> adapterCache = new ConcurrentHashMap<>();
 
   CoreAdapterBuilder(
       DValidator context,
@@ -50,26 +50,21 @@ final class CoreAdapterBuilder {
     this.annotationFactories.add(new FuturePastAdapterFactory(clockSupplier, temporalTolerance));
   }
 
-  /** Return the adapter from cache if exists else return null. */
+  /** Return the adapter from cache if exists creating the adapter if required. */
   @SuppressWarnings("unchecked")
-  <T> ValidationAdapter<T> get(Object cacheKey) {
-    return (ValidationAdapter<T>) adapterCache.get(cacheKey);
-  }
-
-  /** Build for the simple non-annotated type case. */
   <T> ValidationAdapter<T> build(Type type) {
-    return build(type, type);
-  }
-
-  <T> ValidationAdapter<T> annotationAdapter(
-      Class<? extends Annotation> cls, Map<String, Object> attributes, Set<Class<?>> groups) {
-    return buildAnnotation(cls, attributes, groups);
+    var adapter = adapterCache.get(type);
+    if (adapter != null) {
+      return (ValidationAdapter<T>)adapter;
+    }
+    ValidationAdapter<T> newValidator = buildForType(type);
+    adapterCache.put(type, newValidator);
+    return newValidator;
   }
 
-  /** Build given type and annotations. */
-  // TODO understand that lookup chain stuff
+  /** Build for the simple non-annotated type case. */
   @SuppressWarnings("unchecked")
-  <T> ValidationAdapter<T> build(Type type, Object cacheKey) {
+  private <T> ValidationAdapter<T> buildForType(Type type) {
     // Ask each factory to create the validation adapter.
     for (final AdapterFactory factory : factories) {
       final var result = (ValidationAdapter<T>) factory.create(type, context);
@@ -80,6 +75,11 @@ <T> ValidationAdapter<T> build(Type type, Object cacheKey) {
     throw new IllegalArgumentException("No ValidationAdapter for " + type + ". Perhaps needs @Valid or @Valid.Import?");
   }
 
+  <T> ValidationAdapter<T> annotationAdapter(
+      Class<? extends Annotation> cls, Map<String, Object> attributes, Set<Class<?>> groups) {
+    return buildAnnotation(cls, attributes, groups);
+  }
+
   /**
    * Build given type and annotations.
    *

validator/src/main/java/io/avaje/validation/core/DValidator.java

@@ -126,10 +126,6 @@ public Message message(String message, Map<String, Object> attributes) {
   @Override
   public <T> ValidationAdapter<T> adapter(Class<T> cls) {
     final Type cacheKey = canonicalizeClass(requireNonNull(cls));
-    final ValidationAdapter<T> result = builder.get(cacheKey);
-    if (result != null) {
-      return result;
-    }
     return builder.build(cacheKey);
   }
 
@@ -152,13 +148,8 @@ public <T> ValidationAdapter<T> adapter(
 
   @Override
   public <T> ValidationAdapter<T> adapter(Type type) {
-    type = removeSubtypeWildcard(canonicalize(requireNonNull(type)));
-    final Object cacheKey = type;
-    final ValidationAdapter<T> result = builder.get(cacheKey);
-    if (result != null) {
-      return result;
-    }
-    return builder.build(type, cacheKey);
+    final Type cacheKey = removeSubtypeWildcard(canonicalize(requireNonNull(type)));
+    return builder.build(cacheKey);
   }
 
   @Override

validator/src/test/java/io/avaje/validation/core/ValidatorTest.java

@@ -3,6 +3,8 @@
 import io.avaje.validation.ConstraintViolation;
 import io.avaje.validation.ConstraintViolationException;
 import io.avaje.validation.Validator;
+import io.avaje.validation.adapter.ValidationAdapter;
+import io.avaje.validation.adapter.ValidationContext;
 import org.junit.jupiter.api.Test;
 
 import java.time.LocalDate;
@@ -22,6 +24,15 @@ class ValidatorTest {
               .add(Contact.class, ContactValidationAdapter::new)
               .build();
 
+  @Test
+  void cacheForType() {
+    ValidationContext ctx = (ValidationContext) validator;
+
+    ValidationAdapter<Customer> adapter0 = ctx.adapter(Customer.class);
+    ValidationAdapter<Customer> adapter1 = ctx.adapter(Customer.class);
+    assertThat(adapter0).isSameAs(adapter1);
+  }
+
   @Test
   void testAllFail() {
     try {