Bug 559840 - handle connect content for system bundle correctly

Change-Id: I1809c5fe464b23b445eb4fbd42f5c318dff48c6b

3 files changed, 100 insertions, 43 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
index 277f2123e..820fc14ab 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
@@ -58,6 +58,7 @@ import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.ServiceReference;
 import org.osgi.framework.connect.ConnectContent;
 import org.osgi.framework.connect.ConnectContent.ConnectEntry;
@@ -88,7 +89,8 @@ public class ConnectTests extends AbstractBundleTests {
 		doTestConnect(moduleConnector, fwkConfig, test, false);
 	}
 
-	void doTestConnect(ModuleConnector moduleConnector, Map<String, String> fwkConfig, Consumer<Framework> test, boolean enableRuntimeVerification) {
+	void doTestConnect(ModuleConnector moduleConnector, Map<String, String> fwkConfig, Consumer<Framework> test,
+			boolean enableRuntimeVerification) {
 		File config = OSGiTestsActivator.getContext().getDataFile(getName());
 		config.mkdirs();
 		fwkConfig.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath());
@@ -415,7 +417,8 @@ public class ConnectTests extends AbstractBundleTests {
 			}
 		});
 		TestCase.assertEquals("Wrong init store file.", storeFile.get(), initFile.get());
-		assertTrue("Did not find all init configs: " + initConfig.get(), initConfig.get().entrySet().containsAll(config.entrySet()));
+		assertTrue("Did not find all init configs: " + initConfig.get(),
+				initConfig.get().entrySet().containsAll(config.entrySet()));
 		try {
 			initConfig.get().put("k3", "v3");
 			fail("Expected unmodifiable map");
@@ -494,18 +497,18 @@ public class ConnectTests extends AbstractBundleTests {
 	}
 
 	private static void checkConnectTag(Bundle b) {
-		final List<String> namespaces = new ArrayList<>(Arrays.asList(BundleNamespace.BUNDLE_NAMESPACE, HostNamespace.HOST_NAMESPACE, IdentityNamespace.IDENTITY_NAMESPACE));
-
-		b.adapt(BundleRevision.class).getCapabilities(null)
-			.stream()
-			.filter(c -> namespaces.contains(c.getNamespace()))
-			.forEach(c -> {
-				List<String> tags = (List<String>) c.getAttributes().get(IdentityNamespace.CAPABILITY_TAGS_ATTRIBUTE);
-				assertNotNull("No tags found.", tags);
-				assertEquals("Wrong number of tags.", 1, tags.size());
-				assertTrue("Connect tag not found.", tags.contains(ConnectContent.TAG_OSGI_CONNECT));
-				namespaces.remove(c.getNamespace());
-			});
+		final List<String> namespaces = new ArrayList<>(Arrays.asList(BundleNamespace.BUNDLE_NAMESPACE,
+				HostNamespace.HOST_NAMESPACE, IdentityNamespace.IDENTITY_NAMESPACE));
+
+		b.adapt(BundleRevision.class).getCapabilities(null).stream().filter(c -> namespaces.contains(c.getNamespace()))
+				.forEach(c -> {
+					List<String> tags = (List<String>) c.getAttributes()
+							.get(IdentityNamespace.CAPABILITY_TAGS_ATTRIBUTE);
+					assertNotNull("No tags found.", tags);
+					assertEquals("Wrong number of tags.", 1, tags.size());
+					assertTrue("Connect tag not found.", tags.contains(ConnectContent.TAG_OSGI_CONNECT));
+					namespaces.remove(c.getNamespace());
+				});
 		assertTrue("Connect tag namespaces were not removed completely. Found " + namespaces, namespaces.isEmpty());
 	}
 
@@ -515,7 +518,8 @@ public class ConnectTests extends AbstractBundleTests {
 						Collections.singletonMap(Namespace.REQUIREMENT_FILTER_DIRECTIVE, "(tags=osgi.connect)"), //
 						Collections.emptyMap()));
 		osgiConnectTags.forEach(c -> {
-			assertTrue("Unexpected tag on bundle: " + c.getRevision().getBundle(), locations.contains(c.getRevision().getBundle().getLocation()));
+			assertTrue("Unexpected tag on bundle: " + c.getRevision().getBundle(),
+					locations.contains(c.getRevision().getBundle().getLocation()));
 		});
 	}
 
@@ -544,11 +548,14 @@ public class ConnectTests extends AbstractBundleTests {
 					ServiceReference<?>[] registered = b.getRegisteredServices();
 					assertNotNull("No services found.", registered);
 					assertEquals("Wrong number of services.", 1, registered.length);
-					assertEquals("Wrong service property.", Activator.class.getSimpleName() + id, (String) registered[0].getProperty("activator"));
+					assertEquals("Wrong service property.", Activator.class.getSimpleName() + id,
+							(String) registered[0].getProperty("activator"));
 					if (provideLoader) {
-						assertTrue("Expected the same classes.", Activator.class.equals(b.loadClass(Activator.class.getName())));
+						assertTrue("Expected the same classes.",
+								Activator.class.equals(b.loadClass(Activator.class.getName())));
 					} else {
-						assertFalse("Expected different classes.", Activator.class.equals(b.loadClass(Activator.class.getName())));
+						assertFalse("Expected different classes.",
+								Activator.class.equals(b.loadClass(Activator.class.getName())));
 					}
 				}
 			} catch (Throwable t) {
@@ -596,17 +603,21 @@ public class ConnectTests extends AbstractBundleTests {
 					assertTrue("Wrong bundle entry URLs: " + bundleEntryUrls, entries.containsAll(bundleEntryUrls));
 
 					List<String> bundleEntryPaths = new ArrayList<>();
-					for (Enumeration<String> ePaths = b.getEntryPaths("org/eclipse/osgi/tests/bundles/resources"); ePaths.hasMoreElements();) {
+					for (Enumeration<String> ePaths = b
+							.getEntryPaths("org/eclipse/osgi/tests/bundles/resources"); ePaths.hasMoreElements();) {
 						bundleEntryPaths.add(ePaths.nextElement());
 					}
 					assertEquals("Wrong number of bundle entry paths from root.", 1, bundleEntryPaths.size());
-					assertEquals("Wrong bundle entry found at root.", "org/eclipse/osgi/tests/bundles/resources/" + id + ".txt", bundleEntryPaths.get(0));
+					assertEquals("Wrong bundle entry found at root.",
+							"org/eclipse/osgi/tests/bundles/resources/" + id + ".txt", bundleEntryPaths.get(0));
 
 					BundleWiring wiring = b.adapt(BundleWiring.class);
 					assertNotNull("No wiring.", wiring);
-					Collection<String> wiringResourcePaths = wiring.listResources("/", "*", BundleWiring.LISTRESOURCES_LOCAL | BundleWiring.LISTRESOURCES_RECURSE);
+					Collection<String> wiringResourcePaths = wiring.listResources("/", "*",
+							BundleWiring.LISTRESOURCES_LOCAL | BundleWiring.LISTRESOURCES_RECURSE);
 					assertEquals("Wrong number of resource paths.", entries.size(), wiringResourcePaths.size());
-					assertTrue("Wrong resource paths: " + wiringResourcePaths, entries.containsAll(wiringResourcePaths));
+					assertTrue("Wrong resource paths: " + wiringResourcePaths,
+							entries.containsAll(wiringResourcePaths));
 
 					Set<String> wiringEntryUrls = new HashSet<>();
 					for (URL url : wiring.findEntries("/", "*", BundleWiring.FINDENTRIES_RECURSE)) {
@@ -708,7 +719,7 @@ public class ConnectTests extends AbstractBundleTests {
 				} else {
 					checkHeaders(m.getContent().getHeaders().get(), headers1);
 				}
-				// set the new content 
+				// set the new content
 				m.setContent(withManifest ? createSimpleManifestContent(NAME2) : createSimpleHeadersContent(NAME2));
 				b.update();
 				Dictionary<String, String> headers2 = b.getHeaders();
@@ -752,7 +763,8 @@ public class ConnectTests extends AbstractBundleTests {
 				f.start();
 				Bundle b = f.getBundleContext().getBundle(NAME);
 				assertFalse("Content is not closed", m.getContent().isOpen());
-				//Bundle.getHeaders() will eventually call ConnectBundleFile.getConnectHeaders() which opens the connect content
+				// Bundle.getHeaders() will eventually call
+				// ConnectBundleFile.getConnectHeaders() which opens the connect content
 				headers2.set(b.getHeaders());
 				assertTrue("Content is not open", m.getContent().isOpen());
 				f.stop();
@@ -780,8 +792,10 @@ public class ConnectTests extends AbstractBundleTests {
 	}
 
 	void dotestInstallUpdate(boolean installWithInputStream, boolean updateWithInputStream) throws Exception {
-		final InputStream in1 = installWithInputStream ? new URL(installer.getBundleLocation("test")).openStream() : null;
-		final InputStream in2 = updateWithInputStream ? new URL(installer.getBundleLocation("test2")).openStream() : null;
+		final InputStream in1 = installWithInputStream ? new URL(installer.getBundleLocation("test")).openStream()
+				: null;
+		final InputStream in2 = updateWithInputStream ? new URL(installer.getBundleLocation("test2")).openStream()
+				: null;
 		final String NAME1 = installWithInputStream ? "test1" : "bundle1";
 		final String NAME2 = updateWithInputStream ? "test2" : "bundle2";
 
@@ -814,6 +828,33 @@ public class ConnectTests extends AbstractBundleTests {
 		});
 	}
 
+	public void testSystemBundleContent() {
+		TestCountingModuleConnector connector = new TestCountingModuleConnector();
+		Bundle systemBundle = getContext().getBundle(Constants.SYSTEM_BUNDLE_LOCATION);
+		Map<String, String> headers = new HashMap<>(FrameworkUtil.asMap(systemBundle.getHeaders()));
+		headers.put("test.key", "test.value");
+		// remove bundle manifest version to allow java export
+		headers.remove(Constants.BUNDLE_MANIFESTVERSION);
+
+		TestConnectModule systemModule = new TestConnectModule(
+				new TestConnectContent(headers, systemBundle.adapt(BundleWiring.class).getClassLoader()));
+		connector.setModule(Constants.SYSTEM_BUNDLE_LOCATION, systemModule);
+		Consumer<Framework> test = f -> {
+			try {
+				f.init();
+				Dictionary<String, String> h = f.getHeaders();
+				assertEquals("Wrong system BSN", systemBundle.getSymbolicName(), f.getSymbolicName());
+				assertEquals("Wrong test value", "test.value", h.get("test.key"));
+			} catch (Throwable t) {
+				sneakyThrow(t);
+			}
+		};
+
+		// run twice to test clean and persistent start
+		doTestConnect(connector, new HashMap<>(), test);
+		doTestConnect(connector, new HashMap<>(), test);
+	}
+
 	private void checkHeaders(Map<String, String> expected, Dictionary<String, String> actual) {
 		assertEquals("Headers size not equals", expected.size(), actual.size());
 		for (Entry<String, String> entry : expected.entrySet()) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
index 69b566bb4..ce2b51ce6 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
@@ -57,6 +57,7 @@ import org.eclipse.osgi.util.ManifestElement;
 import org.eclipse.osgi.util.NLS;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.connect.ConnectModule;
 
 public final class BundleInfo {
 	public static final String OSGI_BUNDLE_MANIFEST = "META-INF/MANIFEST.MF"; //$NON-NLS-1$
@@ -101,7 +102,7 @@ public final class BundleInfo {
 		public BundleFile getBundleFile() {
 			synchronized (genMonitor) {
 				if (bundleFile == null) {
-					if (getBundleId() == 0 && content == null) {
+					if (getBundleId() == 0 && content == null && contentType != Type.CONNECT) {
 						bundleFile = new SystemBundleFile();
 					} else {
 						bundleFile = getStorage().createBundleFile(content, this, isDirectory, true);
@@ -252,6 +253,14 @@ public final class BundleInfo {
 		}
 
 		void setContent(File content, Type contentType) {
+			if (getBundleId() == 0) {
+				// check connect for content first
+				ConnectModule connected = getStorage().getEquinoxContainer().getConnectModules().connect(getLocation());
+				if (connected != null) {
+					content = null;
+					contentType = Type.CONNECT;
+				}
+			}
 			synchronized (this.genMonitor) {
 				this.content = content;
 				this.isDirectory = content == null ? false : Storage.secureAction.isDirectory(content);
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
index 408cb33fa..730206a6c 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java
@@ -1528,31 +1528,32 @@ public class Storage {
 			boolean isMRJar = (version >= MR_JAR_VERSION) ? in.readBoolean() : false;
 
 			File content = null;
-			if (infoId == 0) {
-				content = getSystemContent();
-				isDirectory = content != null ? content.isDirectory() : false;
-				// Note that we do not do any checking for absolute paths with
-				// the system bundle.  We always take the content as discovered
-				// by getSystemContent()
-			} else if (contentType != Type.CONNECT) {
-				content = new File(contentPath);
-				if (!content.isAbsolute()) {
-					// make sure it has the absolute location instead
-					switch (contentType) {
-						case REFERENCE :
+			if (contentType != Type.CONNECT) {
+				if (infoId == 0) {
+					content = getSystemContent();
+					isDirectory = content != null ? content.isDirectory() : false;
+					// Note that we do not do any checking for absolute paths with
+					// the system bundle. We always take the content as discovered
+					// by getSystemContent()
+				} else {
+					content = new File(contentPath);
+					if (!content.isAbsolute()) {
+						// make sure it has the absolute location instead
+						switch (contentType) {
+						case REFERENCE:
 							// reference installs are relative to the installPath
 							content = new File(installPath, contentPath);
 							break;
-						case DEFAULT :
+						case DEFAULT:
 							// normal installs are relative to the storage area
 							content = getFile(contentPath, true);
 							break;
-						default :
+						default:
 							throw new IllegalArgumentException("Unknown type: " + contentType); //$NON-NLS-1$
+						}
 					}
 				}
 			}
-
 			BundleInfo info = new BundleInfo(this, infoId, infoLocation, nextGenId);
 			Generation generation = info.restoreGeneration(generationId, content, isDirectory, contentType, hasPackageInfo, cachedHeaders, lastModified, isMRJar);
 			result.put(infoId, generation);
@@ -1567,7 +1568,9 @@ public class Storage {
 	private void connectPersistentBundles(List<Generation> generations) {
 		generations.forEach(g -> {
 			try {
-				equinoxContainer.getConnectModules().connect(g.getBundleInfo().getLocation());
+				if (g.getContentType() == Type.CONNECT) {
+					equinoxContainer.getConnectModules().connect(g.getBundleInfo().getLocation());
+				}
 			} catch (IllegalStateException e) {
 				if (!(e.getCause() instanceof BundleException)) {
 					throw e;
@@ -2289,4 +2292,8 @@ public class Storage {
 		}
 		return storageStream;
 	}
+
+	EquinoxContainer getEquinoxContainer() {
+		return equinoxContainer;
+	}
 }