You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2014/01/17 08:25:13 UTC

svn commit: r1559036 - in /sling/trunk/bundles/resourceresolver/src: main/java/org/apache/sling/resourceresolver/impl/ main/java/org/apache/sling/resourceresolver/impl/tree/ test/java/org/apache/sling/resourceresolver/impl/ test/java/org/apache/sling/r...

Author: cziegeler
Date: Fri Jan 17 07:25:12 2014
New Revision: 1559036

URL: http://svn.apache.org/r1559036
Log:
SLING-2698 - resource access security service for resource providers. Avoid unnecessary null checks

Modified:
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/tree/ProviderHandler.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecoratorTestBase.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverMangleNamespacesTest.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/SortedProviderListTest.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/tree/ResourceProviderEntryTest.java

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java Fri Jan 17 07:25:12 2014
@@ -217,7 +217,7 @@ public class ResourceResolverFactoryActi
     private ServiceUserMapper serviceUserMapper;
 
     @Reference
-    private ResourceAccessSecurityTracker resourceAccessSecurityTracker;
+    ResourceAccessSecurityTracker resourceAccessSecurityTracker;
 
     /** ComponentContext */
     private volatile ComponentContext componentContext;

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/tree/ProviderHandler.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/tree/ProviderHandler.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/tree/ProviderHandler.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/tree/ProviderHandler.java Fri Jan 17 07:25:12 2014
@@ -119,18 +119,16 @@ public abstract class ProviderHandler im
 
         Resource returnValue = null;
 
-        if (useResourceAccessSecurity && returnValue != null) {
-            if ( tracker != null ) {
-                final ResourceAccessSecurity resourceAccessSecurity = tracker.getProviderResourceAccessSecurity();
-                if (resourceAccessSecurity != null) {
-                    returnValue = resourceAccessSecurity.getReadableResource(resource);
-                }
+        if (useResourceAccessSecurity && resource != null) {
+            final ResourceAccessSecurity resourceAccessSecurity = tracker.getProviderResourceAccessSecurity();
+            if (resourceAccessSecurity != null) {
+                returnValue = resourceAccessSecurity.getReadableResource(resource);
             }
         } else {
             returnValue = resource;
         }
 
-        if ( returnValue != null && tracker != null ) {
+        if ( returnValue != null ) {
             final ResourceAccessSecurity resourceAccessSecurity = tracker.getApplicationResourceAccessSecurity();
             if (resourceAccessSecurity != null) {
                 returnValue = resourceAccessSecurity.getReadableResource(resource);

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java Fri Jan 17 07:25:12 2014
@@ -128,6 +128,7 @@ public class MockedResourceResolverImplT
         Mockito.when(componentContext.getBundleContext()).thenReturn(
             bundleContext);
         activator.eventAdmin = eventAdmin;
+        activator.resourceAccessSecurityTracker = new ResourceAccessSecurityTracker();
 
         activator.bindResourceProvider(resourceProvider,
             buildResourceProviderProperties("org.apache.sling.resourceresolver.impl.DummyTestProvider",

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecoratorTestBase.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecoratorTestBase.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecoratorTestBase.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecoratorTestBase.java Fri Jan 17 07:25:12 2014
@@ -51,8 +51,8 @@ public abstract class ResourceDecoratorT
     protected static final String QUERY_LANGUAGE = "some.funnny.language";
 
     protected abstract Resource wrapResourceForTest(Resource resource);
-    
-    @Before 
+
+    @Before
     public void setup() {
         final ResourceDecorator d = new ResourceDecorator() {
             public Resource decorate(Resource resource) {
@@ -62,14 +62,14 @@ public abstract class ResourceDecoratorT
             public Resource decorate(Resource resource, HttpServletRequest request) {
                 throw new UnsupportedOperationException("Not supposed to be used in these tests");
             }
-            
+
         };
-        
+
         final ResourceDecoratorTracker t = new ResourceDecoratorTracker();
         t.bindResourceDecorator(d, null);
-        
+
         final ResourceProvider provider = new QueriableResourceProvider() {
-            
+
             public Resource getResource(ResourceResolver resourceResolver, HttpServletRequest request, String path) {
                 return getResource(null, path);
             }
@@ -98,11 +98,11 @@ public abstract class ResourceDecoratorT
                 }
                 return children.iterator();
             }
-            
+
             public Iterator<ValueMap> queryResources(ResourceResolver resolver, String query, String language) {
                 return null;
             }
-            
+
             public Iterator<Resource> findResources(ResourceResolver resolver, String query, String language) {
                 final List<Resource> found = new ArrayList<Resource>();
                 found.add(mockResource("/tmp/C"));
@@ -111,16 +111,16 @@ public abstract class ResourceDecoratorT
                 found.add(mockResource("/var/two"));
                 return found.iterator();
             }
-            
+
         };
-        
+
         final RootResourceProviderEntry rrp = new RootResourceProviderEntry();
         final Map<String, Object> props = new HashMap<String, Object>();
         props.put(Constants.SERVICE_ID, System.currentTimeMillis());
         props.put(ResourceProvider.ROOTS, "/");
         props.put(QueriableResourceProvider.LANGUAGES, QUERY_LANGUAGE);
         rrp.bindResourceProvider(provider, props);
-        
+
         final CommonResourceResolverFactoryImpl  crf = new CommonResourceResolverFactoryImpl(new ResourceResolverFactoryActivator()) {
             @Override
             public ResourceDecoratorTracker getResourceDecoratorTracker() {
@@ -132,16 +132,16 @@ public abstract class ResourceDecoratorT
                 return rrp;
             }
         };
-        resolver = new ResourceResolverImpl(crf, new ResourceResolverContext(false, null, null));
+        resolver = new ResourceResolverImpl(crf, new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
     }
-    
+
     protected void assertExistent(Resource r, boolean existent) {
         assertNotNull("Expecting non-null Resource", r);
         assertEquals("Expecting " + (existent ? "existent" : "non-existent") + " resource",
                 existent,
                 !NonExistingResource.RESOURCE_TYPE_NON_EXISTING.equals(r.getResourceType()));
     }
-    
+
     protected Resource mockResource(String path) {
         final Resource result = Mockito.mock(Resource.class);
         Mockito.when(result.getPath()).thenReturn(path);

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java Fri Jan 17 07:25:12 2014
@@ -57,11 +57,11 @@ public class ResourceResolverImplTest {
     @Before public void setup() {
         commonFactory = new CommonResourceResolverFactoryImpl(new ResourceResolverFactoryActivator());
         resFac = new ResourceResolverFactoryImpl(commonFactory, /* TODO: using Bundle */ null, null);
-        resResolver = new ResourceResolverImpl(commonFactory, new ResourceResolverContext(false, null, null));
+        resResolver = new ResourceResolverImpl(commonFactory, new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
     }
 
     @Test public void testClose() throws Exception {
-        final ResourceResolver rr = new ResourceResolverImpl(commonFactory, new ResourceResolverContext(false, null, null));
+        final ResourceResolver rr = new ResourceResolverImpl(commonFactory, new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
         assertTrue(rr.isLive());
         rr.close();
         assertFalse(rr.isLive());
@@ -358,7 +358,7 @@ public class ResourceResolverImplTest {
                     }
 
                 },
-                new ResourceResolverContext(false, null, null));
+                new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
         resolvers.add(resolver);
 
         // the resources to test
@@ -394,7 +394,7 @@ public class ResourceResolverImplTest {
                     }
 
                 },
-                new ResourceResolverContext(false, null, null));
+                new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
         resolvers.add(resolver);
         final Resource r = new SyntheticResource(resolver, "/a", "a:b") {
             @Override

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverMangleNamespacesTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverMangleNamespacesTest.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverMangleNamespacesTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverMangleNamespacesTest.java Fri Jan 17 07:25:12 2014
@@ -77,7 +77,7 @@ public class ResourceResolverMangleNames
             }
         };
 
-        rr = new ResourceResolverImpl(fac, new ResourceResolverContext(false, null, null));
+        rr = new ResourceResolverImpl(fac, new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker()));
     }
 
     @Test

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/SortedProviderListTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/SortedProviderListTest.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/SortedProviderListTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/SortedProviderListTest.java Fri Jan 17 07:25:12 2014
@@ -36,6 +36,7 @@ import org.apache.sling.api.resource.Res
 import org.apache.sling.api.resource.ResourceProvider;
 import org.apache.sling.api.resource.ResourceProviderFactory;
 import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.resourceresolver.impl.ResourceAccessSecurityTracker;
 import org.apache.sling.resourceresolver.impl.tree.ResourceProviderFactoryHandler;
 import org.apache.sling.resourceresolver.impl.tree.ResourceProviderHandler;
 import org.junit.Test;
@@ -99,7 +100,7 @@ public class SortedProviderListTest {
         final AdaptableResourceProviderImpl rp3 = new AdaptableResourceProviderImpl(new String[] {"/hello"}, 3L);
         final ResourceProviderImpl rp4 = new ResourceProviderImpl(new String[] {"/you"}, 4L);
 
-        final ResourceResolverContext ctx = new ResourceResolverContext(false, null, null);
+        final ResourceResolverContext ctx = new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker());
 
         final SortedProviderList<Adaptable> spl = new SortedProviderList<Adaptable>(Adaptable.class);
         check(spl, ctx);
@@ -131,7 +132,7 @@ public class SortedProviderListTest {
         final AdaptableResourceProviderImpl rp4 = new AdaptableResourceProviderImpl(new String[] {"/a/a"}, 4L);
         final AdaptableResourceProviderImpl rp5 = new AdaptableResourceProviderImpl(new String[] {"/all/or/nothing"}, 5L);
 
-        final ResourceResolverContext ctx = new ResourceResolverContext(false, null, null);
+        final ResourceResolverContext ctx = new ResourceResolverContext(false, null, new ResourceAccessSecurityTracker());
 
         final SortedProviderList<Adaptable> spl = new SortedProviderList<Adaptable>(Adaptable.class);
         check(spl, ctx);

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/tree/ResourceProviderEntryTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/tree/ResourceProviderEntryTest.java?rev=1559036&r1=1559035&r2=1559036&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/tree/ResourceProviderEntryTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/tree/ResourceProviderEntryTest.java Fri Jan 17 07:25:12 2014
@@ -32,6 +32,7 @@ import org.apache.sling.api.resource.Res
 import org.apache.sling.api.resource.ResourceProvider;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.SyntheticResource;
+import org.apache.sling.resourceresolver.impl.ResourceAccessSecurityTracker;
 import org.apache.sling.resourceresolver.impl.helper.ResourceResolverContext;
 import org.junit.Before;
 import org.junit.Test;
@@ -56,7 +57,7 @@ public class ResourceProviderEntryTest {
     @Test public void testRootProvider() {
         assertNull(root.getResource(null, null, "relpath"));
         final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
-        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
         assertEqualsResolver(this.rootResolver, root.getResource(ctx, null, "/"));
         assertEqualsResolver(this.rootResolver, root.getResource(ctx, null, "/rootel"));
         assertEqualsResolver(this.rootResolver, root.getResource(ctx, null, "/rootel/child"));
@@ -68,7 +69,7 @@ public class ResourceProviderEntryTest {
     @Test public void testAdd1Provider() {
         String firstPath = "/rootel";
         final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
-        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
         final ResourceResolver resolver = Mockito.mock(ResourceResolver.class);
         final ResourceProvider first = Mockito.mock(ResourceProvider.class);
         Mockito.when(first.getResource(Mockito.any(ResourceResolver.class), Mockito.startsWith(firstPath))).thenReturn(new TestResource(resolver));
@@ -95,7 +96,7 @@ public class ResourceProviderEntryTest {
         String secondPath = firstPath + "/child";
 
         final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
-        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
         final ResourceResolver firstResolver = Mockito.mock(ResourceResolver.class);
         final ResourceProvider first = Mockito.mock(ResourceProvider.class);
         Mockito.when(first.getResource(Mockito.any(ResourceResolver.class), Mockito.startsWith(firstPath))).thenReturn(new TestResource(firstResolver));
@@ -135,7 +136,7 @@ public class ResourceProviderEntryTest {
         String secondPath = firstPath + "/child";
 
         final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
-        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
         final ResourceResolver firstResolver = Mockito.mock(ResourceResolver.class);
         final ResourceProvider first = Mockito.mock(ResourceProvider.class);
         Mockito.when(first.getResource(Mockito.any(ResourceResolver.class), Mockito.startsWith(firstPath))).thenReturn(new TestResource(firstResolver));
@@ -175,7 +176,7 @@ public class ResourceProviderEntryTest {
         String secondPath = firstPath + "/child";
 
         final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
-        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+        Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
         final ResourceResolver firstResolver = Mockito.mock(ResourceResolver.class);
         final ResourceProvider first = Mockito.mock(ResourceProvider.class);
         Mockito.when(first.getResource(Mockito.any(ResourceResolver.class), Mockito.startsWith(firstPath))).thenReturn(new TestResource(firstResolver));
@@ -224,7 +225,7 @@ public class ResourceProviderEntryTest {
             final ResourceProvider p = Mockito.mock(ResourceProvider.class);
             final ResourceResolverContext ctx = Mockito.mock(ResourceResolverContext.class);
             Mockito.when(p.getResource(Mockito.any(ResourceResolver.class), Mockito.startsWith(path))).thenReturn(new TestResource(resolver));
-            Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(null);
+            Mockito.when(ctx.getResourceAccessSecurityTracker()).thenReturn(new ResourceAccessSecurityTracker());
 
             final Map<String, Object> props = new HashMap<String, Object>();
             props.put(Constants.SERVICE_ID, ++counter);