Add JSON view for blame Similar to how we use SoyMapData/SoyListData directly to avoid unnecessary allocation in the HTML path, use a custom type adapter for JSON serialization rather than converting objects needlessly. Ranges are specified as (start, count) using 1-based numbering, which matches "git blame" and "diff -u" (but not JGit). Change-Id: I4b359cc4b8a4e26c2bfcabfa65c6083829518756
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitJsonData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitJsonData.java index 39d9be4..20cb9cd 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitJsonData.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitJsonData.java
@@ -33,10 +33,16 @@ import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; -class CommitJsonData { +public class CommitJsonData { static final ImmutableSet<Field> DEFAULT_FIELDS = Sets.immutableEnumSet( Field.SHA, Field.TREE, Field.PARENTS, Field.AUTHOR, Field.COMMITTER, Field.MESSAGE); + public static class Ident { + public String name; + public String email; + public String time; + } + static class Commit { String commit; String tree; @@ -48,12 +54,6 @@ List<Diff> treeDiff; } - static class Ident { - String name; - String email; - String time; - } - /** @see DiffEntry */ static class Diff { String type;
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/blame/BlameServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/blame/BlameServlet.java index 04e86a8..f59ba04 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/blame/BlameServlet.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/blame/BlameServlet.java
@@ -31,6 +31,8 @@ import com.google.gitiles.GitilesView; import com.google.gitiles.Renderer; import com.google.gitiles.ViewFilter; +import com.google.gson.GsonBuilder; +import com.google.gson.reflect.TypeToken; import com.google.template.soy.data.SoyListData; import com.google.template.soy.data.SoyMapData; @@ -75,43 +77,21 @@ RevWalk rw = new RevWalk(repo); try { GitilesAccess access = getAccess(req); - RevCommit currCommit = rw.parseCommit(view.getRevision().getId()); - ObjectId currCommitBlobId = resolveBlob(view, rw, currCommit); - if (currCommitBlobId == null) { - res.setStatus(SC_NOT_FOUND); + RegionResult result = getRegions(view, access, repo, rw, res); + if (result == null) { return; } - ObjectId lastCommit = cache.findLastCommit(repo, currCommit, view.getPathPart()); - ObjectId lastCommitBlobId = resolveBlob(view, rw, lastCommit); - - if (!Objects.equal(currCommitBlobId, lastCommitBlobId)) { - log.warn(String.format("Blob %s in last modified commit %s for repo %s starting from %s" - + " does not match original blob %s", - ObjectId.toString(lastCommitBlobId), - ObjectId.toString(lastCommit), - access.getRepositoryName(), - ObjectId.toString(currCommit), - ObjectId.toString(currCommitBlobId))); - lastCommitBlobId = currCommitBlobId; - lastCommit = currCommit; - } - String title = "Blame - " + view.getPathPart(); Map<String, ?> blobData = new BlobSoyData(rw.getObjectReader(), view) - .toSoyData(view.getPathPart(), lastCommitBlobId); + .toSoyData(view.getPathPart(), result.blobId); if (blobData.get("lines") != null) { - List<Region> regions = cache.get(repo, lastCommit, view.getPathPart()); - if (regions.isEmpty()) { - res.setStatus(SC_NOT_FOUND); - return; - } DateFormatter df = new DateFormatter(access, Format.ISO); renderHtml(req, res, "gitiles.blameDetail", ImmutableMap.of( "title", title, "breadcrumbs", view.getBreadcrumbs(), "data", blobData, - "regions", toSoyData(view, rw.getObjectReader(), regions, df))); + "regions", toSoyData(view, rw.getObjectReader(), result.regions, df))); } else { renderHtml(req, res, "gitiles.blameDetail", ImmutableMap.of( "title", title, @@ -123,6 +103,80 @@ } } + @Override + protected void doGetJson(HttpServletRequest req, HttpServletResponse res) throws IOException { + GitilesView view = ViewFilter.getView(req); + Repository repo = ServletUtils.getRepository(req); + + RevWalk rw = new RevWalk(repo); + try { + RegionResult result = getRegions(view, getAccess(req), repo, rw, res); + if (result == null) { + return; + } + // Output from BlameCache is 0-based for lines. We convert to 1-based for + // JSON output later (in RegionAdapter); here we're just filling in the + // transient fields. + int start = 0; + for (Region r : result.regions) { + r.setStart(start); + start += r.getCount(); + } + renderJson(req, res, ImmutableMap.of("regions", result.regions), + new TypeToken<Map<String, List<Region>>>() {}.getType()); + } finally { + rw.release(); + } + } + + @Override + protected GsonBuilder newGsonBuilder(HttpServletRequest req) throws IOException { + return super.newGsonBuilder(req).registerTypeAdapter(Region.class, + new RegionAdapter(new DateFormatter(getAccess(req), Format.ISO))); + } + + private static class RegionResult { + private final List<Region> regions; + private final ObjectId blobId; + + private RegionResult(List<Region> regions, ObjectId blobId) { + this.regions = regions; + this.blobId = blobId; + } + } + + private RegionResult getRegions(GitilesView view, GitilesAccess access, Repository repo, + RevWalk rw, HttpServletResponse res) throws IOException { + RevCommit currCommit = rw.parseCommit(view.getRevision().getId()); + ObjectId currCommitBlobId = resolveBlob(view, rw, currCommit); + if (currCommitBlobId == null) { + res.setStatus(SC_NOT_FOUND); + return null; + } + + ObjectId lastCommit = cache.findLastCommit(repo, currCommit, view.getPathPart()); + ObjectId lastCommitBlobId = resolveBlob(view, rw, lastCommit); + + if (!Objects.equal(currCommitBlobId, lastCommitBlobId)) { + log.warn(String.format("Blob %s in last modified commit %s for repo %s starting from %s" + + " does not match original blob %s", + ObjectId.toString(lastCommitBlobId), + ObjectId.toString(lastCommit), + access.getRepositoryName(), + ObjectId.toString(currCommit), + ObjectId.toString(currCommitBlobId))); + lastCommitBlobId = currCommitBlobId; + lastCommit = currCommit; + } + + List<Region> regions = cache.get(repo, lastCommit, view.getPathPart()); + if (regions.isEmpty()) { + res.setStatus(SC_NOT_FOUND); + return null; + } + return new RegionResult(regions, lastCommitBlobId); + } + private static ObjectId resolveBlob(GitilesView view, RevWalk rw, ObjectId commitId) throws IOException { try {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/blame/Region.java b/gitiles-servlet/src/main/java/com/google/gitiles/blame/Region.java index 13a3dd3..09a39d6 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/blame/Region.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/blame/Region.java
@@ -42,6 +42,10 @@ this.count = end - start; } + void setStart(int start) { + this.start = start; + } + int getStart() { return start; }
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/blame/RegionAdapter.java b/gitiles-servlet/src/main/java/com/google/gitiles/blame/RegionAdapter.java new file mode 100644 index 0000000..a9710ae --- /dev/null +++ b/gitiles-servlet/src/main/java/com/google/gitiles/blame/RegionAdapter.java
@@ -0,0 +1,55 @@ +// 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.blame; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.gitiles.DateFormatter; +import com.google.gson.TypeAdapter; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; + +import org.eclipse.jgit.lib.ObjectId; + +import java.io.IOException; + +class RegionAdapter extends TypeAdapter<Region> { + private final DateFormatter df; + + RegionAdapter(DateFormatter df) { + this.df = checkNotNull(df, "DateFormatter"); + } + + @Override + public void write(JsonWriter out, Region value) throws IOException { + out.beginObject() + .name("start").value(value.getStart() + 1) + .name("count").value(value.getCount()) + .name("path").value(value.getSourcePath()) + .name("commit").value(ObjectId.toString(value.getSourceCommit())) + .name("author").beginObject() + // TODO(dborowitz): Use an adapter from CommitJsonData instead. + .name("name").value(value.getSourceAuthor().getName()) + .name("email").value(value.getSourceAuthor().getEmailAddress()) + .name("time").value(df.format(value.getSourceAuthor())) + .endObject() + .endObject(); + } + + @Override + public Region read(JsonReader in) throws IOException { + throw new UnsupportedOperationException(); + } +}
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/blame/BlameServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/blame/BlameServletTest.java new file mode 100644 index 0000000..19229e9 --- /dev/null +++ b/gitiles-servlet/src/test/java/com/google/gitiles/blame/BlameServletTest.java
@@ -0,0 +1,103 @@ +// 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.blame; + +import static org.junit.Assert.assertEquals; + +import com.google.common.collect.Iterables; +import com.google.gitiles.CommitJsonData.Ident; +import com.google.gitiles.FakeHttpServletRequest; +import com.google.gitiles.FakeHttpServletResponse; +import com.google.gitiles.GitilesServlet; +import com.google.gitiles.TestGitilesServlet; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; + +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.revwalk.RevCommit; +import org.junit.Before; +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +public class BlameServletTest { + private static class RegionJsonData { + int start; + int count; + String path; + String commit; + Ident author; + } + + 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 blameJson() 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().tick(10).parent(c1).add("foo", contents2)); + + Map<String, List<RegionJsonData>> result = getBlameJson("/test/+blame/" + c2.name() + "/foo"); + assertEquals("regions", Iterables.getOnlyElement(result.keySet())); + List<RegionJsonData> regions = result.get("regions"); + assertEquals(2, regions.size()); + + RegionJsonData r1 = regions.get(0); + assertEquals(1, r1.start); + assertEquals(1, r1.count); + assertEquals("foo", r1.path); + assertEquals(c1.name(), r1.commit); + assertEquals("J. Author", r1.author.name); + assertEquals("[email protected]", r1.author.email); + assertEquals("2009-03-13 17:29:48 -0330", r1.author.time); + + RegionJsonData r2 = regions.get(1); + assertEquals(2, r2.start); + assertEquals(1, r2.count); + assertEquals("foo", r2.path); + assertEquals(c2.name(), r2.commit); + assertEquals("J. Author", r2.author.name); + assertEquals("[email protected]", r2.author.email); + assertEquals("2009-03-13 17:29:58 -0330", r2.author.time); + } + + private Map<String, List<RegionJsonData>> getBlameJson(String path) throws Exception { + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo(path); + req.setQueryString("format=JSON"); + + FakeHttpServletResponse res = new FakeHttpServletResponse(); + servlet.service(req, res); + assertEquals(200, res.getStatus()); + String body = res.getActualBodyString(); + String magic = ")]}'\n"; + assertEquals(magic, body.substring(0, magic.length())); + return new Gson().fromJson(body.substring(magic.length()), + new TypeToken<Map<String, List<RegionJsonData>>>() {}.getType()); + } +}