Bug 274272 - [repository] Cache artifact repository index

4 files changed, 162 insertions, 153 deletions

diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepository.java
index 04167f75d..928d34b19 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepository.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepository.java
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Sonatype Inc - ongoing development
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.repository;
 
@@ -130,19 +131,7 @@ public class CompositeArtifactRepository extends AbstractArtifactRepository impl
 	}
 
 	private static URI getActualLocation(URI base, String extension) {
-		final String name = CONTENT_FILENAME + extension;
-		String spec = base.toString();
-		if (spec.endsWith(name))
-			return base;
-		if (spec.endsWith("/")) //$NON-NLS-1$
-			spec += name;
-		else
-			spec += "/" + name; //$NON-NLS-1$
-		try {
-			return new URI(spec);
-		} catch (URISyntaxException e) {
-			return null;
-		}
+		return URIUtil.append(base, CONTENT_FILENAME + extension);
 	}
 
 	private boolean isLocal() {
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepositoryFactory.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepositoryFactory.java
index a37216807..180ed0137 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepositoryFactory.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/CompositeArtifactRepositoryFactory.java
@@ -7,13 +7,10 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Sonatype Inc - ongoing development
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.repository;
 
-import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactRepositoryFactory;
-
-import org.eclipse.equinox.p2.core.ProvisionException;
-
 import java.io.*;
 import java.net.URI;
 import java.util.Map;
@@ -23,83 +20,94 @@ import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.core.helpers.Tracing;
 import org.eclipse.equinox.internal.p2.persistence.CompositeRepositoryIO;
 import org.eclipse.equinox.internal.p2.persistence.CompositeRepositoryState;
-import org.eclipse.equinox.internal.p2.repository.RepositoryTransport;
-import org.eclipse.equinox.internal.p2.repository.Transport;
+import org.eclipse.equinox.internal.p2.repository.CacheManager;
+import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.repository.IRepositoryManager;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactRepository;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactRepositoryManager;
+import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactRepositoryFactory;
 import org.eclipse.osgi.util.NLS;
 
 public class CompositeArtifactRepositoryFactory extends ArtifactRepositoryFactory {
 
+	private static final String JAR_EXTENSION = ".jar"; //$NON-NLS-1$
+	private static final String XML_EXTENSION = ".xml"; //$NON-NLS-1$
+	private static final String PROTOCOL_FILE = "file"; //$NON-NLS-1$
+
 	private IArtifactRepositoryManager getManager() {
 		if (getAgent() != null)
 			return (IArtifactRepositoryManager) getAgent().getService(IArtifactRepositoryManager.SERVICE_NAME);
 		return null;
 	}
 
+	/**
+	 * Returns a file in the local file system that contains the contents of the
+	 * metadata repository at the given location.
+	 */
+	private File getLocalFile(URI location, IProgressMonitor monitor) throws IOException, ProvisionException {
+		File localFile = null;
+		URI jarLocation = CompositeArtifactRepository.getActualLocation(location, true);
+		URI xmlLocation = CompositeArtifactRepository.getActualLocation(location, false);
+		// If the repository is local, we can return the repository file directly
+		if (PROTOCOL_FILE.equals(xmlLocation.getScheme())) {
+			//look for a compressed local file
+			localFile = URIUtil.toFile(jarLocation);
+			if (localFile.exists())
+				return localFile;
+			//look for an uncompressed local file
+			localFile = URIUtil.toFile(xmlLocation);
+			if (localFile.exists())
+				return localFile;
+			String msg = NLS.bind(Messages.io_failedRead, location);
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, msg, null));
+		}
+		//file is not local, create a cache of the repository metadata
+		CacheManager cache = (CacheManager) getAgent().getService(CacheManager.SERVICE_NAME);
+		if (cache == null)
+			throw new IllegalArgumentException("Cache manager service not available"); //$NON-NLS-1$
+		localFile = cache.createCache(location, CompositeArtifactRepository.CONTENT_FILENAME, monitor);
+		if (localFile == null) {
+			//there is no remote file in either form
+			String msg = NLS.bind(Messages.io_failedRead, location);
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, msg, null));
+		}
+		return localFile;
+	}
+
 	public IArtifactRepository load(URI location, int flags, IProgressMonitor monitor) throws ProvisionException {
-		final String PROTOCOL_FILE = "file"; //$NON-NLS-1$
 		long time = 0;
 		final String debugMsg = "Restoring artifact repository "; //$NON-NLS-1$
 		if (Tracing.DEBUG_METADATA_PARSING) {
 			Tracing.debug(debugMsg + location);
 			time = -System.currentTimeMillis();
 		}
-		File localFile = null;
-		boolean local = false;
+
+		SubMonitor sub = SubMonitor.convert(monitor, 400);
 		try {
-			SubMonitor sub = SubMonitor.convert(monitor, 300);
-			OutputStream artifacts = null;
-			// try with compressed
-			boolean compress = true;
-			if (PROTOCOL_FILE.equals(location.getScheme())) {
-				local = true;
-				localFile = URIUtil.toFile(CompositeArtifactRepository.getActualLocation(location, true));
-				if (!localFile.exists()) {
-					localFile = URIUtil.toFile(CompositeArtifactRepository.getActualLocation(location, false));
-					compress = false;
-				}
-			} else {
-				//not local, return null if the caller wanted a modifiable repo
-				if ((flags & IRepositoryManager.REPOSITORY_HINT_MODIFIABLE) > 0) {
-					return null;
-				}
+			//non local repos are not modifiable
+			if (!PROTOCOL_FILE.equals(location.getScheme()) && (flags & IRepositoryManager.REPOSITORY_HINT_MODIFIABLE) > 0)
+				return null;
 
-				//download to local temp file
-				localFile = File.createTempFile(CompositeArtifactRepository.CONTENT_FILENAME, CompositeArtifactRepository.XML_EXTENSION);
-				try {
-					artifacts = new BufferedOutputStream(new FileOutputStream(localFile));
-					IStatus status = getTransport().download(CompositeArtifactRepository.getActualLocation(location, compress), artifacts, sub.newChild(100));
-					if (!status.isOK()) {
-						// retry uncompressed
-						compress = false;
-						status = getTransport().download(CompositeArtifactRepository.getActualLocation(location, compress), artifacts, sub.newChild(100));
-						if (!status.isOK())
-							throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, status.getMessage(), null));
-					}
-				} finally {
-					if (artifacts != null)
-						artifacts.close();
-				}
-			}
-			InputStream descriptorStream = null;
+			File localFile = getLocalFile(location, sub.newChild(300));
+			InputStream inStream = new BufferedInputStream(new FileInputStream(localFile));
+			JarInputStream jarStream = null;
 			try {
-				descriptorStream = new BufferedInputStream(new FileInputStream(localFile));
-				if (compress) {
-					URI actualLocation = CompositeArtifactRepository.getActualLocation(location, false);
-					JarInputStream jInStream = new JarInputStream(descriptorStream);
-					JarEntry jarEntry = jInStream.getNextJarEntry();
-					String filename = URIUtil.lastSegment(actualLocation);
-					while (jarEntry != null && filename != null && !(filename.equals(jarEntry.getName()))) {
-						jarEntry = jInStream.getNextJarEntry();
-					}
-					if (jarEntry == null) {
-						//there is a jar but the entry is missing or invalid, so treat this as an invalid repository
-						throw new IOException(NLS.bind(Messages.io_invalidLocation, actualLocation.getPath()));
+				//if reading from a jar, obtain a stream on the entry with the actual contents
+				if (localFile.getAbsolutePath().endsWith(JAR_EXTENSION)) {
+					jarStream = new JarInputStream(inStream);
+					JarEntry jarEntry = jarStream.getNextJarEntry();
+					String entryName = CompositeArtifactRepository.CONTENT_FILENAME + XML_EXTENSION;
+					while (jarEntry != null && (!entryName.equals(jarEntry.getName()))) {
+						jarEntry = jarStream.getNextJarEntry();
 					}
-					descriptorStream = jInStream;
+
+					//there is a jar but the entry is missing or invalid, so treat this as an invalid repository
+					if (jarEntry == null)
+						throw new IOException(NLS.bind(Messages.io_invalidLocation, location.getPath()));
 				}
+				//parse the repository descriptor file
+				sub.setWorkRemaining(100);
+				InputStream descriptorStream = jarStream != null ? jarStream : inStream;
 				CompositeRepositoryIO io = new CompositeRepositoryIO();
 				CompositeRepositoryState resultState = io.read(localFile.toURL(), descriptorStream, CompositeArtifactRepository.PI_REPOSITORY_TYPE, sub.newChild(100));
 				if (resultState.getLocation() == null)
@@ -111,8 +119,8 @@ public class CompositeArtifactRepositoryFactory extends ArtifactRepositoryFactor
 				}
 				return result;
 			} finally {
-				if (descriptorStream != null)
-					descriptorStream.close();
+				safeClose(jarStream);
+				safeClose(inStream);
 			}
 		} catch (FileNotFoundException e) {
 			String msg = NLS.bind(Messages.io_failedRead, location);
@@ -121,8 +129,8 @@ public class CompositeArtifactRepositoryFactory extends ArtifactRepositoryFactor
 			String msg = NLS.bind(Messages.io_failedRead, location);
 			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_FAILED_READ, msg, e));
 		} finally {
-			if (!local && localFile != null && !localFile.delete())
-				localFile.deleteOnExit();
+			if (monitor != null)
+				monitor.done();
 		}
 	}
 
@@ -130,7 +138,16 @@ public class CompositeArtifactRepositoryFactory extends ArtifactRepositoryFactor
 		return new CompositeArtifactRepository(getManager(), location, name, properties);
 	}
 
-	private Transport getTransport() {
-		return RepositoryTransport.getInstance();
+	/**
+	 * Closes a stream, ignoring any secondary exceptions
+	 */
+	private void safeClose(InputStream stream) {
+		if (stream == null)
+			return;
+		try {
+			stream.close();
+		} catch (IOException e) {
+			//ignore
+		}
 	}
 }
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 1bb37ed04..343ec0a44 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
@@ -9,6 +9,7 @@
  * 	IBM Corporation - initial API and implementation
  * 	Genuitec, LLC - support for multi-threaded downloads
  *  Cloudsmith Inc. - query indexes
+ *  Sonatype Inc - ongoing development
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.repository.simple;
 
@@ -53,6 +54,8 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 
 	public static final boolean MD5_CHECK_ENABLED = !"false".equals(Activator.getContext().getProperty("eclipse.p2.MD5Check")); //$NON-NLS-1$//$NON-NLS-2$
 
+	public static final String CONTENT_FILENAME = "artifacts"; //$NON-NLS-1$
+
 	/** 
 	 * The key for a integer property controls the maximum number
 	 * of threads that should be used when optimizing downloads from a remote
@@ -192,7 +195,6 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 	private static final String ARTIFACT_FOLDER = "artifact.folder"; //$NON-NLS-1$
 	private static final String ARTIFACT_UUID = "artifact.uuid"; //$NON-NLS-1$
 	static final private String BLOBSTORE = ".blobstore/"; //$NON-NLS-1$
-	static final private String CONTENT_FILENAME = "artifacts"; //$NON-NLS-1$
 	static final private String[][] PACKED_MAPPING_RULES = { {"(& (classifier=osgi.bundle) (format=packed))", "${repoUrl}/plugins/${id}_${version}.jar.pack.gz"}, //$NON-NLS-1$//$NON-NLS-2$
 			{"(& (classifier=osgi.bundle))", "${repoUrl}/plugins/${id}_${version}.jar"}, //$NON-NLS-1$//$NON-NLS-2$
 			{"(& (classifier=binary))", "${repoUrl}/binary/${id}_${version}"}, //$NON-NLS-1$ //$NON-NLS-2$
@@ -239,24 +241,12 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		toDelete.delete();
 	}
 
-	public static URI getActualLocation(URI base, boolean compress) throws IOException {
+	public static URI getActualLocation(URI base, boolean compress) {
 		return getActualLocation(base, compress ? JAR_EXTENSION : XML_EXTENSION);
 	}
 
-	private static URI getActualLocation(URI base, String extension) throws IOException {
-		final String name = CONTENT_FILENAME + extension;
-		String spec = base.toString();
-		if (spec.endsWith(name))
-			return base;
-		if (spec.endsWith("/")) //$NON-NLS-1$
-			spec += name;
-		else
-			spec += "/" + name; //$NON-NLS-1$
-		try {
-			return new URI(spec);
-		} catch (URISyntaxException e) {
-			throw new IOException(NLS.bind(Messages.io_invalidLocation, spec));
-		}
+	private static URI getActualLocation(URI base, String extension) {
+		return URIUtil.append(base, CONTENT_FILENAME + extension);
 	}
 
 	public static URI getBlobStoreLocation(URI base) {
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepositoryFactory.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepositoryFactory.java
index d9cd59a79..4ebeb06ad 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepositoryFactory.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepositoryFactory.java
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Sonatype Inc - ongoing development
  *******************************************************************************/
 package org.eclipse.equinox.internal.p2.artifact.repository.simple;
 
@@ -19,8 +20,7 @@ import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
 import org.eclipse.equinox.internal.p2.artifact.repository.Messages;
 import org.eclipse.equinox.internal.p2.core.helpers.Tracing;
-import org.eclipse.equinox.internal.p2.repository.RepositoryTransport;
-import org.eclipse.equinox.internal.p2.repository.Transport;
+import org.eclipse.equinox.internal.p2.repository.CacheManager;
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.repository.IRepositoryManager;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactRepository;
@@ -28,86 +28,90 @@ import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactRepositoryFactory;
 import org.eclipse.osgi.util.NLS;
 
 public class SimpleArtifactRepositoryFactory extends ArtifactRepositoryFactory {
+	private static final String PROTOCOL_FILE = "file"; //$NON-NLS-1$
+	private static final String JAR_EXTENSION = ".jar"; //$NON-NLS-1$
+	private static final String XML_EXTENSION = ".xml"; //$NON-NLS-1$
+
 	public SimpleArtifactRepositoryFactory() {
 		super();
 	}
 
+	/**
+	 * Returns a file in the local file system that contains the contents of the
+	 * metadata repository at the given location.
+	 */
+	private File getLocalFile(URI location, IProgressMonitor monitor) throws IOException, ProvisionException {
+		File localFile = null;
+		URI jarLocation = SimpleArtifactRepository.getActualLocation(location, true);
+		URI xmlLocation = SimpleArtifactRepository.getActualLocation(location, false);
+		// If the repository is local, we can return the repository file directly
+		if (PROTOCOL_FILE.equals(xmlLocation.getScheme())) {
+			//look for a compressed local file
+			localFile = URIUtil.toFile(jarLocation);
+			if (localFile.exists())
+				return localFile;
+			//look for an uncompressed local file
+			localFile = URIUtil.toFile(xmlLocation);
+			if (localFile.exists())
+				return localFile;
+			String msg = NLS.bind(Messages.io_failedRead, location);
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, msg, null));
+		}
+		// file is not local, create a cache of the repository metadata
+		CacheManager cache = (CacheManager) getAgent().getService(CacheManager.SERVICE_NAME);
+		if (cache == null)
+			throw new IllegalArgumentException("Cache manager service not available"); //$NON-NLS-1$
+		localFile = cache.createCache(location, SimpleArtifactRepository.CONTENT_FILENAME, monitor);
+		if (localFile == null) {
+			// there is no remote file in either form - this should not really happen as
+			// createCache should bail out with exception if something is wrong. This is an internal
+			// error.
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, Messages.repoMan_internalError, null));
+		}
+		return localFile;
+	}
+
 	public IArtifactRepository load(URI location, int flags, IProgressMonitor monitor) throws ProvisionException {
-		final String PROTOCOL_FILE = "file"; //$NON-NLS-1$
 		long time = 0;
 		final String debugMsg = "Restoring artifact repository "; //$NON-NLS-1$
 		if (Tracing.DEBUG_METADATA_PARSING) {
 			Tracing.debug(debugMsg + location);
 			time = -System.currentTimeMillis();
 		}
-		File localFile = null;
-		boolean local = false;
+		SubMonitor sub = SubMonitor.convert(monitor, 400);
 		try {
-			SubMonitor sub = SubMonitor.convert(monitor, 300);
-			OutputStream artifacts = null;
-			// try with compressed
-			boolean compress = true;
-			if (PROTOCOL_FILE.equals(location.getScheme())) {
-				local = true;
-				localFile = URIUtil.toFile(SimpleArtifactRepository.getActualLocation(location, true));
-				if (!localFile.exists()) {
-					localFile = URIUtil.toFile(SimpleArtifactRepository.getActualLocation(location, false));
-					compress = false;
-				}
-			} else {
-				//not local, return null if the caller wanted a modifiable repo
-				if ((flags & IRepositoryManager.REPOSITORY_HINT_MODIFIABLE) > 0) {
-					return null;
-				}
-
-				//download to local temp file
-				localFile = File.createTempFile("artifacts", ".xml"); //$NON-NLS-1$ //$NON-NLS-2$
-				try {
-					artifacts = new BufferedOutputStream(new FileOutputStream(localFile));
-					IStatus status = getTransport().download(SimpleArtifactRepository.getActualLocation(location, compress), artifacts, sub.newChild(100));
-					if (!status.isOK()) {
-						// not meaningful to continue on an authentication exception (user will likely just be prompted again)
-						if (status.getCode() == ProvisionException.REPOSITORY_FAILED_AUTHENTICATION)
-							throw new ProvisionException(status);
-						// retry uncompressed
-						compress = false;
-						status = getTransport().download(SimpleArtifactRepository.getActualLocation(location, compress), artifacts, sub.newChild(100));
-						if (!status.isOK())
-							throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_NOT_FOUND, status.getMessage(), null));
-					}
-				} finally {
-					if (artifacts != null)
-						artifacts.close();
-				}
-			}
-			InputStream descriptorStream = null;
+			File localFile = getLocalFile(location, sub.newChild(300));
+			InputStream inStream = new BufferedInputStream(new FileInputStream(localFile));
+			JarInputStream jarStream = null;
 			try {
-				descriptorStream = new BufferedInputStream(new FileInputStream(localFile));
-				if (compress) {
-					URI actualLocation = SimpleArtifactRepository.getActualLocation(location, false);
-					JarInputStream jInStream = new JarInputStream(descriptorStream);
-					JarEntry jarEntry = jInStream.getNextJarEntry();
-					String filename = URIUtil.lastSegment(actualLocation);
-					while (jarEntry != null && filename != null && !(filename.equals(jarEntry.getName()))) {
-						jarEntry = jInStream.getNextJarEntry();
+				//if reading from a jar, obtain a stream on the entry with the actual contents
+				if (localFile.getAbsolutePath().endsWith(JAR_EXTENSION)) {
+					jarStream = new JarInputStream(inStream);
+					JarEntry jarEntry = jarStream.getNextJarEntry();
+					String entryName = SimpleArtifactRepository.CONTENT_FILENAME + XML_EXTENSION;
+					while (jarEntry != null && (!entryName.equals(jarEntry.getName()))) {
+						jarEntry = jarStream.getNextJarEntry();
 					}
-					if (jarEntry == null) {
-						//there is a jar but the entry is missing or invalid, so treat this as an invalid repository
-						throw new IOException(NLS.bind(Messages.io_invalidLocation, actualLocation.getPath()));
-					}
-					descriptorStream = jInStream;
+					//if there is a jar but the entry is missing or invalid, treat this as an invalid repository
+					if (jarEntry == null)
+						throw new IOException(NLS.bind(Messages.io_invalidLocation, location));
 				}
+				//parse the repository descriptor file
+				sub.setWorkRemaining(100);
+				InputStream descriptorStream = jarStream != null ? jarStream : inStream;
 				SimpleArtifactRepositoryIO io = new SimpleArtifactRepositoryIO(getAgent());
 				SimpleArtifactRepository result = (SimpleArtifactRepository) io.read(localFile.toURL(), descriptorStream, sub.newChild(100));
 				result.initializeAfterLoad(location);
+				if (result != null && (flags & IRepositoryManager.REPOSITORY_HINT_MODIFIABLE) > 0 && !result.isModifiable())
+					return null;
 				if (Tracing.DEBUG_METADATA_PARSING) {
 					time += System.currentTimeMillis();
 					Tracing.debug(debugMsg + "time (ms): " + time); //$NON-NLS-1$ 
 				}
 				return result;
 			} finally {
-				if (descriptorStream != null)
-					descriptorStream.close();
+				safeClose(jarStream);
+				safeClose(inStream);
 			}
 		} catch (FileNotFoundException e) {
 			String msg = NLS.bind(Messages.io_failedRead, location);
@@ -116,8 +120,8 @@ public class SimpleArtifactRepositoryFactory extends ArtifactRepositoryFactory {
 			String msg = NLS.bind(Messages.io_failedRead, location);
 			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_FAILED_READ, msg, e));
 		} finally {
-			if (!local && localFile != null && !localFile.delete())
-				localFile.deleteOnExit();
+			if (monitor != null)
+				monitor.done();
 		}
 	}
 
@@ -125,7 +129,16 @@ public class SimpleArtifactRepositoryFactory extends ArtifactRepositoryFactory {
 		return new SimpleArtifactRepository(getAgent(), name, location, properties);
 	}
 
-	private Transport getTransport() {
-		return RepositoryTransport.getInstance();
+	/**
+	 * Closes a stream, ignoring any secondary exceptions
+	 */
+	private void safeClose(InputStream stream) {
+		if (stream == null)
+			return;
+		try {
+			stream.close();
+		} catch (IOException e) {
+			//ignore
+		}
 	}
 }