bug 249035 - identity of ArtifactDescriptor

2 files changed, 25 insertions, 24 deletions

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 4a815e948..672549529 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
@@ -171,7 +171,6 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 	}
 
 	private static final String ARTIFACT_FOLDER = "artifact.folder"; //$NON-NLS-1$
-	private static final String ARTIFACT_REFERENCE = "artifact.reference"; //$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$
@@ -399,7 +398,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 	 * descriptor existed in the repository, and was successfully removed.
 	 */
 	private boolean doRemoveArtifact(IArtifactDescriptor descriptor) {
-		if (((ArtifactDescriptor) descriptor).getRepositoryProperty(ARTIFACT_REFERENCE) == null) {
+		if (((ArtifactDescriptor) descriptor).getRepositoryProperty(ArtifactDescriptor.ARTIFACT_REFERENCE) == null) {
 			File file = getArtifactFile(descriptor);
 			if (file == null)
 				return false;
@@ -631,7 +630,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		try {
 			// if the artifact is just a reference then return the reference location
 			if (descriptor instanceof ArtifactDescriptor) {
-				String artifactReference = ((ArtifactDescriptor) descriptor).getRepositoryProperty(ARTIFACT_REFERENCE);
+				String artifactReference = ((ArtifactDescriptor) descriptor).getRepositoryProperty(ArtifactDescriptor.ARTIFACT_REFERENCE);
 				if (artifactReference != null) {
 					try {
 						return new URI(artifactReference);
@@ -675,17 +674,19 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 
 	public OutputStream getOutputStream(IArtifactDescriptor descriptor) throws ProvisionException {
 		assertModifiable();
-		// Check if the artifact is already in this repository
-		if (contains(descriptor)) {
-			String msg = NLS.bind(Messages.available_already_in, getLocation().toString());
-			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_EXISTS, msg, null));
-		}
 
-		// create a copy of the original descriptor that we can manipulate
+		// Create a copy of the original descriptor that we can manipulate and add to our repo.
 		ArtifactDescriptor newDescriptor = new ArtifactDescriptor(descriptor);
 		if (isFolderBased(descriptor))
 			newDescriptor.setRepositoryProperty(ARTIFACT_FOLDER, Boolean.TRUE.toString());
 
+		// Check if the artifact is already in this repository, check the newDescriptor instead of the original
+		// since the implementation of hash/equals on the descriptor matters here.
+		if (contains(newDescriptor)) {
+			String msg = NLS.bind(Messages.available_already_in, getLocation().toString());
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_EXISTS, msg, null));
+		}
+
 		// Determine writing location
 		URI newLocation = createLocation(newDescriptor);
 		if (newLocation == null)
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/ArtifactDescriptor.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/ArtifactDescriptor.java
index a196a6c3b..0dfc5562e 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/ArtifactDescriptor.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/ArtifactDescriptor.java
@@ -20,6 +20,7 @@ import org.eclipse.equinox.internal.provisional.p2.metadata.IArtifactKey;
  * This represents information about a given artifact stored on a particular byte server.
  */
 public class ArtifactDescriptor implements IArtifactDescriptor {
+	public static final String ARTIFACT_REFERENCE = "artifact.reference"; //$NON-NLS-1$
 
 	private static final ProcessingStepDescriptor[] EMPTY_STEPS = new ProcessingStepDescriptor[0];
 
@@ -117,6 +118,8 @@ public class ArtifactDescriptor implements IArtifactDescriptor {
 		processingSteps = value == null ? EMPTY_STEPS : value;
 	}
 
+	// Implementation of both equals and hash depends on the implementation of
+	// SimpleArtifactRepository#getOutputStream(IArtifactDescriptor)
 	public boolean equals(Object obj) {
 		if (this == obj)
 			return true;
@@ -132,20 +135,16 @@ public class ArtifactDescriptor implements IArtifactDescriptor {
 			return false;
 		if (!Arrays.equals(processingSteps, other.processingSteps))
 			return false;
-		if (properties == null) {
-			if (other.properties != null)
-				return false;
-		} else if (!properties.equals(other.properties))
-			return false;
-		if (properties == null) {
-			if (other.properties != null)
-				return false;
-		} else if (!properties.equals(other.properties))
+
+		//Properties affecting SimpleArtifactRepository#getLocation
+		String locationProperty = getRepositoryProperty(ARTIFACT_REFERENCE);
+		String otherProperty = other.getRepositoryProperty(ARTIFACT_REFERENCE);
+		if ((locationProperty != null && !locationProperty.equals(otherProperty)) || locationProperty != otherProperty)
 			return false;
-		if (repositoryProperties == null) {
-			if (other.repositoryProperties != null)
-				return false;
-		} else if (!repositoryProperties.equals(other.repositoryProperties))
+
+		locationProperty = getRepositoryProperty(FORMAT);
+		otherProperty = other.getRepositoryProperty(FORMAT);
+		if ((locationProperty != null && !locationProperty.equals(otherProperty)) || locationProperty != otherProperty)
 			return false;
 
 		return true;
@@ -167,8 +166,9 @@ public class ArtifactDescriptor implements IArtifactDescriptor {
 		int result = 1;
 		result = prime * result + ((key == null) ? 0 : key.hashCode());
 		result = prime * result + hashCode(processingSteps);
-		result = prime * result + ((properties == null) ? 0 : properties.hashCode());
-		result = prime * result + ((repositoryProperties == null) ? 0 : repositoryProperties.hashCode());
+
+		String[] hashProperties = new String[] {getRepositoryProperty(ARTIFACT_REFERENCE), getRepositoryProperty(FORMAT)};
+		result = prime * result + hashCode(hashProperties);
 		return result;
 	}