Bug 207978 - support for lazy loading and flushing artifact repositories

19 files changed, 292 insertions, 239 deletions

diff --git a/bundles/org.eclipse.equinox.p2.artifact.processors/src/org/eclipse/equinox/internal/p2/artifact/processors/AbstractDeltaProcessorStep.java b/bundles/org.eclipse.equinox.p2.artifact.processors/src/org/eclipse/equinox/internal/p2/artifact/processors/AbstractDeltaProcessorStep.java
index 8f0087692..7963cb4ec 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.processors/src/org/eclipse/equinox/internal/p2/artifact/processors/AbstractDeltaProcessorStep.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.processors/src/org/eclipse/equinox/internal/p2/artifact/processors/AbstractDeltaProcessorStep.java
@@ -11,6 +11,7 @@
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.processors;
 
+import java.net.URL;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.equinox.internal.p2.artifact.optimizers.AbstractDeltaStep;
@@ -45,11 +46,14 @@ public abstract class AbstractDeltaProcessorStep extends AbstractDeltaStep {
 			return;
 		}
 
-		IArtifactRepository[] repositories = repoMgr.getKnownRepositories();
+		URL[] repositories = repoMgr.getKnownRepositories();
 		for (int i = 0; i < repositories.length; i++) {
-			if ("file".equals(repositories[i].getLocation().getProtocol()) && repositories[i].contains(key)) {
-				repository = repositories[i];
-				return;
+			if ("file".equals(repositories[i].getProtocol())) {//$NON-NLS-1$
+				IArtifactRepository currentRepo = repoMgr.loadRepository(repositories[i], null);
+				if (currentRepo != null && currentRepo.contains(key)) {
+					repository = currentRepo;
+					return;
+				}
 			}
 		}
 		status = new Status(IStatus.ERROR, Activator.ID, "No repository available containing key " + key);
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/ArtifactRepositoryManager.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/ArtifactRepositoryManager.java
index 72c175141..eabb883f9 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/ArtifactRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/ArtifactRepositoryManager.java
@@ -9,6 +9,7 @@
  ******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.repository;
 
+import java.lang.ref.SoftReference;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.*;
@@ -25,8 +26,15 @@ import org.osgi.service.prefs.Preferences;
 
 // TODO Need to react to repository going away 
 // TODO the current assumption that the "location" is the dir/root limits us to 
-// having just one repo in a given URL..  
+// having just one repository in a given URL..  
 public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
+	static class RepositoryInfo {
+		String description;
+		URL location;
+		String name;
+		SoftReference repository;
+	}
+
 	private static final String ATTR_FILTER = "filter"; //$NON-NLS-1$
 	private static final String ATTR_SUFFIX = "suffix"; //$NON-NLS-1$
 	private static final String EL_FACTORY = "factory"; //$NON-NLS-1$
@@ -39,18 +47,46 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 	private static final String KEY_VERSION = "version"; //$NON-NLS-1$
 	private static final String NODE_REPOSITORIES = "repositories"; //$NON-NLS-1$
 
-	private List repositories = Collections.synchronizedList(new ArrayList());
+	/**
+	 * Map of String->RepositoryInfo, where String is the repository key
+	 * obtained vai getKey(URL).
+	 */
+	private Map repositories = null;
+	//lock object to be held when referring to the repositories field
+	private final Object repositoryLock = new Object();
 
 	public ArtifactRepositoryManager() {
-		restoreRepositories();
+		//initialize repositories lazily
 	}
 
 	public void addRepository(IArtifactRepository repository) {
-		repositories.add(repository);
+		RepositoryInfo info = new RepositoryInfo();
+		info.repository = new SoftReference(repository);
+		info.name = repository.getName();
+		info.description = repository.getDescription();
+		info.location = repository.getLocation();
+		synchronized (repositoryLock) {
+			if (repositories == null)
+				restoreRepositories();
+			repositories.put(getKey(repository), info);
+		}
 		// save the given repository in the preferences.
 		remember(repository);
 	}
 
+	public void addRepository(URL location) {
+		Assert.isNotNull(location);
+		RepositoryInfo info = new RepositoryInfo();
+		info.location = location;
+		synchronized (repositoryLock) {
+			if (repositories == null)
+				restoreRepositories();
+			repositories.put(getKey(location), info);
+		}
+		// save the given repository in the preferences.
+		remember(info);
+	}
+
 	public IArtifactRequest createDownloadRequest(IArtifactKey key, IPath destination) {
 		return new FileDownloadRequest(key, destination);
 	}
@@ -115,10 +151,6 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 		return results;
 	}
 
-	private void forget(IArtifactRepository toRemove) throws BackingStoreException {
-		getPreferences().node(getKey(toRemove)).removeNode();
-	}
-
 	private String[] getAllSuffixes() {
 		IConfigurationElement[] elements = RegistryFactory.getRegistry().getConfigurationElementsFor(Activator.REPO_PROVIDER_XPT);
 		ArrayList result = new ArrayList(elements.length);
@@ -133,13 +165,29 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 	 * as the name of a preference node.
 	 */
 	private String getKey(IArtifactRepository repository) {
-		return repository.getLocation().toExternalForm().replace('/', '_');
+		return getKey(repository.getLocation());
 	}
 
-	public IArtifactRepository[] getKnownRepositories() {
-		if (repositories == null)
-			restoreRepositories();
-		return (IArtifactRepository[]) repositories.toArray(new IArtifactRepository[repositories.size()]);
+	/*
+	 * Return a string key based on the given repository location which
+	 * is suitable for use as a preference node name.
+	 */
+	private String getKey(URL location) {
+		return location.toExternalForm().replace('/', '_');
+	}
+
+	public URL[] getKnownRepositories() {
+		synchronized (repositoryLock) {
+			if (repositories == null)
+				restoreRepositories();
+			URL[] result = new URL[repositories.size()];
+			int i = 0;
+			for (Iterator it = repositories.values().iterator(); it.hasNext(); i++) {
+				RepositoryInfo info = (RepositoryInfo) it.next();
+				result[i] = info.location;
+			}
+			return result;
+		}
 	}
 
 	/*
@@ -150,14 +198,19 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 	}
 
 	public IArtifactRepository getRepository(URL location) {
-		if (repositories == null)
-			restoreRepositories();
-		for (Iterator iterator = repositories.iterator(); iterator.hasNext();) {
-			IArtifactRepository match = (IArtifactRepository) iterator.next();
-			if (URLUtil.sameURL(match.getLocation(), location))
-				return match;
+		synchronized (repositoryLock) {
+			if (repositories == null)
+				restoreRepositories();
+			for (Iterator it = repositories.values().iterator(); it.hasNext();) {
+				RepositoryInfo info = (RepositoryInfo) it.next();
+				if (URLUtil.sameURL(info.location, location)) {
+					if (info.repository == null)
+						return null;
+					return (IArtifactRepository) info.repository.get();
+				}
+			}
+			return null;
 		}
-		return null;
 	}
 
 	public IArtifactRepository loadRepository(URL location, IProgressMonitor monitor) {
@@ -216,21 +269,49 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 			node.put(KEY_VERSION, value);
 		value = repository.getLocation().toExternalForm();
 		node.put(KEY_URL, value);
-		saveRepositoryList();
+		saveToPreferences();
 	}
 
-	public void removeRepository(IArtifactRepository toRemove) {
-		if (toRemove == null)
-			return;
+	/*
+	 * Save the list of repositories in the preference store.
+	 */
+	private void remember(RepositoryInfo info) {
+		Preferences node = getPreferences().node(getKey(info.location));
+		node.put(KEY_URL, info.location.toExternalForm());
+		if (info.description != null)
+			node.put(KEY_DESCRIPTION, info.description);
+		if (info.name != null)
+			node.put(KEY_NAME, info.name);
+		saveToPreferences();
+	}
 
-		repositories.remove(toRemove);
-		// remove the repository from the preferences
+	public boolean removeRepository(URL toRemove) {
+		Assert.isNotNull(toRemove);
+		final String repoKey = getKey(toRemove);
+		synchronized (repositoryLock) {
+			if (repositories == null)
+				restoreRepositories();
+			if (repositories.remove(repoKey) == null)
+				return false;
+		}
+		// remove the repository from the preference store
 		try {
-			forget(toRemove);
-			saveRepositoryList();
+			getPreferences().node(repoKey).removeNode();
+			saveToPreferences();
 		} catch (BackingStoreException e) {
 			log("Error saving preferences", e); //$NON-NLS-1$
 		}
+		return true;
+	}
+
+	private void restoreDownloadCache() {
+		// TODO while recreating, we may want to have proxies on repo instead of the real repo object to limit what is activated.
+		AgentLocation location = (AgentLocation) ServiceHelper.getService(Activator.getContext(), AgentLocation.class.getName());
+		if (location == null)
+			// TODO should do something here since we are failing to restore.
+			return;
+		SimpleArtifactRepository cache = (SimpleArtifactRepository) createRepository(location.getArtifactRepositoryURL(), "download cache", "org.eclipse.equinox.p2.artifact.repository.simpleRepository"); //$NON-NLS-1$ //$NON-NLS-2$
+		cache.tagAsImplementation();
 	}
 
 	/*
@@ -248,37 +329,25 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 		}
 		for (int i = 0; i < children.length; i++) {
 			Preferences child = node.node(children[i]);
-			String url = child.get(KEY_URL, null);
-			if (url == null)
+			String locationString = child.get(KEY_URL, null);
+			if (locationString == null)
 				continue;
 			String type = child.get(KEY_TYPE, null);
 			try {
-				IArtifactRepository repository = restoreRepository(new URL(url), type);
-				// If we could not restore the repo then remove it from the preferences.
-				if (repository == null)
-					child.removeNode();
-				else
-					repositories.add(repository);
+				RepositoryInfo info = new RepositoryInfo();
+				info.location = new URL(locationString);
+				info.name = child.get(KEY_NAME, null);
+				info.description = child.get(KEY_DESCRIPTION, null);
+				repositories.put(getKey(info.location), info);
 			} catch (MalformedURLException e) {
-				log("Error while restoring repository: " + url, e); //$NON-NLS-1$
-			} catch (BackingStoreException e) {
-				log("Error restoring repositories from preferences", e); //$NON-NLS-1$
+				log("Error while restoring repository: " + locationString, e); //$NON-NLS-1$
 			}
 		}
 		// now that we have loaded everything, remember them
-		saveRepositoryList();
+		saveToPreferences();
 	}
 
-	private void restoreRepositories() {
-		repositories = new ArrayList();
-		// TODO while recreating, we may want to have proxies on repo instead of the real repo object to limit what is activated.
-		AgentLocation location = (AgentLocation) ServiceHelper.getService(Activator.getContext(), AgentLocation.class.getName());
-		if (location == null)
-			// TODO should do something here since we are failing to restore.
-			return;
-		SimpleArtifactRepository cache = (SimpleArtifactRepository) createRepository(location.getArtifactRepositoryURL(), "download cache", "org.eclipse.equinox.p2.artifact.repository.simpleRepository"); //$NON-NLS-1$ //$NON-NLS-2$
-		cache.tagAsImplementation();
-
+	private void restoreFromSystemProperty() {
 		String locationString = Activator.getContext().getProperty("eclipse.p2.artifactRepository"); //$NON-NLS-1$
 		if (locationString != null) {
 			StringTokenizer tokenizer = new StringTokenizer(locationString, ","); //$NON-NLS-1$
@@ -290,38 +359,24 @@ public class ArtifactRepositoryManager implements IArtifactRepositoryManager {
 				}
 			}
 		}
-		// restore the persisted list of repositories
-		restoreFromPreferences();
 	}
 
-	private IArtifactRepository restoreRepository(URL location, String type) {
-		if (getRepository(location) != null) {
-			//already restored
-			return null;
-		}
-
-		IExtension extension = RegistryFactory.getRegistry().getExtension(Activator.REPO_PROVIDER_XPT, type);
-		if (extension == null) {
-			// TODO: remove next milestone or otherwise the next time we need a metadata format change 
-			// this is to allow us to get away with missing or bad types in older artifact repos
-			return loadRepository(location, "artifacts.xml"); //$NON-NLS-1$
-		}
-
-		IArtifactRepositoryFactory factory = null;
-		try {
-			factory = (IArtifactRepositoryFactory) createExecutableExtension(extension, EL_FACTORY);
-		} catch (CoreException e) {
-			log("Failed to restore artifact repository extension: " + location, e); //$NON-NLS-1$
+	/**
+	 * Restores the repository list.
+	 */
+	protected void restoreRepositories() {
+		synchronized (repositoryLock) {
+			repositories = new HashMap();
+			restoreDownloadCache();
+			restoreFromSystemProperty();
+			restoreFromPreferences();
 		}
-		if (factory == null)
-			return null;
-		return factory.load(location);
 	}
 
 	/*
 	 * Save the list of repositories to the file-system.
 	 */
-	private void saveRepositoryList() {
+	private void saveToPreferences() {
 		try {
 			getPreferences().flush();
 		} catch (BackingStoreException e) {
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
index 428f4b5be..a327fbaab 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
@@ -530,9 +530,10 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		return mappingRules;
 	}
 
-	public String setProperty(String key, String value) {
-		String oldValue = super.setProperty(key, value);
-		save();
+	public String setProperty(String key, String newValue) {
+		String oldValue = super.setProperty(key, newValue);
+		if (oldValue != newValue && (oldValue == null || !oldValue.equals(newValue)))
+			save();
 		return oldValue;
 	}
 
@@ -541,7 +542,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 	}
 
 	public void tagAsImplementation() {
-		properties.put(IRepository.IMPLEMENTATION_ONLY_KEY, Boolean.TRUE.toString());
+		setProperty(IRepository.IMPLEMENTATION_ONLY_KEY, Boolean.TRUE.toString());
 	}
 
 	/**
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/p2/artifact/repository/IArtifactRepositoryManager.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/p2/artifact/repository/IArtifactRepositoryManager.java
index 457279022..3dea3ba56 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/p2/artifact/repository/IArtifactRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/p2/artifact/repository/IArtifactRepositoryManager.java
@@ -20,52 +20,12 @@ public interface IArtifactRepositoryManager {
 	public static final IArtifactRequest[] NO_ARTIFACT_REQUEST = new IArtifactRequest[0];
 
 	/**
-	 * Return the list of artifact repositories known by this manager.
-	 * @return the list of known repositories
+	 * Adds a repository to the list of artifact repositories tracked by the repository
+	 * manager.
+	 * 
+	 * @param location The location of the artifact repository to add
 	 */
-	public IArtifactRepository[] getKnownRepositories();
-
-	/**
-	 * Loads the repository at the given location.  The location is expected to contain 
-	 * data that describes a valid artifact repository of a known type.  If this manager
-	 * already knows a repository at the given location then that repository is returned.
-	 * @param location the location in which to look for a repository description
-	 * @monitor 
-	 * @return a repository object for the given location or <code>null</code> if a repository
-	 * could not be found or loaded.
-	 */
-	public IArtifactRepository loadRepository(URL location, IProgressMonitor monitor);
-
-	/**
-	 * Add the given repository to the set of repositories managed by this manager.
-	 * @param repository the repository to add
-	 */
-	public void addRepository(IArtifactRepository repository);
-
-	/**
-	 * Return the artifact repository at the given location if known by this manager.  
-	 * Otherwise return <code>null</code>
-	 * @param location the location of the repository to return
-	 * @return the found repository
-	 */
-	public IArtifactRepository getRepository(URL location);
-
-	/**
-	 * Remove the given repository from this manager.  Do nothing if the repository
-	 * is not currently managed.
-	 * @param toRemove the repository to remove
-	 */
-	public void removeRepository(IArtifactRepository toRemove);
-
-	/**
-	 * Creates and returns an artifact repository of the given type at the given location.
-	 * If a repository already exists at that location <code>null</code> is returned.
-	 * @param location the location for the new repository
-	 * @param name the name of the new repo
-	 * @param type the kind of repository to create
-	 * @return the discovered or created repository
-	 */
-	public IArtifactRepository createRepository(URL location, String name, String type);
+	public void addRepository(URL location);
 
 	/**
 	 * Return a new request to download the given artifact and store it at the given destination.
@@ -93,4 +53,48 @@ public interface IArtifactRepositoryManager {
 	 */
 	public IArtifactRequest createMirrorRequest(IArtifactKey key, IArtifactRepository destination, Properties destinationDescriptorProperties, Properties destinationRepositoryProperties);
 
-}
+	/**
+	 * Creates and returns an artifact repository of the given type at the given location.
+	 * If a repository already exists at that location <code>null</code> is returned.
+	 * @param location the location for the new repository
+	 * @param name the name of the new repository
+	 * @param type the kind of repository to create
+	 * @return the discovered or created repository
+	 */
+	public IArtifactRepository createRepository(URL location, String name, String type);
+
+	/**
+	 * Returns the artifact repository locations known to the repository manager.
+	 * <p>
+	 * Note that the repository manager does not guarantee that a valid repository
+	 * exists at any of the returned locations at any particular moment in time.
+	 * A subsequent attempt to load a repository at any of the given locations may
+	 * or may not succeed.
+	 * 
+	 * @return the locations of the repositories managed by this repository manager.
+	 */
+	public URL[] getKnownRepositories();
+
+	/**
+	 * Loads the repository at the given location.  The location is expected to contain 
+	 * data that describes a valid artifact repository of a known type.  If this manager
+	 * already knows a repository at the given location then that repository is returned.
+	 * 
+	 * @param location the location in which to look for a repository description
+	 * @param monitor a progress monitor, or <code>null</code> if progress
+	 *    reporting is not desired
+	 * @return a repository object for the given location or <code>null</code> if a repository
+	 * could not be found or loaded.
+	 */
+	public IArtifactRepository loadRepository(URL location, IProgressMonitor monitor);
+
+	/**
+	 * Remove the given repository from this manager.  Do nothing if the repository
+	 * is not currently managed.
+	 * 
+	 * @param location the location of the repository to remove
+	 * @return <code>true</code> if a repository was removed, and 
+	 * <code>false</code> otherwise.
+	 */
+	public boolean removeRepository(URL location);
+}
\ No newline at end of file
diff --git a/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvCommandProvider.java b/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvCommandProvider.java
index 9b6f7267b..3185c2cc0 100644
--- a/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvCommandProvider.java
+++ b/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvCommandProvider.java
@@ -210,11 +210,11 @@ public class ProvCommandProvider implements CommandProvider {
 	public void _provlar(CommandInterpreter interpreter) {
 		String urlString = processArgument(interpreter.nextArgument());
 		if (urlString == null) {
-			IArtifactRepository[] repositories = ProvisioningHelper.getArtifactRepositories();
+			URL[] repositories = ProvisioningHelper.getArtifactRepositories();
 			if (repositories == null)
 				return;
 			for (int i = 0; i < repositories.length; i++)
-				interpreter.println(repositories[i].getLocation());
+				interpreter.println(repositories[i]);
 			return;
 		}
 		URL repoURL = toURL(interpreter, urlString);
diff --git a/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvisioningHelper.java b/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvisioningHelper.java
index ae3684380..1dc19c78c 100644
--- a/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvisioningHelper.java
+++ b/bundles/org.eclipse.equinox.p2.console/src/org/eclipse/equinox/internal/p2/console/ProvisioningHelper.java
@@ -81,14 +81,7 @@ public class ProvisioningHelper {
 		if (manager == null)
 			// TODO log here
 			return;
-		IArtifactRepository[] repos = manager.getKnownRepositories();
-		for (int i = 0; i < repos.length; i++) {
-			IArtifactRepository repo = repos[i];
-			if (repo.getLocation().equals(location)) {
-				manager.removeRepository(repo);
-				return;
-			}
-		}
+		manager.removeRepository(location);
 	}
 
 	public static Profile addProfile(String profileId, Properties properties) {
@@ -234,24 +227,21 @@ public class ProvisioningHelper {
 		}
 	}
 
-	public static IArtifactRepository[] getArtifactRepositories() {
+	public static URL[] getArtifactRepositories() {
 		IArtifactRepositoryManager manager = (IArtifactRepositoryManager) ServiceHelper.getService(Activator.getContext(), IArtifactRepositoryManager.class.getName());
 		if (manager == null)
 			// TODO log here
 			return null;
-		IArtifactRepository[] repos = manager.getKnownRepositories();
+		URL[] repos = manager.getKnownRepositories();
 		if (repos.length > 0)
 			return repos;
 		return null;
 	}
 
 	public static IArtifactRepository getArtifactRepository(URL repoURL) {
-		IArtifactRepository[] repositories = getArtifactRepositories();
-		if (repositories == null)
+		IArtifactRepositoryManager manager = (IArtifactRepositoryManager) ServiceHelper.getService(Activator.getContext(), IArtifactRepositoryManager.class.getName());
+		if (manager == null)
 			return null;
-		for (int i = 0; i < repositories.length; i++)
-			if (repoURL.equals(repositories[i].getLocation()))
-				return repositories[i];
-		return null;
+		return manager.loadRepository(repoURL, null);
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/spi/p2/core/repository/AbstractRepository.java b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/spi/p2/core/repository/AbstractRepository.java
index 6180ded02..8e27b768d 100644
--- a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/spi/p2/core/repository/AbstractRepository.java
+++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/spi/p2/core/repository/AbstractRepository.java
@@ -24,13 +24,13 @@ import org.eclipse.equinox.p2.core.repository.IRepository;
 * </p>
 */
 public abstract class AbstractRepository extends PlatformObject implements IRepository {
-	protected String name;
-	protected String type;
-	protected String version;
 	protected String description;
-	protected String provider;
 	protected transient URL location;
+	protected String name;
 	protected Map properties = new OrderedProperties();
+	protected String provider;
+	protected String type;
+	protected String version;
 
 	protected AbstractRepository(String name, String type, String version, URL location, String description, String provider) {
 		this.name = name;
@@ -42,27 +42,11 @@ public abstract class AbstractRepository extends PlatformObject implements IRepo
 	}
 
 	/**
-	 * Returns the name of the repository.
-	 * @return the name of the repository.
-	 */
-	public synchronized String getName() {
-		return name;
-	}
-
-	/**
-	 * Returns a string representing the type of the repository.
-	 * @return the type of the repository.
-	 */
-	public String getType() {
-		return type;
-	}
-
-	/**
-	 * Returns a string representing the version for the repository type.
-	 * @return the version of the type of the repository.
+	 * Returns a brief description of the repository.
+	 * @return the description of the repository.
 	 */
-	public String getVersion() {
-		return version;
+	public synchronized String getDescription() {
+		return description;
 	}
 
 	/**
@@ -76,11 +60,19 @@ public abstract class AbstractRepository extends PlatformObject implements IRepo
 	}
 
 	/**
-	 * Returns a brief description of the repository.
-	 * @return the description of the repository.
+	 * Returns the name of the repository.
+	 * @return the name of the repository.
 	 */
-	public synchronized String getDescription() {
-		return description;
+	public synchronized String getName() {
+		return name;
+	}
+
+	/**
+	 * Returns a read-only collection of the properties of the repository.
+	 * @return the properties of this repository.
+	 */
+	public Map getProperties() {
+		return OrderedProperties.unmodifiableProperties(properties);
 	}
 
 	/**
@@ -92,27 +84,31 @@ public abstract class AbstractRepository extends PlatformObject implements IRepo
 	}
 
 	/**
-	 * Returns a read-only collection of the properties of the repository.
-	 * @return the properties of this repository.
+	 * Returns a string representing the type of the repository.
+	 * @return the type of the repository.
 	 */
-	public Map getProperties() {
-		return OrderedProperties.unmodifiableProperties(properties);
+	public String getType() {
+		return type;
 	}
 
-	public synchronized void setName(String value) {
-		this.name = value;
+	/**
+	 * Returns a string representing the version for the repository type.
+	 * @return the version of the type of the repository.
+	 */
+	public String getVersion() {
+		return version;
 	}
 
-	public synchronized void setDescription(String description) {
-		this.description = description;
+	public boolean isModifiable() {
+		return false;
 	}
 
-	public synchronized void setProvider(String provider) {
-		this.provider = provider;
+	public synchronized void setDescription(String description) {
+		this.description = description;
 	}
 
-	public boolean isModifiable() {
-		return false;
+	public synchronized void setName(String value) {
+		this.name = value;
 	}
 
 	public String setProperty(String key, String value) {
@@ -120,4 +116,8 @@ public abstract class AbstractRepository extends PlatformObject implements IRepo
 			throw new UnsupportedOperationException("Repository not modifiable"); //$NON-NLS-1$
 		return (String) (value == null ? properties.remove(key) : properties.put(key, value));
 	}
+
+	public synchronized void setProvider(String provider) {
+		this.provider = provider;
+	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimpleDirector.java b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimpleDirector.java
index 1c4c56f39..1746cf0e5 100644
--- a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimpleDirector.java
+++ b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimpleDirector.java
@@ -46,10 +46,10 @@ public class SimpleDirector implements IDirector {
 	}
 
 	private void initializeRollbackRepository() {
-		new FormerState(getRollbackLocation());
+		new FormerState(getRollbackRepositoryLocation());
 	}
 
-	public URL getRollbackLocation() {
+	public URL getRollbackRepositoryLocation() {
 		AgentLocation agentLocation = (AgentLocation) ServiceHelper.getService(DirectorActivator.context, AgentLocation.class.getName());
 		try {
 			return new URL(agentLocation.getDataArea(DirectorActivator.PI_DIRECTOR), ROLLBACK_LOCATION);
diff --git a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimplePlanner.java b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimplePlanner.java
index 3c152331d..786d688f2 100644
--- a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimplePlanner.java
+++ b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/internal/p2/director/SimplePlanner.java
@@ -221,6 +221,8 @@ public class SimplePlanner implements IPlanner {
 
 		for (int i = 0; i < metadataRepositories.length; i++) {
 			IMetadataRepository repository = repoMgr.loadRepository(metadataRepositories[i], null);
+			if (repository == null)
+				continue;
 			Collector matches = repository.query(new InstallableUnitQuery(null, VersionRange.emptyRange), new Collector(), null);
 			for (Iterator it = matches.iterator(); it.hasNext();)
 				results.add(it.next());
diff --git a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/p2/director/IDirector.java b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/p2/director/IDirector.java
index ffacc7367..7f9eb2ca2 100644
--- a/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/p2/director/IDirector.java
+++ b/bundles/org.eclipse.equinox.p2.director/src/org/eclipse/equinox/p2/director/IDirector.java
@@ -51,9 +51,9 @@ public interface IDirector {
 
 	public IStatus revert(IInstallableUnit previous, Profile profile, URL[] metadataRepositories, IProgressMonitor monitor);
 
-	//TODO And many more operations for uninstallation and the rest ! See bug 179819
-
-	//TODO we might want to register a service for this instead?
-	// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=211810
-	public URL getRollbackLocation();
+	/**
+	 * Returns the location of the director's rollback repository, where information about
+	 * previous profile states is stored.
+	 */
+	public URL getRollbackRepositoryLocation();
 }
diff --git a/bundles/org.eclipse.equinox.p2.directorywatcher/src/org/eclipse/equinox/p2/directorywatcher/RepositoryListener.java b/bundles/org.eclipse.equinox.p2.directorywatcher/src/org/eclipse/equinox/p2/directorywatcher/RepositoryListener.java
index 635082c17..aede17329 100644
--- a/bundles/org.eclipse.equinox.p2.directorywatcher/src/org/eclipse/equinox/p2/directorywatcher/RepositoryListener.java
+++ b/bundles/org.eclipse.equinox.p2.directorywatcher/src/org/eclipse/equinox/p2/directorywatcher/RepositoryListener.java
@@ -84,7 +84,7 @@ public class RepositoryListener extends DirectoryChangeListener {
 
 		IArtifactRepository repository = null;
 		try {
-			repository = manager.getRepository(stateDirURL);
+			repository = manager.loadRepository(stateDirURL, null);
 			if (repository == null) {
 				repository = manager.createRepository(stateDirURL, "artifact listener " + repositoryName, "org.eclipse.equinox.p2.artifact.repository.simpleRepository");
 				repository.setProperty(IRepository.IMPLEMENTATION_ONLY_KEY, Boolean.TRUE.toString());
diff --git a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/p2/engine/phases/Sizing.java b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/p2/engine/phases/Sizing.java
index 32748804a..a565e070f 100644
--- a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/p2/engine/phases/Sizing.java
+++ b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/p2/engine/phases/Sizing.java
@@ -8,6 +8,7 @@
  ******************************************************************************/
 package org.eclipse.equinox.p2.engine.phases;
 
+import java.net.URL;
 import java.util.*;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
@@ -62,12 +63,15 @@ public class Sizing extends Phase {
 		}
 
 		IArtifactRepositoryManager repoMgr = (IArtifactRepositoryManager) ServiceHelper.getService(EngineActivator.getContext(), IArtifactRepositoryManager.class.getName());
-		IArtifactRepository[] repositories = repoMgr.getKnownRepositories();
+		URL[] repositories = repoMgr.getKnownRepositories();
 
 		for (Iterator iterator = artifactsToObtain.iterator(); iterator.hasNext();) {
 			IArtifactRequest artifactRequest = (IArtifactRequest) iterator.next();
 			for (int i = 0; i < repositories.length; i++) {
-				IArtifactDescriptor[] descriptors = repositories[i].getArtifactDescriptors(artifactRequest.getArtifactKey());
+				IArtifactRepository repo = repoMgr.loadRepository(repositories[i], null);
+				if (repo == null)
+					continue;
+				IArtifactDescriptor[] descriptors = repo.getArtifactDescriptors(artifactRequest.getArtifactKey());
 				if (descriptors.length > 0) {
 					if (descriptors[0].getProperty(IArtifactDescriptor.ARTIFACT_SIZE) != null)
 						sizeOnDisk += Long.parseLong(descriptors[0].getProperty(IArtifactDescriptor.ARTIFACT_SIZE));
@@ -83,7 +87,7 @@ public class Sizing extends Phase {
 	}
 
 	protected IStatus initializePhase(IProgressMonitor monitor, Profile profile, Map parameters) {
-		parameters.put(PARM_ARTIFACT_REQUESTS, new ArrayList()); 
+		parameters.put(PARM_ARTIFACT_REQUESTS, new ArrayList());
 		return null;
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/LocalMetadataRepository.java b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/LocalMetadataRepository.java
index b7fab5311..5c79f9d67 100644
--- a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/LocalMetadataRepository.java
+++ b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/LocalMetadataRepository.java
@@ -135,9 +135,10 @@ public class LocalMetadataRepository extends AbstractMetadataRepository {
 		}
 	}
 
-	public String setProperty(String key, String value) {
-		String oldValue = super.setProperty(key, value);
-		save();
+	public String setProperty(String key, String newValue) {
+		String oldValue = super.setProperty(key, newValue);
+		if (oldValue != newValue && (oldValue == null || !oldValue.equals(newValue)))
+			save();
 		return oldValue;
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/MetadataRepositoryManager.java b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/MetadataRepositoryManager.java
index 9d7762419..70c6e5a4a 100644
--- a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/MetadataRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/internal/p2/metadata/repository/MetadataRepositoryManager.java
@@ -43,8 +43,12 @@ public class MetadataRepositoryManager implements IMetadataRepositoryManager {
 	private static final String KEY_VERSION = "version"; //$NON-NLS-1$
 	private static final String NODE_REPOSITORIES = "repositories"; //$NON-NLS-1$
 
+	/**
+	 * Map of String->RepositoryInfo, where String is the repository key
+	 * obtained vai getKey(URL).
+	 */
 	private Map repositories = null;
-	//lock object to be held when referring repositories field
+	//lock object to be held when referring to the repositories field
 	private final Object repositoryLock = new Object();
 
 	public MetadataRepositoryManager() {
diff --git a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/repository/IMetadataRepositoryManager.java b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/repository/IMetadataRepositoryManager.java
index 302b6449d..f68d288bb 100644
--- a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/repository/IMetadataRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/repository/IMetadataRepositoryManager.java
@@ -39,7 +39,12 @@ public interface IMetadataRepositoryManager extends IQueryable {
 	public IMetadataRepository createRepository(URL location, String name, String type);
 
 	/**
-	 * Returns the locations of the repositories managed by this repository manager.
+	 * Returns the metadata repository locations known to the repository manager.
+	 * <p>
+	 * Note that the repository manager does not guarantee that a valid repository
+	 * exists at any of the returned locations at any particular moment in time.
+	 * A subsequent attempt to load a repository at any of the given locations may
+	 * or may not succeed.
 	 * 
 	 * @return the locations of the repositories managed by this repository manager.
 	 */
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/FoldersRepositoryTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/FoldersRepositoryTest.java
index 936073cf2..a7adbb02d 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/FoldersRepositoryTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/FoldersRepositoryTest.java
@@ -52,10 +52,8 @@ public class FoldersRepositoryTest extends TestCase {
 		AbstractProvisioningTest.delete(testRepo);
 		testRepo.mkdir();
 
-		IArtifactRepository repo = manager.loadRepository(testRepo.toURL(), null);
-		if (repo != null)
-			manager.removeRepository(repo);
-		repo = manager.createRepository(testRepo.toURL(), "testRepo", "org.eclipse.equinox.p2.artifact.repository.simpleRepository");
+		manager.removeRepository(testRepo.toURL());
+		IArtifactRepository repo = manager.createRepository(testRepo.toURL(), "testRepo", "org.eclipse.equinox.p2.artifact.repository.simpleRepository");
 		File pluginsFolder = new File(testRepo, "plugins");
 		pluginsFolder.mkdir();
 
@@ -94,7 +92,7 @@ public class FoldersRepositoryTest extends TestCase {
 		assertEquals(0, repo.getArtifactKeys().length);
 		assertEquals(0, pluginsFolder.listFiles(filter).length);
 
-		manager.removeRepository(repo);
+		manager.removeRepository(repo.getLocation());
 		AbstractProvisioningTest.delete(testRepo);
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/touchpoint/eclipse/Util.java b/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/touchpoint/eclipse/Util.java
index 8e3cc3e40..23b8c7062 100644
--- a/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/touchpoint/eclipse/Util.java
+++ b/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/touchpoint/eclipse/Util.java
@@ -14,6 +14,7 @@ import org.eclipse.equinox.internal.p2.core.helpers.Headers;
 import org.eclipse.equinox.internal.p2.core.helpers.ServiceHelper;
 import org.eclipse.equinox.p2.artifact.repository.*;
 import org.eclipse.equinox.p2.core.location.AgentLocation;
+import org.eclipse.equinox.p2.core.repository.IRepository;
 import org.eclipse.equinox.p2.engine.Profile;
 import org.eclipse.equinox.p2.metadata.IArtifactKey;
 import org.eclipse.equinox.p2.metadata.TouchpointData;
@@ -65,11 +66,11 @@ public class Util {
 			// 	the given repo location is not an existing repo so we have to create something
 			String repositoryName = location + " - bundle pool"; //$NON-NLS-1$
 			bundlePool = manager.createRepository(location, repositoryName, REPOSITORY_TYPE);
+			if (bundlePool == null)
+				throw new IllegalArgumentException("Bundle pool repository not writeable: " + location); //$NON-NLS-1$
+			((IRepository) bundlePool).setProperty(IRepository.IMPLEMENTATION_ONLY_KEY, Boolean.valueOf(true).toString());
 		}
 
-		if (bundlePool == null) {
-			throw new IllegalArgumentException("BundlePool repository not writeable: " + location); //$NON-NLS-1$
-		}
 		return (IFileArtifactRepository) bundlePool;
 	}
 
@@ -78,13 +79,14 @@ public class Util {
 		bundleRepositories.add(Util.getBundlePoolRepository(profile));
 
 		IArtifactRepositoryManager manager = getArtifactRepositoryManager();
-		IArtifactRepository[] knownRepositories = manager.getKnownRepositories();
+		URL[] knownRepositories = manager.getKnownRepositories();
 		for (int i = 0; i < knownRepositories.length; i++) {
-			IArtifactRepository repository = knownRepositories[i];
+			IArtifactRepository repository = manager.loadRepository(knownRepositories[i], null);
+			if (repository == null)
+				continue;
 			String profileExtension = (String) repository.getProperties().get(PROFILE_EXTENSION);
 			if (profileExtension != null && profileExtension.equals(profile.getProfileId()))
 				bundleRepositories.add(repository);
-
 		}
 
 		return new AggregatedBundleRepository(bundleRepositories);
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/operations/ProvisioningUtil.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/operations/ProvisioningUtil.java
index c48b3e0aa..b0b9f03a5 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/operations/ProvisioningUtil.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/operations/ProvisioningUtil.java
@@ -55,7 +55,7 @@ public class ProvisioningUtil {
 		IMetadataRepositoryManager manager = (IMetadataRepositoryManager) ServiceHelper.getService(ProvUIActivator.getContext(), IMetadataRepositoryManager.class.getName());
 		if (manager == null)
 			throw new ProvisionException(ProvUIMessages.ProvisioningUtil_NoRepositoryManager);
-		URL location = director.getRollbackLocation();
+		URL location = director.getRollbackRepositoryLocation();
 		IMetadataRepository repo = manager.loadRepository(location, monitor);
 		if (repo == null) {
 			throw new ProvisionException(NLS.bind(ProvUIMessages.ProvisioningUtil_LoadRepositoryFailure, location.toExternalForm()));
@@ -93,27 +93,8 @@ public class ProvisioningUtil {
 		if (manager == null) {
 			throw new ProvisionException(ProvUIMessages.ProvisioningUtil_NoRepositoryManager);
 		}
-		IArtifactRepository[] repos = manager.getKnownRepositories();
-		for (int i = 0; i < repos.length; i++) {
-			IArtifactRepository repo = repos[i];
-			if (repo.getLocation().equals(location)) {
-				manager.removeRepository(repo);
-				EventObject event = new EventObject(IProvisioningListener.REPO_REMOVED);
-				ProvUIActivator.getDefault().notifyListeners(event);
-				return;
-			}
-		}
-	}
-
-	public static IArtifactRepository[] getArtifactRepositories() throws ProvisionException {
-		IArtifactRepositoryManager manager = (IArtifactRepositoryManager) ServiceHelper.getService(ProvUIActivator.getContext(), IArtifactRepositoryManager.class.getName());
-		if (manager == null)
-			throw new ProvisionException(ProvUIMessages.ProvisioningUtil_NoRepositoryManager);
-		IArtifactRepository[] repos = manager.getKnownRepositories();
-		if (repos != null) {
-			return repos;
-		}
-		return new IArtifactRepository[0];
+		if (manager.removeRepository(location))
+			ProvUIActivator.getDefault().notifyListeners(new EventObject(IProvisioningListener.REPO_REMOVED));
 	}
 
 	public static void addProfile(Profile profile, IProgressMonitor monitor) throws ProvisionException {
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/query/QueryableArtifactRepositoryManager.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/query/QueryableArtifactRepositoryManager.java
index cebe4bbed..0779e2d0c 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/query/QueryableArtifactRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/query/QueryableArtifactRepositoryManager.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.equinox.p2.ui.query;
 
+import java.net.URL;
 import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.core.helpers.ServiceHelper;
 import org.eclipse.equinox.internal.p2.ui.ProvUIMessages;
@@ -32,13 +33,14 @@ public class QueryableArtifactRepositoryManager implements IQueryable {
 			ProvUI.reportStatus(new Status(IStatus.ERROR, ProvUIActivator.PLUGIN_ID, ProvUIMessages.ProvisioningUtil_NoRepositoryManager));
 			return result;
 		}
-		IArtifactRepository[] repos = manager.getKnownRepositories();
+		URL[] repos = manager.getKnownRepositories();
 		if (monitor == null)
 			monitor = new NullProgressMonitor();
-		monitor.beginTask(ProvUIMessages.QueryableArtifactRepositoryManager_RepositoryQueryProgress, repos.length);
+		monitor.beginTask(ProvUIMessages.QueryableArtifactRepositoryManager_RepositoryQueryProgress, repos.length * 2);
 		for (int i = 0; i < repos.length; i++) {
-			if (query.isMatch(repos[i]))
-				result.accept(new ArtifactRepositoryElement(repos[i]));
+			IArtifactRepository repo = manager.loadRepository(repos[i], new SubProgressMonitor(monitor, 1));
+			if (repo != null && query.isMatch(repo))
+				result.accept(new ArtifactRepositoryElement(repo));
 			monitor.worked(1);
 		}
 		monitor.done();