Allow individual bundles to be specified as being in a given region.

If you have a bundle that is not part of a feature it is now possible to
configure the apiregions runtime to make that bundle part of a specified
region. By default bundles that are not in a region can only see the
'global' region.
To make the bundle part of a different region, specify the following
framework property:
  sling.feature.apiregions.bundles=
    {bsn:version}={groupid:artifactId:version};{region},
    {bsn:version}={groupid:artifactId:version};{region}
So the Bundle-SymbolicName and version needs to be mapped to the
associated artifact ID for the bundle artifact and the region needs to
specified.
For example:
  org.foo.bar:1.2.3=org.foo:bar:1.2.3;myregion

The bundle will be added to a synthesized region.
diff --git a/pom.xml b/pom.xml
index abeb95e..313bfb0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -65,6 +65,7 @@
                         <exclude>*.md</exclude>
                         <exclude>src/test/resources/*</exclude>
                         <exclude>src/test/resources/props1/*</exclude>
+                        <exclude>src/test/resources/props2/*</exclude>
                     </excludes>
                 </configuration>
             </plugin>
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 b0d93e9..badaa5c 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
@@ -37,24 +37,30 @@
 import java.util.Dictionary;
 import java.util.HashMap;
 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;
 
 class RegionEnforcer implements ResolverHookFactory {
-    private static final String CLASSLOADER_PSEUDO_PROTOCOL = "classloader://";
-
     public static final String GLOBAL_REGION = "global";
 
+    static final String CLASSLOADER_PSEUDO_PROTOCOL = "classloader://";
     static final String PROPERTIES_RESOURCE_PREFIX = "sling.feature.apiregions.resource.";
     static final String PROPERTIES_FILE_LOCATION = "sling.feature.apiregions.location";
+    static final String SYNTHESIZED_BUNDLES_KEY = "sling.feature.apiregions.bundles";
+    static final String SYNTHESIZED_FEATURE = "org.apache.sling:org.apache.sling.feature.synthesized:0.0.0-SNAPSHOT";
 
     static final String IDBSNVER_FILENAME = "idbsnver.properties";
     static final String BUNDLE_FEATURE_FILENAME = "bundles.properties";
     static final String FEATURE_REGION_FILENAME = "features.properties";
     static final String REGION_PACKAGE_FILENAME = "regions.properties";
 
+    static final Logger LOG = Logger.getLogger(ResolverHookImpl.class.getName());
+
     final Map<Map.Entry<String, Version>, List<String>> bsnVerMap;
     final Map<String, Set<String>> bundleFeatureMap;
     final Map<String, Set<String>> featureRegionMap;
@@ -92,11 +98,58 @@
         }
 
         enabledRegions = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(regionsProp.split(","))));
+
+        loadRegionsFromProperties(context, bsnVerMap, bundleFeatureMap, featureRegionMap);
+        // TODO fix all collections
     }
 
-    private Map<Map.Entry<String, Version>, List<String>> populateBSNVerMap(URI idbsnverFile) throws IOException {
+    private static void loadRegionsFromProperties(BundleContext context,
+            Map<Entry<String, Version>, List<String>> bsnVerMap,
+            Map<String, Set<String>> bundleFeatureMap,
+            Map<String, Set<String>> featureRegionMap) {
+        String prop = context.getProperty(SYNTHESIZED_BUNDLES_KEY);
+        if (prop == null)
+            return;
+
+        for (String bundle : prop.split(",")) {
+            String[] bundleinfo = bundle.split("=");
+            if (bundleinfo.length != 2) {
+                LOG.severe("Incorrect bundle info '" + bundle + "' in " + prop);
+                continue;
+            }
+
+            String bsnver = bundleinfo[0];
+            String info = bundleinfo[1];
+
+            String[] bsnver1 = bsnver.split(":");
+            if (bsnver1.length != 2) {
+                LOG.severe("Incorrect bsn and version '" + bsnver + "' in " + prop);
+                continue;
+            }
+
+            String bsn = bsnver1[0];
+            String ver = bsnver1[1];
+
+            String[] aidregion = info.split(";");
+            if (aidregion.length != 2) {
+                LOG.severe("Incorrect artifact and region '" + aidregion + "' in " + prop);
+                continue;
+            }
+
+            String aid = aidregion[0];
+            String region = aidregion[1];
+
+            addBsnVerArtifact(bsnVerMap, bsn, ver, aid);
+            addValuesToMap(bundleFeatureMap, aid, SYNTHESIZED_FEATURE);
+            addValuesToMap(featureRegionMap, SYNTHESIZED_FEATURE, region);
+
+            LOG.info("Added bundle " +  bsnver + " as " + aid + " to feature " + region);
+        }
+    }
+
+    private static Map<Map.Entry<String, Version>, List<String>> populateBSNVerMap(URI idbsnverFile) throws IOException {
         if (idbsnverFile == null) {
-            return Collections.emptyMap();
+            return new HashMap<>();
         }
 
         Map<Map.Entry<String, Version>, List<String>> m = new HashMap<>();
@@ -108,39 +161,41 @@
 
         for (String n : p.stringPropertyNames()) {
             String[] bsnver = p.getProperty(n).split("~");
-            Map.Entry<String, Version> key = new AbstractMap.SimpleEntry<>(bsnver[0], Version.valueOf(bsnver[1]));
-            List<String> l = m.get(key);
-            if (l == null) {
-                l = new ArrayList<>();
-                m.put(key, l);
-            }
-            l.add(n);
+            addBsnVerArtifact(m, bsnver[0], bsnver[1], n);
         }
 
-        Map<Map.Entry<String, Version>, List<String>> m2 = new HashMap<>();
-
-        for (Map.Entry<Map.Entry<String, Version>, List<String>> entry : m.entrySet()) {
-            m2.put(entry.getKey(), Collections.unmodifiableList(entry.getValue()));
-        }
-
-        return Collections.unmodifiableMap(m2);
+        return m;
     }
 
-    private Map<String, Set<String>> populateBundleFeatureMap(URI bundlesFile) throws IOException {
+    private static void addBsnVerArtifact(
+            Map<Map.Entry<String, Version>, List<String>> bsnVerMap,
+            String bundleSymbolicName, String bundleVersion,
+            String artifactId) {
+        Version version = Version.valueOf(bundleVersion);
+        Map.Entry<String, Version> bsnVer = new AbstractMap.SimpleEntry<>(bundleSymbolicName, version);
+        List<String> l = bsnVerMap.get(bsnVer);
+        if (l == null) {
+            l = new ArrayList<>();
+            bsnVerMap.put(bsnVer, l);
+        }
+        l.add(artifactId);
+    }
+
+    private static Map<String, Set<String>> populateBundleFeatureMap(URI bundlesFile) throws IOException {
         return loadMap(bundlesFile);
     }
 
-    private Map<String, Set<String>> populateFeatureRegionMap(URI featuresFile) throws IOException {
+    private static Map<String, Set<String>> populateFeatureRegionMap(URI featuresFile) throws IOException {
         return loadMap(featuresFile);
     }
 
-    private Map<String, Set<String>> populateRegionPackageMap(URI regionsFile) throws IOException {
+    private static Map<String, Set<String>> populateRegionPackageMap(URI regionsFile) throws IOException {
         return loadMap(regionsFile);
     }
 
-    private Map<String, Set<String>> loadMap(URI propsFile) throws IOException {
+    private static Map<String, Set<String>> loadMap(URI propsFile) throws IOException {
         if (propsFile == null) {
-            return Collections.emptyMap();
+            return new HashMap<>();
         }
         Map<String, Set<String>> m = new HashMap<>();
 
@@ -151,10 +206,19 @@
 
         for (String n : p.stringPropertyNames()) {
             String[] features = p.getProperty(n).split(",");
-            m.put(n, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(features))));
+            addValuesToMap(m, n, features);
         }
 
-        return Collections.unmodifiableMap(m);
+        return m;
+    }
+
+    private static void addValuesToMap(Map<String, Set<String>> map, String key, 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));
     }
 
     private URI getDataFileURI(BundleContext ctx, String name) throws IOException, URISyntaxException {
diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java b/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
index 427c357..0bdda79 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
@@ -37,11 +37,8 @@
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.logging.Level;
-import java.util.logging.Logger;
 
 class ResolverHookImpl implements ResolverHook {
-    private static final Logger LOG = Logger.getLogger(ResolverHookImpl.class.getName());
-
     final Map<Map.Entry<String, Version>, List<String>> bsnVerMap;
     final Map<String, Set<String>> bundleFeatureMap;
     final Map<String, Set<String>> featureRegionMap;
@@ -158,9 +155,26 @@
                 }
                 bcRegionMap.put(bc, capRegions);
 
-                HashSet<String> sharedRegions = new HashSet<>(reqRegions);
+                List<String> sharedRegions = new ArrayList<>(reqRegions);
                 sharedRegions.retainAll(capRegions);
 
+                // Add any regions before the sharedRegions back in, as later regions inherit from earlier ones
+                List<String> capRegionList = new ArrayList<>(capRegions);
+                for (String region : new ArrayList<>(sharedRegions)) {
+                    boolean foundRegion = false;
+                    for (int i = capRegionList.size() - 1; i >= 0; i--) {
+                        String capRegion = capRegionList.get(i);
+                        if (region.equals(capRegion)) {
+                            foundRegion = true;
+                            continue;
+                        }
+                        if (foundRegion) {
+                            // Add the found region to the front of the list of shared regions
+                            sharedRegions.add(0, capRegion);
+                        }
+                    }
+                }
+
                 Object pkg = bc.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
                 if (pkg instanceof String) {
                     String packageName = (String) pkg;
@@ -186,7 +200,9 @@
 
         List<BundleCapability> removedCandidates = new ArrayList<>(candidates);
         // Remove any capabilities that are not covered
-        if (candidates.retainAll(coveredCaps)) {
+        candidates.retainAll(coveredCaps);
+
+        if (candidates.isEmpty()) {
             removedCandidates.removeAll(candidates);
 
             StringBuilder sb = new StringBuilder();
@@ -203,7 +219,7 @@
                 sb.append("]");
             }
 
-            LOG.log(Level.INFO,
+            RegionEnforcer.LOG.log(Level.WARNING,
                     "API-Regions removed candidates {0} for requirement {1} as the requirement is in the following regions: {2}",
                     new Object[] {sb, requirement, reqRegions});
         }
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 7fd0be6..2962cb4 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
@@ -25,10 +25,13 @@
 
 import java.io.File;
 import java.util.AbstractMap;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
+import java.util.Set;
 
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.BUNDLE_FEATURE_FILENAME;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.FEATURE_REGION_FILENAME;
@@ -36,6 +39,7 @@
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.PROPERTIES_FILE_LOCATION;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.PROPERTIES_RESOURCE_PREFIX;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.REGION_PACKAGE_FILENAME;
+import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.SYNTHESIZED_BUNDLES_KEY;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -159,7 +163,8 @@
     @Test
     public void testClassloaderURLs() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
-        Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).thenReturn("classloader://props1");
+        Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
+            thenReturn("classloader://props1");
 
         RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
         assertTrue(re.bsnVerMap.size() > 0);
@@ -167,4 +172,49 @@
         assertTrue(re.featureRegionMap.size() > 0);
         assertTrue(re.regionPackageMap.size() > 0);
     }
+
+    @Test
+    public void testOrderingOfRegionsInFeatures() throws Exception {
+        BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
+            thenReturn("classloader://props2");
+
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        assertEquals(Arrays.asList("r0", "r1", "r2", "r3"),
+                new ArrayList<>(re.featureRegionMap.get("org.sling:something:1.2.3")));
+    }
+
+    @Test
+    public void testFrameworkPropertyBundleRegions() throws Exception {
+        BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getProperty(SYNTHESIZED_BUNDLES_KEY)).thenReturn(
+                "org.foo.bar:1.2.3=org.foo:bar:1.2.3;myregion," +
+                "org.foo.jar:1.0.0=org.foo:jar:1.0.0;myregion");
+
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+
+        assertTrue(re.bsnVerMap.size() == 2);
+        List<String> al1 = re.bsnVerMap.get(new AbstractMap.SimpleEntry<>(
+                "org.foo.bar", Version.valueOf("1.2.3")));
+        assertEquals(1, al1.size());
+        String a1 = al1.iterator().next();
+
+        List<String> al2 = re.bsnVerMap.get(new AbstractMap.SimpleEntry<>(
+                "org.foo.jar", Version.valueOf("1.0.0")));
+        assertEquals(1, al2.size());
+        String a2 = al2.iterator().next();
+
+        Set<String> fl1 = re.bundleFeatureMap.get(a1);
+        assertEquals(1, fl1.size());
+        String f1 = fl1.iterator().next();
+        Set<String> fl2 = re.bundleFeatureMap.get(a2);
+        assertEquals(1, fl2.size());
+        String f2 = fl2.iterator().next();
+        assertEquals(f1, f2);
+
+        Set<String> rl = re.featureRegionMap.get(f1);
+        assertEquals(1, rl.size());
+        String r = rl.iterator().next();
+        assertEquals("myregion", r);
+    }
 }
diff --git a/src/test/java/org/apache/sling/feature/apiregions/impl/ResolverHookImplTest.java b/src/test/java/org/apache/sling/feature/apiregions/impl/ResolverHookImplTest.java
index d92935f..f22c212 100644
--- a/src/test/java/org/apache/sling/feature/apiregions/impl/ResolverHookImplTest.java
+++ b/src/test/java/org/apache/sling/feature/apiregions/impl/ResolverHookImplTest.java
@@ -34,6 +34,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -43,6 +44,75 @@
 
 public class ResolverHookImplTest {
     @Test
+    public void testProvidingFeatureHasNoRegionsSoEveryoneCanAccess() {
+        Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+                "providing.bundle", new Version(1,0,0)), Collections.singletonList("b1"));
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+                "requiring.bundle", new Version(1,0,0)), Collections.singletonList("b2"));
+
+        Map<String, Set<String>> bfmap = new HashMap<>();
+        bfmap.put("b1", Collections.singleton("f1"));
+        bfmap.put("b2", Collections.singleton("f2"));
+
+        Map<String, Set<String>> frmap = new HashMap<>();
+        frmap.put("f2", Collections.singleton("r2"));
+
+        Map<String, Set<String>> rpmap = new HashMap<>();
+
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap);
+
+        // b2 is in r2, it requires a capability that is provided by b1.
+        // b1 is not in any region so it can provide access.
+        BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+        BundleCapability cap1 = mockCapability("org.apache.sling.test", "b1", bsnvermap);
+        List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1));
+        rh.filterMatches(req1, candidates1);
+        assertEquals(Collections.singletonList(cap1), candidates1);
+    }
+
+    @Test
+    public void testRegionInheritance() {
+        Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+                "providing.bundle", new Version(9,9,9)), Collections.singletonList("b1"));
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+                "requiring.bundle", new Version(123,456,789)), Collections.singletonList("b2"));
+
+        Map<String, Set<String>> bfmap = new HashMap<>();
+        bfmap.put("b1", Collections.singleton("f1"));
+        bfmap.put("b2", Collections.singleton("f2"));
+
+        Map<String, Set<String>> frmap = new HashMap<>();
+        frmap.put("f1", new LinkedHashSet<>(Arrays.asList("r0", "r1", "r2", "r3")));
+        frmap.put("f2", Collections.singleton("r2"));
+
+        Map<String, Set<String>> rpmap = new HashMap<>();
+        rpmap.put("r0", Collections.<String>emptySet());
+        rpmap.put("r1", Collections.singleton("org.apache.sling.test"));
+        rpmap.put("r3", Collections.singleton("org.apache.sling.toast"));
+
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap);
+
+        // b2 is in r2, it requires a capability that is provided by b1 in r1. However since b1 is also
+        // in r2, r2 inherits all the capabilities provided by regions before r2, which includes r2. So
+        // b2, which is in r2 should resolve.
+        BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+        BundleCapability cap1 = mockCapability("org.apache.sling.test", "b1", bsnvermap);
+        List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1));
+        rh.filterMatches(req1, candidates1);
+        assertEquals(Collections.singletonList(cap1), candidates1);
+
+        // b2 is in r2, it requires a capability that is provided by b1 in r3. r3 is listed after r2 in
+        // b1 so it is not inherited by r2. The requirement should not resolve.
+        BundleRequirement req2 = mockRequirement("b2", bsnvermap);
+        BundleCapability cap2 = mockCapability("org.apache.sling.toast", "b1", bsnvermap);
+        List<BundleCapability> candidates2 = new ArrayList<>(Arrays.asList(cap2));
+        rh.filterMatches(req2, candidates2);
+        assertEquals(Collections.emptyList(), candidates2);
+    }
+
+    @Test
     public void testFilterMatches() {
         Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
         bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>("system.bundle", new Version(3,2,1)),
diff --git a/src/test/resources/props2/bundles.properties b/src/test/resources/props2/bundles.properties
new file mode 100644
index 0000000..eac3761
--- /dev/null
+++ b/src/test/resources/props2/bundles.properties
@@ -0,0 +1,3 @@
+#Generated at Sat Nov 03 10:58:58 GMT 2018
+#Sat Nov 03 10:58:58 GMT 2018
+org.sling\:b1\:1=org.sling\:something\:1.2.3
diff --git a/src/test/resources/props2/features.properties b/src/test/resources/props2/features.properties
new file mode 100644
index 0000000..c2cb887
--- /dev/null
+++ b/src/test/resources/props2/features.properties
@@ -0,0 +1,3 @@
+#Generated at Sat Nov 03 11:10:29 GMT 2018
+#Sat Nov 03 11:10:29 GMT 2018
+org.sling\:something\:1.2.3=r0,r1,r2,r3
diff --git a/src/test/resources/props2/idbsnver.properties b/src/test/resources/props2/idbsnver.properties
new file mode 100644
index 0000000..8262273
--- /dev/null
+++ b/src/test/resources/props2/idbsnver.properties
@@ -0,0 +1,4 @@
+#Generated at Sat Nov 03 10:26:37 GMT 2018
+#Sat Nov 03 10:26:37 GMT 2018
+org.sling\:b1\:1=b1~1
+
diff --git a/src/test/resources/props2/regions.properties b/src/test/resources/props2/regions.properties
new file mode 100644
index 0000000..2808442
--- /dev/null
+++ b/src/test/resources/props2/regions.properties
@@ -0,0 +1,8 @@
+#Generated at Sat Nov 03 11:10:29 GMT 2018
+#Sat Nov 03 11:10:29 GMT 2018
+r0=
+r1=
+r2=
+r3=
+r4=
+r5=