summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSam Davis2014-04-11 18:12:36 (EDT)
committerSam Davis2014-05-06 19:48:08 (EDT)
commitabeb2c68826c25d1ff50012367e56872f1be919b (patch)
treef543e4aa3d942bd5a846f96ee9d08f7412c84a99
parentc42f66f1037c9b26205df992cf1049298f9a2ba9 (diff)
downloadorg.eclipse.mylyn.reviews-abeb2c68826c25d1ff50012367e56872f1be919b.zip
org.eclipse.mylyn.reviews-abeb2c68826c25d1ff50012367e56872f1be919b.tar.gz
org.eclipse.mylyn.reviews-abeb2c68826c25d1ff50012367e56872f1be919b.tar.bz2
428477: improve image comparison support and add testsrefs/changes/56/25356/7
Change-Id: I0b4f191ba3a26b1f9bb01b51e000817572d4220e Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=428477 Signed-off-by: Sam Davis <sam.davis@tasktop.com>
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java25
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java10
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritProject.java13
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java145
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java9
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.gifbin0 -> 1011 bytes
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.pngbin0 -> 841 bytes
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit2.pngbin0 -> 610 bytes
8 files changed, 193 insertions, 9 deletions
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
index d29215b..b21fe54 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
@@ -410,11 +410,11 @@ public class GerritClient extends ReviewsClient {
});
if (patchScript.isBinary()) {
if (patchScript.getChangeType() != ChangeType.ADDED) {
- String keyBaseEncoded = encode(key.toString() + "^1"); //$NON-NLS-1$
+ String keyBaseEncoded = encode(leftId + "," + key.getFileName() + "^0"); //$NON-NLS-1$ //$NON-NLS-2$
patchScript.setBinaryA(fetchBinaryContent("/cat/" + keyBaseEncoded, monitor)); //$NON-NLS-1$
}
if (patchScript.getChangeType() != ChangeType.DELETED) {
- String keyTargetEncoded = encode(key.toString() + "^0"); //$NON-NLS-1$
+ String keyTargetEncoded = encode(rightId + "," + key.getFileName() + "^0"); //$NON-NLS-1$ //$NON-NLS-2$
patchScript.setBinaryB(fetchBinaryContent("/cat/" + keyTargetEncoded, monitor)); //$NON-NLS-1$
}
}
@@ -425,15 +425,26 @@ public class GerritClient extends ReviewsClient {
final TypeToken<Byte[]> byteArrayType = new TypeToken<Byte[]>() {
};
byte[] bin = executeGetRestRequest(url, byteArrayType.getType(), monitor);
- if (isVersion27OrLater(monitor)) {
- return bin;
- } else if (isVersion26OrLater(monitor)) {
+ if (isZippedContent(bin)) {
return unzip(bin);
+ } else {
+ return bin;
}
- return null;
}
- private static byte[] unzip(byte[] zip) throws GerritException {
+ /**
+ * Checks for the 4 byte header that identifies a ZIP file
+ *
+ * @noreference This method is not intended to be referenced by clients.
+ */
+ public static boolean isZippedContent(byte[] bin) {
+ return bin != null && bin.length > 4 && bin[0] == 'P' && bin[1] == 'K' && bin[2] == 3 && bin[3] == 4;
+ }
+
+ /**
+ * @noreference This method is not intended to be referenced by clients.
+ */
+ public static byte[] unzip(byte[] zip) throws GerritException {
ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zip));
try {
zis.getNextEntry(); // expecting a single entry
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
index 3901147..04c75c5 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/GerritClientTest.java
@@ -303,4 +303,14 @@ public class GerritClientTest extends TestCase {
assertEquals(reviewId, client.toReviewId(changeId, null));
}
+ @Test
+ public void testIsZippedContent() throws Exception {
+ assertTrue(GerritClient.isZippedContent("PK\u0003\u0004somezippedcontent".getBytes()));
+ assertFalse(GerritClient.isZippedContent("PK\u0003notzippedcontent".getBytes()));
+ assertFalse(GerritClient.isZippedContent("PKnotzippedcontent".getBytes()));
+ assertFalse(GerritClient.isZippedContent("notzippedcontent".getBytes()));
+ assertFalse(GerritClient.isZippedContent("PK".getBytes()));
+ assertFalse(GerritClient.isZippedContent("".getBytes()));
+ }
+
}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritProject.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritProject.java
index a79cf68..8cb4ea7 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritProject.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/support/GerritProject.java
@@ -115,7 +115,18 @@ public class GerritProject {
public void addFile(String fileName, String text) throws Exception {
Git gitProject = getGitProject();
CommonTestUtil.write(new File(getFolder(), fileName).getAbsolutePath(), new StringBuffer(text));
- gitProject.add().addFilepattern(".").call();
+ gitProject.add().addFilepattern(fileName).call();
+ }
+
+ public void addFile(String fileName, File file) throws Exception {
+ Git gitProject = getGitProject();
+ CommonTestUtil.copy(file, new File(getFolder(), fileName));
+ gitProject.add().addFilepattern(fileName).call();
+ }
+
+ public void removeFile(String fileName) throws Exception {
+ Git gitProject = getGitProject();
+ gitProject.rm().addFilepattern(fileName).call();
}
public CommitResult commitAndPush(CommitCommand command) throws Exception {
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
index 50bebd1..4ad57fa 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
@@ -13,31 +13,47 @@ package org.eclipse.mylyn.internal.gerrit.core.remote;
import static org.eclipse.mylyn.internal.gerrit.core.remote.TestRemoteObserverConsumer.retrieveForLocalKey;
import static org.eclipse.mylyn.internal.gerrit.core.remote.TestRemoteObserverConsumer.retrieveForRemoteKey;
+import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
import java.util.Collections;
import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jgit.api.CommitCommand;
+import org.eclipse.mylyn.commons.sdk.util.CommonTestUtil;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritChange;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritClient;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
import org.eclipse.mylyn.internal.gerrit.core.client.PatchSetContent;
+import org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchScriptX;
import org.eclipse.mylyn.reviews.core.model.IComment;
import org.eclipse.mylyn.reviews.core.model.IFileItem;
import org.eclipse.mylyn.reviews.core.model.IFileVersion;
import org.eclipse.mylyn.reviews.core.model.IReview;
import org.eclipse.mylyn.reviews.core.model.IReviewItem;
import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
+import org.hamcrest.Matcher;
import org.junit.Test;
import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.reviewdb.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.Patch;
+import com.google.gerrit.reviewdb.Patch.Key;
import com.google.gerrit.reviewdb.PatchSet;
/**
@@ -105,6 +121,134 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
}
@Test
+ public void testFetchBinaryContent() throws Exception {
+ fetchBinaryContent("test.png", "testdata/binary/gerrit.png");
+ }
+
+ @Test
+ public void testFetchZippedBinaryContent() throws Exception {
+ // test servers are configured so that gif files are zipped (the mimetype is not marked safe)
+ fetchBinaryContent("test.gif", "testdata/binary/gerrit.gif");
+ }
+
+ private void fetchBinaryContent(String fileName, String path) throws Exception {
+ byte[] fileContent = commitFile(fileName, path);
+
+ assertThat(getReview().getSets().size(), is(2));
+
+ PatchSetDetail detail = retrievePatchSetDetail(Integer.toString(2));
+ assertThat(detail.getInfo().getKey().get(), is(2));
+
+ IReviewItemSet patchSet = getReview().getSets().get(1);
+ List<IFileItem> fileItems = patchSet.getItems();
+ assertThat(fileItems.size(), is(0));
+ retrievePatchSetContents(patchSet);
+ assertThat(fileItems.size(), is(3));
+
+ IFileItem fileItem = fileItems.get(1);
+ assertThat(fileItem.getName(), is(fileName));
+ assertThat(fileItem.getBase().getContent(), nullValue());
+ assertThat(fileItem.getBase().getBinaryContent(), nullValue());
+ assertThat(fileItem.getTarget().getContent(), nullValue());
+ assertThat(fileItem.getTarget().getBinaryContent(), is(fileContent));
+ }
+
+ @Test
+ public void testUnzipBinaryContent() throws Exception {
+ File imageFile = CommonTestUtil.getFile(this, "testdata/binary/gerrit.png");
+ byte[] zippedBytes = zip(imageFile);
+ assertThat(GerritClient.isZippedContent(zippedBytes), is(true));
+
+ byte[] unzippedBytes = GerritClient.unzip(zippedBytes);
+ byte[] imageBytes = FileUtils.readFileToByteArray(imageFile);
+ assertThat(GerritClient.isZippedContent(imageBytes), is(false));
+ assertThat(unzippedBytes, is(imageBytes));
+ }
+
+ private byte[] zip(File file) throws IOException, FileNotFoundException {
+ ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ ZipOutputStream out = new ZipOutputStream(bytes);
+ out.putNextEntry(new ZipEntry(file.getName()));
+ IOUtils.copy(new FileInputStream(file), out);
+ out.closeEntry();
+ return bytes.toByteArray();
+ }
+
+ @Test
+ public void testCompareBinaryContent() throws Exception {
+ String fileName = "test.png";
+ byte[] fileContent2 = commitFile(fileName, "testdata/binary/gerrit.png");
+ byte[] fileContent3 = commitFile(fileName, "testdata/binary/gerrit2.png");
+ removeFile(fileName);
+
+ PatchSetDetail detail1 = retrievePatchSetDetail("1");
+ PatchSetDetail detail2 = retrievePatchSetDetail("2");
+ PatchSetDetail detail3 = retrievePatchSetDetail("3");
+ PatchSetDetail detail4 = retrievePatchSetDetail("4");
+
+ // compare modified images
+ PatchScriptX patchScript = loadPatchSetContent(fileName, detail2, detail3);
+ assertPatchContent(patchScript, equalTo(fileContent2), equalTo(fileContent3));
+
+ patchScript = loadPatchSetContent(fileName, detail3, detail2);
+ assertPatchContent(patchScript, equalTo(fileContent3), equalTo(fileContent2));
+
+ // compare deleted image
+ patchScript = loadPatchSetContent(fileName, detail2, detail4);
+ assertPatchContent(patchScript, equalTo(fileContent2), empty());
+
+ patchScript = loadPatchSetContent(fileName, detail3, detail1);
+ assertPatchContent(patchScript, equalTo(fileContent3), empty());
+
+ // compare added image
+ patchScript = loadPatchSetContent(fileName, detail1, detail2);
+ assertPatchContent(patchScript, empty(), equalTo(fileContent2));
+
+ patchScript = loadPatchSetContent(fileName, detail4, detail3);
+ assertPatchContent(patchScript, empty(), equalTo(fileContent3));
+ }
+
+ private PatchScriptX loadPatchSetContent(String fileName, PatchSetDetail base, PatchSetDetail target)
+ throws GerritException {
+ PatchSetContent content = new PatchSetContent(base.getPatchSet(), target.getPatchSet());
+ reviewHarness.client.loadPatchSetContent(content, new NullProgressMonitor());
+ return content.getPatchScript(createPatchKey(fileName, target.getInfo().getKey().get()));
+ }
+
+ private Key createPatchKey(String fileName, int patchSet) throws GerritException {
+ Change.Id changeId = new Change.Id(reviewHarness.client.id(reviewHarness.shortId));
+ PatchSet.Id patchSetId = new PatchSet.Id(changeId, patchSet);
+ return new Patch.Key(patchSetId, fileName);
+ }
+
+ private void assertPatchContent(PatchScriptX patchScript, Matcher<byte[]> base, Matcher<byte[]> target) {
+ assertThat(patchScript.getBinaryA(), is(base));
+ assertThat(patchScript.getBinaryB(), is(target));
+ }
+
+ private static Matcher<byte[]> empty() {
+ return nullValue(byte[].class);
+ }
+
+ private byte[] commitFile(String fileName, String path) throws Exception {
+ File file = CommonTestUtil.getFile(this, path);
+ CommitCommand command = reviewHarness.createCommitCommand();
+ reviewHarness.addFile(fileName, file);
+ reviewHarness.commitAndPush(command);
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse();
+ return FileUtils.readFileToByteArray(file);
+ }
+
+ private void removeFile(String fileName) throws Exception {
+ CommitCommand command = reviewHarness.createCommitCommand();
+ reviewHarness.removeFile(fileName);
+ reviewHarness.commitAndPush(command);
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse();
+ }
+
+ @Test
public void testPatchSetComments() throws Exception {
CommitCommand command2 = reviewHarness.createCommitCommand();
reviewHarness.addFile("testComments.txt", "line1\nline2\nline3\nline4\nline5\nline6\nline7\n");
@@ -159,7 +303,6 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
assertThat(details.size(), is(1));
PatchSetDetail detail = details.get(0);
PatchSetContent content = new PatchSetContent((PatchSet) null, detail);
- assertThat(content, notNullValue());
final Change.Id changeId = new Change.Id(reviewHarness.client.id(reviewHarness.shortId));
final PatchSet.Id patchSetId = new PatchSet.Id(changeId, 1);
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
index 607f4bd..b80d231 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/ReviewHarness.java
@@ -20,6 +20,7 @@ import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThat;
+import java.io.File;
import java.util.Date;
import org.apache.commons.lang.StringUtils;
@@ -149,6 +150,14 @@ class ReviewHarness {
gerritHarness.project().addFile(fileName, text);
}
+ public void addFile(String fileName, File file) throws Exception {
+ gerritHarness.project().addFile(fileName, file);
+ }
+
+ public void removeFile(String fileName) throws Exception {
+ gerritHarness.project().removeFile(fileName);
+ }
+
public CommitResult commitAndPush(CommitCommand command) throws Exception {
return gerritHarness.project().commitAndPush(command);
}
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.gif b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.gif
new file mode 100644
index 0000000..d635353
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.gif
Binary files differ
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.png b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.png
new file mode 100644
index 0000000..9d021f8
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit.png
Binary files differ
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit2.png b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit2.png
new file mode 100644
index 0000000..83245e2
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/binary/gerrit2.png
Binary files differ