You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2019/02/12 16:16:23 UTC

[sling-org-apache-sling-feature-apiregions] branch master updated: Create a framework property to merge a specified region with the global region

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

davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-apiregions.git


The following commit(s) were added to refs/heads/master by this push:
     new 85ffc7a  Create a framework property to merge a specified region with the global region
85ffc7a is described below

commit 85ffc7acc962b84cc675d1f58e8dab35934219b3
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Tue Feb 12 16:14:18 2019 +0000

    Create a framework property to merge a specified region with the global region
    
    To make a given region visible to any bundle, even ones that don't
    reside in a feature model, the region can be merged with the global
    region. This can be useful to enable deprecated APIs for bundles that
    don't live in a feature model.
---
 .../feature/apiregions/impl/RegionEnforcer.java    | 80 ++++++++++++++--------
 .../apiregions/impl/RegionEnforcerTest.java        | 79 ++++++++++++++++++++-
 src/test/resources/regions2.properties             |  2 +
 3 files changed, 132 insertions(+), 29 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/RegionEnforcer.java b/src/main/java/org/apache/sling/feature/apiregions/impl/RegionEnforcer.java
index f4ecaec..456f7f4 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/RegionEnforcer.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/RegionEnforcer.java
@@ -40,6 +40,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
 import java.util.logging.Logger;
@@ -48,6 +49,7 @@ class RegionEnforcer implements ResolverHookFactory {
     public static final String GLOBAL_REGION = "global";
 
     static final String CLASSLOADER_PSEUDO_PROTOCOL = "classloader://";
+    static final String APIREGIONS_TOGLOBAL = "sling.feature.apiregions.toglobal";
     static final String PROPERTIES_RESOURCE_PREFIX = "sling.feature.apiregions.resource.";
     static final String PROPERTIES_FILE_LOCATION = "sling.feature.apiregions.location";
 
@@ -67,43 +69,66 @@ class RegionEnforcer implements ResolverHookFactory {
     RegionEnforcer(BundleContext context, Dictionary<String, Object> regProps, String regionsProp)
             throws IOException, URISyntaxException {
         URI idbsnverFile = getDataFileURI(context, IDBSNVER_FILENAME);
-        bsnVerMap = populateBSNVerMap(idbsnverFile);
-        if (idbsnverFile != null) {
-            // Register the location as a service property for diagnostic purposes
-            regProps.put(IDBSNVER_FILENAME, idbsnverFile.toString());
-        }
+        // Register the location as a service property for diagnostic purposes
+        regProps.put(IDBSNVER_FILENAME, idbsnverFile.toString());
+        Map<Entry<String, Version>, List<String>> bvm = populateBSNVerMap(idbsnverFile);
 
         URI bundlesFile = getDataFileURI(context, BUNDLE_FEATURE_FILENAME);
-        bundleFeatureMap = populateBundleFeatureMap(bundlesFile);
-        if (bundlesFile != null) {
-            // Register the location as a service property for diagnostic purposes
-            regProps.put(BUNDLE_FEATURE_FILENAME, bundlesFile.toString());
-        }
+        // Register the location as a service property for diagnostic purposes
+        regProps.put(BUNDLE_FEATURE_FILENAME, bundlesFile.toString());
+        Map<String, Set<String>> bfm = populateBundleFeatureMap(bundlesFile);
 
         URI featuresFile = getDataFileURI(context, FEATURE_REGION_FILENAME);
-        featureRegionMap = populateFeatureRegionMap(featuresFile);
-        if (featuresFile != null) {
-            // Register the location as a service property for diagnostic purposes
-            regProps.put(FEATURE_REGION_FILENAME, featuresFile.toString());
-        }
+        // Register the location as a service property for diagnostic purposes
+        regProps.put(FEATURE_REGION_FILENAME, featuresFile.toString());
+        Map<String, Set<String>> frm = populateFeatureRegionMap(featuresFile);
 
         URI regionsFile = getDataFileURI(context, REGION_PACKAGE_FILENAME);
-        regionPackageMap = populateRegionPackageMap(regionsFile);
-        if (regionsFile != null) {
-            // Register the location as a service property for diagnostic purposes
-            regProps.put(REGION_PACKAGE_FILENAME, regionsFile.toString());
+        // Register the location as a service property for diagnostic purposes
+        regProps.put(REGION_PACKAGE_FILENAME, regionsFile.toString());
+        Map<String, Set<String>> rpm = populateRegionPackageMap(regionsFile);
+
+        String toglobal = context.getProperty(APIREGIONS_TOGLOBAL);
+        if (toglobal != null) {
+            moveRegionsToGlobal(toglobal, rpm);
+            regProps.put(APIREGIONS_TOGLOBAL, toglobal);
         }
 
         enabledRegions = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(regionsProp.split(","))));
 
-        // TODO fix all collections
+        // Make all maps and their contents unmodifiable
+        bsnVerMap = unmodifiableMapToList(bvm);
+        bundleFeatureMap = unmodifiableMapToSet(bfm);
+        featureRegionMap = unmodifiableMapToSet(frm);
+        regionPackageMap = unmodifiableMapToSet(rpm);
     }
 
-    private static Map<Map.Entry<String, Version>, List<String>> populateBSNVerMap(URI idbsnverFile) throws IOException {
-        if (idbsnverFile == null) {
-            return new HashMap<>();
+    private static <K,V> Map<K, List<V>> unmodifiableMapToList(Map<K, List<V>> m) {
+        for (Map.Entry<K, List<V>> entry : m.entrySet()) {
+            m.put(entry.getKey(), Collections.unmodifiableList(entry.getValue()));
+        }
+        return Collections.unmodifiableMap(m);
+    }
+
+    private static <K,V> Map<K, Set<V>> unmodifiableMapToSet(Map<K, Set<V>> m) {
+        for (Map.Entry<K, Set<V>> entry : m.entrySet()) {
+            m.put(entry.getKey(), Collections.unmodifiableSet(entry.getValue()));
+        }
+        return Collections.unmodifiableMap(m);
+    }
+
+    private void moveRegionsToGlobal(String toglobal, Map<String, Set<String>> rpm) {
+        for (String region : toglobal.split(",")) {
+            Set<String> packages = rpm.get(region);
+            if (packages == null)
+                continue;
+
+            addValuesToMap(rpm, GLOBAL_REGION, packages);
+            rpm.remove(region);
         }
+    }
 
+    private static Map<Map.Entry<String, Version>, List<String>> populateBSNVerMap(URI idbsnverFile) throws IOException {
         Map<Map.Entry<String, Version>, List<String>> m = new HashMap<>();
 
         Properties p = new Properties();
@@ -146,9 +171,6 @@ class RegionEnforcer implements ResolverHookFactory {
     }
 
     private static Map<String, Set<String>> loadMap(URI propsFile) throws IOException {
-        if (propsFile == null) {
-            return new HashMap<>();
-        }
         Map<String, Set<String>> m = new HashMap<>();
 
         Properties p = new Properties();
@@ -165,12 +187,16 @@ class RegionEnforcer implements ResolverHookFactory {
     }
 
     private static void addValuesToMap(Map<String, Set<String>> map, String key, String ... values) {
+        addValuesToMap(map, key, Arrays.asList(values));
+
+    }
+    private static void addValuesToMap(Map<String, Set<String>> map, String key, Collection<String> values) {
         Set<String> bf = map.get(key);
         if (bf == null) {
             bf = new LinkedHashSet<>(); // It's important that the insertion order is maintained.
             map.put(key, bf);
         }
-        bf.addAll(Arrays.asList(values));
+        bf.addAll(values);
     }
 
     private URI getDataFileURI(BundleContext ctx, String name) throws IOException, URISyntaxException {
diff --git a/src/test/java/org/apache/sling/feature/apiregions/impl/RegionEnforcerTest.java b/src/test/java/org/apache/sling/feature/apiregions/impl/RegionEnforcerTest.java
index 45192bd..7cc7c8b 100644
--- a/src/test/java/org/apache/sling/feature/apiregions/impl/RegionEnforcerTest.java
+++ b/src/test/java/org/apache/sling/feature/apiregions/impl/RegionEnforcerTest.java
@@ -30,7 +30,11 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
+import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.APIREGIONS_TOGLOBAL;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.BUNDLE_FEATURE_FILENAME;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.FEATURE_REGION_FILENAME;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.IDBSNVER_FILENAME;
@@ -46,9 +50,8 @@ public class RegionEnforcerTest {
     public void testRegionEnforcerNoConfiguration() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
 
-        RegionEnforcer re;
         try {
-            re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+            new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
             fail("Expected exception. Enforcer is enabled but is missing configuration");
         } catch (Exception e) {
             // good
@@ -138,6 +141,23 @@ public class RegionEnforcerTest {
     }
 
     @Test
+    public void testMoveRegionsToGlobal() throws Exception {
+        String e = getClass().getResource("/empty.properties").getFile();
+        String f = getClass().getResource("/regions2.properties").getFile();
+        BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getProperty(APIREGIONS_TOGLOBAL)).thenReturn("obsolete,deprecated");
+        Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
+        Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
+        Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
+        Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).thenReturn(f);
+
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        assertEquals(1, re.regionPackageMap.size());
+        assertEquals(new HashSet<>(Arrays.asList("xyz", "a.b.c", "d.e.f", "test")),
+                re.regionPackageMap.get("global"));
+    }
+
+    @Test
     public void testBegin() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).
@@ -199,4 +219,59 @@ public class RegionEnforcerTest {
         assertEquals(Arrays.asList("r0", "r1", "r2", "r3"),
                 new ArrayList<>(re.featureRegionMap.get("org.sling:something:1.2.3")));
     }
+
+    @Test
+    public void testUnModifiableMaps() throws Exception {
+        BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
+            thenReturn("classloader://props1");
+
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        assertTrue(re.bsnVerMap.size() > 0);
+        assertBSNVerMapUnmodifiable(re.bsnVerMap);
+        assertTrue(re.bundleFeatureMap.size() > 0);
+        assertMapUnmodifiable(re.bundleFeatureMap);
+        assertTrue(re.featureRegionMap.size() > 0);
+        assertMapUnmodifiable(re.featureRegionMap);
+        assertTrue(re.regionPackageMap.size() > 0);
+        assertMapUnmodifiable(re.regionPackageMap);
+    }
+
+    private void assertBSNVerMapUnmodifiable(Map<Map.Entry<String, Version>, List<String>> m) {
+        Map.Entry<Map.Entry<String, Version>, List<String>> entry = m.entrySet().iterator().next();
+        try {
+            List<String> c = entry.getValue();
+            c.add("test");
+            fail("Changing a value should have thrown an exception");
+        } catch (Exception ex) {
+            // good
+        }
+
+        try {
+            m.put(new AbstractMap.SimpleEntry<>("hi", Version.parseVersion("1.2.3")),
+                    Collections.singletonList("xyz"));
+            fail("Adding a new value should have thrown an exception");
+        } catch (Exception ex) {
+            // good
+        }
+    }
+
+    private void assertMapUnmodifiable(Map<String, Set<String>> m) {
+        Map.Entry<String, Set<String>> entry = m.entrySet().iterator().next();
+        try {
+            Set<String> s = entry.getValue();
+            s.add("testing");
+            fail("Changing a value should have thrown an exception");
+        } catch (Exception ex) {
+            // good
+        }
+
+        try {
+            m.put("foo", Collections.<String>emptySet());
+            fail("Adding a new value should have thrown an exception");
+        } catch (Exception ex) {
+            // good
+        }
+    }
+
 }
diff --git a/src/test/resources/regions2.properties b/src/test/resources/regions2.properties
new file mode 100644
index 0000000..e61c0b1
--- /dev/null
+++ b/src/test/resources/regions2.properties
@@ -0,0 +1,2 @@
+deprecated=xyz
+global=d.e.f,test,a.b.c