Cleaned up the get attribute expression a little bit

src/main/java/com/mitchellbosecke/pebble/node/expression/GetAttributeExpression.java

@@ -11,14 +11,14 @@
 
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.util.Map;
 
 import com.mitchellbosecke.pebble.error.AttributeNotFoundException;
 import com.mitchellbosecke.pebble.error.PebbleException;
 import com.mitchellbosecke.pebble.extension.NodeVisitor;
-import com.mitchellbosecke.pebble.template.ClassAttributeCacheEntry;
 import com.mitchellbosecke.pebble.template.EvaluationContext;
 import com.mitchellbosecke.pebble.template.PebbleTemplateImpl;
 
@@ -44,63 +44,49 @@ public GetAttributeExpression(Expression<?> node, String attributeName) {
 	public Object evaluate(PebbleTemplateImpl self, EvaluationContext context) throws PebbleException {
 		Object object = node.evaluate(self, context);
 
-		if (object == null) {
-			if (context.isStrictVariables()) {
-				throw new NullPointerException(String.format("Can not get attribute [%s] of null object.",
-						attributeName));
-			} else {
-				return null;
-			}
-		}
-
-		// hold onto original name for error reporting
-		String originalAttributeName = attributeName;
-
-		Class<?> clazz = object.getClass();
-
 		Object result = null;
+		boolean found = false;
 
-		// first we check maps, as they are a bit of an exception
-		if (object instanceof Map && ((Map<?, ?>) object).containsKey(attributeName)) {
-			return ((Map<?, ?>) object).get(attributeName);
-		}
+		if (object != null) {
 
-		Member member = null;
-		if (attributeName != null) {
-			// check if it's cached
-			ClassAttributeCacheEntry cacheEntry = context.getAttributeCache().get(clazz);
-			if (cacheEntry == null) {
-				cacheEntry = new ClassAttributeCacheEntry();
-				context.getAttributeCache().put(clazz, cacheEntry);
+			// first we check maps, as they are a bit of an exception
+			if (!found) {
+				if (object instanceof Map && ((Map<?, ?>) object).containsKey(attributeName)) {
+					result = ((Map<?, ?>) object).get(attributeName);
+					found = true;
+				}
 			}
 
-			if (cacheEntry.hasAttribute(attributeName)) {
-				member = cacheEntry.getAttribute(attributeName);
-			} else {
-				member = findMember(object, attributeName);
-				cacheEntry.putAttribute(attributeName, member);
-			}
+			if (!found) {
+				Member member = null;
+				try {
+					member = findMember(object, attributeName);
+					if (member != null) {
+
+						if (member instanceof Method) {
+							result = ((Method) member).invoke(object);
+						} else if (member instanceof Field) {
+							result = ((Field) member).get(object);
+						}
+						found = true;
+					}
+				} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+					e.printStackTrace();
+					throw new PebbleException(e, "Could not access attribute [" + attributeName + "]");
+				}
 
+			}
 		}
 
-		if (member == null && context.isStrictVariables()) {
+		if (!found && context.isStrictVariables()) {
 			throw new AttributeNotFoundException(
 					null,
 					String.format(
 							"Attribute [%s] of [%s] does not exist or can not be accessed and strict variables is set to true.",
-							originalAttributeName, clazz));
-		}
-
-		try {
-			if (member instanceof Method) {
-				result = ((Method) member).invoke(object);
-			} else if (member instanceof Field) {
-				result = ((Field) member).get(object);
-			}
-		} catch (Exception e) {
-			throw new RuntimeException(e);
+							attributeName, object.getClass().getName()));
 		}
 		return result;
+
 	}
 
 	@Override
@@ -116,60 +102,65 @@ public String getAttribute() {
 		return attributeName;
 	}
 
-	private Member findMember(Object object, String attributeName) {
+	private Member findMember(Object object, String attributeName) throws IllegalAccessException {
 
 		Class<?> clazz = object.getClass();
 
-		Member member = null;
+		boolean found = false;
+		Member result = null;
 
 		// capitalize first letter of attribute for the following attempts
 		String attributeCapitalized = Character.toUpperCase(attributeName.charAt(0)) + attributeName.substring(1);
 
 		// check get method
-		if (member == null) {
+		if (!found) {
 			try {
-				member = clazz.getMethod("get" + attributeCapitalized);
+				result = clazz.getMethod("get" + attributeCapitalized);
+				found = true;
 			} catch (NoSuchMethodException | SecurityException e) {
 			}
 		}
 
 		// check is method
-		if (member == null) {
+		if (!found) {
 			try {
-				member = clazz.getMethod("is" + attributeCapitalized);
+				result = clazz.getMethod("is" + attributeCapitalized);
+				found = true;
 			} catch (NoSuchMethodException | SecurityException e) {
 			}
 		}
 
 		// check has method
-		if (member == null) {
+		if (!found) {
 			try {
-				member = clazz.getMethod("has" + attributeCapitalized);
+				result = clazz.getMethod("has" + attributeCapitalized);
+				found = true;
 			} catch (NoSuchMethodException | SecurityException e) {
 			}
 		}
 
 		// check if attribute is a public method
-		if (member == null) {
+		if (!found) {
 			try {
-				member = clazz.getMethod(attributeName);
+				result = clazz.getMethod(attributeName);
+				found = true;
 			} catch (NoSuchMethodException | SecurityException e) {
 			}
 		}
 
 		// public field
-		if (member == null) {
+		if (!found) {
 			try {
-				member = clazz.getField(attributeName);
+				result = clazz.getField(attributeName);
+				found = true;
 			} catch (NoSuchFieldException | SecurityException e) {
 			}
 		}
 
-		if (member != null) {
-			((AccessibleObject) member).setAccessible(true);
+		if (result != null) {
+			((AccessibleObject) result).setAccessible(true);
 		}
-
-		return member;
+		return result;
 	}
 
 }

src/main/java/com/mitchellbosecke/pebble/template/EvaluationContext.java

@@ -1,7 +1,6 @@
 package com.mitchellbosecke.pebble.template;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -42,11 +41,6 @@ public class EvaluationContext {
 	 */
 	private Stack<Scope> scopes;
 
-	/**
-	 * We cache the attributes of objects for performance purposes.
-	 */
-	private Map<Class<?>, ClassAttributeCacheEntry> attributeCache;
-
 	/**
 	 * The locale of this template. Will be used by LocaleAware filters,
 	 * functions, etc.
@@ -78,7 +72,6 @@ public EvaluationContext(PebbleTemplateImpl self, boolean strictVariables, Local
 		this.executorService = executorService;
 		this.inheritanceChain = new InheritanceChain(self);
 		this.scopes = new Stack<>();
-		this.attributeCache = new HashMap<>();
 
 		// add an initial scope
 		this.scopes.add(new Scope(null));
@@ -93,7 +86,6 @@ public EvaluationContext(PebbleTemplateImpl self, boolean strictVariables, Local
 	public EvaluationContext copyWithoutInheritanceChain(PebbleTemplateImpl self) {
 		EvaluationContext result = new EvaluationContext(self, strictVariables, locale, filters, tests, functions,
 				executorService);
-		result.setAttributeCache(attributeCache);
 		result.setScopes(scopes);
 		return result;
 	}
@@ -178,10 +170,6 @@ public Locale getLocale() {
 		return locale;
 	}
 
-	public Map<Class<?>, ClassAttributeCacheEntry> getAttributeCache() {
-		return attributeCache;
-	}
-
 	public Map<String, Test> getTests() {
 		return tests;
 	}
@@ -214,10 +202,6 @@ public Stack<Scope> getScopes() {
 		return scopes;
 	}
 
-	public void setAttributeCache(Map<Class<?>, ClassAttributeCacheEntry> attributeCache) {
-		this.attributeCache = attributeCache;
-	}
-
 	public void setScopes(Stack<Scope> scopes) {
 		this.scopes = scopes;
 	}

src/test/java/com/mitchellbosecke/pebble/BenchmarkTest.java

@@ -0,0 +1,51 @@
+package com.mitchellbosecke.pebble;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Ignore;
+import org.junit.Test;
+
+import com.mitchellbosecke.pebble.error.PebbleException;
+import com.mitchellbosecke.pebble.loader.Loader;
+import com.mitchellbosecke.pebble.loader.StringLoader;
+import com.mitchellbosecke.pebble.template.PebbleTemplate;
+
+@Ignore
+public class BenchmarkTest extends AbstractTest {
+
+	@Test
+	public void benchmark() throws PebbleException, IOException {
+		Loader stringLoader = new StringLoader();
+		PebbleEngine pebble = new PebbleEngine(stringLoader);
+
+		PebbleTemplate template = pebble.getTemplate("hello {{ object.firstName }} {{ object.lastName }}");
+		Map<String, Object> context = new HashMap<>();
+		context.put("object", new SimpleObject());
+
+		Writer writer = new StringWriter();
+		int numberOfEvaluations = 1000_000;
+
+		for (int i = 0; i < numberOfEvaluations; i++) {
+			template.evaluate(writer, context);
+		}
+	}
+
+	public class SimpleObject {
+		public String firstName = "Steve";
+
+		private String lastName = "Johnson";
+
+		public String getFirstName(){
+			return firstName;
+		}
+
+		public String getLastName() {
+			return lastName;
+		}
+	}
+
+}

src/test/java/com/mitchellbosecke/pebble/CoreTestsTest.java

@@ -224,8 +224,7 @@ public void testNegativeTestOnAttribute() throws PebbleException, IOException {
 		assertEquals("no", writer.toString());
 	}
 
-	private static class Classroom {
-		@SuppressWarnings("unused")
+	public static class Classroom {
 		public static List<Object> students = new ArrayList<>();
 	}