diff --git a/src/main/java/ognl/ASTProperty.java b/src/main/java/ognl/ASTProperty.java index 196313f3..4ebc32f0 100644 --- a/src/main/java/ognl/ASTProperty.java +++ b/src/main/java/ognl/ASTProperty.java @@ -72,7 +72,7 @@ public int getIndexedPropertyType(OgnlContext context, Object source) if (property instanceof String) { return OgnlRuntime.getIndexedPropertyType((source == null) ? null - : OgnlRuntime.getCompiler().getInterfaceClass(source.getClass()), (String) property); + : OgnlRuntime.getInterfaceClass(source.getClass()), (String) property); } } @@ -447,7 +447,7 @@ public String toSetSourceString(OgnlContext context, Object target) { } } - PropertyDescriptor pd = OgnlRuntime.getPropertyDescriptor(OgnlRuntime.getCompiler().getInterfaceClass(context.getCurrentObject().getClass()), name); + PropertyDescriptor pd = OgnlRuntime.getPropertyDescriptor(OgnlRuntime.getInterfaceClass(context.getCurrentObject().getClass()), name); if (pd != null) { Method pdMethod = lastChild(context) ? pd.getWriteMethod() : pd.getReadMethod(); diff --git a/src/main/java/ognl/OgnlCache.java b/src/main/java/ognl/OgnlCache.java index 8881ac99..7d60efb8 100644 --- a/src/main/java/ognl/OgnlCache.java +++ b/src/main/java/ognl/OgnlCache.java @@ -158,6 +158,9 @@ public class OgnlCache { private final Cache methodPermCache = cacheFactory.createCache(methodPermCacheEntryFactory); + private final ClassCache> interfaceClassCache = + cacheFactory.createClassCache(key -> OgnlRuntime.getCompiler().getInterfaceClass(key)); + public Class[] getMethodParameterTypes(Method method) throws CacheException { return methodParameterTypesCache.get(method); } @@ -182,6 +185,14 @@ public Map getPropertyDescriptor(Class clazz) thr return propertyDescriptorCache.get(clazz); } + public Class getInterfaceClass(Class clazz) throws CacheException { + return interfaceClassCache.get(clazz); + } + + void clearInterfaceClassCache() { + interfaceClassCache.clear(); + } + /** * @deprecated since 3.4.6, forRemoval = true */ @@ -227,6 +238,7 @@ public void setClassCacheInspector(ClassCacheInspector inspector) { // _methodCache.setClassInspector( inspector ); // _invokePermissionCache.setClassInspector( inspector ); fieldCache.setClassInspector(inspector); + interfaceClassCache.setClassInspector(inspector); } public Class[] getGenericMethodParameterTypes(GenericMethodParameterTypeCacheEntry key) throws CacheException { @@ -255,6 +267,7 @@ public void clear() { invokePermissionCache.clear(); fieldCache.clear(); methodAccessCache.clear(); + interfaceClassCache.clear(); } public ElementsAccessor getElementsAccessor(Class clazz) throws OgnlException { diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index c84bf311..0f8dd25b 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -557,12 +557,26 @@ public static String getNumericLiteral(Class type) { public static void setCompiler(OgnlExpressionCompiler compiler) { _compiler = compiler; + // The cached interface classes are answers of the previous compiler + cache.clearInterfaceClassCache(); } public static OgnlExpressionCompiler getCompiler() { return _compiler; } + /** + * Same as calling {@link OgnlExpressionCompiler#getInterfaceClass(Class)} on the current compiler, but + * remembering the result for each class in the {@link OgnlCache}. The answer only depends on the class and + * on the compiler, and interpreted evaluation asks for it on every link of a property chain. + * + * @param clazz The class to find a compatible interface for. + * @return what the current compiler's {@code getInterfaceClass(clazz)} returns. + */ + static Class getInterfaceClass(Class clazz) { + return cache.getInterfaceClass(clazz); + } + public static void compileExpression(OgnlContext context, Node expression, Object root) throws Exception { _compiler.compileExpression(context, expression, root); diff --git a/src/test/java/ognl/OgnlRuntimeInterfaceClassCacheTest.java b/src/test/java/ognl/OgnlRuntimeInterfaceClassCacheTest.java new file mode 100644 index 00000000..05b70845 --- /dev/null +++ b/src/test/java/ognl/OgnlRuntimeInterfaceClassCacheTest.java @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package ognl; + +import ognl.enhance.ExpressionCompiler; +import ognl.enhance.OgnlExpressionCompiler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +/** + * Issue #650: the interface class of a chain link's source is looked up once per class. + */ +class OgnlRuntimeInterfaceClassCacheTest { + + public static class Customer { + public String getName() { + return "Alice"; + } + } + + public static class Order { + private final Customer customer = new Customer(); + + public Customer getCustomer() { + return customer; + } + } + + private static class CountingCompiler extends ExpressionCompiler { + + private final ConcurrentHashMap, AtomicInteger> calls = new ConcurrentHashMap<>(); + + @Override + public Class getInterfaceClass(Class clazz) { + calls.computeIfAbsent(clazz, c -> new AtomicInteger()).incrementAndGet(); + return super.getInterfaceClass(clazz); + } + + int calls(Class clazz) { + AtomicInteger count = calls.get(clazz); + return count == null ? 0 : count.get(); + } + } + + private static class ConstantCompiler extends ExpressionCompiler { + + @Override + public Class getInterfaceClass(Class clazz) { + return Object.class; + } + } + + private OgnlExpressionCompiler originalCompiler; + private CountingCompiler compiler; + + @BeforeEach + void setUp() { + originalCompiler = OgnlRuntime.getCompiler(); + compiler = new CountingCompiler(); + OgnlRuntime.setCompiler(compiler); + } + + @AfterEach + void tearDown() { + OgnlRuntime.setClassCacheInspector(null); + OgnlRuntime.setCompiler(originalCompiler); + } + + @Test + void chainLooksUpInterfaceClassOncePerClass() throws Exception { + Order order = new Order(); + OgnlContext context = Ognl.createDefaultContext(order, new DefaultMemberAccess(false)); + Object tree = Ognl.parseExpression("customer.name"); + + for (int i = 0; i < 5; i++) { + assertEquals("Alice", Ognl.getValue(tree, context, order)); + } + + assertEquals(1, compiler.calls(Order.class)); + } + + @Test + void returnsWhatTheCompilerReturns() { + assertSame(List.class, OgnlRuntime.getInterfaceClass(ArrayList.class)); + assertSame(List.class, OgnlRuntime.getInterfaceClass(ArrayList.class)); + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + + assertEquals(1, compiler.calls(ArrayList.class)); + assertEquals(1, compiler.calls(Order.class)); + } + + @Test + void newCompilerIsAskedAgain() { + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + + OgnlExpressionCompiler other = new ConstantCompiler(); + OgnlRuntime.setCompiler(other); + + assertSame(Object.class, OgnlRuntime.getInterfaceClass(Order.class)); + + OgnlRuntime.setCompiler(compiler); + + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + assertEquals(2, compiler.calls(Order.class)); + } + + @Test + void classCacheInspectorCanKeepAClassOutOfTheCache() { + OgnlRuntime.setClassCacheInspector(type -> type != Order.class); + + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + assertSame(Order.class, OgnlRuntime.getInterfaceClass(Order.class)); + assertSame(Customer.class, OgnlRuntime.getInterfaceClass(Customer.class)); + assertSame(Customer.class, OgnlRuntime.getInterfaceClass(Customer.class)); + + assertEquals(2, compiler.calls(Order.class)); + assertEquals(1, compiler.calls(Customer.class)); + } + + @Test + void clearCacheDropsInterfaceClasses() { + OgnlRuntime.getInterfaceClass(Order.class); + + OgnlRuntime.clearCache(); + OgnlRuntime.getInterfaceClass(Order.class); + + assertEquals(2, compiler.calls(Order.class)); + } + + @Test + void clearAdditionalCacheDropsInterfaceClasses() { + OgnlRuntime.getInterfaceClass(Order.class); + + OgnlRuntime.clearAdditionalCache(); + OgnlRuntime.getInterfaceClass(Order.class); + + assertEquals(2, compiler.calls(Order.class)); + } + +}