477635: prevent linking reviews to reviews

Change-Id: Icefa1691a4b1e0b655e00418300194262e18a2b5
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=477635

2 files changed, 40 insertions, 10 deletions

diff --git a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewMappingStoreTest.java b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewMappingStoreTest.java
index 0f4831fe4..32a1e265d 100644
--- a/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewMappingStoreTest.java
+++ b/org.eclipse.mylyn.reviews.core.tests/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewMappingStoreTest.java
@@ -27,6 +27,7 @@ import org.eclipse.mylyn.internal.tasks.core.TaskRepositoryManager;
 import org.eclipse.mylyn.internal.tasks.core.TaskTask;
 import org.eclipse.mylyn.internal.tasks.core.data.TaskDataManager;
 import org.eclipse.mylyn.reviews.core.spi.ReviewsConnector;
+import org.eclipse.mylyn.tasks.core.AbstractRepositoryConnector;
 import org.eclipse.mylyn.tasks.core.ITask;
 import org.eclipse.mylyn.tasks.core.TaskRepository;
 import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
@@ -65,6 +66,10 @@ public class TaskReviewMappingStoreTest {
 			+ " associated reviews Change-Id: I3a38d375688aad7be36bfd58c3311d692eb51ed Task-Url: " + taskUrl2
 			+ " Signed-off-by: Blaine Lewis <Blaine1@ualberta.ca>";
 
+	final String descriptionWithReviewAndTaskUrl = "477635: [UCOSP] contribute reviews section to task editor showing"
+			+ " associated reviews " + reviewUrl2 + " Change-Id: I3a38d375688aad7be36bfd58c3311d692eb51ed Task-Url: "
+			+ taskUrl1 + " Signed-off-by: Blaine Lewis <Blaine1@ualberta.ca>";
+
 	final String descriptionWithNoTaskUrl = "477635: [UCOSP] contribute reviews section to task editor showing"
 			+ " associated reviews Change-Id: I3a38d375688aad7be36bfd58c3311d692eb51ed "
 			+ " Signed-off-by: Blaine Lewis <Blaine1@ualberta.ca>";
@@ -92,21 +97,23 @@ public class TaskReviewMappingStoreTest {
 		reviewNoTask = new TaskTask("reviewKind", reviewUrlNoTask, "5");
 		reviewNoTask.setUrl(reviewUrlNoTask);
 
-		task1 = new TaskTask("reviewKind", taskUrl1, "1");
+		task1 = new TaskTask("taskKind", taskUrl1, "1");
 		task1.setUrl(taskUrl1);
 
-		task2 = new TaskTask("reviewKind", taskUrl2, "2");
+		task2 = new TaskTask("taskKind", taskUrl2, "2");
 		task2.setUrl(taskUrl2);
 	}
 
 	public TaskReviewsMappingsStore getEmptyTaskReviewStore() {
 		TaskRepositoryManager repositoryManager = mock(TaskRepositoryManager.class);
-		ReviewsConnector connector = mock(ReviewsConnector.class);
+		ReviewsConnector reviewConnector = mock(ReviewsConnector.class);
+		AbstractRepositoryConnector taskConnector = mock(AbstractRepositoryConnector.class);
 
 		taskDataManager = mock(TaskDataManager.class);
 
-		when(repositoryManager.getConnectorForRepositoryTaskUrl(Matchers.anyString())).thenReturn(connector);
-		when(repositoryManager.getRepositoryConnector(Matchers.anyString())).thenReturn(connector);
+		when(repositoryManager.getConnectorForRepositoryTaskUrl(Matchers.anyString())).thenReturn(reviewConnector);
+		when(repositoryManager.getRepositoryConnector("reviewKind")).thenReturn(reviewConnector);
+		when(repositoryManager.getRepositoryConnector("taskKind")).thenReturn(taskConnector);
 
 		TaskReviewsMappingsStore taskReviewsMappingStore = new MockTaskReviewsMappingsStore(mock(TaskList.class),
 				repositoryManager);
@@ -180,6 +187,20 @@ public class TaskReviewMappingStoreTest {
 	}
 
 	@Test
+	public void testReviewNotLinkedToReview() throws CoreException {
+		TaskReviewsMappingsStore taskReviewsMappingStore = getEmptyTaskReviewStore();
+		TaskData reviewData1 = addReviewData(review1, descriptionWithReviewAndTaskUrl);
+
+		TaskContainerDelta delta = new TaskContainerDelta(review1, Kind.ADDED);
+		taskReviewsMappingStore.addTaskAssocation(review1, reviewData1);
+		taskReviewsMappingStore.containersChanged(ImmutableSet.of(delta));
+
+		assertFalse(taskReviewsMappingStore.getReviewUrls(reviewUrl2).contains(reviewUrl1));
+		assertTrue(taskReviewsMappingStore.getReviewUrls(taskUrl1).contains(reviewUrl1));
+		assertTrue(taskReviewsMappingStore.getTaskUrl(review1).equals(taskUrl1));
+	}
+
+	@Test
 	public void testRemove() throws CoreException {
 
 		TaskReviewsMappingsStore taskReviewsMappingStore = getEmptyTaskReviewStore();
@@ -283,6 +304,10 @@ public class TaskReviewMappingStoreTest {
 				return task1;
 			case taskUrl2:
 				return task2;
+			case reviewUrl1:
+				return review1;
+			case reviewUrl2:
+				return review2;
 			}
 			return null;
 		}
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewsMappingsStore.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewsMappingsStore.java
index 7dbf387d1..f36006932 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewsMappingsStore.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/TaskReviewsMappingsStore.java
@@ -29,7 +29,7 @@ import com.google.common.collect.SetMultimap;
  * TaskEditorReviewPart to give a table of the reviews pertaining to a task. The class is limited as it maps one task to
  * many reviews. It is however possible (albeit strange) to have multiple tasks for one review. This is a limitation by
  * design.
- * 
+ *
  * @author Blaine Lewis
  */
 
@@ -93,8 +93,12 @@ public class TaskReviewsMappingsStore implements ITaskListChangeListener {
 					ITask task = getTaskByUrl(token);
 
 					if (task != null) {
-						updateMapping(review, task.getUrl() != null ? task.getUrl() : token);
-						return;
+						AbstractRepositoryConnector connector = repositoryManager
+								.getRepositoryConnector(task.getConnectorKind());
+						if (!(connector instanceof ReviewsConnector)) {
+							updateMapping(review, task.getUrl() != null ? task.getUrl() : token);
+							return;
+						}
 					}
 				} catch (MalformedURLException e) {
 					//Do nothing, this is expected behavior when there is no URL
@@ -132,9 +136,10 @@ public class TaskReviewsMappingsStore implements ITaskListChangeListener {
 				ITask review = (ITask) reviewRepoElement;
 
 				//We need to check it in case the mapping was removed from the task
-				AbstractRepositoryConnector connector = repositoryManager.getRepositoryConnector(review.getConnectorKind());
+				AbstractRepositoryConnector connector = repositoryManager
+						.getRepositoryConnector(review.getConnectorKind());
 
-				if (review != null && connector instanceof ReviewsConnector) {
+				if (connector instanceof ReviewsConnector) {
 					deleteMappingsTo(review);
 				}
 			}