You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by fm...@apache.org on 2014/01/28 09:21:44 UTC

svn commit: r1561988 - in /sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl: ClientContextImpl.java FeatureManager.java

Author: fmeschbe
Date: Tue Jan 28 08:21:43 2014
New Revision: 1561988

URL: http://svn.apache.org/r1561988
Log:
Simplify feature management and ClientContext setup

Modified:
    sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/ClientContextImpl.java
    sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/FeatureManager.java

Modified: sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/ClientContextImpl.java
URL: http://svn.apache.org/viewvc/sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/ClientContextImpl.java?rev=1561988&r1=1561987&r2=1561988&view=diff
==============================================================================
--- sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/ClientContextImpl.java (original)
+++ sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/ClientContextImpl.java Tue Jan 28 08:21:43 2014
@@ -22,6 +22,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+
 import org.apache.sling.api.resource.ResourceDecorator;
 import org.apache.sling.featureflags.ClientContext;
 import org.apache.sling.featureflags.Feature;
@@ -34,18 +36,22 @@ public class ClientContextImpl implement
 
     private final ExecutionContext featureContext;
 
-    private final List<Feature> enabledFeatures;
+    private final Map<String, Feature> enabledFeatures;
 
-    private final List<ResourceDecorator> mapperFeatures = new ArrayList<ResourceDecorator>();
+    private final List<ResourceDecorator> resourceDecorators;
 
-    public ClientContextImpl(final ExecutionContext featureContext, final List<Feature> features) {
-        this.enabledFeatures = Collections.unmodifiableList(features);
-        for (final Feature f : this.enabledFeatures) {
+    public ClientContextImpl(final ExecutionContext featureContext, final Map<String, Feature> features) {
+        ArrayList<ResourceDecorator> resourceDecorators = new ArrayList<ResourceDecorator>(features.size());
+        for (final Feature f : features.values()) {
             if (f instanceof ResourceDecorator) {
-                mapperFeatures.add((ResourceDecorator) f);
+                resourceDecorators.add((ResourceDecorator) f);
             }
         }
+        resourceDecorators.trimToSize();
+
         this.featureContext = featureContext;
+        this.enabledFeatures = Collections.unmodifiableMap(features);
+        this.resourceDecorators = Collections.unmodifiableList(resourceDecorators);
     }
 
     public ExecutionContext getFeatureContext() {
@@ -54,20 +60,15 @@ public class ClientContextImpl implement
 
     @Override
     public boolean isEnabled(final String featureName) {
-        for(final Feature f : this.enabledFeatures) {
-            if ( featureName.equals(f.getName()) ) {
-                return true;
-            }
-        }
-        return false;
+        return this.enabledFeatures.get(featureName) != null;
     }
 
     @Override
     public Collection<Feature> getEnabledFeatures() {
-        return this.enabledFeatures;
+        return this.enabledFeatures.values();
     }
 
     public List<ResourceDecorator> getResourceDecorators() {
-        return this.mapperFeatures;
+        return this.resourceDecorators;
     }
 }

Modified: sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/FeatureManager.java
URL: http://svn.apache.org/viewvc/sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/FeatureManager.java?rev=1561988&r1=1561987&r2=1561988&view=diff
==============================================================================
--- sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/FeatureManager.java (original)
+++ sling/whiteboard/fmeschbe/featureflags/feature-flags/src/main/java/org/apache/sling/featureflags/impl/FeatureManager.java Tue Jan 28 08:21:43 2014
@@ -25,7 +25,6 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
 
 import javax.servlet.Filter;
 import javax.servlet.Servlet;
@@ -64,9 +63,23 @@ public class FeatureManager {
 
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
 
+    private final ThreadLocal<ClientContext> perThreadClientContext = new ThreadLocal<ClientContext>();
+
+    private final ClientContext defaultClientContext = new ClientContext() {
+        @Override
+        public boolean isEnabled(final String featureName) {
+            return false;
+        }
+
+        @Override
+        public Collection<Feature> getEnabledFeatures() {
+            return Collections.emptyList();
+        }
+    };
+
     private final Map<String, List<FeatureDescription>> allFeatures = new HashMap<String, List<FeatureDescription>>();
 
-    private Map<String, FeatureDescription> activeFeatures = new TreeMap<String, FeatureDescription>();
+    private Map<String, Feature> activeFeatures = Collections.emptyMap();
 
     private List<ServiceRegistration> services;
 
@@ -108,7 +121,11 @@ public class FeatureManager {
         }
     }
 
-    protected void bindFeature(final Feature f, final Map<String, Object> props) {
+    //--- Feature binding
+
+    // bind method for Feature services
+    @SuppressWarnings("unused")
+    private void bindFeature(final Feature f, final Map<String, Object> props) {
         synchronized (this.allFeatures) {
             final String name = f.getName();
             final FeatureDescription info = new FeatureDescription(f, props);
@@ -125,7 +142,9 @@ public class FeatureManager {
         }
     }
 
-    protected void unbindFeature(final Feature f, final Map<String, Object> props) {
+    // unbind method for Feature services
+    @SuppressWarnings("unused")
+    private void unbindFeature(final Feature f, final Map<String, Object> props) {
         synchronized (this.allFeatures) {
             final String name = f.getName();
             final FeatureDescription info = new FeatureDescription(f, props);
@@ -141,12 +160,13 @@ public class FeatureManager {
         }
     }
 
+    // calculates map of active features (eliminating Feature name
+    // collisions). Must be called while synchronized on this.allFeatures
     private void calculateActiveProviders() {
-        final Map<String, FeatureDescription> activeMap = new TreeMap<String, FeatureDescription>();
+        final Map<String, Feature> activeMap = new HashMap<String, Feature>();
         for (final Map.Entry<String, List<FeatureDescription>> entry : this.allFeatures.entrySet()) {
             final FeatureDescription desc = entry.getValue().get(0);
-
-            activeMap.put(entry.getKey(), desc);
+            activeMap.put(entry.getKey(), desc.feature);
             if (entry.getValue().size() > 1) {
                 logger.warn("More than one feature service for feature {}", entry.getKey());
             }
@@ -154,22 +174,9 @@ public class FeatureManager {
         this.activeFeatures = activeMap;
     }
 
-    private final ThreadLocal<ClientContext> perThreadClientContext = new ThreadLocal<ClientContext>();
-
-    private final ClientContext defaultClientContext = new ClientContext() {
-
-        @Override
-        public boolean isEnabled(final String featureName) {
-            return false;
-        }
-
-        @Override
-        public Collection<Feature> getEnabledFeatures() {
-            return Collections.emptyList();
-        }
-    };
+    //--- Client Context management and access
 
-    public ClientContext getCurrentClientContext() {
+    ClientContext getCurrentClientContext() {
         ClientContext result = perThreadClientContext.get();
         if (result == null) {
             result = defaultClientContext;
@@ -177,7 +184,7 @@ public class FeatureManager {
         return result;
     }
 
-    public ClientContext setCurrentClientContext(final ServletRequest request) {
+    ClientContext setCurrentClientContext(final ServletRequest request) {
         final ClientContext current = perThreadClientContext.get();
         if (request instanceof HttpServletRequest) {
             final ExecutionContext providerContext = new ExecutionContextImpl((HttpServletRequest) request);
@@ -187,7 +194,7 @@ public class FeatureManager {
         return current;
     }
 
-    public void unsetCurrentClientContext(final ClientContext previous) {
+    void unsetCurrentClientContext(final ClientContext previous) {
         if (previous != null) {
             perThreadClientContext.set(previous);
         } else {
@@ -195,7 +202,7 @@ public class FeatureManager {
         }
     }
 
-    public ClientContext createClientContext(final ResourceResolver resolver) {
+    ClientContext createClientContext(final ResourceResolver resolver) {
         if (resolver == null) {
             throw new IllegalArgumentException("Resolver must not be null.");
         }
@@ -204,7 +211,7 @@ public class FeatureManager {
         return ctx;
     }
 
-    public ClientContext createClientContext(final HttpServletRequest request) {
+    ClientContext createClientContext(final HttpServletRequest request) {
         if (request == null) {
             throw new IllegalArgumentException("Request must not be null.");
         }
@@ -214,42 +221,33 @@ public class FeatureManager {
     }
 
     private ClientContextImpl createClientContext(final ExecutionContext providerContext) {
-        final List<Feature> enabledFeatures = new ArrayList<Feature>();
-
-        for (final Map.Entry<String, FeatureDescription> entry : this.activeFeatures.entrySet()) {
-            final Feature f = entry.getValue().feature;
-
-            if (entry.getValue().feature.isEnabled(providerContext)) {
-                enabledFeatures.add(f);
+        final Map<String, Feature> enabledFeatures = new HashMap<String, Feature>();
+        for (final Map.Entry<String, Feature> entry : this.activeFeatures.entrySet()) {
+            if (entry.getValue().isEnabled(providerContext)) {
+                enabledFeatures.put(entry.getKey(), entry.getValue());
             }
         }
 
-        final ClientContextImpl ctx = new ClientContextImpl(providerContext, enabledFeatures);
-        return ctx;
+        return new ClientContextImpl(providerContext, enabledFeatures);
     }
 
-    public Feature[] getAvailableFeatures() {
-        final List<Feature> result = new ArrayList<Feature>();
-        for (final Map.Entry<String, FeatureDescription> entry : this.activeFeatures.entrySet()) {
-            final Feature f = entry.getValue().feature;
-            result.add(f);
-        }
-        return result.toArray(new Feature[result.size()]);
+    //--- Feature access
+
+    Feature[] getAvailableFeatures() {
+        final Map<String, Feature> activeFeatures = this.activeFeatures;
+        return activeFeatures.values().toArray(new Feature[activeFeatures.size()]);
     }
 
-    public Feature getFeature(final String name) {
-        final FeatureDescription desc = this.activeFeatures.get(name);
-        if (desc != null) {
-            return desc.feature;
-        }
-        return null;
+    Feature getFeature(final String name) {
+        return this.activeFeatures.get(name);
     }
 
-    public String[] getAvailableFeatureNames() {
-        return this.activeFeatures.keySet().toArray(new String[this.activeFeatures.size()]);
+    String[] getAvailableFeatureNames() {
+        final Map<String, Feature> activeFeatures = this.activeFeatures;
+        return activeFeatures.keySet().toArray(new String[activeFeatures.size()]);
     }
 
-    public boolean isAvailable(final String featureName) {
+    boolean isAvailable(final String featureName) {
         return this.activeFeatures.containsKey(featureName);
     }