Don't create new JinjavaBeanELResolver instances per Jinjava or

JinjavaConfig for bean cache performance

Author: jasmith-hs

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

@@ -180,11 +180,9 @@ default AllowlistReturnTypeValidator getReturnTypeValidator() {
 
   @Value.Default
   default ELResolver getElResolver() {
-    return JinjavaInterpreterResolver.createDefaultResolver(
-      isDefaultReadOnlyResolver(),
-      getMethodValidator(),
-      getReturnTypeValidator()
-    );
+    return isDefaultReadOnlyResolver()
+      ? JinjavaInterpreterResolver.DEFAULT_RESOLVER_READ_ONLY
+      : JinjavaInterpreterResolver.DEFAULT_RESOLVER_READ_WRITE;
   }
 
   @Value.Default

src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java

@@ -4,15 +4,11 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.hubspot.jinjava.el.ext.AbstractCallableMethod;
-import com.hubspot.jinjava.el.ext.AllowlistMethodValidator;
-import com.hubspot.jinjava.el.ext.AllowlistReturnTypeValidator;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
 import com.hubspot.jinjava.el.ext.ExtendedParser;
 import com.hubspot.jinjava.el.ext.JinjavaBeanELResolver;
 import com.hubspot.jinjava.el.ext.JinjavaListELResolver;
-import com.hubspot.jinjava.el.ext.MethodValidatorConfig;
 import com.hubspot.jinjava.el.ext.NamedParameter;
-import com.hubspot.jinjava.el.ext.ReturnTypeValidatorConfig;
 import com.hubspot.jinjava.interpret.DeferredValueException;
 import com.hubspot.jinjava.interpret.DisabledException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
@@ -56,39 +52,25 @@
 
 public class JinjavaInterpreterResolver extends SimpleResolver {
 
-  public static ELResolver createDefaultResolver(
-    boolean readOnly,
-    AllowlistMethodValidator allowlistMethodValidator,
-    AllowlistReturnTypeValidator allowlistReturnTypeValidator
-  ) {
-    return new CompositeELResolver() {
-      {
-        add(new ArrayELResolver(readOnly));
-        add(new JinjavaListELResolver(readOnly));
-        add(new TypeConvertingMapELResolver(readOnly));
-        add(new ResourceBundleELResolver());
-        add(
-          new JinjavaBeanELResolver(
-            readOnly,
-            allowlistMethodValidator,
-            allowlistReturnTypeValidator
-          )
-        );
-      }
-    };
-  }
-
-  public static final ELResolver DEFAULT_RESOLVER_READ_ONLY = createDefaultResolver(
-    true,
-    AllowlistMethodValidator.create(MethodValidatorConfig.of()),
-    AllowlistReturnTypeValidator.create(ReturnTypeValidatorConfig.of())
-  );
-
-  public static final ELResolver DEFAULT_RESOLVER_READ_WRITE = createDefaultResolver(
-    false,
-    AllowlistMethodValidator.create(MethodValidatorConfig.of()),
-    AllowlistReturnTypeValidator.create(ReturnTypeValidatorConfig.of())
-  );
+  public static final ELResolver DEFAULT_RESOLVER_READ_ONLY = new CompositeELResolver() {
+    {
+      add(new ArrayELResolver(true));
+      add(new JinjavaListELResolver(true));
+      add(new TypeConvertingMapELResolver(true));
+      add(new ResourceBundleELResolver());
+      add(new JinjavaBeanELResolver(true));
+    }
+  };
+
+  public static final ELResolver DEFAULT_RESOLVER_READ_WRITE = new CompositeELResolver() {
+    {
+      add(new ArrayELResolver(false));
+      add(new JinjavaListELResolver(false));
+      add(new TypeConvertingMapELResolver(false));
+      add(new ResourceBundleELResolver());
+      add(new JinjavaBeanELResolver(false));
+    }
+  };
 
   private final JinjavaInterpreter interpreter;
   private final ObjectUnwrapper objectUnwrapper;

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

@@ -2,14 +2,24 @@
 
 import com.google.common.base.CaseFormat;
 import com.google.common.collect.ImmutableSet;
+import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.interpret.DeferredValueException;
+import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.util.EagerReconstructionUtils;
+import java.beans.IntrospectionException;
+import java.beans.Introspector;
+import java.beans.MethodDescriptor;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import javax.el.ELContext;
+import javax.el.ELException;
 import javax.el.MethodNotFoundException;
 
 /**
@@ -39,35 +49,18 @@ public class JinjavaBeanELResolver extends BeanELResolver {
     .add("merge")
     .build();
 
-  private final AllowlistMethodValidator allowlistMethodValidator;
-  private final AllowlistReturnTypeValidator allowlistReturnTypeValidator;
+  private final ConcurrentHashMap<Class<?>, BeanMethods> beanMethodsCache;
 
   public JinjavaBeanELResolver() {
-    this(
-      true,
-      AllowlistMethodValidator.create(MethodValidatorConfig.of()),
-      AllowlistReturnTypeValidator.create(ReturnTypeValidatorConfig.of())
-    );
-  }
-
-  public JinjavaBeanELResolver(
-    AllowlistMethodValidator allowlistMethodValidator,
-    AllowlistReturnTypeValidator allowlistReturnTypeValidator
-  ) {
-    this(true, allowlistMethodValidator, allowlistReturnTypeValidator);
+    this(true);
   }
 
   /**
    * Creates a new read/write {@link JinjavaBeanELResolver}.
    */
-  public JinjavaBeanELResolver(
-    boolean readOnly,
-    AllowlistMethodValidator allowlistMethodValidator,
-    AllowlistReturnTypeValidator allowlistReturnTypeValidator
-  ) {
+  public JinjavaBeanELResolver(boolean readOnly) {
     super(readOnly);
-    this.allowlistMethodValidator = allowlistMethodValidator;
-    this.allowlistReturnTypeValidator = allowlistReturnTypeValidator;
+    this.beanMethodsCache = new ConcurrentHashMap<Class<?>, BeanMethods>();
   }
 
   @Override
@@ -77,9 +70,9 @@ public Class<?> getType(ELContext context, Object base, Object property) {
 
   @Override
   public Object getValue(ELContext context, Object base, Object property) {
-    return allowlistReturnTypeValidator.validateReturnType(
-      super.getValue(context, base, transformPropertyName(property))
-    );
+    return getJinjavaConfig()
+      .getReturnTypeValidator()
+      .validateReturnType(super.getValue(context, base, transformPropertyName(property)));
   }
 
   @Override
@@ -118,9 +111,9 @@ public Object invoke(
       );
     }
 
-    return allowlistReturnTypeValidator.validateReturnType(
-      super.invoke(context, base, method, paramTypes, params)
-    );
+    return getJinjavaConfig()
+      .getReturnTypeValidator()
+      .validateReturnType(super.invoke(context, base, method, paramTypes, params));
   }
 
   @Override
@@ -167,17 +160,30 @@ protected Method findMethod(
               )
           );
     }
-    return allowlistMethodValidator.validateMethod(method);
+    return getJinjavaConfig().getMethodValidator().validateMethod(method);
   }
 
   @Override
   protected Method getWriteMethod(Object base, Object property) {
-    return allowlistMethodValidator.validateMethod(super.getWriteMethod(base, property));
+    return getJinjavaConfig()
+      .getMethodValidator()
+      .validateMethod(super.getWriteMethod(base, property));
   }
 
   @Override
   protected Method getReadMethod(Object base, Object property) {
-    return allowlistMethodValidator.validateMethod(super.getReadMethod(base, property));
+    return getJinjavaConfig()
+      .getMethodValidator()
+      .validateMethod(super.getReadMethod(base, property));
+  }
+
+  private static JinjavaConfig getJinjavaConfig() {
+    return Objects
+      .requireNonNull(
+        JinjavaInterpreter.getCurrent(),
+        "JinjavaInterpreter.closeablePushCurrent must be used if using a JinjavaInterpreter directly"
+      )
+      .getConfig();
   }
 
   private static boolean checkAssignableParameterTypes(Object[] params, Method method) {
@@ -221,4 +227,52 @@ private String transformPropertyName(Object property) {
     }
     return CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, propertyStr);
   }
+
+  protected final class BeanMethods {
+
+    private final Map<String, List<BeanMethod>> map = new HashMap<>();
+
+    public BeanMethods(Class<?> baseClass) {
+      MethodDescriptor[] descriptors;
+      try {
+        descriptors = Introspector.getBeanInfo(baseClass).getMethodDescriptors();
+      } catch (IntrospectionException e) {
+        throw new ELException(e);
+      }
+      for (MethodDescriptor descriptor : descriptors) {
+        map.compute(
+          descriptor.getName(),
+          (k, v) -> {
+            if (v == null) {
+              v = new LinkedList<>();
+            }
+            v.add(new BeanMethod(descriptor));
+            return v;
+          }
+        );
+      }
+    }
+
+    public List<BeanMethod> getBeanMethods(String methodName) {
+      return map.get(methodName);
+    }
+  }
+
+  protected final class BeanMethod {
+
+    private final MethodDescriptor descriptor;
+
+    private Method method;
+
+    public BeanMethod(MethodDescriptor descriptor) {
+      this.descriptor = descriptor;
+    }
+
+    public Method getMethod() {
+      if (method == null) {
+        method = findAccessibleMethod(descriptor.getMethod());
+      }
+      return method;
+    }
+  }
 }