Bug 293340 - [api][repository] Review of the repository package

5 files changed, 174 insertions, 34 deletions

diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepository.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepository.java
index 69128c9bf..dd5c01f67 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepository.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepository.java
@@ -20,6 +20,7 @@ import org.eclipse.equinox.p2.query.IQueryable;
  * A p2 repository contains either metadata or artifacts related to software
  * provisioning. This base interface defines properties common to all types
  * of repositories.
+ * @param <T> The type of contents contained in this repository
  * 
  * @noimplement This interface is not intended to be implemented by clients.
  * @since 2.0
@@ -171,6 +172,14 @@ public interface IRepository<T> extends IAdaptable, IQueryable<T> {
 	public Map<String, String> getProperties();
 
 	/**
+	 * Returns the repository property with the given key, or <code>null</code>
+	 * if no such property is defined
+	 * @param key the property key
+	 * @return the property value, or <code>null</code>
+	 */
+	public String getProperty(String key);
+
+	/**
 	 * Returns the provisioning agent that manages this repository
 	 * @return A provisioning agent.
 	 */
@@ -185,16 +194,6 @@ public interface IRepository<T> extends IAdaptable, IQueryable<T> {
 	public boolean isModifiable();
 
 	/**
-	 * Set the name of the repository.
-	 */
-	public void setName(String name);
-
-	/**
-	 * Sets the description of the repository.
-	 */
-	public void setDescription(String description);
-
-	/**
 	 * Sets the value of the property with the given key. Returns the old property
 	 * associated with that key, if any.  Setting a value of <code>null</code> will
 	 * remove the corresponding key from the properties of this repository.
@@ -204,9 +203,4 @@ public interface IRepository<T> extends IAdaptable, IQueryable<T> {
 	 * @return The old property value, or <code>null</code> if there was no old value
 	 */
 	public String setProperty(String key, String value);
-
-	/**
-	 * Sets the name of the provider of the repository.
-	 */
-	public void setProvider(String provider);
 }
diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepositoryManager.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepositoryManager.java
index 908c1bc93..fd65f00ed 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/IRepositoryManager.java
@@ -24,7 +24,7 @@ import org.eclipse.equinox.p2.query.IQueryable;
  * <p>
  * All {@link URI} instances provided to a repository manager must be absolute.
  * </p>
- * 
+ * @param <T> the type of contents in the repositories controlled by this manager
  * @noimplement This interface is not intended to be implemented by clients.
  * @since 2.0
  */
diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/IMetadataRepositoryManager.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/IMetadataRepositoryManager.java
index a75359e26..56458b8eb 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/IMetadataRepositoryManager.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/IMetadataRepositoryManager.java
@@ -36,6 +36,10 @@ public interface IMetadataRepositoryManager extends IRepositoryManager<IInstalla
 	 * Repository type for a simple repository based on a URL or local file system location.
 	 */
 	public static final String TYPE_SIMPLE_REPOSITORY = "org.eclipse.equinox.p2.metadata.repository.simpleRepository"; //$NON-NLS-1$
+
+	/**
+	 * Repository type for a composite repository based on a URL or local file system location.
+	 */
 	public static final String TYPE_COMPOSITE_REPOSITORY = "org.eclipse.equinox.p2.metadata.repository.compositeRepository"; //$NON-NLS-1$
 
 	/**
@@ -59,6 +63,7 @@ public interface IMetadataRepositoryManager extends IRepositoryManager<IInstalla
 	 * <li>There was an error writing to the given repository location.</li>
 	 * <li>A repository already exists at that location.</li>
 	 * </ul>
+	 * @throws OperationCanceledException if this operation has been canceled
 	 */
 	public IMetadataRepository createRepository(URI location, String name, String type, Map<String, String> properties) throws ProvisionException, OperationCanceledException;
 
@@ -77,6 +82,7 @@ public interface IMetadataRepositoryManager extends IRepositoryManager<IInstalla
 	 * @param monitor a progress monitor, or <code>null</code> if progress
 	 *    reporting is not desired
 	 * @return The loaded metadata repository
+	 * @throws OperationCanceledException if this operation has been canceled
 	 * @throws ProvisionException if the repository could not be created.  Reasons include:
 	 * <ul>
 	 * <li>There is no existing repository at that location.</li>
@@ -105,6 +111,7 @@ public interface IMetadataRepositoryManager extends IRepositoryManager<IInstalla
 	 * @param monitor a progress monitor, or <code>null</code> if progress
 	 *    reporting is not desired
 	 * @return The loaded metadata repository
+	 * @throws OperationCanceledException if this operation has been canceled
 	 * @throws ProvisionException if the repository could not be created.  Reasons include:
 	 * <ul>
 	 * <li>There is no existing repository at that location.</li>
@@ -124,6 +131,7 @@ public interface IMetadataRepositoryManager extends IRepositoryManager<IInstalla
 	 * @param monitor a progress monitor, or <code>null</code> if progress
 	 *    reporting is not desired
 	 * @return The refreshed metadata repository
+	 * @throws OperationCanceledException if this operation has been canceled
 	 * @throws ProvisionException if the repository could not be refreshed.  Reasons include:
 	 * <ul>
 	 * <li>The location is not known to the repository manager.</li>
diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/spi/AbstractMetadataRepository.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/spi/AbstractMetadataRepository.java
index ce9bcf149..f86fd03d0 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/spi/AbstractMetadataRepository.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/metadata/spi/AbstractMetadataRepository.java
@@ -29,53 +29,105 @@ import org.eclipse.equinox.p2.repository.spi.AbstractRepository;
  */
 public abstract class AbstractMetadataRepository extends AbstractRepository<IInstallableUnit> implements IMetadataRepository {
 
-	//TODO Consider removing from abstract class, this is currently an implementation detail of the simple metadata repo parser
+	/**
+	 * A class that encapsulates the persisted state of a repository. This is used as a convenience
+	 * when loading and storing repositories.
+	 * @see AbstractMetadataRepository#initialize(RepositoryState)
+	 */
 	public static class RepositoryState {
+		/**
+		 * The persisted name of the repository.
+		 */
 		public String Name;
+		/**
+		 * The persisted type of the repository.
+		 */
 		public String Type;
+		/**
+		 * The persisted version of the repository.
+		 */
 		public Version Version;
+		/**
+		 * The persisted provider of the repository.
+		 */
 		public String Provider;
+		/**
+		 * The persisted description of the repository.
+		 */
 		public String Description;
+		/**
+		 * The persisted location of the repository.
+		 */
 		public URI Location;
+		/**
+		 * The persisted properties of the repository.
+		 */
 		public Map<String, String> Properties;
+		/**
+		 * The persisted set of installable units of the repository.
+		 */
 		public IInstallableUnit[] Units;
+		/**
+		 * The persisted array of repository references
+		 */
 		public IRepositoryReference[] Repositories;
 	}
 
+	/**
+	 * Creates a new metadata repository that uses the provided agent.
+	 * 
+	 * @param agent the provisioning agent to be used by this repository
+	 */
 	public AbstractMetadataRepository(IProvisioningAgent agent) {
 		super(agent, "noName", "noType", "noVersion", null, null, null, null); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 	}
 
-	//TODO Consider removing from abstract class, this is currently an implementation detail of the simple metadata repo parser
+	/**
+	 * Initializes this class based on the provided previously persisted state
+	 * 
+	 * @param state the persisted repository state
+	 */
 	public abstract void initialize(RepositoryState state);
 
+	/**
+	 * Creates a new metadata repository with the provided repository information
+	 * 
+	 * @param agent the provisioning agent to be used by this repository
+	 * @param name the repository name
+	 * @param type the repository type
+	 * @param version the repository version
+	 * @param location the repository location
+	 * @param description the repository description
+	 * @param provider the repository provider
+	 * @param properties the repository properties
+	 */
 	protected AbstractMetadataRepository(IProvisioningAgent agent, String name, String type, String version, URI location, String description, String provider, Map<String, String> properties) {
 		super(agent, name, type, version, location, description, provider, properties);
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.equinox.p2.repository.metadata.IMetadataRepository#addInstallableUnits(java.util.Collection)
+	/**
+	 * {@inheritDoc}
 	 */
 	public void addInstallableUnits(Collection<IInstallableUnit> installableUnits) {
 		assertModifiable();
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.equinox.p2.repository.metadata.IMetadataRepository#addReference(java.net.URI, java.lang.String, int, int)
+	/**
+	 * {@inheritDoc}
 	 */
 	public void addReferences(Collection<? extends IRepositoryReference> references) {
 		assertModifiable();
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.equinox.p2.repository.metadata.IMetadataRepository#removeAll()
+	/**
+	 * {@inheritDoc}
 	 */
 	public void removeAll() {
 		assertModifiable();
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.equinox.p2.repository.metadata.IMetadataRepository#removeInstallableUnits(java.util.Collection)
+	/**
+	 * {@inheritDoc}
 	 */
 	public boolean removeInstallableUnits(Collection<IInstallableUnit> installableUnits) {
 		assertModifiable();
diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/spi/AbstractRepository.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/spi/AbstractRepository.java
index e8bcdc6ae..e833c1cb8 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/spi/AbstractRepository.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/p2/repository/spi/AbstractRepository.java
@@ -27,15 +27,27 @@ import org.eclipse.equinox.p2.repository.IRepository;
  * @since 2.0
 */
 public abstract class AbstractRepository<T> extends PlatformObject implements IRepository<T> {
-	protected final IProvisioningAgent agent;
-	protected String description;
-	protected transient URI location;
-	protected String name;
-	protected Map<String, String> properties = new OrderedProperties();
-	protected String provider;
-	protected String type;
-	protected String version;
+	private final IProvisioningAgent agent;
+	private String description;
+	private transient URI location;
+	private String name;
+	private Map<String, String> properties = new OrderedProperties();
+	private String provider;
+	private String type;
+	private String version;
 
+	/**
+	 * Creates a new repository with the given attributes.
+	 * 
+	 * @param agent the provisioning agent to be used by this repository
+	 * @param name the repository name
+	 * @param type the repository type
+	 * @param version the repository version
+	 * @param location the repository location
+	 * @param description the repository description
+	 * @param provider the repository provider
+	 * @param properties the repository properties
+	 */
 	protected AbstractRepository(IProvisioningAgent agent, String name, String type, String version, URI location, String description, String provider, Map<String, String> properties) {
 		this.agent = agent;
 		this.name = name;
@@ -93,6 +105,13 @@ public abstract class AbstractRepository<T> extends PlatformObject implements IR
 	}
 
 	/**
+	 * {@inheritDoc}
+	 */
+	public String getProperty(String key) {
+		return properties.get(key);
+	}
+
+	/**
 	 * Returns the name of the provider of the repository.
 	 * @return the provider of this repository.
 	 */
@@ -100,6 +119,11 @@ public abstract class AbstractRepository<T> extends PlatformObject implements IR
 		return provider;
 	}
 
+	/**
+	 * Returns the provisioning agent used by this repository
+	 * 
+	 * @return the provisioning agent
+	 */
 	public IProvisioningAgent getProvisioningAgent() {
 		return agent;
 	}
@@ -120,24 +144,86 @@ public abstract class AbstractRepository<T> extends PlatformObject implements IR
 		return version;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public boolean isModifiable() {
 		return false;
 	}
 
+	/**
+	 * Sets the description of this repository
+	 * 
+	 * @param description the repository description
+	 */
 	public synchronized void setDescription(String description) {
 		this.description = description;
 	}
 
+	/**
+	 * Sets the name of this repository
+	 * 
+	 * @param value the repository name
+	 */
 	public synchronized void setName(String value) {
 		this.name = value;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public synchronized String setProperty(String key, String value) {
 		assertModifiable();
+		if (key.equals(IRepository.PROP_NAME)) {
+			String oldName = getName();
+			setName(value);
+			return oldName;
+		}
 		return (value == null ? properties.remove(key) : properties.put(key, value));
 	}
 
+	/**
+	 * Sets the provider of this repository
+	 * 
+	 * @param provider the repository provider
+	 */
 	public synchronized void setProvider(String provider) {
 		this.provider = provider;
 	}
+
+	/**
+	 * Sets the type of this repository
+	 * 
+	 * @param type the repository type
+	 */
+	protected synchronized void setType(String type) {
+		this.type = type;
+	}
+
+	/**
+	 * Sets the location of this repository
+	 * 
+	 * @param location the repository location
+	 */
+	protected synchronized void setLocation(URI location) {
+		this.location = location;
+	}
+
+	/**
+	 * Sets the version of this repository
+	 * 
+	 * @param version the repository version
+	 */
+	protected synchronized void setVersion(String version) {
+		this.version = version;
+	}
+
+	/**
+	 * Sets the properties of this repository
+	 * 
+	 * @param properties the repository provider
+	 */
+	protected synchronized void setProperties(Map<String, String> properties) {
+		this.properties = properties;
+	}
 }