SLING-9004 API Regions runtime should have a configured set of default regions

Make it possible to configure the API Regions runtime with a default set
of regions that all bundles are in, regardless of what feature they are
in or if they are in no feature at all.

To configure the default set of regions set the following framework
property

    sling.feature.apiregions.default=region1,region2

The value of this property is a comma-separated list of regions that all
bundles will be in.

This commit also reverts what was committed for SLING-8976 to use
bundles.properties and features.properties and not
bundleOrigins.properties and regionOrigins.properties.
diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java b/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
index 190e29b..27189c3 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
@@ -67,13 +67,14 @@
         if (hookRegistration != null)
             return; // There is already a hook, no need to re-register
 
-        String regions = bundleContext.getProperty(REGIONS_PROPERTY_NAME);
-        if (regions == null)
+        if (bundleContext.getProperty(REGIONS_PROPERTY_NAME) == null) {
+            RegionEnforcer.LOG.log(Level.WARNING, "API Regions not enabled. To enable set framework property: " + REGIONS_PROPERTY_NAME);
             return; // Component not enabled
+        }
 
         Dictionary<String, Object> props = new Hashtable<>();
         try {
-            RegionEnforcer enforcer = new RegionEnforcer(bundleContext, props, regions);
+            RegionEnforcer enforcer = new RegionEnforcer(bundleContext, props);
             hookRegistration = bundleContext.registerService(ResolverHookFactory.class, enforcer, props);
         } catch (Exception e) {
             RegionEnforcer.LOG.log(Level.SEVERE, "Problem activating API Regions runtime enforcement component", e);
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 5c2310e..d1a1f40 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
@@ -50,12 +50,13 @@
 
     static final String CLASSLOADER_PSEUDO_PROTOCOL = "classloader://";
     static final String APIREGIONS_JOINGLOBAL = "sling.feature.apiregions.joinglobal";
+    static final String DEFAULT_REGIONS = "sling.feature.apiregions.default";
     static final String PROPERTIES_RESOURCE_PREFIX = "sling.feature.apiregions.resource.";
     static final String PROPERTIES_FILE_LOCATION = "sling.feature.apiregions.location";
 
     static final String IDBSNVER_FILENAME = "idbsnver.properties";
-    static final String BUNDLE_FEATURE_FILENAME = "bundleOrigins.properties";
-    static final String FEATURE_REGION_FILENAME = "regionOrigins.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());
@@ -64,9 +65,9 @@
     final Map<String, Set<String>> bundleFeatureMap;
     final Map<String, Set<String>> featureRegionMap;
     final Map<String, Set<String>> regionPackageMap;
-    final Set<String> enabledRegions;
+    final Set<String> defaultRegions;
 
-    RegionEnforcer(BundleContext context, Dictionary<String, Object> regProps, String regionsProp)
+    RegionEnforcer(BundleContext context, Dictionary<String, Object> regProps)
             throws IOException, URISyntaxException {
         URI idbsnverFile = getDataFileURI(context, IDBSNVER_FILENAME);
         // Register the location as a service property for diagnostic purposes
@@ -94,7 +95,21 @@
             regProps.put(APIREGIONS_JOINGLOBAL, toglobal);
         }
 
-        enabledRegions = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(regionsProp.split(","))));
+        String defRegProp = context.getProperty(DEFAULT_REGIONS);
+        if (defRegProp != null) {
+            Set<String> defRegs = new HashSet<>();
+            for (String region : Arrays.asList(defRegProp.split(","))) {
+                if (region.length() > 0) {
+                    defRegs.add(region);
+                }
+            }
+            defaultRegions = Collections.unmodifiableSet(defRegs);
+            if (defaultRegions.size() > 0) {
+                regProps.put(DEFAULT_REGIONS, defaultRegions.toString());
+            }
+        } else {
+            defaultRegions = Collections.emptySet();
+        }
 
         // Make all maps and their contents unmodifiable
         bsnVerMap = unmodifiableMapToList(bvm);
@@ -230,8 +245,6 @@
 
     @Override
     public ResolverHook begin(Collection<BundleRevision> triggers) {
-        if (enabledRegions.isEmpty())
-            return null;
-        return new ResolverHookImpl(bsnVerMap, bundleFeatureMap, featureRegionMap, regionPackageMap);
+        return new ResolverHookImpl(bsnVerMap, bundleFeatureMap, featureRegionMap, regionPackageMap, defaultRegions);
     }
 }
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 0ec8c16..208a0cf 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
@@ -43,13 +43,15 @@
     final Map<String, Set<String>> bundleFeatureMap;
     final Map<String, Set<String>> featureRegionMap;
     final Map<String, Set<String>> regionPackageMap;
+    final Set<String> defaultRegions;
 
     ResolverHookImpl(Map<Entry<String, Version>, List<String>> bsnVerMap, Map<String, Set<String>> bundleFeatureMap,
-            Map<String, Set<String>> featureRegionMap, Map<String, Set<String>> regionPackageMap) {
+            Map<String, Set<String>> featureRegionMap, Map<String, Set<String>> regionPackageMap, Set<String> defaultRegions) {
         this.bsnVerMap = bsnVerMap;
         this.bundleFeatureMap = bundleFeatureMap;
         this.featureRegionMap = featureRegionMap;
         this.regionPackageMap = regionPackageMap;
+        this.defaultRegions = defaultRegions;
     }
 
     @Override
@@ -73,7 +75,7 @@
         String reqBundleName = reqBundle.getSymbolicName();
         Version reqBundleVersion = reqBundle.getVersion();
 
-        Set<String> reqRegions = new HashSet<>();
+        Set<String> reqRegions = new HashSet<>(defaultRegions);
         List<String> reqFeatures = new ArrayList<>();
         List<String> aids = bsnVerMap.get(new AbstractMap.SimpleEntry<String, Version>(reqBundleName, reqBundleVersion));
         if (aids != null) {
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 938f3a6..ac1d77d 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
@@ -22,8 +22,12 @@
 import org.mockito.Mockito;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Version;
+import org.osgi.framework.hooks.resolver.ResolverHook;
 
 import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.net.URISyntaxException;
 import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -36,6 +40,7 @@
 
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.APIREGIONS_JOINGLOBAL;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.BUNDLE_FEATURE_FILENAME;
+import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.DEFAULT_REGIONS;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.FEATURE_REGION_FILENAME;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.IDBSNVER_FILENAME;
 import static org.apache.sling.feature.apiregions.impl.RegionEnforcer.PROPERTIES_FILE_LOCATION;
@@ -51,7 +56,7 @@
         BundleContext ctx = Mockito.mock(BundleContext.class);
 
         try {
-            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
@@ -69,7 +74,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).thenReturn(e);
 
         Hashtable<String, Object> props = new Hashtable<>();
-        RegionEnforcer re = new RegionEnforcer(ctx, props, "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, props);
         assertEquals(2, re.bsnVerMap.size());
         assertEquals(Collections.singletonList("g:b1:1"),
                 re.bsnVerMap.get(new AbstractMap.SimpleEntry<String,Version>("b1", new Version(1,0,0))));
@@ -89,7 +94,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).thenReturn(e);
 
         Hashtable<String, Object> props = new Hashtable<>();
-        RegionEnforcer re = new RegionEnforcer(ctx, props, "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, props);
         assertEquals(3, re.bundleFeatureMap.size());
         assertEquals(Collections.singleton("org.sling:something:1.2.3:slingosgifeature:myclassifier"),
                 re.bundleFeatureMap.get("org.sling:b1:1"));
@@ -111,7 +116,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).thenReturn(e);
 
         Hashtable<String, Object> props = new Hashtable<>();
-        RegionEnforcer re = new RegionEnforcer(ctx, props, "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, props);
         assertEquals(2, re.featureRegionMap.size());
         assertEquals(Collections.singleton("global"),
                 re.featureRegionMap.get("an.other:feature:123"));
@@ -131,7 +136,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).thenReturn(f);
 
         Hashtable<String, Object> props = new Hashtable<>();
-        RegionEnforcer re = new RegionEnforcer(ctx, props, "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, props);
         assertEquals(2, re.regionPackageMap.size());
         assertEquals(Collections.singleton("xyz"),
                 re.regionPackageMap.get("internal"));
@@ -151,7 +156,7 @@
         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>(), "*");
+        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"));
@@ -169,7 +174,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + REGION_PACKAGE_FILENAME)).
             thenReturn(getClass().getResource("/regions1.properties").getFile());
 
-        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>());
         assertTrue(re.bsnVerMap.size() > 0);
         assertTrue(re.bundleFeatureMap.size() > 0);
         assertTrue(re.featureRegionMap.size() > 0);
@@ -189,7 +194,7 @@
                 getFile()).getParentFile().toURI().toString();
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).thenReturn(location);
 
-        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>());
         assertTrue(re.bsnVerMap.size() > 0);
         assertTrue(re.bundleFeatureMap.size() > 0);
         assertTrue(re.featureRegionMap.size() > 0);
@@ -202,7 +207,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props1");
 
-        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>());
         assertTrue(re.bsnVerMap.size() > 0);
         assertTrue(re.bundleFeatureMap.size() > 0);
         assertTrue(re.featureRegionMap.size() > 0);
@@ -215,7 +220,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props2");
 
-        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        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")));
     }
@@ -226,7 +231,7 @@
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props1");
 
-        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>(), "*");
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>());
         assertTrue(re.bsnVerMap.size() > 0);
         assertBSNVerMapUnmodifiable(re.bsnVerMap);
         assertTrue(re.bundleFeatureMap.size() > 0);
@@ -237,6 +242,30 @@
         assertMapUnmodifiable(re.regionPackageMap);
     }
 
+    @Test
+    public void testDefaultRegions() throws Exception {
+        testDefaultRegions("foo.bar,foo.zar", new HashSet<>(Arrays.asList("foo.bar", "foo.zar")));
+        testDefaultRegions("test", Collections.singleton("test"));
+        testDefaultRegions("", Collections.emptySet());
+        testDefaultRegions(null, Collections.emptySet());
+    }
+
+    @SuppressWarnings("unchecked")
+    private void testDefaultRegions(String defProp, Set<String> expected)
+            throws IOException, URISyntaxException, NoSuchFieldException, IllegalAccessException {
+        BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getProperty(DEFAULT_REGIONS)).thenReturn(defProp);
+        Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
+        thenReturn("classloader://props1");
+
+        RegionEnforcer re = new RegionEnforcer(ctx, new Hashtable<String, Object>());
+        ResolverHook hook = re.begin(Collections.emptySet());
+        Field f = ResolverHookImpl.class.getDeclaredField("defaultRegions");
+        f.setAccessible(true);
+
+        assertEquals(expected, f.get(hook));
+    }
+
     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 {
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 13c1248..2910492 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
@@ -60,7 +60,7 @@
 
         Map<String, Set<String>> rpmap = new HashMap<>();
 
-        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap);
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("*"));
 
         // b2 is in r2, it requires a capability that is provided by b1.
         // b1 is not in any region so it can provide access.
@@ -92,7 +92,7 @@
         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);
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton(""));
 
         // 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
@@ -158,7 +158,7 @@
         rpmap.put(RegionEnforcer.GLOBAL_REGION, Collections.singleton("org.bar.tar"));
         rpmap.put("r3", Collections.singleton("xyz"));
 
-        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap);
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.emptySet());
 
         // Check that we can get the capability from another bundle in the same region
         // where that region exports the package
@@ -286,7 +286,7 @@
 
         ResolverHookImpl rh = new ResolverHookImpl(
                 Collections.<Map.Entry<String, Version>, List<String>>emptyMap(),
-                Collections.<String, Set<String>>emptyMap(), featureRegionMap, regionPackageMap);
+                Collections.<String, Set<String>>emptyMap(), featureRegionMap, regionPackageMap, Collections.emptySet());
 
         assertEquals(Collections.emptyList(), rh.getRegionsForPackage(null, "f1"));
         assertEquals(Collections.emptyList(), rh.getRegionsForPackage("org.foo", "f1"));
@@ -299,6 +299,52 @@
             rh.getRegionsForPackage("a.b.c", "f2"));
     }
 
+    @Test
+    public void testDefaultRegions() {
+        Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>("b98", new Version(1, 0, 0)),
+                Collections.singletonList("b98"));
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>("b99", new Version(1, 2, 3)),
+                Collections.singletonList("b99"));
+        bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>("b100", new Version(4, 5, 6)),
+                Collections.singletonList("b100"));
+
+        Map<String, Set<String>> bfmap = new HashMap<>();
+        bfmap.put("b98", Collections.singleton("f2"));
+        bfmap.put("b100", Collections.singleton("f1"));
+
+        Map<String, Set<String>> frmap = new HashMap<>();
+        frmap.put("f1", Collections.singleton("r1"));
+        frmap.put("f2", Collections.singleton("r2"));
+
+        Map<String, Set<String>> rpmap = new HashMap<>();
+        rpmap.put("r1", Collections.singleton("org.test"));
+        rpmap.put("r2", Collections.singleton("org.test"));
+
+        ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap,
+                new HashSet<>(Arrays.asList("r1", "r3")));
+
+        // b99 is not in any region itself and tries to resolve to b100 which is in r1
+        // b99 can resolve to b100 because 'r1' is listed as a default region in the
+        // ResolverHook.
+        BundleRequirement req = mockRequirement("b99", bsnvermap);
+        BundleCapability cap = mockCapability("org.test", "b100", bsnvermap);
+        List<BundleCapability> candidates = new ArrayList<>(Arrays.asList(cap));
+        rh.filterMatches(req, candidates);
+        assertEquals("b99 should be able to wire to b100, as the default region is r1, which is what b100 is in. ",
+                Collections.singletonList(cap), candidates);
+
+        // b99 is not in any region and tries to resolve to b98, which is in r2
+        // b99 can't resolve because b2 is not in the default regions and it's not in
+        // any other regions itself.
+        BundleRequirement req1 = mockRequirement("b99", bsnvermap);
+        BundleCapability cap1 = mockCapability("org.test", "b98", bsnvermap);
+        List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1));
+        rh.filterMatches(req1, candidates1);
+        assertEquals("b99 should not be able to wire to b98, as this is in region r2, which is not in the default regions r1 and r3",
+                0, candidates1.size());
+    }
+
     private BundleCapability mockCapability(String pkgName, String bid, Map<Entry<String, Version>, List<String>> bsnvermap) {
         for (Map.Entry<Map.Entry<String, Version>, List<String>> entry : bsnvermap.entrySet()) {
             if (entry.getValue().contains(bid)) {
diff --git a/src/test/resources/props1/bundleOrigins.properties b/src/test/resources/props1/bundles.properties
similarity index 100%
rename from src/test/resources/props1/bundleOrigins.properties
rename to src/test/resources/props1/bundles.properties
diff --git a/src/test/resources/props1/regionOrigins.properties b/src/test/resources/props1/features.properties
similarity index 100%
rename from src/test/resources/props1/regionOrigins.properties
rename to src/test/resources/props1/features.properties
diff --git a/src/test/resources/props2/bundleOrigins.properties b/src/test/resources/props2/bundles.properties
similarity index 100%
rename from src/test/resources/props2/bundleOrigins.properties
rename to src/test/resources/props2/bundles.properties
diff --git a/src/test/resources/props2/regionOrigins.properties b/src/test/resources/props2/features.properties
similarity index 100%
rename from src/test/resources/props2/regionOrigins.properties
rename to src/test/resources/props2/features.properties
diff --git a/src/test/resources/props3/bundleOrigins.properties b/src/test/resources/props3/bundles.properties
similarity index 100%
rename from src/test/resources/props3/bundleOrigins.properties
rename to src/test/resources/props3/bundles.properties
diff --git a/src/test/resources/props3/regionOrigins.properties b/src/test/resources/props3/features.properties
similarity index 100%
rename from src/test/resources/props3/regionOrigins.properties
rename to src/test/resources/props3/features.properties