You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by tj...@apache.org on 2017/02/03 22:17:00 UTC

svn commit: r1781611 - in /felix/trunk/resolver/src: main/java/org/apache/felix/resolver/FelixWiring.java main/java/org/apache/felix/resolver/ResolverImpl.java test/java/org/apache/felix/resolver/test/ResolverTest.java

Author: tjwatson
Date: Fri Feb  3 22:16:59 2017
New Revision: 1781611

URL: http://svn.apache.org/viewvc?rev=1781611&view=rev
Log:
[FELIX-5514] Issues with substituted exports with require bundle

A framework implementation already has to calculate substituted packages
for the bundle wirings according to the wires returned by the resolver.
Instead of always having the resolver recalculate the substituted
exports I added a new interface (FelixWiring) which has a method to get
the substitution wires.

In cases where the resolve context does not provide instances of
FelixWiring there is code to do the calculation again based on the wires
in the Wiring.  I added 6 new testcases to cover the different cases for
when bundles are already resolved using FelixWiring objects or just
plain Wiring objects and also cases when a substitutable export is
really substituted or not.

Added:
    felix/trunk/resolver/src/main/java/org/apache/felix/resolver/FelixWiring.java
Modified:
    felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
    felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java

Added: felix/trunk/resolver/src/main/java/org/apache/felix/resolver/FelixWiring.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/FelixWiring.java?rev=1781611&view=auto
==============================================================================
--- felix/trunk/resolver/src/main/java/org/apache/felix/resolver/FelixWiring.java (added)
+++ felix/trunk/resolver/src/main/java/org/apache/felix/resolver/FelixWiring.java Fri Feb  3 22:16:59 2017
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */package org.apache.felix.resolver;
+
+import java.util.Collection;
+import org.osgi.resource.Wire;
+
+public interface FelixWiring
+{
+    Collection<Wire> getSubstitutionWires();
+}

Modified: felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java?rev=1781611&r1=1781610&r2=1781611&view=diff
==============================================================================
--- felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java (original)
+++ felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java Fri Feb  3 22:16:59 2017
@@ -899,7 +899,6 @@ public class ResolverImpl implements Res
         }
         else if (candCap.getNamespace().equals(BundleNamespace.BUNDLE_NAMESPACE))
         {
-
             // Get the candidate's package space to determine which packages
             // will be visible to the current resource.
             if (visitedRequiredBundles.add(candCap.getResource()))
@@ -913,6 +912,13 @@ public class ResolverImpl implements Res
                         currentReq,
                         blame.m_cap);
                 }
+                // now merge in substitutes
+                for (Blame blame : resourcePkgMap.get(
+                    candCap.getResource()).m_substitePkgs.values())
+                {
+                    mergeCandidatePackage(packages.m_requiredPkgs, currentReq,
+                        blame.m_cap);
+                }
             }
 
             // If the candidate requires any other bundles with reexport visibility,
@@ -1143,7 +1149,8 @@ public class ResolverImpl implements Res
             {
                 public void run()
                 {
-                    calculateExportedPackages(session, allCandidates, resource, packages.m_exportedPkgs);
+                    calculateExportedPackages(session, allCandidates, resource,
+                        packages.m_exportedPkgs, packages.m_substitePkgs);
                 }
             });
         }
@@ -1573,7 +1580,7 @@ public class ResolverImpl implements Res
             ResolveSession session,
             Candidates allCandidates,
             Resource resource,
-            OpenHashMap<String, Blame> exports)
+        OpenHashMap<String, Blame> exports, OpenHashMap<String, Blame> substitutes)
     {
         // Get all exported packages.
         Wiring wiring = session.getContext().getWirings().get(resource);
@@ -1598,9 +1605,34 @@ public class ResolverImpl implements Res
         // already excludes imported substitutable exports, but
         // for resolving resources we must look in the candidate
         // map to determine which exports are substitutable.
-        if (!exports.isEmpty())
+        if (wiring != null)
         {
-            if (wiring == null)
+            Collection<Wire> substitutionWires;
+            if (wiring instanceof FelixWiring)
+            {
+                substitutionWires = ((FelixWiring) wiring).getSubstitutionWires();
+            }
+            else
+            {
+                substitutionWires = getSubstitutionWires(wiring);
+            }
+            for (Wire wire : substitutionWires)
+            {
+                Capability cap = wire.getCapability();
+                if (!cap.getResource().equals(wire.getProvider()))
+                {
+                    cap = new WrappedCapability(wire.getProvider(), cap);
+                }
+                substitutes.put(
+                    // Using a null on requirement instead of the wire requirement here.
+                    // It is unclear if we want to treat the substitution requirement as a permutation req here.
+                    (String) cap.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE),
+                    new Blame(cap, null));
+            }
+        }
+        else
+        {
+            if (!exports.isEmpty())
             {
                 for (Requirement req : resource.getRequirements(null))
                 {
@@ -1610,7 +1642,13 @@ public class ResolverImpl implements Res
                         if (cand != null)
                         {
                             String pkgName = (String) cand.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
-                            exports.remove(pkgName);
+                            Blame blame = exports.remove(pkgName);
+                            if (blame != null)
+                            {
+                                // Using a null on requirement instead of the wire requirement here.
+                                // It is unclear if we want to treat the substitution requirement as a permutation req here.
+                                substitutes.put(pkgName, new Blame(cand, null));
+                            }
                         }
                     }
                 }
@@ -1619,6 +1657,49 @@ public class ResolverImpl implements Res
         return exports;
     }
 
+    private static Collection<Wire> getSubstitutionWires(Wiring wiring)
+    {
+        Set<String> exportNames = new HashSet<String>();
+        for (Capability cap : wiring.getResource().getCapabilities(null))
+        {
+            if (PackageNamespace.PACKAGE_NAMESPACE.equals(cap.getNamespace()))
+            {
+                exportNames.add(
+                    (String) cap.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE));
+            }
+        }
+        // Add fragment exports
+        for (Wire wire : wiring.getProvidedResourceWires(null))
+        {
+            if (HostNamespace.HOST_NAMESPACE.equals(wire.getCapability().getNamespace()))
+            {
+                for (Capability cap : wire.getRequirement().getResource().getCapabilities(
+                    null))
+                {
+                    if (PackageNamespace.PACKAGE_NAMESPACE.equals(cap.getNamespace()))
+                    {
+                        exportNames.add((String) cap.getAttributes().get(
+                            PackageNamespace.PACKAGE_NAMESPACE));
+                    }
+                }
+            }
+        }
+        Collection<Wire> substitutionWires = new ArrayList<Wire>();
+        for (Wire wire : wiring.getRequiredResourceWires(null))
+        {
+            if (PackageNamespace.PACKAGE_NAMESPACE.equals(
+                wire.getCapability().getNamespace()))
+            {
+                if (exportNames.contains(wire.getCapability().getAttributes().get(
+                    PackageNamespace.PACKAGE_NAMESPACE)))
+                {
+                    substitutionWires.add(wire);
+                }
+            }
+        }
+        return substitutionWires;
+    }
+
     private static boolean isCompatible(
         Blame currentBlame, Capability candCap,
         Map<Resource, Packages> resourcePkgMap)
@@ -2022,6 +2103,7 @@ public class ResolverImpl implements Res
     public static class Packages
     {
         public final OpenHashMap<String, Blame> m_exportedPkgs;
+        public final OpenHashMap<String, Blame> m_substitePkgs;
         public final OpenHashMap<String, List<Blame>> m_importedPkgs;
         public final OpenHashMap<String, List<Blame>> m_requiredPkgs;
         public final OpenHashMap<String, ArrayMap<Capability, UsedBlames>> m_usedPkgs;
@@ -2034,6 +2116,7 @@ public class ResolverImpl implements Res
             int nbReqs = resource.getRequirements(null).size();
 
             m_exportedPkgs = new OpenHashMap<String, Blame>(nbCaps);
+            m_substitePkgs = new OpenHashMap<String, Blame>(nbCaps);
             m_importedPkgs = new OpenHashMap<String, List<Blame>>(nbReqs) {
                 public List<Blame> compute(String s) {
                     return new ArrayList<Blame>();

Modified: felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java?rev=1781611&r1=1781610&r2=1781611&view=diff
==============================================================================
--- felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java (original)
+++ felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java Fri Feb  3 22:16:59 2017
@@ -33,6 +33,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.felix.resolver.FelixWiring;
 import org.apache.felix.resolver.Logger;
 import org.apache.felix.resolver.ResolverImpl;
 import org.apache.felix.resolver.test.util.BundleCapability;
@@ -856,6 +857,197 @@ public class ResolverTest
 
     }
 
+    @Test
+    public void testScenario17_1() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(false, false, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(
+            Collections.<Resource, Wiring> emptyMap(), candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    @Test
+    public void testScenario17_2() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(false, false, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(wirings, candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    @Test
+    public void testScenario17_3() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(true, false, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(
+            Collections.<Resource, Wiring> emptyMap(), candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    @Test
+    public void testScenario17_4() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(true, false, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(wirings, candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    @Test
+    public void testScenario17_5() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(false, true, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(wirings, candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    @Test
+    public void testScenario17_6() throws Exception
+    {
+        Map<Resource, Wiring> wirings = new HashMap<Resource, Wiring>();
+        Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+        Map<String, Resource> resources = new HashMap<String, Resource>();
+        populateScenario17(true, true, wirings, candMap, resources);
+        ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+        ResolveContextImpl rci = new ResolveContextImpl(wirings, candMap,
+            Collections.singletonList((Resource) resources.get("requiresMisc")),
+            Collections.<Resource> emptyList());
+
+        resolver.resolve(rci);
+    }
+
+    private void populateScenario17(boolean realSubstitute, boolean felixWiring,
+        Map<Resource, Wiring> wirings,
+        Map<Requirement, List<Capability>> candMap, Map<String, Resource> resources)
+    {
+        ResourceImpl core = new ResourceImpl("core");
+        resources.put("core", core);
+        Capability core_pkgCap = addCap(core, PackageNamespace.PACKAGE_NAMESPACE, "pkg1");
+        Capability core_bundleCap = addCap(core, BundleNamespace.BUNDLE_NAMESPACE,
+            "core");
+        Requirement core_pkgReq = addReq(core, PackageNamespace.PACKAGE_NAMESPACE,
+            "pkg1");
+
+        ResourceImpl misc = new ResourceImpl("misc");
+        resources.put("misc", misc);
+        Capability misc_pkgCap = addCap(misc, PackageNamespace.PACKAGE_NAMESPACE, "pkg1");
+        Capability misc_bundleCap = addCap(misc, BundleNamespace.BUNDLE_NAMESPACE,
+            "misc");
+        Requirement misc_bundleReq = addReq(misc, BundleNamespace.BUNDLE_NAMESPACE,
+            "core");
+
+        ResourceImpl importsCore = new ResourceImpl("importsCore");
+        resources.put("ImportsCore", importsCore);
+        Capability importsCore_pkgCap = addCap(importsCore,
+            PackageNamespace.PACKAGE_NAMESPACE, "pkg2", "pkg1");
+        Requirement importsCore_pkgReq = addReq(importsCore,
+            PackageNamespace.PACKAGE_NAMESPACE, "pkg1");
+
+        ResourceImpl requiresMisc = new ResourceImpl("requiresMisc");
+        resources.put("requiresMisc", requiresMisc);
+        Requirement requiresMisc_pkgReq = addReq(requiresMisc,
+            PackageNamespace.PACKAGE_NAMESPACE, "pkg2");
+        Requirement requiresMisc_bundleReq = addReq(requiresMisc,
+            BundleNamespace.BUNDLE_NAMESPACE, "misc");
+
+        ResourceImpl substitutesCore = new ResourceImpl("substitutesCore");
+        resources.put("substitutesCore", substitutesCore);
+        Capability substitutesCore_pkgCap = addCap(substitutesCore,
+            PackageNamespace.PACKAGE_NAMESPACE, "pkg1");
+
+        candMap.put(core_pkgReq, Collections.singletonList(
+            realSubstitute ? substitutesCore_pkgCap : core_pkgCap));
+        candMap.put(misc_bundleReq, Collections.singletonList(core_bundleCap));
+        candMap.put(importsCore_pkgReq, Collections.singletonList(
+            realSubstitute ? substitutesCore_pkgCap : core_pkgCap));
+        candMap.put(requiresMisc_pkgReq, Collections.singletonList(importsCore_pkgCap));
+        candMap.put(requiresMisc_bundleReq, Collections.singletonList(misc_bundleCap));
+
+        Map<Resource, List<Wire>> wires = new HashMap<Resource, List<Wire>>();
+        wires.put(substitutesCore, new ArrayList<Wire>());
+        wires.put(core, new ArrayList<Wire>());
+        if (realSubstitute)
+        {
+            wires.get(core).add(new SimpleWire(core_pkgReq, substitutesCore_pkgCap));
+        }
+        wires.put(misc, new ArrayList<Wire>());
+        wires.get(misc).add(new SimpleWire(misc_bundleReq, core_bundleCap));
+
+        Map<Resource, List<Wire>> invertedWires = new HashMap<Resource, List<Wire>>();
+        invertedWires.put(substitutesCore, new ArrayList<Wire>());
+        if (realSubstitute)
+        {
+            invertedWires.get(substitutesCore).add(
+                new SimpleWire(core_pkgReq, substitutesCore_pkgCap));
+        }
+        invertedWires.put(core, new ArrayList<Wire>());
+        invertedWires.get(core).add(new SimpleWire(misc_bundleReq, core_bundleCap));
+        invertedWires.put(misc, new ArrayList<Wire>());
+
+        wirings.put(substitutesCore, new SimpleWiring(substitutesCore,
+            Arrays.asList(substitutesCore_pkgCap), wires, invertedWires));
+
+        Wiring coreWiring;
+        if (felixWiring)
+        {
+            if (realSubstitute)
+            {
+                coreWiring = new SimpleFelixWiring(core,
+                    Arrays.asList(core_bundleCap, core_pkgCap), wires, invertedWires,
+                    Arrays.<Wire> asList(
+                        new SimpleWire(core_pkgReq, substitutesCore_pkgCap)));
+            }
+            else
+            {
+                coreWiring = new SimpleFelixWiring(core,
+                    Arrays.asList(core_bundleCap, core_pkgCap), wires, invertedWires,
+                    Collections.<Wire> emptyList());
+            }
+        }
+        else
+        {
+            coreWiring = new SimpleWiring(core,
+                Arrays.asList(core_bundleCap, core_pkgCap), wires, invertedWires);
+        }
+
+        wirings.put(core, coreWiring);
+        wirings.put(misc, new SimpleWiring(misc,
+            Arrays.asList(misc_bundleCap, misc_pkgCap), wires, invertedWires));
+    }
+
     private static String getResourceName(Resource r)
     {
         return r.getCapabilities(IdentityNamespace.IDENTITY_NAMESPACE).get(0).getAttributes()
@@ -1290,6 +1482,23 @@ public class ResolverTest
         }
     }
 
+    private static class SimpleFelixWiring extends SimpleWiring implements FelixWiring
+    {
+        final Collection<Wire> substitutionWires;
+
+        public SimpleFelixWiring(Resource resource, List<Capability> resourceCapabilities, Map<Resource, List<Wire>> wires, Map<Resource, List<Wire>> invertedWires, Collection<Wire> substitutionWires)
+        {
+            super(resource, resourceCapabilities, wires, invertedWires);
+            this.substitutionWires = substitutionWires;
+        }
+
+        public Collection<Wire> getSubstitutionWires()
+        {
+            return substitutionWires;
+        }
+
+    }
+
     private static class SimpleWiring implements Wiring {
         final Resource resource;
         final Map<Resource, List<Wire>> wires;
@@ -1379,3 +1588,4 @@ public class ResolverTest
         }
     }
 }
+