Author: kwin
Date: Fri May 13 10:35:03 2016
New Revision: 1743648

URL: http://svn.apache.org/viewvc?rev=1743648&view=rev
Log:
SLING-5726 allow both ValueMapInjector and ResourcePathInjector to act on 
adaptable SlingHttpServletRequest in all circumstances

Modified:
    sling/site/trunk/content/documentation/bundles/models.mdtext
    
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
    
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
    
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
    
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
    
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java

Modified: sling/site/trunk/content/documentation/bundles/models.mdtext
URL: 
http://svn.apache.org/viewvc/sling/site/trunk/content/documentation/bundles/models.mdtext?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- sling/site/trunk/content/documentation/bundles/models.mdtext (original)
+++ sling/site/trunk/content/documentation/bundles/models.mdtext Fri May 13 
10:35:03 2016
@@ -366,7 +366,7 @@ In addition all [injector-specific annot
 Title              |  Name (for `@Source`)   | Service Ranking     | Available 
Since (Implementation Version) | Description  | Applicable To (including using 
`@Via`) | Accepts Null Name? | Array Support | Parameterized Type Support
 -----------------  | ----------------------- | ------------------- | 
---------------------------------------- | ------------ | 
-------------------------------------- | ------------------ | ------------- | 
--------------------------
 Script Bindings    | `script-bindings`       | 1000                | 1.0.0     
                               | Lookup objects in the script bindings object 
by name. | A ServletRequest object which has the `Sling Bindings` attribute 
defined | no | no conversion is done | If a parameterized type is passed, the 
bindings value must be of a compatible type of the parameterized type.
-Value Map          | `valuemap`              | 2000                | 1.0.0     
                               | Gets a property from a `ValueMap` by name. | 
Any object which is or can be adapted to a `ValueMap`| no | Primitive arrays 
wrapped/unwrapped as necessary. Wrapper object arrays are unwrapped/wrapped as 
necessary. | Parameterized `List` and `Collection` injection points are 
injected by getting an array of the component type and creating an unmodifiable 
`List` from the array.
+Value Map          | `valuemap`              | 2000                | 1.0.0     
                               | Gets a property from a `ValueMap` by name. | 
Any object which is or can be adapted to a `ValueMap`. Since version 
[1.2.10](https://issues.apache.org/jira/browse/SLING-5726) also 
`SlingHttpServletRequest`. | no | Primitive arrays wrapped/unwrapped as 
necessary. Wrapper object arrays are unwrapped/wrapped as necessary. | 
Parameterized `List` and `Collection` injection points are injected by getting 
an array of the component type and creating an unmodifiable `List` from the 
array.
 Child Resources    | `child-resources`       | 3000                | 1.0.0     
                               | Gets a child resource by name. | `Resource` 
objects | no | none | if a parameterized type is passed, a `List<Resource>` is 
returned (the contents of which may be adapted to the target type).
 Request Attributes | `request-attributes`    | 4000                | 1.0.0     
                               | Get a request attribute by name. | 
`ServletRequest` objects | no | no conversion is done | If a parameterized type 
is passed, the request attribute must be of a compatible type of the 
parameterized type.
 OSGi Services      | `osgi-services`         | 5000                | 1.0.0     
                               | Lookup services based on class name. Since 
Sling Models Impl 1.2.8 
([SLING-5664](https://issues.apache.org/jira/browse/SLING-5664)) the service 
with the highest service ranking is returned. In case multiple services are 
returned, they are ordered descending by their service ranking (i.e. the one 
with the highest ranking first). | Any object | yes | yes | Parameterized 
`List` and `Collection` injection points are injected by getting an array of 
the services and creating an unmodifiable `List` from the array.

Modified: 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
 (original)
+++ 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
 Fri May 13 10:35:03 2016
@@ -44,12 +44,30 @@ abstract class AbstractInjector {
         return resolver;
     }
 
+    /** 
+     * Retrieve the ValueMap from the given adaptable. This succeeds, if the 
adaptable is either
+     * <ul>
+     * <li>a {@link ValueMap},</li>
+     * <li>a {@link SlingHttpServletRequest}, in which case the returned 
{@link ValueMap} is the one derived from the request's resource or</li>
+     * <li>adaptable to a {@link ValueMap}.</li>
+     * </ul>
+     * Otherwise {@code null} is returned.
+     * @param adaptable
+     * @return a ValueMap or {@code null}.
+     */
     protected ValueMap getValueMap(Object adaptable) {
         if (adaptable instanceof ValueMap) {
             return (ValueMap) adaptable;
+        } else if (adaptable instanceof SlingHttpServletRequest) {
+            final Resource resource = 
((SlingHttpServletRequest)adaptable).getResource();
+            // resource may be null for mocked adaptables, therefore do a 
check here
+            if (resource != null) {
+                return resource.adaptTo(ValueMap.class);
+            } else {
+                return null;
+            }
         } else if (adaptable instanceof Adaptable) {
-            ValueMap map = ((Adaptable) adaptable).adaptTo(ValueMap.class);
-            return map;
+            return ((Adaptable) adaptable).adaptTo(ValueMap.class);
         } else {
             return null;
         }

Modified: 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
 (original)
+++ 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
 Fri May 13 10:35:03 2016
@@ -84,6 +84,7 @@ public class ResourcePathInjector extend
 
         ResourceResolver resolver = getResourceResolver(adaptable);
         if (resolver == null) {
+            LOG.debug("Could not get resolver from adaptable {}", adaptable);
             return null;
         }
         List<Resource> resources = getResources(resolver, resourcePaths, name);

Modified: 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
 (original)
+++ 
sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
 Fri May 13 10:35:03 2016
@@ -169,12 +169,7 @@ public class ValueMapInjector extends Ab
             if (StringUtils.isNotBlank(annotation.via())) {
                 return annotation.via();
             }
-            // automatically go via resource, if this is the httprequest
-            if (adaptable instanceof SlingHttpServletRequest) {
-                return "resource";
-            } else {
-                return null;
-            }
+            return null;
         }
         
         @Override

Modified: 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
 (original)
+++ 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
 Fri May 13 10:35:03 2016
@@ -57,10 +57,10 @@ public class ResourcePathInjectionTest {
     private ModelAdapterFactory factory;
 
     @Mock
-    private Resource adaptable;
+    private Resource adaptableResource;
     
     @Mock
-    SlingHttpServletRequest nonResourceAdaptable;
+    SlingHttpServletRequest adaptableRequest;
 
     @Mock
     private Resource byPathResource;
@@ -98,13 +98,16 @@ public class ResourcePathInjectionTest {
         when(componentCtx.getBundleContext()).thenReturn(bundleContext);
         when(componentCtx.getProperties()).thenReturn(new Hashtable<String, 
Object>());
 
-        when(adaptable.getResourceResolver()).thenReturn(resourceResolver);
-        when(adaptable.adaptTo(ValueMap.class)).thenReturn(properties);
+        
when(adaptableResource.getResourceResolver()).thenReturn(resourceResolver);
+        when(adaptableResource.adaptTo(ValueMap.class)).thenReturn(properties);
 
         
when(resourceResolver.getResource("/some/path")).thenReturn(byPathResource);
         
when(resourceResolver.getResource("/some/path2")).thenReturn(byPathResource2);
         
when(resourceResolver.getResource("/some/other/path")).thenReturn(byPropertyValueResource);
         
when(resourceResolver.getResource("/some/other/path2")).thenReturn(byPropertyValueResource2);
+        
+        when(adaptableRequest.getResource()).thenReturn(byPathResource);
+        
when(adaptableRequest.getResourceResolver()).thenReturn(resourceResolver);
 
         factory = new ModelAdapterFactory();
         factory.activate(componentCtx);
@@ -116,8 +119,8 @@ public class ResourcePathInjectionTest {
     }
 
     @Test
-    public void testPathInjection() {
-        ResourcePathModel model = factory.getAdapter(adaptable, 
ResourcePathModel.class);
+    public void testPathInjectionFromResource() {
+        ResourcePathModel model = factory.getAdapter(adaptableResource, 
ResourcePathModel.class);
         assertNotNull(model);
         assertEquals(byPathResource, model.getFromPath());
         assertEquals(byPropertyValueResource, model.getByDerefProperty());
@@ -126,15 +129,20 @@ public class ResourcePathInjectionTest {
     }
 
     @Test
-    public void testPathInjectionWithNonResourceAdaptable() {
-        ResourcePathModel model = factory.getAdapter(nonResourceAdaptable, 
ResourcePathModel.class);
-        // should be null because mandatory fields could not be injected
-        assertNull(model);
+    public void testPathInjectionFromRequest() {
+        // return the same properties through this request's resource, as 
through adaptableResource
+        
doReturn(adaptableResource.adaptTo(ValueMap.class)).when(byPathResource).adaptTo(ValueMap.class);
+        ResourcePathModel model = factory.getAdapter(adaptableRequest, 
ResourcePathModel.class);
+        assertNotNull(model);
+        assertEquals(byPathResource, model.getFromPath());
+        assertEquals(byPropertyValueResource, model.getByDerefProperty());
+        assertEquals(byPathResource2, model.getFromPath2());
+        assertEquals(byPropertyValueResource2, model.getByDerefProperty2());
     }
 
     @Test
     public void testOptionalPathInjectionWithNonResourceAdaptable() {
-        ResourcePathAllOptionalModel model = 
factory.getAdapter(nonResourceAdaptable, ResourcePathAllOptionalModel.class);
+        ResourcePathAllOptionalModel model = 
factory.getAdapter(adaptableRequest, ResourcePathAllOptionalModel.class);
         // should not be null because resource paths fields are optional
         assertNotNull(model);
         // but the field itself are null
@@ -146,7 +154,7 @@ public class ResourcePathInjectionTest {
 
     @Test
     public void testMultiplePathInjection() {
-        ResourcePathModel model = factory.getAdapter(adaptable, 
ResourcePathModel.class);
+        ResourcePathModel model = factory.getAdapter(adaptableResource, 
ResourcePathModel.class);
         assertNotNull(model);
         List<Resource> resources=model.getMultipleResources();
         assertNotNull(resources);
@@ -172,16 +180,16 @@ public class ResourcePathInjectionTest {
     public void testPartialInjectionFailure1() {
         
when(resourceResolver.getResource("/some/other/path")).thenReturn(null);
         
-        ResourcePathPartialModel model = factory.getAdapter(adaptable, 
ResourcePathPartialModel.class);
+        ResourcePathPartialModel model = factory.getAdapter(adaptableResource, 
ResourcePathPartialModel.class);
         assertNull(model);
     }
 
     @Test
-    public void testPartialInjectionFailure2() {       
+    public void testPartialInjectionFailure2() {
         
when(resourceResolver.getResource("/some/other/path")).thenReturn(null);
         
when(resourceResolver.getResource("/some/other/path2")).thenReturn(null);
         
-        ResourcePathPartialModel model = factory.getAdapter(adaptable, 
ResourcePathPartialModel.class);
+        ResourcePathPartialModel model = factory.getAdapter(adaptableResource, 
ResourcePathPartialModel.class);
         assertNull(model);
     }
 

Modified: 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java?rev=1743648&r1=1743647&r2=1743648&view=diff
==============================================================================
--- 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
 (original)
+++ 
sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
 Fri May 13 10:35:03 2016
@@ -32,7 +32,7 @@ import org.apache.sling.models.annotatio
 public class ResourcePathAllOptionalModel {
 
     @Inject
-    @Path("/some/path")
+    @Path("/some/invalidpath")
     @Optional
     private Resource fromPath;
 
@@ -42,11 +42,11 @@ public class ResourcePathAllOptionalMode
     private Resource derefProperty;
     
     @Inject
-    @Path(paths={"/some/path", "/some/path2"})
+    @Path(paths={"/some/invalidpath", "/some/invalidpath2"})
     @Optional
     private Resource manyFromPathNonList;
 
-    @ResourcePath(path = "/some/path2", optional=true)
+    @ResourcePath(path = "/some/invalidpath2", optional=true)
     private Resource fromPath2;
 
     @ResourcePath(name = "anotherPropertyContainingAPath", optional=true)


Reply via email to