Extract test objects to separate classes. Allow arrays. Add

AnnotationIntrospector

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/LegacyOverrides.java

@@ -10,7 +10,7 @@
 @ImmutableSetEncodingEnabled
 @ImmutableListEncodingEnabled
 @ImmutableMapEncodingEnabled
-public interface LegacyOverrides {
+public interface LegacyOverrides extends WithLegacyOverrides {
   LegacyOverrides NONE = new Builder().build();
   LegacyOverrides THREE_POINT_0 = new Builder()
     .withEvaluateMapKeys(true)

src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java

@@ -4,8 +4,11 @@
 import com.google.common.collect.ForwardingList;
 import com.google.common.collect.ForwardingMap;
 import com.google.common.collect.ForwardingSet;
+import com.hubspot.jinjava.interpret.NullValue;
 import com.hubspot.jinjava.lib.exptest.ExpTest;
 import com.hubspot.jinjava.lib.filter.Filter;
+import com.hubspot.jinjava.lib.fn.MacroFunction;
+import com.hubspot.jinjava.lib.fn.eager.EagerMacroFunction;
 import com.hubspot.jinjava.objects.DummyObject;
 import com.hubspot.jinjava.objects.Namespace;
 import com.hubspot.jinjava.objects.SafeString;
@@ -71,6 +74,7 @@ String[] allowedDeclaredMethodsFromClasses() {
       DummyObject.class.getCanonicalName(),
       Namespace.class.getCanonicalName(),
       SafeString.class.getCanonicalName(),
+      NullValue.class.getCanonicalName(),
     };
 
     @Override
@@ -112,9 +116,18 @@ String[] allowedReturnTypeClasses() {
     String[] allowedDeclaredMethodsFromClasses() {
       return ARRAY;
     }
+
+    @Override
+    boolean enableArrays() {
+      return true;
+    }
   },
   JinjavaTagConstructs {
-    private static final String[] ARRAY = { ForLoop.class.getCanonicalName() };
+    private static final String[] ARRAY = {
+      ForLoop.class.getCanonicalName(),
+      MacroFunction.class.getCanonicalName(),
+      EagerMacroFunction.class.getCanonicalName(),
+    };
 
     @Override
     String[] allowedReturnTypeClasses() {
@@ -173,4 +186,8 @@ String[] allowedReturnTypeCanonicalClassPrefixes() {
   String[] allowedReturnTypeClasses() {
     return new String[0];
   }
+
+  boolean enableArrays() {
+    return false;
+  }
 }

src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java

@@ -10,6 +10,7 @@ public final class AllowlistReturnTypeValidator {
 
   private final ImmutableSet<String> allowedCanonicalClassPrefixes;
   private final ImmutableSet<String> allowedCanonicalClassNames;
+  private final boolean allowArrays;
   private final ImmutableList<ReturnTypeValidator> additionalValidators;
 
   public static AllowlistReturnTypeValidator create(
@@ -30,6 +31,7 @@ private AllowlistReturnTypeValidator(
       returnTypeValidatorConfig.allowedCanonicalClassPrefixes();
     this.allowedCanonicalClassNames =
       returnTypeValidatorConfig.allowedCanonicalClassNames();
+    this.allowArrays = returnTypeValidatorConfig.allowArrays();
     this.additionalValidators = additionalValidators;
     this.allowedReturnTypesCache = new ConcurrentHashMap<>();
   }
@@ -39,8 +41,8 @@ public Object validateReturnType(Object o) {
       return null;
     }
     Class<?> clazz = o.getClass();
-    while (clazz.isArray()) {
-      clazz = clazz.getComponentType();
+    if (clazz.isArray() && allowArrays) {
+      return o;
     }
     String canonicalClassName = clazz.getCanonicalName();
     boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent(
@@ -60,4 +62,17 @@ public Object validateReturnType(Object o) {
     }
     return o;
   }
+
+  public boolean allowReturnTypeClass(Class<?> clazz) {
+    if (clazz.isArray() && allowArrays) {
+      return true;
+    }
+    String canonicalClassName = clazz.getCanonicalName();
+    return allowedReturnTypesCache.computeIfAbsent(
+      canonicalClassName,
+      c ->
+        allowedCanonicalClassNames.contains(canonicalClassName) ||
+        allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith)
+    );
+  }
 }

src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidatorConfig.java

@@ -30,6 +30,11 @@ public abstract class ReturnTypeValidatorConfig {
 
   public abstract ImmutableSet<String> allowedCanonicalClassNames();
 
+  @Value.Default
+  public boolean allowArrays() {
+    return false;
+  }
+
   @Value.Check
   void banClassesAndMethods() {
     if (
@@ -75,6 +80,9 @@ public Builder addAllowlistGroups(AllowlistGroup... allowlistGroups) {
             allowlistGroup.allowedReturnTypeCanonicalClassPrefixes()
           )
           .addAllowedCanonicalClassNames(allowlistGroup.allowedReturnTypeClasses());
+        if (allowlistGroup.enableArrays()) {
+          this.setAllowArrays(true);
+        }
       }
       return this;
     }

src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java

@@ -19,6 +19,7 @@
 import com.hubspot.jinjava.doc.annotations.JinjavaParam;
 import com.hubspot.jinjava.doc.annotations.JinjavaSnippet;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import com.hubspot.jinjava.objects.collections.ArrayBacked;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
@@ -75,7 +76,7 @@ public String getName() {
     return "reverse";
   }
 
-  static class ReverseArrayIterator<T> implements Iterator<T> {
+  static class ReverseArrayIterator<T> implements Iterator<T>, ArrayBacked<T> {
 
     private final T[] array;
     private int index;
@@ -101,5 +102,10 @@ public T next() {
     public boolean hasNext() {
       return index >= 0;
     }
+
+    @Override
+    public T[] backingArray() {
+      return array;
+    }
   }
 }

src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java

@@ -0,0 +1,5 @@
+package com.hubspot.jinjava.objects.collections;
+
+public interface ArrayBacked<T> {
+  T[] backingArray();
+}

src/main/java/com/hubspot/jinjava/objects/serialization/PyishObjectMapper.java

@@ -8,6 +8,8 @@
 import com.fasterxml.jackson.databind.ObjectWriter;
 import com.fasterxml.jackson.databind.PropertyNamingStrategies;
 import com.fasterxml.jackson.databind.SerializerProvider;
+import com.fasterxml.jackson.databind.introspect.Annotated;
+import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
 import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
 import com.google.common.annotations.Beta;
@@ -53,6 +55,25 @@ private static ObjectMapper getPyishObjectMapper() {
           .addSerializer(PyishSerializable.class, PyishSerializer.INSTANCE)
       );
     mapper.getSerializerProvider().setNullKeySerializer(new NullKeySerializer());
+    mapper.setAnnotationIntrospector(
+      new JacksonAnnotationIntrospector() {
+        @Override
+        protected boolean _isIgnorable(Annotated a) {
+          return (
+            super._isIgnorable(a) ||
+            !JinjavaInterpreter
+              .getCurrentMaybe()
+              .map(interpreter ->
+                interpreter
+                  .getConfig()
+                  .getReturnTypeValidator()
+                  .allowReturnTypeClass(a.getRawType())
+              )
+              .orElse(false)
+          );
+        }
+      }
+    );
     return mapper;
   }
 

src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java

@@ -14,6 +14,7 @@
 import com.hubspot.jinjava.interpret.PartiallyDeferredValue;
 import com.hubspot.jinjava.interpret.TemplateSyntaxException;
 import com.hubspot.jinjava.interpret.UnknownTokenException;
+import com.hubspot.jinjava.objects.collections.ArrayBacked;
 import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
 import com.hubspot.jinjava.objects.serialization.PyishSerializable;
 import com.hubspot.jinjava.tree.ExpressionNode;
@@ -325,6 +326,9 @@ private static boolean isResolvableObjectRec(
     if (isPrimitive(val)) {
       return true;
     }
+    if (val instanceof ArrayBacked<?> arrayBacked) {
+      val = arrayBacked.backingArray();
+    }
     try {
       if (val instanceof Collection || val instanceof Map) {
         int size = val instanceof Collection

src/test/java/com/hubspot/jinjava/FilterOverrideTest.java

@@ -2,40 +2,22 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import com.hubspot.jinjava.interpret.JinjavaInterpreter;
-import com.hubspot.jinjava.lib.filter.Filter;
+import com.hubspot.jinjava.testobjects.FilterOverrideTestObjects;
 import java.util.HashMap;
 import org.junit.Test;
 
 public class FilterOverrideTest {
 
   @Test
   public void itAllowsUsersToOverrideBuiltInFilters() {
-    Jinjava jinjava = new Jinjava();
+    Jinjava jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build());
     String template = "{{ 5 | add(6) }}";
 
     assertThat(jinjava.render(template, new HashMap<>())).isEqualTo("11");
 
-    jinjava.getGlobalContext().registerClasses(DescriptiveAddFilter.class);
+    jinjava
+      .getGlobalContext()
+      .registerClasses(FilterOverrideTestObjects.DescriptiveAddFilter.class);
     assertThat(jinjava.render(template, new HashMap<>())).isEqualTo("5 + 6 = 11");
   }
-
-  public static class DescriptiveAddFilter implements Filter {
-
-    @Override
-    public String getName() {
-      return "add";
-    }
-
-    @Override
-    public Object filter(Object var, JinjavaInterpreter interpreter, String... args) {
-      return (
-        var +
-        " + " +
-        args[0] +
-        " = " +
-        (Integer.parseInt(var.toString()) + Integer.parseInt(args[0]))
-      );
-    }
-  }
 }