Bug 217724 Substitutable exports and require-bundle
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/SubstitutableExportsTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/SubstitutableExportsTest.java
index 7f287f2..6ed070b 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/SubstitutableExportsTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/SubstitutableExportsTest.java
@@ -1217,6 +1217,68 @@
 		return state;
 	}
 
+	private State getNonOverlapingSubstituteBasicState() throws BundleException {
+		// Basic substitutable export test with A, B, C all exporting and importing x,y packages
+		// D, E, F all requiring A, B, C respectively to access x, y packages
+		// all should get packages x and y from A
+		State state = buildEmptyState();
+		Hashtable manifest = new Hashtable();
+		long bundleID = 0;
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "A"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.EXPORT_PACKAGE, "x; y; version=1.0"); //$NON-NLS-1$
+		manifest.put(Constants.IMPORT_PACKAGE, "x; y; version=1.0; nomatch=nomatch"); //$NON-NLS-1$
+		BundleDescription a = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "B"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.EXPORT_PACKAGE, "x; y; version=1.0"); //$NON-NLS-1$
+		manifest.put(Constants.IMPORT_PACKAGE, "x; y; version=1.0"); //$NON-NLS-1$
+		BundleDescription b = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "C"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.EXPORT_PACKAGE, "x; y; version=1.0"); //$NON-NLS-1$
+		manifest.put(Constants.IMPORT_PACKAGE, "x; y; version=1.0"); //$NON-NLS-1$
+		BundleDescription c = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "D"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.REQUIRE_BUNDLE, "A"); //$NON-NLS-1$
+		BundleDescription d = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "E"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.REQUIRE_BUNDLE, "B"); //$NON-NLS-1$
+		BundleDescription e = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "F"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0.0"); //$NON-NLS-1$
+		manifest.put(Constants.REQUIRE_BUNDLE, "C"); //$NON-NLS-1$
+		BundleDescription f = state.getFactory().createBundleDescription(state, manifest, (String) manifest.get(Constants.BUNDLE_SYMBOLICNAME) + (String) manifest.get(Constants.BUNDLE_VERSION), bundleID++);
+
+		state.addBundle(a);
+		state.addBundle(b);
+		state.addBundle(c);
+		state.addBundle(d);
+		state.addBundle(e);
+		state.addBundle(f);
+		return state;
+	}
+
 	public void testSubstitutableExports001() throws BundleException {
 		State state = getSubstituteBasicState();
 		state.resolve();
@@ -2668,4 +2730,52 @@
 			assertContains("nVisible not correct", nVisible, mnExpected[index]); //$NON-NLS-1$
 		}
 	}
+
+	public void testSubstitutableExports024() throws BundleException {
+		State state = getNonOverlapingSubstituteBasicState();
+		state.resolve();
+		BundleDescription a = state.getBundle(0);
+		BundleDescription b = state.getBundle(1);
+		BundleDescription c = state.getBundle(2);
+		BundleDescription d = state.getBundle(3);
+		BundleDescription e = state.getBundle(4);
+		BundleDescription f = state.getBundle(5);
+
+		assertTrue("1.0", a.isResolved()); //$NON-NLS-1$
+		assertTrue("1.1", b.isResolved()); //$NON-NLS-1$
+		assertTrue("1.2", c.isResolved()); //$NON-NLS-1$
+		assertTrue("1.3", d.isResolved()); //$NON-NLS-1$
+		assertTrue("1.4", e.isResolved()); //$NON-NLS-1$
+		assertTrue("1.5", f.isResolved()); //$NON-NLS-1$
+
+		ExportPackageDescription[] aVisible = state.getStateHelper().getVisiblePackages(a);
+		ExportPackageDescription[] bVisible = state.getStateHelper().getVisiblePackages(b);
+		ExportPackageDescription[] cVisible = state.getStateHelper().getVisiblePackages(c);
+		ExportPackageDescription[] dVisible = state.getStateHelper().getVisiblePackages(d);
+		ExportPackageDescription[] eVisible = state.getStateHelper().getVisiblePackages(e);
+		ExportPackageDescription[] fVisible = state.getStateHelper().getVisiblePackages(f);
+
+		assertNotNull("aVisible is null", aVisible); //$NON-NLS-1$
+		assertNotNull("bVisible is null", bVisible); //$NON-NLS-1$
+		assertNotNull("cVisible is null", cVisible); //$NON-NLS-1$
+		assertNotNull("dVisible is null", dVisible); //$NON-NLS-1$
+		assertNotNull("eVisible is null", eVisible); //$NON-NLS-1$
+		assertNotNull("fVisible is null", fVisible); //$NON-NLS-1$
+
+		assertEquals("aVisible wrong number", 0, aVisible.length); //$NON-NLS-1$
+		assertEquals("bVisible wrong number", 2, bVisible.length); //$NON-NLS-1$
+		assertEquals("cVisible wrong number", 2, cVisible.length); //$NON-NLS-1$
+		assertEquals("dVisible wrong number", 2, dVisible.length); //$NON-NLS-1$
+		assertEquals("eVisible wrong number", 2, eVisible.length); //$NON-NLS-1$
+		assertEquals("fVisible wrong number", 2, fVisible.length); //$NON-NLS-1$
+
+		ExportPackageDescription[] aExports = a.getSelectedExports();
+		assertEquals("aVisible not correct", aExports, a.getExportPackages()); //$NON-NLS-1$
+		assertEquals("bVisible not correct", aExports, bVisible); //$NON-NLS-1$
+		assertEquals("cVisible not correct", aExports, cVisible); //$NON-NLS-1$
+		assertEquals("dVisible not correct", aExports, dVisible); //$NON-NLS-1$
+		assertEquals("eVisible not correct", aExports, eVisible); //$NON-NLS-1$
+		assertEquals("fVisible not correct", aExports, fVisible); //$NON-NLS-1$
+	}
+
 }
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/module/ResolverImpl.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/module/ResolverImpl.java
index 61d5412..5485f71 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/module/ResolverImpl.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/module/ResolverImpl.java
@@ -1254,8 +1254,9 @@
 			return true; // Already wired (due to grouping dependencies) so just return
 		}
 		boolean result = false;
+		ResolverExport[] substitutableExps = imp.getBundle().getExports(imp.getName());
 		Object[] exports = resolverExports.get(imp.getName());
-		exportsloop: for (int i = 0; i < exports.length; i++) {
+		for (int i = 0; i < exports.length; i++) {
 			ResolverExport export = (ResolverExport) exports[i];
 			if (DEBUG_IMPORTS)
 				ResolverImpl.log("CHECKING: " + export.getExporter().getBundle() + ", " + export.getName()); //$NON-NLS-1$ //$NON-NLS-2$
@@ -1269,22 +1270,19 @@
 				export.getExporter().addRef(imp.getBundle());
 				// first add the possible supplier; this is done before resolving the supplier bundle to prevent endless cycle loops.
 				imp.addPossibleSupplier(export);
-				ResolverExport[] importerExps = null;
 				if (imp.getBundle() != export.getExporter()) {
-					// Save the exports of this package from the importer in case we need to add them back
-					importerExps = imp.getBundle().getExports(imp.getName());
-					for (int j = 0; j < importerExps.length; j++)
-						if (importerExps[j].getSubstitute() == null)
-							importerExps[j].setSubstitute(export); // Import wins, drop export
+					for (int j = 0; j < substitutableExps.length; j++)
+						if (substitutableExps[j].getSubstitute() == null)
+							substitutableExps[j].setSubstitute(export); // Import wins, drop export
 					// if in dev mode then allow a constraint to resolve to an unresolved bundle
 					if ((originalState != ResolverBundle.RESOLVED && !resolveBundle(export.getExporter(), cycle) && !developmentMode) || export.getSubstitute() != null) {
 						// remove the possible supplier
 						imp.removePossibleSupplier(export);
 						// add back the exports of this package from the importer
 						if (imp.getSelectedSupplier() == null)
-							for (int j = 0; j < importerExps.length; j++)
-								if (importerExps[j].getSubstitute() == export)
-									importerExps[j].setSubstitute(null);
+							for (int j = 0; j < substitutableExps.length; j++)
+								if (substitutableExps[j].getSubstitute() == export)
+									substitutableExps[j].setSubstitute(null);
 						continue; // Bundle hasn't resolved || export has not been selected and is unavailable
 					}
 				} else if (export.getSubstitute() != null)
@@ -1310,6 +1308,8 @@
 			return true;
 		if (imp.isOptional())
 			return true; // If the import is optional then just return true
+		if (substitutableExps.length > 0 && substitutableExps[0].getSubstitute() == null)
+			return true; // If we still have an export that is not substituted return true
 		return false;
 	}