Bug 216511 Should exception be thrown when adding a bundle that already belongs to another state?

5 files changed, 185 insertions, 5 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
index 48f00b955..562020802 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2007 IBM Corporation and others.
+ * Copyright (c) 2003, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -2950,6 +2950,166 @@ public class StateResolverTest extends AbstractStateTest {
 		assertTrue("1.0", testNativeBundle.isResolved());
 	}
 
+	public void testMultiStateAdd01() throws BundleException {
+		State state1 = buildEmptyState();
+
+		// test the selection algorithm of the resolver to pick the bundles which
+		// resolve the largest set of bundles
+		Hashtable manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "sdk; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "platform; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription sdk10 = state1.getFactory().createBundleDescription(state1, manifest, "sdk10", 0);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "platform; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription platform10 = state1.getFactory().createBundleDescription(state1, manifest, "platform10", 1);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "rcp; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		BundleDescription rcp10 = state1.getFactory().createBundleDescription(state1, manifest, "rcp10", 2);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "gef; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,1.0]\"");
+		BundleDescription gef10 = state1.getFactory().createBundleDescription(state1, manifest, "gef10", 3);
+
+		state1.addBundle(sdk10);
+		state1.addBundle(platform10);
+		state1.addBundle(rcp10);
+		state1.addBundle(gef10);
+		state1.resolve();
+		assertTrue("1.0", sdk10.isResolved());
+		assertTrue("1.1", platform10.isResolved());
+		assertTrue("1.2", rcp10.isResolved());
+		assertTrue("1.3", gef10.isResolved());
+
+		State state2 = buildEmptyState();
+		try {
+			state2.addBundle(rcp10);
+			fail("Expected IllegalStateException on adding to multiple states");
+		} catch (IllegalStateException e) {
+			// expected
+		}
+	}
+
+	public void testMultiStateAdd02() throws BundleException {
+		State state1 = buildEmptyState();
+
+		// test the selection algorithm of the resolver to pick the bundles which
+		// resolve the largest set of bundles
+		Hashtable manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "sdk; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "platform; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription sdk10 = state1.getFactory().createBundleDescription(state1, manifest, "sdk10", 0);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "platform; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription platform10 = state1.getFactory().createBundleDescription(state1, manifest, "platform10", 1);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "rcp; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		BundleDescription rcp10 = state1.getFactory().createBundleDescription(state1, manifest, "rcp10", 2);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "gef; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,1.0]\"");
+		BundleDescription gef10 = state1.getFactory().createBundleDescription(state1, manifest, "gef10", 3);
+
+		state1.addBundle(sdk10);
+		state1.addBundle(platform10);
+		state1.addBundle(rcp10);
+		state1.addBundle(gef10);
+		state1.resolve();
+		assertTrue("1.0", sdk10.isResolved());
+		assertTrue("1.1", platform10.isResolved());
+		assertTrue("1.2", rcp10.isResolved());
+		assertTrue("1.3", gef10.isResolved());
+
+		// remove the rcp10 bundle.  The bundle will be removal pending
+		// this should still throw an exception until the removal is no longer pending
+		state1.removeBundle(rcp10);
+		State state2 = buildEmptyState();
+		try {
+			state2.addBundle(rcp10);
+			fail("Expected IllegalStateException on adding to multiple states");
+		} catch (IllegalStateException e) {
+			// expected
+		}
+	}
+
+	public void testMultiStateAdd03() throws BundleException {
+		State state1 = buildEmptyState();
+
+		// test the selection algorithm of the resolver to pick the bundles which
+		// resolve the largest set of bundles
+		Hashtable manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "sdk; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "platform; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription sdk10 = state1.getFactory().createBundleDescription(state1, manifest, "sdk10", 0);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "platform; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,2.0]\"");
+		BundleDescription platform10 = state1.getFactory().createBundleDescription(state1, manifest, "platform10", 1);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "rcp; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		BundleDescription rcp10 = state1.getFactory().createBundleDescription(state1, manifest, "rcp10", 2);
+
+		manifest = new Hashtable();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "gef; " + Constants.SINGLETON_DIRECTIVE + ":=true");
+		manifest.put(Constants.BUNDLE_VERSION, "1.0");
+		manifest.put(Constants.REQUIRE_BUNDLE, "rcp; bundle-version=\"[1.0,1.0]\"");
+		BundleDescription gef10 = state1.getFactory().createBundleDescription(state1, manifest, "gef10", 3);
+
+		state1.addBundle(sdk10);
+		state1.addBundle(platform10);
+		state1.addBundle(rcp10);
+		state1.addBundle(gef10);
+		state1.resolve();
+		assertTrue("1.0", sdk10.isResolved());
+		assertTrue("1.1", platform10.isResolved());
+		assertTrue("1.2", rcp10.isResolved());
+		assertTrue("1.3", gef10.isResolved());
+
+		// remove the rcp10 bundle.  The bundle will be removal pending
+		// this should still throw an exception until the removal is no longer pending
+		state1.removeBundle(rcp10);
+		state1.resolve(new BundleDescription[] {rcp10});
+		State state2 = buildEmptyState();
+
+		try {
+			state2.addBundle(rcp10);
+		} catch (IllegalStateException e) {
+			fail("Unexpected IllegalStateException on adding to state", e);
+		}
+	}
+
 	public static class CatchAllValue {
 		public CatchAllValue(String s) {
 			//do nothing
diff --git a/bundles/org.eclipse.osgi/core/adaptor/org/eclipse/osgi/service/resolver/State.java b/bundles/org.eclipse.osgi/core/adaptor/org/eclipse/osgi/service/resolver/State.java
index 845888bcd..304244ed8 100644
--- a/bundles/org.eclipse.osgi/core/adaptor/org/eclipse/osgi/service/resolver/State.java
+++ b/bundles/org.eclipse.osgi/core/adaptor/org/eclipse/osgi/service/resolver/State.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2007 IBM Corporation and others.
+ * Copyright (c) 2003, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -27,9 +27,18 @@ import org.osgi.framework.Version;
 public interface State {
 	/**
 	 * Adds the given bundle to this state.
+	 * <p>
+	 * If the bundle already exists in another state then an <code>IllegalStateException</code>
+	 * will be thrown.  Note that even if you remove a <code>BundleDescription</code> from
+	 * one <code>State</code> object using {@link State#removeBundle(BundleDescription)} it 
+	 * may still be considered as removing pending if other bundles in that state depend on the
+	 * bundle you removed.  To complete a pending removal a call must be done to 
+	 * {@link State#resolve(BundleDescription[])} with the removed bundle.
+	 * </p>
 	 * 
 	 * @param description the description to add
 	 * @return a boolean indicating whether the bundle was successfully added
+	 * @throws IllegalStateException if the bundle already exists in another state
 	 */
 	public boolean addBundle(BundleDescription description);
 
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
index 6ade24af2..9ac6e28e2 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2007 IBM Corporation and others.
+ * Copyright (c) 2003, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -466,6 +466,13 @@ public abstract class StateImpl implements State {
 	}
 
 	boolean basicAddBundle(BundleDescription description) {
+		StateImpl origState = (StateImpl) description.getContainingState();
+		if (origState != null && origState != this) {
+			if (origState.removalPendings.contains(description))
+				throw new IllegalStateException(NLS.bind(StateMsg.BUNDLE_PENDING_REMOVE_STATE, description.toString()));
+			if (origState.getBundle(description.getBundleId()) == description)
+				throw new IllegalStateException(NLS.bind(StateMsg.BUNDLE_IN_OTHER_STATE, description.toString()));
+		}
 		((BundleDescriptionImpl) description).setContainingState(this);
 		((BundleDescriptionImpl) description).setStateBit(BundleDescriptionImpl.REMOVAL_PENDING, false);
 		if (bundleDescriptions.add((BundleDescriptionImpl) description)) {
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMessages.properties b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMessages.properties
index 79f06e2d3..6c17d2f96 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMessages.properties
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMessages.properties
@@ -1,5 +1,5 @@
 ###############################################################################
-# Copyright (c) 2004, 2007 IBM Corporation and others.
+# Copyright (c) 2004, 2008 IBM Corporation and others.
 # All rights reserved. This program and the accompanying materials
 # are made available under the terms of the Eclipse Public License v1.0
 # which accompanies this distribution, and is available at
@@ -11,6 +11,8 @@
 
 #State/Resolver Messages for EN locale
 BUNDLE_NOT_IN_STATE=The bundle is not in the state: {0}
+BUNDLE_IN_OTHER_STATE=The bundle belongs to another state: {0}
+BUNDLE_PENDING_REMOVE_STATE = The bundle is pending remove in another state: {0}
 COMMIT_INVALID_TIMESTAMP=Cannot commit: invalid timestamp
 
 HEADER_REQUIRED=The \"{0}\" header must be specified
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMsg.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMsg.java
index 51adfae04..32ab014f9 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMsg.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateMsg.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2007 IBM Corporation and others.
+ * Copyright (c) 2004, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -16,6 +16,8 @@ public class StateMsg extends NLS {
 	private static final String BUNDLE_NAME = "org.eclipse.osgi.internal.resolver.StateMessages"; //$NON-NLS-1$
 
 	public static String BUNDLE_NOT_IN_STATE;
+	public static String BUNDLE_IN_OTHER_STATE;
+	public static String BUNDLE_PENDING_REMOVE_STATE;
 
 	public static String COMMIT_INVALID_TIMESTAMP;
 	public static String HEADER_REQUIRED;