You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2021/12/03 14:28:57 UTC

[sling-org-apache-sling-models-impl] branch feature/SLING-5762-adapt-value-maps-and-resource-paths-from-requests created (now 964334d)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a change to branch feature/SLING-5762-adapt-value-maps-and-resource-paths-from-requests
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-impl.git.


      at 964334d  SLING-5762 inject value map properties and resource paths from SlingHttpServletRequest

This branch includes the following new commits:

     new 964334d  SLING-5762 inject value map properties and resource paths from SlingHttpServletRequest

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[sling-org-apache-sling-models-impl] 01/01: SLING-5762 inject value map properties and resource paths from SlingHttpServletRequest

Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch feature/SLING-5762-adapt-value-maps-and-resource-paths-from-requests
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-impl.git

commit 964334dc384b43dfda694ad213794fc57ce4af9f
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Fri Dec 3 15:28:46 2021 +0100

    SLING-5762 inject value map properties and resource paths from
    SlingHttpServletRequest
    
    This is internally using the request's resource value map
---
 .../models/impl/injectors/AbstractInjector.java    | 22 ++++++++++--
 .../impl/injectors/ResourcePathInjector.java       |  1 +
 .../models/impl/injectors/ValueMapInjector.java    |  7 +---
 .../models/impl/ResourcePathInjectionTest.java     | 39 +++++++++++++---------
 .../classes/ResourcePathAllOptionalModel.java      |  6 ++--
 5 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java b/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
index 8ef3797..cce4938 100644
--- a/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
+++ b/src/main/java/org/apache/sling/models/impl/injectors/AbstractInjector.java
@@ -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;
         }
diff --git a/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java b/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
index 9bfe445..f49a1d7 100644
--- a/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
+++ b/src/main/java/org/apache/sling/models/impl/injectors/ResourcePathInjector.java
@@ -80,6 +80,7 @@ public class ResourcePathInjector extends AbstractInjector implements Injector,
 
         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);
diff --git a/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java b/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
index 06e91b1..fc3791b 100644
--- a/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
+++ b/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
@@ -178,12 +178,7 @@ public class ValueMapInjector extends AbstractInjector implements Injector, Inje
             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
diff --git a/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java b/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
index fca062f..73d1d66 100644
--- a/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
+++ b/src/test/java/org/apache/sling/models/impl/ResourcePathInjectionTest.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doReturn;
 
 import java.util.HashMap;
 import java.util.List;
@@ -52,10 +53,10 @@ public class ResourcePathInjectionTest {
     private ModelAdapterFactory factory;
 
     @Mock
-    private Resource adaptable;
+    private Resource adaptableResource;
 
     @Mock
-    SlingHttpServletRequest nonResourceAdaptable;
+    SlingHttpServletRequest adaptableRequest;
 
     @Mock
     private Resource byPathResource;
@@ -90,14 +91,17 @@ public class ResourcePathInjectionTest {
 
         ValueMap properties = new ValueMapDecorator(map);
 
-        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 = AdapterFactoryTest.createModelAdapterFactory();
         factory.bindInjector(new SelfInjector(), new ServicePropertiesMap(1, Integer.MAX_VALUE));
         factory.bindInjector(new ValueMapInjector(), new ServicePropertiesMap(2, 2000));
@@ -108,8 +112,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());
@@ -118,15 +122,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
@@ -138,7 +147,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);
@@ -164,7 +173,7 @@ 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);
     }
 
@@ -173,13 +182,13 @@ public class ResourcePathInjectionTest {
         lenient().when(resourceResolver.getResource("/some/other/path")).thenReturn(null);
         lenient().when(resourceResolver.getResource("/some/other/path2")).thenReturn(null);
 
-        ResourcePathPartialModel model = factory.getAdapter(adaptable, ResourcePathPartialModel.class);
+        ResourcePathPartialModel model = factory.getAdapter(adaptableResource, ResourcePathPartialModel.class);
         assertNull(model);
     }
 
     @Test
     public void TestWithArrayWrapping() {
-        ResourcePathModelWrapping model = factory.getAdapter(adaptable, ResourcePathModelWrapping.class);
+        ResourcePathModelWrapping model = factory.getAdapter(adaptableResource, ResourcePathModelWrapping.class);
         assertNotNull(model);
         assertTrue(model.getFromPath().length > 0);
         assertTrue(model.getMultipleResources().length > 0);
diff --git a/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java b/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
index a7dcb62..38fc7f9 100644
--- a/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
+++ b/src/test/java/org/apache/sling/models/testmodels/classes/ResourcePathAllOptionalModel.java
@@ -32,7 +32,7 @@ import org.apache.sling.models.annotations.injectorspecific.ResourcePath;
 public class ResourcePathAllOptionalModel {
 
     @Inject
-    @Path("/some/path")
+    @Path("/some/invalidpath")
     @Optional
     private Resource fromPath;
 
@@ -42,11 +42,11 @@ public class ResourcePathAllOptionalModel {
     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)