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 2015/03/13 08:14:40 UTC

svn commit: r1666360 - in /sling/trunk/bundles/extensions/serviceusermapper/src: main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java

Author: cziegeler
Date: Fri Mar 13 07:14:40 2015
New Revision: 1666360

URL: http://svn.apache.org/r1666360
Log:
SLING-4495 : Improve ServiceUserMapperImpl#isValidUser 

Modified:
    sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
    sling/trunk/bundles/extensions/serviceusermapper/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java

Modified: sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java?rev=1666360&r1=1666359&r2=1666360&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java (original)
+++ sling/trunk/bundles/extensions/serviceusermapper/src/main/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImpl.java Fri Mar 13 07:14:40 2015
@@ -19,7 +19,6 @@
 package org.apache.sling.serviceusermapping.impl;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Dictionary;
 import java.util.HashMap;
@@ -27,15 +26,15 @@ import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Vector;
 import java.util.SortedMap;
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.apache.felix.scr.annotations.Activate;
-import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Modified;
 import org.apache.felix.scr.annotations.Property;
 import org.apache.felix.scr.annotations.PropertyUnbounded;
@@ -105,7 +104,7 @@ public class ServiceUserMapperImpl imple
 
     private Mapping[] activeMappings = new Mapping[0];
 
-    private Vector <ServiceUserValidator> validators = new Vector<ServiceUserValidator>();
+    private final List<ServiceUserValidator> validators = new CopyOnWriteArrayList<ServiceUserValidator>();
 
     private SortedMap<Mapping, ServiceRegistration> activeMappingRegistrations = new TreeMap<Mapping, ServiceRegistration>();
 
@@ -144,22 +143,20 @@ public class ServiceUserMapperImpl imple
             bundleContext = null;
         }
     }
-    
+
     /**
      * bind the serviceUserValidator
      * @param serviceUserValidator
-     * @param properties
      */
-    protected void bindServiceUserValidator(final ServiceUserValidator serviceUserValidator, final Map<String, Object> properties){
+    protected void bindServiceUserValidator(final ServiceUserValidator serviceUserValidator) {
         validators.add(serviceUserValidator);
     }
-    
+
     /**
      * unbind the serviceUserValidator
      * @param serviceUserValidator
-     * @param properties
      */
-    protected void unbindServiceUserValidator(final ServiceUserValidator serviceUserValidator, final Map<String, Object> properties){
+    protected void unbindServiceUserValidator(final ServiceUserValidator serviceUserValidator) {
         validators.remove(serviceUserValidator);
     }
 
@@ -211,27 +208,24 @@ public class ServiceUserMapperImpl imple
             }
         }
 
-
         activeMappings = mappings.toArray(new Mapping[mappings.size()]);
 
         updateServiceMappings(mappings);
-
     }
 
 
-    void updateServiceMappings(List<Mapping> newMappings) {
-        
+    void updateServiceMappings(final List<Mapping> newMappings) {
+
         // do not do anything if not activated
         if (bundleContext == null) {
             return;
         }
 
-        SortedSet<Mapping> orderedActiveMappings = new TreeSet<Mapping>(newMappings);
-
+        final SortedSet<Mapping> orderedActiveMappings = new TreeSet<Mapping>(newMappings);
 
-        Iterator<Map.Entry<Mapping, ServiceRegistration>> it = activeMappingRegistrations.entrySet().iterator();
+        final Iterator<Map.Entry<Mapping, ServiceRegistration>> it = activeMappingRegistrations.entrySet().iterator();
         while (it.hasNext()) {
-            Map.Entry<Mapping, ServiceRegistration> registrationEntry = it.next();
+            final Map.Entry<Mapping, ServiceRegistration> registrationEntry = it.next();
 
             if (!orderedActiveMappings.contains(registrationEntry.getKey())) {
                 registrationEntry.getValue().unregister();
@@ -240,24 +234,24 @@ public class ServiceUserMapperImpl imple
         }
 
 
-        for (Mapping mapping: orderedActiveMappings) {
+        for (final Mapping mapping: orderedActiveMappings) {
             if (!activeMappingRegistrations.containsKey(mapping)) {
-                Dictionary<String, Object> properties = new Hashtable<String, Object>();
+                final Dictionary<String, Object> properties = new Hashtable<String, Object>();
                 if (mapping.getSubServiceName() != null) {
                     properties.put(ServiceUserMapped.SUBSERVICENAME, mapping.getSubServiceName());
                 }
 
                 properties.put(Mapping.SERVICENAME, mapping.getServiceName());
-                ServiceRegistration registration = bundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
+                final ServiceRegistration registration = bundleContext.registerService(ServiceUserMappedImpl.SERVICEUSERMAPPED,
                         new ServiceUserMappedImpl(), properties);
                 activeMappingRegistrations.put(mapping, registration);
             }
         }
     }
 
-    private String internalGetUserId(String serviceName, String subServiceName) {
+    private String internalGetUserId(final String serviceName, final String subServiceName) {
         // try with serviceInfo first
-        for (Mapping mapping : this.activeMappings) {
+        for (final Mapping mapping : this.activeMappings) {
             final String userId = mapping.map(serviceName, subServiceName);
             if (userId != null) {
                 return userId;
@@ -276,22 +270,22 @@ public class ServiceUserMapperImpl imple
         return this.defaultUser;
     }
 
-    private boolean isValidUser(String userId, String serviceName, String subServiceName) {
+    private boolean isValidUser(final String userId, final String serviceName, final String subServiceName) {
         if (userId == null) {
             return false;
         }
-        if (validators != null && validators.size() > 0) {
-            for (ServiceUserValidator validator : validators) {
-                boolean valid = validator.isValid(userId, serviceName, subServiceName);
-                if (!valid) {
-                    return false;
+        if ( !validators.isEmpty() ) {
+            for (final ServiceUserValidator validator : validators) {
+                if ( validator.isValid(userId, serviceName, subServiceName) ) {
+                    return true;
                 }
             }
+            return false;
         }
         return true;
     }
 
-    static String getServiceName(Bundle bundle) {
+    static String getServiceName(final Bundle bundle) {
         return bundle.getSymbolicName();
     }
 }

Modified: sling/trunk/bundles/extensions/serviceusermapper/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/serviceusermapper/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java?rev=1666360&r1=1666359&r2=1666360&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/serviceusermapper/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java (original)
+++ sling/trunk/bundles/extensions/serviceusermapper/src/test/java/org/apache/sling/serviceusermapping/impl/ServiceUserMapperImplTest.java Fri Mar 13 07:14:40 2015
@@ -18,6 +18,10 @@
  */
 package org.apache.sling.serviceusermapping.impl;
 
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.Map;
@@ -34,10 +38,6 @@ import org.osgi.framework.Constants;
 import org.osgi.framework.ServiceReference;
 import org.osgi.framework.ServiceRegistration;
 
-import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 public class ServiceUserMapperImplTest {
     private static final String BUNDLE_SYMBOLIC1 = "bundle1";
 
@@ -96,7 +96,7 @@ public class ServiceUserMapperImplTest {
         TestCase.assertEquals(SAMPLE_SUB, sum.getServiceUserID(BUNDLE1, SUB));
         TestCase.assertEquals(ANOTHER_SUB, sum.getServiceUserID(BUNDLE2, SUB));
     }
-    
+
     @Test
     public void test_getServiceUserID_WithServiceUserValidator() {
         @SuppressWarnings("serial")
@@ -115,7 +115,7 @@ public class ServiceUserMapperImplTest {
         final ServiceUserMapperImpl sum = new ServiceUserMapperImpl();
         sum.configure(null, config);
         ServiceUserValidator serviceUserValidator = new ServiceUserValidator() {
-            
+
             public boolean isValid(String serviceUserId, String serviceName,
                     String subServiceName) {
                 if (SAMPLE.equals(serviceUserId)) {
@@ -124,7 +124,7 @@ public class ServiceUserMapperImplTest {
                 return true;
             }
         };
-        sum.bindServiceUserValidator(serviceUserValidator, null);
+        sum.bindServiceUserValidator(serviceUserValidator);
 
         TestCase.assertEquals(null, sum.getServiceUserID(BUNDLE1, null));
         TestCase.assertEquals(ANOTHER, sum.getServiceUserID(BUNDLE2, null));