SLING-6318 - internal optimization to avoid repeated retrieval of ValueMap from Resource
authorJustin Edelson <justin@apache.org>
Wed, 30 Nov 2016 17:09:57 +0000 (17:09 +0000)
committerJustin Edelson <justin@apache.org>
Wed, 30 Nov 2016 17:09:57 +0000 (17:09 +0000)
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1772085 13f79535-47bb-0310-9956-ffa450edef68

src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
src/main/java/org/apache/sling/models/impl/injectors/BindingsInjector.java
src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
src/main/java/org/apache/sling/models/impl/injectors/ValuePreparer.java [new file with mode: 0644]
src/test/java/org/apache/sling/models/impl/RequestInjectionTest.java
src/test/java/org/apache/sling/models/impl/ResourceModelClassesTest.java
src/test/java/org/apache/sling/models/impl/ResourceModelInterfacesTest.java
src/test/java/org/apache/sling/models/testmodels/classes/BindingsModel.java
src/test/java/org/apache/sling/models/testmodels/classes/constructorinjection/BindingsModel.java

index 9602799..5fdf72c 100644 (file)
@@ -43,6 +43,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.PostConstruct;
 
 import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -73,6 +74,7 @@ import org.apache.sling.models.factory.ModelClassException;
 import org.apache.sling.models.factory.ModelFactory;
 import org.apache.sling.models.factory.PostConstructException;
 import org.apache.sling.models.factory.ValidationException;
+import org.apache.sling.models.impl.injectors.ValuePreparer;
 import org.apache.sling.models.impl.model.ConstructorParameter;
 import org.apache.sling.models.impl.model.InjectableElement;
 import org.apache.sling.models.impl.model.InjectableField;
@@ -109,6 +111,9 @@ import org.slf4j.LoggerFactory;
 @SuppressWarnings("deprecation")
 public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFactory {
 
+    // hard code this value since we always know exactly how many there are
+    private static final int VALUE_PREPARERS_COUNT = 2;
+
     private static class DisposalCallbackRegistryImpl implements DisposalCallbackRegistry {
 
         private List<DisposalCallback> callbacks = new ArrayList<DisposalCallback>();
@@ -402,7 +407,8 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
     private
     @CheckForNull
     RuntimeException injectElement(final InjectableElement element, final Object adaptable,
-                                   final @Nonnull DisposalCallbackRegistry registry, final InjectCallback callback) {
+                                   final @Nonnull DisposalCallbackRegistry registry, final InjectCallback callback,
+                                   final @Nonnull Map<ValuePreparer, Object> preparedValues) {
 
         InjectAnnotationProcessor annotationProcessor = null;
         String source = element.getSource();
@@ -425,7 +431,7 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
         }
 
         String name = getName(element, annotationProcessor);
-        Object injectionAdaptable = getAdaptable(adaptable, element, annotationProcessor);
+        final Object injectionAdaptable = getAdaptable(adaptable, element, annotationProcessor);
 
         RuntimeException lastInjectionException = null;
         if (injectionAdaptable != null) {
@@ -445,7 +451,29 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
             // find the right injector
             for (Injector injector : injectorsToProcess) {
                 if (name != null || injector instanceof AcceptsNullName) {
-                    Object value = injector.getValue(injectionAdaptable, name, element.getType(), element.getAnnotatedElement(), registry);
+                    Object preparedValue = injectionAdaptable;
+
+                    // only do the ValuePreparer optimization for the original adaptable
+                    if (injector instanceof ValuePreparer && adaptable == injectionAdaptable) {
+                        final ValuePreparer preparer = (ValuePreparer) injector;
+                        Object fromMap = preparedValues.get(preparer);
+                        if (fromMap != null) {
+                            if (ObjectUtils.NULL.equals(fromMap)) {
+                                preparedValue = null;
+                            } else {
+                                preparedValue = fromMap;
+                            }
+                        } else {
+                            preparedValue = preparer.prepareValue(injectionAdaptable);
+                            if (preparedValue == null) {
+                                preparedValues.put(preparer, ObjectUtils.NULL);
+                            } else {
+                                preparedValues.put(preparer, preparedValue);
+                            }
+                        }
+                    }
+
+                    Object value = injector.getValue(preparedValue, name, element.getType(), element.getAnnotatedElement(), registry);
                     if (value != null) {
                         lastInjectionException = callback.inject(element, value);
                         if (lastInjectionException == null) {
@@ -503,9 +531,11 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
         DisposalCallbackRegistryImpl registry = new DisposalCallbackRegistryImpl();
         registerCallbackRegistry(handler, registry);
 
+        final Map<ValuePreparer, Object> preparedValues = new HashMap<ValuePreparer, Object>(VALUE_PREPARERS_COUNT);
+
         MissingElementsException missingElements = new MissingElementsException("Could not create all mandatory methods for interface of model " + modelClass);
         for (InjectableMethod method : injectableMethods) {
-            RuntimeException t = injectElement(method, adaptable, registry, callback);
+            RuntimeException t = injectElement(method, adaptable, registry, callback, preparedValues);
             if (t != null) {
                 missingElements.addMissingElementExceptions(new MissingElementException(method.getAnnotatedElement(), t));
             }
@@ -531,6 +561,8 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
             return new Result<ModelType>(new ModelClassException("Unable to find a useable constructor for model " + modelClass.getType()));
         }
 
+        final Map<ValuePreparer, Object> preparedValues = new HashMap<ValuePreparer, Object>(VALUE_PREPARERS_COUNT);
+
         final ModelType object;
         if (constructorToUse.getConstructor().getParameterTypes().length == 0) {
             // no parameters for constructor injection? instantiate it right away
@@ -539,7 +571,7 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
             // instantiate with constructor injection
             // if this fails, make sure resources that may be claimed by injectors are cleared up again
             try {
-                Result<ModelType> result = newInstanceWithConstructorInjection(constructorToUse, adaptable, modelClass, registry);
+                Result<ModelType> result = newInstanceWithConstructorInjection(constructorToUse, adaptable, modelClass, registry, preparedValues);
                 if (!result.wasSuccessful()) {
                     registry.onDisposed();
                     return result;
@@ -564,8 +596,9 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
 
         InjectableField[] injectableFields = modelClass.getInjectableFields();
         MissingElementsException missingElements = new MissingElementsException("Could not inject all required fields into " + modelClass.getType());
+
         for (InjectableField field : injectableFields) {
-            RuntimeException t = injectElement(field, adaptable, registry, callback);
+            RuntimeException t = injectElement(field, adaptable, registry, callback, preparedValues);
             if (t != null) {
                 missingElements.addMissingElementExceptions(new MissingElementException(field.getAnnotatedElement(), t));
             }
@@ -618,7 +651,7 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
     }
 
     private <ModelType> Result<ModelType> newInstanceWithConstructorInjection(final ModelClassConstructor<ModelType> constructor, final Object adaptable,
-            final ModelClass<ModelType> modelClass, final DisposalCallbackRegistry registry)
+            final ModelClass<ModelType> modelClass, final DisposalCallbackRegistry registry, final @Nonnull Map<ValuePreparer, Object> preparedValues)
             throws InstantiationException, InvocationTargetException, IllegalAccessException {
         ConstructorParameter[] parameters = constructor.getConstructorParameters();
 
@@ -627,7 +660,7 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
 
         MissingElementsException missingElements = new MissingElementsException("Required constructor parameters were not able to be injected on model " + modelClass.getType());
         for (int i = 0; i < parameters.length; i++) {
-            RuntimeException t = injectElement(parameters[i], adaptable, registry, callback);
+            RuntimeException t = injectElement(parameters[i], adaptable, registry, callback, preparedValues);
             if (t != null) {
                 missingElements.addMissingElementExceptions(new MissingElementException(parameters[i].getAnnotatedElement(), t));
             }
index e9c76d3..73ab708 100644 (file)
@@ -40,7 +40,7 @@ import org.slf4j.LoggerFactory;
 @Component
 @Service
 @Property(name = Constants.SERVICE_RANKING, intValue = 1000)
-public class BindingsInjector implements Injector, StaticInjectAnnotationProcessorFactory {
+public class BindingsInjector implements Injector, StaticInjectAnnotationProcessorFactory, ValuePreparer {
 
     private static final Logger log = LoggerFactory.getLogger(BindingsInjector.class);
 
@@ -64,7 +64,9 @@ public class BindingsInjector implements Injector, StaticInjectAnnotationProcess
     }
 
     private SlingBindings getBindings(Object adaptable) {
-        if (adaptable instanceof ServletRequest) {
+        if (adaptable instanceof SlingBindings) {
+            return (SlingBindings) adaptable;
+        } else if (adaptable instanceof ServletRequest) {
             ServletRequest request = (ServletRequest) adaptable;
             return (SlingBindings) request.getAttribute(SlingBindings.class.getName());
         } else {
@@ -82,6 +84,11 @@ public class BindingsInjector implements Injector, StaticInjectAnnotationProcess
         return null;
     }
 
+    @Override
+    public Object prepareValue(Object adaptable) {
+        return getBindings(adaptable);
+    }
+
     private static class ScriptVariableAnnotationProcessor extends AbstractInjectAnnotationProcessor2 {
 
         private final ScriptVariable annotation;
index c76a553..68532c9 100644 (file)
@@ -48,7 +48,7 @@ import org.slf4j.LoggerFactory;
 @Service
 @Property(name = Constants.SERVICE_RANKING, intValue = 2000)
 @SuppressWarnings("deprecation")
-public class ValueMapInjector extends AbstractInjector implements Injector, InjectAnnotationProcessorFactory {
+public class ValueMapInjector extends AbstractInjector implements Injector, InjectAnnotationProcessorFactory, ValuePreparer {
 
     private static final Logger log = LoggerFactory.getLogger(ValueMapInjector.class);
 
@@ -134,6 +134,11 @@ public class ValueMapInjector extends AbstractInjector implements Injector, Inje
     }
 
     @Override
+    public Object prepareValue(final Object adaptable) {
+        return getValueMap(adaptable);
+    }
+
+    @Override
     public InjectAnnotationProcessor createAnnotationProcessor(Object adaptable, AnnotatedElement element) {
         // check if the element has the expected annotation
         ValueMapValue annotation = element.getAnnotation(ValueMapValue.class);
diff --git a/src/main/java/org/apache/sling/models/impl/injectors/ValuePreparer.java b/src/main/java/org/apache/sling/models/impl/injectors/ValuePreparer.java
new file mode 100644 (file)
index 0000000..772aafa
--- /dev/null
@@ -0,0 +1,25 @@
+/*
+ * 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 org.apache.sling.models.impl.injectors;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
+public interface ValuePreparer {
+
+    @CheckForNull Object prepareValue(@Nonnull Object adaptable);
+}
index 23b8e35..0fae27d 100644 (file)
@@ -19,6 +19,8 @@ package org.apache.sling.models.impl;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.times;
 
 import java.util.Hashtable;
 
@@ -34,6 +36,7 @@ import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.osgi.framework.BundleContext;
 import org.osgi.service.component.ComponentContext;
+import org.slf4j.LoggerFactory;
 
 @RunWith(MockitoJUnitRunner.class)
 public class RequestInjectionTest {
@@ -59,6 +62,7 @@ public class RequestInjectionTest {
 
         SlingBindings bindings = new SlingBindings();
         bindings.setSling(sling);
+        bindings.setLog(LoggerFactory.getLogger("test"));
         when(request.getAttribute(SlingBindings.class.getName())).thenReturn(bindings);
 
         factory = new ModelAdapterFactory();
@@ -73,6 +77,8 @@ public class RequestInjectionTest {
         BindingsModel model = factory.getAdapter(request, BindingsModel.class);
         assertNotNull(model.getSling());
         assertEquals(sling, model.getSling());
+        assertEquals("test", model.getLog().getName());
+        verify(request, times(1)).getAttribute(SlingBindings.class.getName());
     }
 
     @Test
@@ -81,6 +87,9 @@ public class RequestInjectionTest {
                 = factory.getAdapter(request, org.apache.sling.models.testmodels.classes.constructorinjection.BindingsModel.class);
         assertNotNull(model.getSling());
         assertEquals(sling, model.getSling());
+        assertEquals("test", model.getLog().getName());
+
+        verify(request, times(1)).getAttribute(SlingBindings.class.getName());
     }
 
 }
index 22439f5..b80b684 100644 (file)
@@ -105,6 +105,8 @@ public class ResourceModelClassesTest {
         assertEquals("three", array[0]);
 
         assertTrue(model.isPostConstructCalled());
+
+        verify(res, times(1)).adaptTo(ValueMap.class);
     }
 
     @Test
index c980785..acdbd86 100644 (file)
@@ -87,6 +87,8 @@ public class ResourceModelInterfacesTest {
         assertNull(model.getSecond());
         assertEquals("third-value", model.getThirdProperty());
         assertTrue(model.isFourth());
+
+        verify(res, times(1)).adaptTo(ValueMap.class);
     }
 
     @Test
index 342b9cf..81f1a65 100644 (file)
@@ -22,6 +22,7 @@ import javax.inject.Named;
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.scripting.SlingScriptHelper;
 import org.apache.sling.models.annotations.Model;
+import org.slf4j.Logger;
 
 @Model(adaptables=SlingHttpServletRequest.class)
 public class BindingsModel {
@@ -29,9 +30,16 @@ public class BindingsModel {
     @Inject
     @Named("sling")
     private SlingScriptHelper sling;
+
+    @Inject
+    private Logger log;
     
     public SlingScriptHelper getSling() {
         return sling;
     }
 
+    public Logger getLog() {
+        return log;
+    }
+
 }
index 12e7be7..518f892 100644 (file)
@@ -22,12 +22,17 @@ import javax.inject.Named;
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.scripting.SlingScriptHelper;
 import org.apache.sling.models.annotations.Model;
+import org.slf4j.Logger;
 
 @Model(adaptables = SlingHttpServletRequest.class)
 public class BindingsModel {
 
     private final SlingScriptHelper sling;
 
+
+    @Inject
+    private Logger log;
+
     @Inject
     public BindingsModel(@Named("sling") SlingScriptHelper sling) {
         this.sling = sling;
@@ -37,4 +42,7 @@ public class BindingsModel {
         return sling;
     }
 
+    public Logger getLog() {
+        return log;
+    }
 }
\ No newline at end of file