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 2020/02/11 13:32:10 UTC
[sling-org-apache-sling-feature-apiregions] branch master updated:
SLING-9024 API Regions runtime should prefer named regions from 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 e0280c7 SLING-9024 API Regions runtime should prefer named regions from the Global region
new bb5b198 Merge pull request #3 from bosschaert/SLING-9024-2
e0280c7 is described below
commit e0280c73ef85a003f396848ad37951a1911d4546
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Fri Jan 24 09:57:40 2020 +0000
SLING-9024 API Regions runtime should prefer named regions from the Global region
---
.../feature/apiregions/impl/ResolverHookImpl.java | 105 +++++++--
.../apiregions/impl/ResolverHookImplTest.java | 243 ++++++++++++++++++++-
2 files changed, 326 insertions(+), 22 deletions(-)
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 a8f3b67..3b9279c 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
@@ -32,6 +32,7 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -95,7 +96,7 @@ class ResolverHookImpl implements ResolverHook {
// Bundle is not coming from a feature
}
- Set<BundleCapability> coveredCaps = new HashSet<>();
+ Map<BundleCapability, String> coveredCaps = new HashMap<>();
Map<BundleCapability, String> bcFeatureMap = new HashMap<>();
String packageName = null;
@@ -107,13 +108,13 @@ class ResolverHookImpl implements ResolverHook {
long capBundleID = capBundle.getBundleId();
if (capBundleID == 0) {
// always allow capability from the system bundle
- coveredCaps.add(bc);
+ coveredCaps.put(bc, null); // null value means same bundle, same feature or system bundlee
continue nextCapability;
}
if (capBundleID == reqBundleID) {
// always allow capability from same bundle
- coveredCaps.add(bc);
+ coveredCaps.put(bc, null); // null value means same bundle, same feature or system bundle
continue nextCapability;
}
@@ -123,7 +124,7 @@ class ResolverHookImpl implements ResolverHook {
List<String> capBundleArtifacts = bsnVerMap.get(new AbstractMap.SimpleEntry<String, Version>(capBundleName, capBundleVersion));
if (capBundleArtifacts == null) {
// Capability is not in any feature, everyone can access
- coveredCaps.add(bc);
+ coveredCaps.put(bc, RegionEnforcer.GLOBAL_REGION);
continue nextCapability;
}
@@ -139,21 +140,21 @@ class ResolverHookImpl implements ResolverHook {
for (String capFeat : capFeatures) {
if (capFeat == null) {
- // always allow capability not coming from a feature
- coveredCaps.add(bc);
+ // everyone can access capability not coming from a feature
+ coveredCaps.put(bc, RegionEnforcer.GLOBAL_REGION);
continue nextCapability;
}
if (reqFeatures.contains(capFeat)) {
// Within a single feature everything can wire to everything else
- coveredCaps.add(bc);
+ coveredCaps.put(bc, null); // null value means same bundle, same feature or system bundle
continue nextCapability;
}
Set<String> capRegions = featureRegionMap.get(capFeat);
- if (capRegions == null) {
+ if (capRegions == null || capRegions.size() == 0) {
// If the feature hosting the capability has no regions defined, everyone can access
- coveredCaps.add(bc);
+ coveredCaps.put(bc, RegionEnforcer.GLOBAL_REGION);
continue nextCapability;
}
bcFeatureMap.put(bc, capFeat);
@@ -165,28 +166,32 @@ class ResolverHookImpl implements ResolverHook {
if (pkg instanceof String) {
packageName = (String) pkg;
- Set<String> globalPackages = regionPackageMap.get(RegionEnforcer.GLOBAL_REGION);
- if (globalPackages != null && globalPackages.contains(packageName)) {
- // If the export is in the global region everyone can access
- coveredCaps.add(bc);
- continue nextCapability;
- }
-
+ // Look at specific regions first as they take precedence over the global region
for (String region : sharedRegions) {
Set<String> regionPackages = regionPackageMap.get(region);
if (regionPackages != null && regionPackages.contains(packageName)) {
// If the export is in a region that the feature is also in, then allow
- coveredCaps.add(bc);
+ coveredCaps.put(bc, region);
continue nextCapability;
}
}
+
+ // Now check the global region
+ Set<String> globalPackages = regionPackageMap.get(RegionEnforcer.GLOBAL_REGION);
+ if (globalPackages != null && globalPackages.contains(packageName)) {
+ // If the export is in the global region everyone can access
+ coveredCaps.put(bc, RegionEnforcer.GLOBAL_REGION);
+ continue nextCapability;
+ }
}
}
}
+ pruneCoveredCaps(reqRegions, coveredCaps);
+
List<BundleCapability> removedCandidates = new ArrayList<>(candidates);
// Remove any capabilities that are not covered
- candidates.retainAll(coveredCaps);
+ candidates.retainAll(coveredCaps.keySet());
if (candidates.isEmpty()) {
removedCandidates.removeAll(candidates);
@@ -199,15 +204,73 @@ class ResolverHookImpl implements ResolverHook {
else
sb.append(", ");
+ String capFeat = bcFeatureMap.get(bc);
sb.append(bc.toString());
sb.append("[Regions: ");
- sb.append(getRegionsForPackage(packageName, bcFeatureMap.get(bc)));
+ sb.append(getRegionsForPackage(packageName, capFeat));
+ sb.append(", Feature: ");
+ sb.append(capFeat);
sb.append("]");
}
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});
+ "API-Regions removed candidates {0} for requirement {1} as the requirement is in the following regions: {2} and in feature: {3}",
+ new Object[] {sb, requirement, reqRegions, reqFeatures});
+ }
+ }
+
+ /*
+ * If there are multiple choices of capabilities and some of the capabilities are in the global
+ * region while others are in another named region, take out the capabilities from the global
+ * region so that the requirement gets wired to the more 'specifc' one than the global one.
+ * Capabilities from bundle 0 (the system bundle), the same bundle as the requirer and from the
+ * same feature as the requirer should always be kept. These are marked in the capMap with a
+ * {@code null} region value.
+ */
+ private void pruneCoveredCaps(Set<String> reqRegions, Map<BundleCapability,String> capMap) {
+ Set<String> reqNonGlobalRegions = new HashSet<>(reqRegions);
+ reqNonGlobalRegions.remove(RegionEnforcer.GLOBAL_REGION);
+
+ if (capMap.size() <= 1) {
+ // Shortcut: there is only 0 or 1 capability, nothing to do
+ return;
+ }
+
+ if (reqRegions.size() == 0
+ || Collections.singleton(RegionEnforcer.GLOBAL_REGION).equals(reqRegions)) {
+ // No regions (other than global) for the requirement: do nothing
+ return;
+ }
+
+ List<BundleCapability> specificCaps = new ArrayList<>();
+ for (Iterator<Map.Entry<BundleCapability,String>> it = capMap.entrySet().iterator(); it.hasNext(); ) {
+ Map.Entry<BundleCapability,String> entry = it.next();
+
+ String capRegion = entry.getValue();
+ if (capRegion == null) {
+ // This one came from the same bundle, the same feature or bundle 0 -> always allow
+ specificCaps.add(entry.getKey());
+ continue;
+ }
+
+ if (reqNonGlobalRegions.contains(capRegion)) {
+ // the requirement has the region from the capability
+ specificCaps.add(entry.getKey());
+ }
+ }
+
+ if (specificCaps.size() == 0) {
+ // There are no capabilities that are either in the same bundle, same feature or overlapping specific
+ // feature. We should just allow all, including the global region
+ return;
+ }
+
+ // There are specific capabilities, therefore we should remove the Global region is any from the capabilities
+ // We have collected the capabilities we want to keep in specificCaps
+ for (Iterator<BundleCapability> it = capMap.keySet().iterator(); it.hasNext(); ) {
+ if (!specificCaps.contains(it.next())) {
+ it.remove();
+ }
}
}
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 cde362f..fcc86e8 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
@@ -62,7 +62,7 @@ public class ResolverHookImplTest {
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.
+ // b1 is in a feature, but that feature 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));
@@ -71,6 +71,247 @@ public class ResolverHookImplTest {
}
@Test
+ public void testProvidingBundleHasNoFeatureSoEveryoneCanAccess() {
+ 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("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, Collections.singleton("global"));
+
+ // b2 is in r2, it requires a capability that is provided by b1.
+ // b1 is not in any feature 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 testProvidingFeatureHasNoRegionsSoEveryoneCanAccess3() {
+ 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("f1", Collections.emptySet());
+ frmap.put("f2", Collections.singleton("r2"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 is in r2, it requires a capability that is provided by b1.
+ // b1 is in a feature that has an empty region set, 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 testProvidingFeatureHasNoExportsSoOtherFeatureCannotAccess() {
+ 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("f1", Collections.singleton("r2"));
+ frmap.put("f2", Collections.singleton("r2"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 is in r2, it requires a capability that is provided by b1.
+ // b1 is also in r2 but is not exporting the package in there, so b2 should not see the package.
+ 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("The exported package is not exported in any region, and b1 is in r2 so b2 should not see the package as its feature-private",
+ 0, candidates1.size());
+ }
+
+ @Test
+ public void testRegionHasPrecedenceOverGlobal() {
+ Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.otherfeature", new Version(1,0,0)), Collections.singletonList("b10"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.infeature", new Version(1,0,0)), Collections.singletonList("b11"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal", new Version(1,0,0)), Collections.singletonList("b19"));
+ 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("b10", Collections.singleton("f10"));
+ bfmap.put("b11", Collections.singleton("f11"));
+ bfmap.put("b2", Collections.singleton("f2"));
+
+ Map<String, Set<String>> frmap = new HashMap<>();
+ frmap.put("f10", new HashSet<>(Arrays.asList("r2", "r3")));
+ frmap.put("f11", new HashSet<>(Arrays.asList("r1", "r2")));
+ frmap.put("f2", Collections.singleton("r1"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+ rpmap.put("r1", Collections.singleton("org.foo.bar"));
+ rpmap.put("r2", new HashSet<>(Arrays.asList("xxx", "yyy", "zzz")));
+ rpmap.put("r3", new HashSet<>(Arrays.asList("org.foo.bar", "zzz")));
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 needs to resolve to 'org.foo.bar' package. b2 is in region r1.
+ // The package is provided by 3 bundles:
+ // b10 provides it but is not in a matching region
+ // b11 provides it and has a matching region
+ // b19 provides it in the global region
+ // Only b11 should provide the capability. Even though b19 provides it from the global region, if there is an overlapping
+ // specific region then the global region should not be used
+ BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+ BundleCapability cap1 = mockCapability("org.foo.bar", "b10", bsnvermap);
+ BundleCapability cap2 = mockCapability("org.foo.bar", "b11", bsnvermap);
+ BundleCapability cap3 = mockCapability("org.foo.bar", "b19", bsnvermap);
+ List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1, cap2, cap3));
+ rh.filterMatches(req1, candidates1);
+
+ assertEquals("Only the capability coming from bundle b11 should be selected, b10 is in a different region and b19 is in global "
+ + "which should be excluded as there is a capability in a matching region.",
+ Collections.singletonList(cap2), candidates1);
+ }
+
+ @Test
+ public void testOwnFeatureHasPrecendenceOverGlobal() {
+ Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.infeature", new Version(1,0,0)), Collections.singletonList("b11"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal", new Version(1,0,0)), Collections.singletonList("b19"));
+ 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("b11", Collections.singleton("f1"));
+ bfmap.put("b2", Collections.singleton("f1"));
+
+ Map<String, Set<String>> frmap = new HashMap<>();
+ frmap.put("f1", Collections.singleton("r1"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+ rpmap.put("r1", Collections.emptySet());
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 needs to resolve 'org.foo.bar' and there are 2 candidates:
+ // b11 is in the same feature as b2
+ // b19 is in the no feature
+ // In this case b11 should be selected as its in the same region
+ BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+ BundleCapability cap1 = mockCapability("org.foo.bar", "b11", bsnvermap);
+ BundleCapability cap2 = mockCapability("org.foo.bar", "b19", bsnvermap);
+ List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1, cap2));
+ rh.filterMatches(req1, candidates1);
+
+ assertEquals("Only b11 should be selected as its from the same region as b2",
+ Collections.singletonList(cap1), candidates1);
+ }
+
+ @Test
+ public void testMultipleGlobalOptionsOnly() {
+ Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.otherfeature", new Version(1,0,0)), Collections.singletonList("b10"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal1", new Version(1,0,0)), Collections.singletonList("b18"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal2", new Version(1,0,0)), Collections.singletonList("b19"));
+ 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("b10", Collections.singleton("f10"));
+ bfmap.put("b2", Collections.singleton("f2"));
+
+ Map<String, Set<String>> frmap = new HashMap<>();
+ frmap.put("f10", new HashSet<>(Arrays.asList("r2", "r3")));
+ frmap.put("f2", Collections.singleton("r1"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+ rpmap.put("r1", Collections.emptySet());
+ rpmap.put("r2", new HashSet<>(Arrays.asList("xxx", "yyy", "zzz")));
+ rpmap.put("r3", new HashSet<>(Arrays.asList("org.foo.bar", "zzz")));
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 needs to resolve to 'org.foo.bar' package. b2 is in region r1.
+ // However nobody in r1 exports the package
+ // The package is exported by b10 in r3 but b2 is not in that region.
+ // The package is exported by b18 and b19 which are both in the global region
+ // Both these candidates should remain available.
+ BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+ BundleCapability cap1 = mockCapability("org.foo.bar", "b10", bsnvermap);
+ BundleCapability cap2 = mockCapability("org.foo.bar", "b18", bsnvermap);
+ BundleCapability cap3 = mockCapability("org.foo.bar", "b19", bsnvermap);
+ List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1, cap2, cap3));
+ rh.filterMatches(req1, candidates1);
+
+ assertEquals("Since there are no specific candidates in a matching region, the candidates from "
+ + "the global region should be allowed.",
+ Arrays.asList(cap2, cap3), candidates1);
+ }
+
+ @Test
+ public void testMultipleGlobalOptionsOnly2() {
+ Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal1", new Version(1,0,0)), Collections.singletonList("b18"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "providing.bundle.inglobal2", new Version(1,0,0)), Collections.singletonList("b19"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "requiring.bundle", new Version(1,0,0)), Collections.singletonList("b2"));
+
+ Map<String, Set<String>> bfmap = new HashMap<>();
+ Map<String, Set<String>> frmap = new HashMap<>();
+ Map<String, Set<String>> rpmap = new HashMap<>();
+
+ ResolverHookImpl rh = new ResolverHookImpl(bsnvermap, bfmap, frmap, rpmap, Collections.singleton("global"));
+
+ // b2 needs to resolve to 'org.foo.bar' package. b2 is in no region.
+ // b18 and b19 export the package in the global region. They should both be allowed.
+ BundleRequirement req1 = mockRequirement("b2", bsnvermap);
+ BundleCapability cap1 = mockCapability("org.foo.bar", "b18", bsnvermap);
+ BundleCapability cap2 = mockCapability("org.foo.bar", "b19", bsnvermap);
+ List<BundleCapability> candidates1 = new ArrayList<>(Arrays.asList(cap1, cap2));
+ rh.filterMatches(req1, candidates1);
+
+ assertEquals(Arrays.asList(cap1, cap2), candidates1);
+ }
+
+ @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)),