Merge pull request #2 from bosschaert/SLING-9004
SLING-9004 API Regions runtime should have a configured set of default regions
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
