summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLeo Dos Santos2014-01-14 17:57:24 (EST)
committerMiles Parker2014-02-07 13:40:26 (EST)
commitea0aebfa41766833d10f6def977ca9d45f0b3cdb (patch)
treea17e1c4c824a2949cec185037a3e32582aa3bf34
parent701fe7e7f7d37e2725c0ea464a199180085fbeaf (diff)
downloadorg.eclipse.mylyn.reviews-ea0aebfa41766833d10f6def977ca9d45f0b3cdb.zip
org.eclipse.mylyn.reviews-ea0aebfa41766833d10f6def977ca9d45f0b3cdb.tar.gz
org.eclipse.mylyn.reviews-ea0aebfa41766833d10f6def977ca9d45f0b3cdb.tar.bz2
410770: [regression] duplicate inline comments shown in compare editorrefs/changes/32/20632/11
Change-Id: I7330cd421487118ac2a6967c0eb267daf9e1b653 Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=410770 Signed-off-by: Leo Dos Santos <leo.dos.santos@tasktop.com>
-rw-r--r--org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/ModelUtil.java52
-rw-r--r--org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Change.java10
-rw-r--r--org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Comment.java10
-rw-r--r--org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewItemSet.java13
-rw-r--r--org.eclipse.mylyn.reviews.tests/META-INF/MANIFEST.MF3
-rw-r--r--org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/AllReviewsTests.java2
-rw-r--r--org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/ui/ReviewAnnotationModelTest.java159
-rw-r--r--org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentAnnotation.java30
-rw-r--r--org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java8
9 files changed, 251 insertions, 36 deletions
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/ModelUtil.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/ModelUtil.java
new file mode 100644
index 0000000..5873837
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/ModelUtil.java
@@ -0,0 +1,52 @@
+/*******************************************************************************
+ * Copyright (c) 2014 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.reviews.core.spi;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.eclipse.emf.ecore.EAttribute;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EReference;
+
+public class ModelUtil {
+
+ private final static int PRIME = 31;
+
+ public static int ecoreHash(EObject object) {
+ return ecoreHash(0, object);
+ }
+
+ public static int ecoreHash(int current, EObject object) {
+ return ecoreHash(current, object, new HashSet<EObject>());
+ }
+
+ private static int ecoreHash(int current, EObject object, Set<EObject> references) {
+ if (object != null) {
+ references.add(object);
+ for (EAttribute attribute : object.eClass().getEAllAttributes()) {
+ Object value = object.eGet(attribute);
+ if (value != null) {
+ current = PRIME * current + value.hashCode();
+ }
+ }
+ for (EReference reference : object.eClass().getEAllReferences()) {
+ Object value = object.eGet(reference);
+ if (value instanceof EObject && !references.contains(value)) {
+ current = ecoreHash(current, (EObject) value, references);
+ }
+ }
+ }
+ return current;
+ }
+
+}
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Change.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Change.java
index 9a1df4b..5ce45b4 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Change.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Change.java
@@ -218,11 +218,12 @@ public class Change extends EObjectImpl implements IChange {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setCreationDate(Date newCreationDate) {
Date oldCreationDate = creationDate;
- creationDate = newCreationDate;
+ //Protect against case where java.sql.Timestamp is used
+ creationDate = new Date(newCreationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.CHANGE__CREATION_DATE,
oldCreationDate, creationDate));
@@ -241,11 +242,12 @@ public class Change extends EObjectImpl implements IChange {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setModificationDate(Date newModificationDate) {
Date oldModificationDate = modificationDate;
- modificationDate = newModificationDate;
+ //Protect against case where java.sql.Timestamp is used
+ modificationDate = new Date(newModificationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.CHANGE__MODIFICATION_DATE,
oldModificationDate, modificationDate));
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Comment.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Comment.java
index 62f19ff..4af18a5 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Comment.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/Comment.java
@@ -314,11 +314,12 @@ public class Comment extends EObjectImpl implements IComment {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setCreationDate(Date newCreationDate) {
Date oldCreationDate = creationDate;
- creationDate = newCreationDate;
+ //Protect against case where java.sql.Timestamp is used
+ creationDate = new Date(newCreationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.COMMENT__CREATION_DATE,
oldCreationDate, creationDate));
@@ -337,11 +338,12 @@ public class Comment extends EObjectImpl implements IComment {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setModificationDate(Date newModificationDate) {
Date oldModificationDate = modificationDate;
- modificationDate = newModificationDate;
+ //Protect against case where java.sql.Timestamp is used
+ modificationDate = new Date(newModificationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.COMMENT__MODIFICATION_DATE,
oldModificationDate, modificationDate));
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewItemSet.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewItemSet.java
index c3afe2e..c4236aa 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewItemSet.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewItemSet.java
@@ -150,11 +150,12 @@ public class ReviewItemSet extends ReviewItem implements IReviewItemSet {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setCreationDate(Date newCreationDate) {
Date oldCreationDate = creationDate;
- creationDate = newCreationDate;
+ //Protect against case where java.sql.Timestamp is used
+ creationDate = new Date(newCreationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.REVIEW_ITEM_SET__CREATION_DATE,
oldCreationDate, creationDate));
@@ -173,11 +174,12 @@ public class ReviewItemSet extends ReviewItem implements IReviewItemSet {
/**
* <!-- begin-user-doc --> <!-- end-user-doc -->
*
- * @generated
+ * @generated NOT
*/
public void setModificationDate(Date newModificationDate) {
Date oldModificationDate = modificationDate;
- modificationDate = newModificationDate;
+ //Protect against case where java.sql.Timestamp is used
+ modificationDate = new Date(newModificationDate.getTime());
if (eNotificationRequired()) {
eNotify(new ENotificationImpl(this, Notification.SET, ReviewsPackage.REVIEW_ITEM_SET__MODIFICATION_DATE,
oldModificationDate, modificationDate));
@@ -290,8 +292,7 @@ public class ReviewItemSet extends ReviewItem implements IReviewItemSet {
public void setParentReview(IReview newParentReview) {
if (newParentReview != eInternalContainer()
|| (eContainerFeatureID() != ReviewsPackage.REVIEW_ITEM_SET__PARENT_REVIEW && newParentReview != null)) {
- if (EcoreUtil.isAncestor(this, newParentReview))
- {
+ if (EcoreUtil.isAncestor(this, newParentReview)) {
throw new IllegalArgumentException("Recursive containment not allowed for " + toString()); //$NON-NLS-1$
}
NotificationChain msgs = null;
diff --git a/org.eclipse.mylyn.reviews.tests/META-INF/MANIFEST.MF b/org.eclipse.mylyn.reviews.tests/META-INF/MANIFEST.MF
index 677bca7..dd4a2ac 100644
--- a/org.eclipse.mylyn.reviews.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.mylyn.reviews.tests/META-INF/MANIFEST.MF
@@ -24,7 +24,8 @@ Require-Bundle: org.hamcrest;bundle-version="1.1.0",
org.eclipse.core.resources,
org.eclipse.emf.ecore;bundle-version="2.5.0",
org.eclipse.emf.edit;bundle-version="2.5.0",
- org.apache.commons.io
+ org.apache.commons.io,
+ org.eclipse.jface.text
Export-Package: org.eclipse.mylyn.reviews.spi.edit.remote;x-internal:=true,
org.eclipse.mylyn.reviews.tests;x-internal:=true,
org.eclipse.mylyn.reviews.tests.ui;x-internal:=true
diff --git a/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/AllReviewsTests.java b/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/AllReviewsTests.java
index 19beb55..65560ec 100644
--- a/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/AllReviewsTests.java
+++ b/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/AllReviewsTests.java
@@ -15,6 +15,7 @@ import junit.framework.Test;
import junit.framework.TestSuite;
import org.eclipse.mylyn.reviews.spi.edit.remote.AbstractRemoteEditFactoryProviderTest;
+import org.eclipse.mylyn.reviews.tests.ui.ReviewAnnotationModelTest;
import org.eclipse.mylyn.reviews.tests.ui.ReviewUiTest;
import org.eclipse.mylyn.reviews.tests.ui.UiDataLocatorTest;
@@ -28,6 +29,7 @@ public class AllReviewsTests {
suite.addTestSuite(ReviewUiTest.class);
suite.addTestSuite(UiDataLocatorTest.class);
suite.addTestSuite(AbstractRemoteEditFactoryProviderTest.class);
+ suite.addTestSuite(ReviewAnnotationModelTest.class);
return suite;
}
diff --git a/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/ui/ReviewAnnotationModelTest.java b/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/ui/ReviewAnnotationModelTest.java
new file mode 100644
index 0000000..3b2bf11
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.tests/src/org/eclipse/mylyn/reviews/tests/ui/ReviewAnnotationModelTest.java
@@ -0,0 +1,159 @@
+/*******************************************************************************
+ * Copyright (c) 2014 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.reviews.tests.ui;
+
+import java.sql.Timestamp;
+import java.util.Date;
+import java.util.Iterator;
+
+import junit.framework.TestCase;
+
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.emf.common.notify.Notification;
+import org.eclipse.emf.common.notify.impl.NotificationImpl;
+import org.eclipse.jface.text.Document;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.mylyn.internal.reviews.ui.annotations.ReviewAnnotationModel;
+import org.eclipse.mylyn.reviews.core.model.IComment;
+import org.eclipse.mylyn.reviews.core.model.IFileVersion;
+import org.eclipse.mylyn.reviews.core.model.ILineLocation;
+import org.eclipse.mylyn.reviews.core.model.ILineRange;
+import org.eclipse.mylyn.reviews.core.model.ILocation;
+import org.eclipse.mylyn.reviews.core.model.IReviewItem;
+import org.eclipse.mylyn.reviews.core.model.IReviewsFactory;
+import org.eclipse.mylyn.reviews.core.model.IUser;
+import org.eclipse.mylyn.reviews.ui.ReviewBehavior;
+import org.eclipse.team.core.history.IFileRevision;
+
+/**
+ * @author Leo Dos Santos
+ */
+public class ReviewAnnotationModelTest extends TestCase {
+
+ private final static String DEFAULT_TEXT = "This change looks good.";
+
+ private final static long DEFAULT_TIMESTAMP = 1388577600000L;
+
+ private IDocument doc;
+
+ private IReviewItem review;
+
+ private ReviewAnnotationModel model;
+
+ private class MockReviewBehaviour extends ReviewBehavior {
+
+ public MockReviewBehaviour() {
+ super(null);
+ }
+
+ @Override
+ public IStatus addComment(IReviewItem fileItem, IComment comment, IProgressMonitor monitor) {
+ return null;
+ }
+
+ @Override
+ public IFileRevision getFileRevision(IFileVersion reviewFileVersion) {
+ return null;
+ }
+
+ }
+
+ @Override
+ protected void setUp() throws Exception {
+ doc = new Document("A test document, nothing special.");
+ review = IReviewsFactory.INSTANCE.createFileItem();
+ generateComment(DEFAULT_TEXT, new Date(DEFAULT_TIMESTAMP));
+
+ model = new ReviewAnnotationModel();
+ model.setItem(review, new MockReviewBehaviour());
+ model.connect(doc);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ model.disconnect(doc);
+ }
+
+ public void testConnect() {
+ Iterator iter = model.getAnnotationIterator();
+ assertEquals(1, getCount(iter));
+
+ model.disconnect(doc);
+ iter = model.getAnnotationIterator();
+ assertEquals(0, getCount(iter));
+
+ model.connect(doc);
+ iter = model.getAnnotationIterator();
+ assertEquals(1, getCount(iter));
+ }
+
+ public void testNotifyChanged() {
+ Iterator iter = model.getAnnotationIterator();
+ assertEquals(1, getCount(iter));
+
+ // Comments sometimes come in with Dates and sometimes with Timestamps,
+ // so we need to be able to handle both.
+ IComment clone = generateComment(DEFAULT_TEXT, new Timestamp(DEFAULT_TIMESTAMP));
+ Notification notification = new NotificationImpl(Notification.ADD, null, clone);
+ model.getItem().eNotify(notification);
+
+ iter = model.getAnnotationIterator();
+ assertEquals(1, getCount(iter));
+
+ IComment newComment = generateComment("Actually, maybe it needs more work.", new Date());
+ notification = new NotificationImpl(Notification.ADD, null, newComment);
+ model.getItem().eNotify(notification);
+
+ iter = model.getAnnotationIterator();
+ assertEquals(2, getCount(iter));
+ }
+
+ private int getCount(Iterator iter) {
+ int count = 0;
+ while (iter.hasNext()) {
+ count++;
+ iter.next();
+ }
+ return count;
+ }
+
+ private IUser generateUser() {
+ IUser user = IReviewsFactory.INSTANCE.createUser();
+ user.setId("1");
+ user.setDisplayName("Leo Dos Santos");
+ user.setEmail("leo@testkop.com");
+ return user;
+ }
+
+ private IComment generateComment(String text, Date date) {
+ IComment comment = IReviewsFactory.INSTANCE.createComment();
+ comment.getLocations().add(generateLocation());
+ comment.setDescription(text);
+ comment.setCreationDate(date);
+ comment.setDraft(false);
+ comment.setId("12345");
+ comment.setAuthor(generateUser());
+ comment.setItem(review);
+ return comment;
+ }
+
+ private ILocation generateLocation() {
+ ILineRange range = IReviewsFactory.INSTANCE.createLineRange();
+ range.setStart(0);
+ range.setEnd(10);
+ ILineLocation location = IReviewsFactory.INSTANCE.createLineLocation();
+ location.getRanges().add(range);
+ return location;
+ }
+
+}
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentAnnotation.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentAnnotation.java
index eeed026..4e5f41b 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentAnnotation.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentAnnotation.java
@@ -12,19 +12,21 @@
package org.eclipse.mylyn.internal.reviews.ui.annotations;
+import org.eclipse.emf.ecore.util.EcoreUtil;
import org.eclipse.jface.text.Position;
import org.eclipse.jface.text.source.Annotation;
import org.eclipse.mylyn.reviews.core.model.IComment;
+import org.eclipse.mylyn.reviews.core.spi.ModelUtil;
/**
- * Class to represent a comment in a Crucible review
+ * Class to represent a comment in a review
*
* @author Shawn Minto
* @author Steffen Pingel
*/
public class CommentAnnotation extends Annotation {
- public static final String COMMENT_ANNOTATION_ID = "org.eclipse.mylyn.reviews.ui.comment.Annotation";
+ public static final String COMMENT_ANNOTATION_ID = "org.eclipse.mylyn.reviews.ui.comment.Annotation"; //$NON-NLS-1$
private final Position position;
@@ -51,11 +53,8 @@ public class CommentAnnotation extends Annotation {
@Override
public int hashCode() {
- final int prime = 31;
- int result = 1;
- result = prime * result + ((comment == null) ? 0 : comment.hashCode());
- result = prime * result + ((position == null) ? 0 : position.hashCode());
- return result;
+ int result = ((position == null) ? 0 : position.hashCode());
+ return ModelUtil.ecoreHash(result, comment);
}
@Override
@@ -66,17 +65,10 @@ public class CommentAnnotation extends Annotation {
if (obj == null) {
return false;
}
- if (!(obj instanceof CommentAnnotation)) {
- return false;
- }
- final CommentAnnotation other = (CommentAnnotation) obj;
- if (comment == null) {
- if (other.comment != null) {
- return false;
- }
- } else if (!comment.equals(other.comment)) {
+ if (getClass() != obj.getClass()) {
return false;
}
+ CommentAnnotation other = (CommentAnnotation) obj;
if (position == null) {
if (other.position != null) {
return false;
@@ -84,7 +76,9 @@ public class CommentAnnotation extends Annotation {
} else if (!position.equals(other.position)) {
return false;
}
- return true;
+ if (comment != null && other.comment != null && comment.getId() != null && other.comment.getId() != null) {
+ return comment.getId().equals(other.comment.getId());
+ }
+ return EcoreUtil.equals(comment, other.comment);
}
-
}
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
index baebb6a..f437a96 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
@@ -15,6 +15,7 @@ package org.eclipse.mylyn.internal.reviews.ui.annotations;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
@@ -51,7 +52,7 @@ public class ReviewAnnotationModel implements IAnnotationModel {
private final Set<IAnnotationModelListener> annotationModelListeners = new HashSet<IAnnotationModelListener>(2);
- private final List<CommentAnnotation> annotations = new ArrayList<CommentAnnotation>();
+ private final Set<CommentAnnotation> annotations = new LinkedHashSet<CommentAnnotation>();
private ReviewBehavior behavior;
@@ -229,8 +230,9 @@ public class ReviewAnnotationModel implements IAnnotationModel {
}
length = Math.max(1, length);
CommentAnnotation ca = new CommentAnnotation(offset, length, comment);
- annotations.add(ca);
- event.annotationAdded(ca);
+ if (annotations.add(ca)) {
+ event.annotationAdded(ca);
+ }
} catch (BadLocationException e) {
StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Unable to add annotation.",
e));