This is an automated email from the ASF dual-hosted git repository.
davidb pushed a commit to branch apiregionsbug
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-apiregions.git
commit 36251afcbe35d76a3893d7962e85b3806b3deb90
Author: David Bosschaert <bossc...@adobe.com>
AuthorDate: Fri Sep 1 15:04:39 2023 +0100
Expose the bug
---
.../apiregions/impl/ResolverHookImplTest.java | 72 +++++++++++++++++++---
1 file changed, 63 insertions(+), 9 deletions(-)
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 3f27cb9..46a80e9 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
@@ -18,14 +18,7 @@
*/
package org.apache.sling.feature.apiregions.impl;
-import org.junit.Test;
-import org.mockito.Mockito;
-import org.osgi.framework.Bundle;
-import org.osgi.framework.Version;
-import org.osgi.framework.namespace.PackageNamespace;
-import org.osgi.framework.wiring.BundleCapability;
-import org.osgi.framework.wiring.BundleRequirement;
-import org.osgi.framework.wiring.BundleRevision;
+import static org.junit.Assert.assertEquals;
import java.util.AbstractMap;
import java.util.ArrayList;
@@ -39,7 +32,14 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
-import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.Version;
+import org.osgi.framework.namespace.PackageNamespace;
+import org.osgi.framework.wiring.BundleCapability;
+import org.osgi.framework.wiring.BundleRequirement;
+import org.osgi.framework.wiring.BundleRevision;
public class ResolverHookImplTest {
@Test
@@ -812,6 +812,60 @@ public class ResolverHookImplTest {
assertEquals("There were no candidates, there still are none", 0,
candidates1.size());
}
+ /* This test exposes a bug that exports the package from the wrong bundle.
The situation can be reproduced as follows:
+ * - Have a feature f1 that contains a bundle that exports a package, e.g.
org.soup. Make sure that the feature doesn't
+ * export the package into an API region.
+ * - Have another feature f2 that contains a different bundle that exports
the same package. In this case the feature
+ * exports the package into the 'global' region.
+ * Now when the resolver hook needs to handle the wiring for f2 it gets 2
candidates, because to OSGi both exporting
+ * bundles are candidates.
+ * The resolver hook finds that org.soup is exported into the global
region (by f2). Then it looks at what bundles in
+ * the global region and finds that both f1 and f2 are in the global
region because they are by default.
+ * Then the resolver hook thinks that both candidates are acceptible
because both are in the global region, so it
+ * produces a wiring to the candidate from f1, even though f1 doesn't
export the package.
+ *
+ * It should really just return the candidate from f2, but currently the
data provided to the API regions doesn't have
+ * the richness to know this.
+ */
+ @Test
+ public void testABCXYZ() {
+ Map<Entry<String, Version>, List<String>> bsnvermap = new HashMap<>();
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "b1", new Version(1,0,0)), Collections.singletonList("b1"));
+ bsnvermap.put(new AbstractMap.SimpleEntry<String,Version>(
+ "b2", new Version(1,0,0)), Collections.singletonList("b2"));
+
+ Map<String, Set<String>> bfmap = new HashMap<>();
+ bfmap.put("b1", Collections.singleton("g:feat-int:1"));
+ bfmap.put("b2", Collections.singleton("g:feat-global:1"));
+
+ Map<String, List<String>> frmap = new HashMap<>();
+ frmap.put("g:feat-int:1", Arrays.asList("global",
"org.foo.bar.deprecated"));
+ frmap.put("g:feat-global:1", Arrays.asList("global",
"org.foo.bar.deprecated"));
+
+ Map<String, Set<String>> rpmap = new HashMap<>();
+ rpmap.put("global", Collections.singleton("spaghetti.soup"));
+
+ RegionConfiguration rc = new RegionConfiguration(bsnvermap, bfmap,
frmap, rpmap,
+ new HashSet<>(Arrays.asList("global",
"org.foo.bar.deprecated")));
+
+ ResolverHookImpl rh = new ResolverHookImpl(rc);
+ BundleRequirement req = mockRequirement("b2", bsnvermap);
+ List<BundleCapability> candidates = new ArrayList<>();
+
+ BundleCapability capInt = mockCapability("spaghetti.soup", "b1",
bsnvermap);
+ BundleCapability capGlob = mockCapability("spaghetti.soup", "b2",
bsnvermap);
+ candidates.add(capInt);
+ candidates.add(capGlob);
+
+ rh.filterMatches(req, candidates);
+ assertEquals("We should only get the second candidate here because
that is the one that is exporting " +
+ "spaghetti.soup. However we get both, because the data
currently doesn't expose what feature exported a package " +
+ "it just states that a package is exported in a certain
region. So if there is a bundle in a feature that is part " +
+ "of that region and happens to export it, it is automatically
included, even if that package isn't exported in that feature",
+ 1, candidates.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)) {
