486145: tasks should not receive incoming notifications for attributes

that aren't visible

Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=486145

Change-Id: I57980475082de716c2ca24a11e0bc4b31121cdc5
Signed-off-by: Brandon Dong <brandon.dong@tasktop.com>

5 files changed, 218 insertions, 29 deletions

diff --git a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/SynchronizationManger.java b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/SynchronizationManger.java
index cbdfd1592..f3d6be684 100644
--- a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/SynchronizationManger.java
+++ b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/SynchronizationManger.java
@@ -68,7 +68,6 @@ public class SynchronizationManger {
 						it.remove();
 					}
 				}
-				diff.setHasChanged(diff.getChangedAttributes().size() > 0 || diff.getNewComments().size() > 0);
 			}
 		}
 
@@ -93,8 +92,8 @@ public class SynchronizationManger {
 		List<String> attributeIds = new ArrayList<String>();
 
 		IExtensionRegistry registry = Platform.getExtensionRegistry();
-		IExtensionPoint connectorsExtensionPoint = registry.getExtensionPoint(ITasksCoreConstants.ID_PLUGIN
-				+ ".synchronizationParticipants"); //$NON-NLS-1$
+		IExtensionPoint connectorsExtensionPoint = registry
+				.getExtensionPoint(ITasksCoreConstants.ID_PLUGIN + ".synchronizationParticipants"); //$NON-NLS-1$
 		IExtension[] extensions = connectorsExtensionPoint.getExtensions();
 		for (IExtension extension : extensions) {
 			IConfigurationElement[] elements = extension.getConfigurationElements();
@@ -109,20 +108,15 @@ public class SynchronizationManger {
 								participant.setId(element.getAttribute("id")); //$NON-NLS-1$
 								participants.add(participant);
 							} else {
-								status.add(new Status(
-										IStatus.ERROR,
-										ITasksCoreConstants.ID_PLUGIN,
-										NLS.bind(
-												"Connector core ''{0}'' does not extend expected class for extension contributed by {1}", //$NON-NLS-1$
-												object.getClass().getCanonicalName(), element.getContributor()
-														.getName())));
+								status.add(new Status(IStatus.ERROR, ITasksCoreConstants.ID_PLUGIN, NLS.bind(
+										"Connector core ''{0}'' does not extend expected class for extension contributed by {1}", //$NON-NLS-1$
+										object.getClass().getCanonicalName(), element.getContributor().getName())));
 							}
 						} catch (Throwable e) {
-							status.add(new Status(
-									IStatus.ERROR,
-									ITasksCoreConstants.ID_PLUGIN,
-									NLS.bind(
-											"Connector core failed to load for extension contributed by {0}", element.getContributor().getName()), e)); //$NON-NLS-1$
+							status.add(new Status(IStatus.ERROR, ITasksCoreConstants.ID_PLUGIN,
+									NLS.bind("Connector core failed to load for extension contributed by {0}", //$NON-NLS-1$
+											element.getContributor().getName()),
+									e));
 						}
 					}
 				} else if ("suppressIncoming".equals(element.getName())) { //$NON-NLS-1$
diff --git a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/TaskDataDiff.java b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/TaskDataDiff.java
index 74fefffa4..83fb7c4b6 100644
--- a/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/TaskDataDiff.java
+++ b/org.eclipse.mylyn.tasks.core/src/org/eclipse/mylyn/internal/tasks/core/data/TaskDataDiff.java
@@ -38,12 +38,12 @@ public class TaskDataDiff implements ITaskDataDiff {
 
 	private final Set<ITaskComment> newComments = new LinkedHashSet<ITaskComment>();
 
+	private final Set<TaskAttribute> newAttachments = new LinkedHashSet<TaskAttribute>();
+
 	private final Set<ITaskAttributeDiff> changedAttributes = new LinkedHashSet<ITaskAttributeDiff>();
 
 	private final RepositoryModel repositoryModel;
 
-	public boolean hasChanged;
-
 	private final TaskRepository repository;
 
 	public TaskDataDiff(RepositoryModel repositoryModel, TaskData newTaskData, TaskData oldTaskData) {
@@ -53,16 +53,16 @@ public class TaskDataDiff implements ITaskDataDiff {
 		this.repository = newTaskData.getAttributeMapper().getTaskRepository();
 		this.newTaskData = newTaskData;
 		this.oldTaskData = oldTaskData;
-		this.hasChanged = true;
 		parse();
 	}
 
 	public boolean hasChanged() {
-		return hasChanged;
+		return !changedAttributes.isEmpty() || !newComments.isEmpty() || !newAttachments.isEmpty();
 	}
 
+	@Deprecated
 	public void setHasChanged(boolean hasChanged) {
-		this.hasChanged = hasChanged;
+
 	}
 
 	public TaskRepository getRepository() {
@@ -81,6 +81,10 @@ public class TaskDataDiff implements ITaskDataDiff {
 		return newComments;
 	}
 
+	public Collection<TaskAttribute> getNewAttachments() {
+		return newAttachments;
+	}
+
 	public Collection<ITaskAttributeDiff> getChangedAttributes() {
 		return changedAttributes;
 	}
@@ -127,6 +131,8 @@ public class TaskDataDiff implements ITaskDataDiff {
 		String type = attribute.getMetaData().getType();
 		if (TaskAttribute.TYPE_COMMENT.equals(type)) {
 			addChangedComment(oldAttribute, newAttribute);
+		} else if (TaskAttribute.TYPE_ATTACHMENT.equals(type)) {
+			newAttachments.add(attribute);
 		} else if (TaskAttribute.TYPE_OPERATION.equals(type)) {
 			// ignore
 		} else if (ignoreKind || attribute.getMetaData().getKind() != null) {
@@ -145,5 +151,4 @@ public class TaskDataDiff implements ITaskDataDiff {
 			}
 		}
 	}
-
 }
diff --git a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/AllTasksTests.java b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/AllTasksTests.java
index a819eb752..72def97b5 100644
--- a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/AllTasksTests.java
+++ b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/AllTasksTests.java
@@ -11,9 +11,6 @@
 
 package org.eclipse.mylyn.tasks.tests;
 
-import junit.framework.Test;
-import junit.framework.TestSuite;
-
 import org.eclipse.mylyn.commons.sdk.util.ManagedTestSuite;
 import org.eclipse.mylyn.commons.sdk.util.TestConfiguration;
 import org.eclipse.mylyn.tasks.tests.bugs.SupportHandlerManagerTest;
@@ -30,6 +27,7 @@ import org.eclipse.mylyn.tasks.tests.core.TaskJobFactoryTest;
 import org.eclipse.mylyn.tasks.tests.core.TaskListUnmatchedContainerTest;
 import org.eclipse.mylyn.tasks.tests.core.TaskRepositoryLocationTest;
 import org.eclipse.mylyn.tasks.tests.core.TaskRepositoryTest;
+import org.eclipse.mylyn.tasks.tests.data.SynchronizationMangerTest;
 import org.eclipse.mylyn.tasks.tests.data.TaskAttributeMapperTest;
 import org.eclipse.mylyn.tasks.tests.data.TaskAttributeTest;
 import org.eclipse.mylyn.tasks.tests.data.TaskDataDiffTest;
@@ -54,6 +52,9 @@ import org.eclipse.mylyn.tasks.tests.ui.editor.TaskEditorPartDescriptorTest;
 import org.eclipse.mylyn.tasks.tests.ui.editor.TaskMigratorTest;
 import org.eclipse.mylyn.tasks.tests.ui.editor.TaskUrlHyperlinkDetectorTest;
 
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
 /**
  * @author Mik Kersten
  * @author Shawn Minto
@@ -166,6 +167,7 @@ public class AllTasksTests {
 		suite.addTestSuite(RepositoryConnectorContributorTest.class);
 		suite.addTestSuite(TaskInitializationDataTest.class);
 		suite.addTestSuite(TaskDataDiffTest.class);
+		suite.addTestSuite(SynchronizationMangerTest.class);
 		suite.addTestSuite(TaskEditorExtensionsTest.class);
 		suite.addTestSuite(AbstractRepositoryConnectorTest.class);
 		suite.addTestSuite(TaskJobFactoryTest.class);
diff --git a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/SynchronizationMangerTest.java b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/SynchronizationMangerTest.java
new file mode 100644
index 000000000..063a49d0c
--- /dev/null
+++ b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/SynchronizationMangerTest.java
@@ -0,0 +1,190 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.tasks.tests.data;
+
+import java.util.Collections;
+
+import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.mylyn.internal.tasks.core.RepositoryModel;
+import org.eclipse.mylyn.internal.tasks.core.TaskList;
+import org.eclipse.mylyn.internal.tasks.core.TaskRepositoryManager;
+import org.eclipse.mylyn.internal.tasks.core.TaskTask;
+import org.eclipse.mylyn.internal.tasks.core.data.SynchronizationManger;
+import org.eclipse.mylyn.internal.tasks.core.data.TaskAttributeDiff;
+import org.eclipse.mylyn.internal.tasks.core.data.TaskDataDiff;
+import org.eclipse.mylyn.tasks.core.IRepositoryManager;
+import org.eclipse.mylyn.tasks.core.ITask;
+import org.eclipse.mylyn.tasks.core.TaskRepository;
+import org.eclipse.mylyn.tasks.core.data.TaskAttribute;
+import org.eclipse.mylyn.tasks.core.data.TaskAttributeMapper;
+import org.eclipse.mylyn.tasks.core.data.TaskData;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableSet;
+
+import junit.framework.TestCase;
+
+public class SynchronizationMangerTest extends TestCase {
+
+	private SynchronizationManger manager;
+
+	private RepositoryModel model;
+
+	private TaskRepository repository;
+
+	private TaskAttributeMapper mapper;
+
+	private TaskData newData;
+
+	private TaskData oldData;
+
+	@Override
+	protected void setUp() throws Exception {
+		ITask task = new TaskTask("kind", "url", "1");
+		repository = new TaskRepository(task.getConnectorKind(), task.getRepositoryUrl());
+		mapper = new TaskAttributeMapper(repository);
+		IRepositoryManager repositoryManager = new TaskRepositoryManager();
+		repositoryManager.addRepository(repository);
+		TaskList taskList = new TaskList();
+		taskList.addTask(task);
+		model = new RepositoryModel(taskList, repositoryManager);
+
+		newData = new TaskData(mapper, repository.getConnectorKind(), repository.getUrl(), "1");
+		oldData = new TaskData(mapper, repository.getConnectorKind(), repository.getUrl(), "1");
+		manager = new SynchronizationManger(model);
+	}
+
+	@Test
+	public void testHasChangedAttributes() {
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertFalse(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithoutKind() {
+		TaskAttribute attributeCustom = newData.getRoot().createAttribute("custom");
+		attributeCustom.setValue("1");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertFalse(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithKind() {
+		TaskAttribute attributeCustom = newData.getRoot().createAttribute("custom");
+		attributeCustom.setValue("1");
+		attributeCustom.getMetaData().setKind("kind");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithChangedValues() {
+		TaskAttribute attributeOld = oldData.getRoot().createAttribute(TaskAttribute.SUMMARY);
+		attributeOld.setValue("1");
+		TaskAttribute attributeNew = newData.getRoot().createAttribute(TaskAttribute.SUMMARY);
+		attributeNew.setValue("2");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithChangedCustomValues() {
+		TaskAttribute attributeOld = oldData.getRoot().createAttribute("custom");
+		attributeOld.setValue("1");
+		attributeOld.getMetaData().setKind("kind");
+		TaskAttribute attributeNew = newData.getRoot().createAttribute("custom");
+		attributeNew.setValue("2");
+		attributeNew.getMetaData().setKind("kind");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithUnchangedCustomValues() {
+		TaskAttribute attributeOld = oldData.getRoot().createAttribute("custom");
+		attributeOld.setValue("1");
+		attributeOld.getMetaData().setKind("kind");
+		TaskAttribute attributeNew = newData.getRoot().createAttribute("custom");
+		attributeNew.setValue("1");
+		attributeNew.getMetaData().setKind("kind");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertFalse(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesMultiple() {
+		TaskAttribute attributeCustom = newData.getRoot().createAttribute("custom");
+		attributeCustom.setValue("1");
+		TaskAttribute attributeSummary = newData.getRoot().createAttribute(TaskAttribute.SUMMARY);
+		attributeSummary.setValue("1");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithComments() {
+		TaskAttribute attributeComment = newData.getRoot().createAttribute("custom");
+		attributeComment.setValue("1");
+		attributeComment.getMetaData().setType(TaskAttribute.TYPE_COMMENT);
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testGetChangedAttributesWithComments() {
+		TaskAttribute attributeComment = newData.getRoot().createAttribute("custom");
+		attributeComment.setValue("1");
+		attributeComment.getMetaData().setType(TaskAttribute.TYPE_COMMENT);
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertEquals(1, diff.getNewComments().size());
+		assertEquals(attributeComment, diff.getNewComments().iterator().next().getTaskAttribute());
+	}
+
+	@Test
+	public void testHasChangedAttributesWithAttachments() {
+		TaskAttribute attributeAttachment = newData.getRoot().createAttribute("custom");
+		attributeAttachment.setValue("1");
+		attributeAttachment.getMetaData().setType(TaskAttribute.TYPE_ATTACHMENT);
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testGetChangedAttributesWithAttachments() {
+		TaskAttribute attributeAttachment = newData.getRoot().createAttribute("custom");
+		attributeAttachment.setValue("1");
+		attributeAttachment.getMetaData().setType(TaskAttribute.TYPE_ATTACHMENT);
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertEquals(Collections.singleton(attributeAttachment), diff.getNewAttachments());
+	}
+
+	@Test
+	public void testGetChangedAttributesMultipleKind() {
+		TaskAttribute attributeCustom = newData.getRoot().createAttribute("custom");
+		attributeCustom.setValue("1");
+		attributeCustom.getMetaData().setKind("kind");
+		TaskAttribute attributeSummary = newData.getRoot().createAttribute(TaskAttribute.SUMMARY);
+		attributeSummary.setValue("1");
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertEquals(ImmutableSet.of(new TaskAttributeDiff(null, attributeSummary),
+				new TaskAttributeDiff(null, attributeCustom)), diff.getChangedAttributes());
+		assertTrue(diff.hasChanged());
+	}
+
+	@Test
+	public void testHasChangedAttributesSummaryEmptyValue() {
+		newData.getRoot().createAttribute(TaskAttribute.SUMMARY);
+		TaskDataDiff diff = manager.createDiff(newData, oldData, new NullProgressMonitor());
+		assertFalse(diff.hasChanged());
+	}
+
+}
diff --git a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/TaskDataDiffTest.java b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/TaskDataDiffTest.java
index 56b141fe2..f9efe2377 100644
--- a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/TaskDataDiffTest.java
+++ b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/data/TaskDataDiffTest.java
@@ -16,8 +16,6 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 
-import junit.framework.TestCase;
-
 import org.eclipse.mylyn.internal.tasks.core.RepositoryModel;
 import org.eclipse.mylyn.internal.tasks.core.TaskList;
 import org.eclipse.mylyn.internal.tasks.core.TaskRepositoryManager;
@@ -33,6 +31,8 @@ import org.eclipse.mylyn.tasks.core.data.TaskAttributeMapper;
 import org.eclipse.mylyn.tasks.core.data.TaskData;
 import org.junit.Test;
 
+import junit.framework.TestCase;
+
 /**
  * @author Steffen Pingel
  */
@@ -110,7 +110,7 @@ public class TaskDataDiffTest extends TestCase {
 	@Test
 	public void testHasChanges() {
 		TaskDataDiff diff = new TaskDataDiff(model, newData, oldData);
-		assertTrue(diff.hasChanged());
+		assertFalse(diff.hasChanged());
 	}
 
 	@Test
@@ -118,7 +118,5 @@ public class TaskDataDiffTest extends TestCase {
 		newData.getRoot().createAttribute(TaskAttribute.SUMMARY).setValue("text");
 		TaskDataDiff diff = new TaskDataDiff(model, newData, oldData);
 		assertTrue(diff.hasChanged());
-		diff.setHasChanged(false);
-		assertFalse(diff.hasChanged());
 	}
 }