Add text view for diff Rather than define a complex JSON format for representing diffs (or, more likely, copying Gerrit Code Review), use the standard textual diff format, base64-encoded to avoid decoding by browsers. Tools already exist to parse this output. Bug: 55 Change-Id: I985ca6e70488ce1ea426b8c9773e57c11567d8a6
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java index 4e2c4ba..5b091f2 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java
@@ -18,6 +18,7 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.base.Charsets; +import com.google.common.io.BaseEncoding; import com.google.gitiles.CommitData.Field; import com.google.gitiles.DateFormatter.Format; @@ -38,6 +39,7 @@ import java.io.IOException; import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.util.Arrays; import java.util.Map; import java.util.Set; @@ -58,7 +60,7 @@ } @Override - protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException { + protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { GitilesView view = ViewFilter.getView(req); Repository repo = ServletUtils.getRepository(req); @@ -115,7 +117,8 @@ try (OutputStream out = res.getOutputStream()) { out.write(html[0].getBytes(Charsets.UTF_8)); - formatHtmlDiff(out, view, repo, oldTree, newTree, view.getPathPart()); + DiffFormatter diff = new HtmlDiffFormatter(renderer, view, out); + formatDiff(repo, oldTree, newTree, view.getPathPart(), diff); out.write(html[1].getBytes(Charsets.UTF_8)); } } finally { @@ -126,6 +129,33 @@ } } + @Override + protected void doGetText(HttpServletRequest req, HttpServletResponse res) + throws IOException { + GitilesView view = ViewFilter.getView(req); + Repository repo = ServletUtils.getRepository(req); + + RevWalk walk = new RevWalk(repo); + try { + AbstractTreeIterator oldTree; + AbstractTreeIterator newTree; + try { + oldTree = getTreeIterator(walk, view.getOldRevision().getId()); + newTree = getTreeIterator(walk, view.getRevision().getId()); + } catch (MissingObjectException | IncorrectObjectTypeException e) { + res.setStatus(SC_NOT_FOUND); + return; + } + + try (OutputStream out = BaseEncoding.base64() + .encodingStream(new OutputStreamWriter(res.getOutputStream()))) { + formatDiff(repo, oldTree, newTree, view.getPathPart(), new DiffFormatter(out)); + } + } finally { + walk.release(); + } + } + private static TreeWalk newTreeWalk(RevWalk walk, GitilesView view) throws IOException { if (view.getPathPart().isEmpty()) { return null; @@ -164,11 +194,8 @@ return new String[] {html.substring(0, lt), html.substring(gt + 1)}; } - private void formatHtmlDiff(OutputStream out, GitilesView view, - Repository repo, AbstractTreeIterator oldTree, - AbstractTreeIterator newTree, String path) - throws IOException { - DiffFormatter diff = new HtmlDiffFormatter(renderer, view, out); + private static void formatDiff(Repository repo, AbstractTreeIterator oldTree, + AbstractTreeIterator newTree, String path, DiffFormatter diff) throws IOException { try { if (!path.equals("")) { diff.setPathFilter(PathFilter.create(path));
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/DiffServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/DiffServletTest.java new file mode 100644 index 0000000..92a52ce --- /dev/null +++ b/gitiles-servlet/src/test/java/com/google/gitiles/DiffServletTest.java
@@ -0,0 +1,126 @@ +// Copyright (C) 2014 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gitiles; + +import static com.google.common.collect.Iterables.getOnlyElement; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; + +import com.google.common.collect.ImmutableList; +import com.google.common.io.BaseEncoding; + +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +import org.eclipse.jgit.diff.DiffEntry.Side; +import org.eclipse.jgit.diff.Edit; +import org.eclipse.jgit.diff.Edit.Type; +import org.eclipse.jgit.diff.RawText; +import org.eclipse.jgit.internal.storage.dfs.DfsRepository; +import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; +import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.patch.FileHeader; +import org.eclipse.jgit.patch.Patch; +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Before; +import org.junit.Test; + +public class DiffServletTest { + private TestRepository<DfsRepository> repo; + private GitilesServlet servlet; + + @Before + public void setUp() throws Exception { + DfsRepository r = new InMemoryRepository(new DfsRepositoryDescription("test")); + repo = new TestRepository<DfsRepository>(r); + servlet = TestGitilesServlet.create(repo); + } + + @Test + public void diffFileNoParents() throws Exception { + String contents = "foo\ncontents\n"; + RevCommit c = repo.update("master", repo.commit().add("foo", contents)); + + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo("/test/+diff/" + c.name() + "^!/foo"); + req.setQueryString("format=TEXT"); + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + + Patch p = parsePatch(res.getActualBody()); + FileHeader f = getOnlyElement(p.getFiles()); + assertEquals(ChangeType.ADD, f.getChangeType()); + assertEquals(DiffEntry.DEV_NULL, f.getPath(Side.OLD)); + assertEquals("foo", f.getPath(Side.NEW)); + + RawText rt = new RawText(contents.getBytes(UTF_8)); + Edit e = getOnlyElement(getOnlyElement(f.getHunks()).toEditList()); + assertEquals(Type.INSERT, e.getType()); + assertEquals(contents, rt.getString(e.getBeginB(), e.getEndB(), false)); + } + + @Test + public void diffFileOneParent() throws Exception { + String contents1 = "foo\n"; + String contents2 = "foo\ncontents\n"; + RevCommit c1 = repo.update("master", repo.commit().add("foo", contents1)); + RevCommit c2 = repo.update("master", repo.commit().parent(c1).add("foo", contents2)); + + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo("/test/+diff/" + c2.name() + "^!/foo"); + req.setQueryString("format=TEXT"); + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + + Patch p = parsePatch(res.getActualBody()); + FileHeader f = getOnlyElement(p.getFiles()); + assertEquals(ChangeType.MODIFY, f.getChangeType()); + assertEquals("foo", f.getPath(Side.OLD)); + assertEquals("foo", f.getPath(Side.NEW)); + + RawText rt2 = new RawText(contents2.getBytes(UTF_8)); + Edit e = getOnlyElement(getOnlyElement(f.getHunks()).toEditList()); + assertEquals(Type.INSERT, e.getType()); + assertEquals("contents\n", rt2.getString(e.getBeginB(), e.getEndB(), false)); + } + + @Test + public void diffDirectory() throws Exception { + String contents = "contents\n"; + RevCommit c = repo.update("master", repo.commit() + .add("dir/foo", contents) + .add("dir/bar", contents) + .add("baz", contents)); + + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo("/test/+diff/" + c.name() + "^!/dir"); + req.setQueryString("format=TEXT"); + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + + Patch p = parsePatch(res.getActualBody()); + assertEquals(2, p.getFiles().size()); + assertEquals("dir/bar", p.getFiles().get(0).getPath(Side.NEW)); + assertEquals("dir/foo", p.getFiles().get(1).getPath(Side.NEW)); + } + + private static Patch parsePatch(byte[] enc) { + byte[] buf = BaseEncoding.base64().decode(new String(enc, UTF_8)); + Patch p = new Patch(); + p.parse(buf, 0, buf.length); + assertEquals(ImmutableList.of(), p.getErrors()); + return p; + } +}