Add parent parameter to GetContent

PolyGerrit has a hard time showing the diff view for binary files like
images. To get the base, it has to request it separately using the
parent commit SHA1, which is expensive.

This commit adds a 'parent' parameter to GetContent in the same way that
it is done for DownloadContent. This way, PolyGerrit can use the same
endpoint and avoid expensive permission checks since users who can see a
change are also allowed to see the base commit.

Change-Id: Id6b4d0ea994282d9b4942f9ec1a8f4071a1064b1

Documentation/rest-api-changes.txt

@@ -4636,6 +4636,11 @@ is the 1-based index of the parent's position in the commit object.
 
 Gets the content of a file from a certain revision.
 
+The optional, integer-valued `parent` parameter can be specified to request
+the named file from a parent commit of the specified revision. The value is
+the 1-based index of the parent's position in the commit object. If the
+parameter is omitted or the value is non-positive, the patch set is referenced.
+
 .Request
 ----
   GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/files/gerrit-server%2Fsrc%2Fmain%2Fjava%2Fcom%2Fgoogle%2Fgerrit%2Fserver%2Fproject%2FRefControl.java/content HTTP/1.0

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -21,6 +21,7 @@ import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.N
 import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
 import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.HEAD;
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
@@ -110,6 +111,7 @@ import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -1237,4 +1239,18 @@ public abstract class AbstractDaemonTest {
     projectsToWatch.add(pwi);
     gApi.accounts().self().setWatchedProjects(projectsToWatch);
   }
+
+  protected void assertContent(PushOneCommit.Result pushResult, String path, String expectedContent)
+      throws Exception {
+    BinaryResult bin =
+        gApi.changes()
+            .id(pushResult.getChangeId())
+            .revision(pushResult.getCommit().name())
+            .file(path)
+            .content();
+    ByteArrayOutputStream os = new ByteArrayOutputStream();
+    bin.writeTo(os);
+    String res = new String(os.toByteArray(), UTF_8);
+    assertThat(res).isEqualTo(expectedContent);
+  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -1062,20 +1062,6 @@ public class RevisionIT extends AbstractDaemonTest {
     return eTag;
   }
 
-  private void assertContent(PushOneCommit.Result pushResult, String path, String expectedContent)
-      throws Exception {
-    BinaryResult bin =
-        gApi.changes()
-            .id(pushResult.getChangeId())
-            .revision(pushResult.getCommit().name())
-            .file(path)
-            .content();
-    ByteArrayOutputStream os = new ByteArrayOutputStream();
-    bin.writeTo(os);
-    String res = new String(os.toByteArray(), UTF_8);
-    assertThat(res).isEqualTo(expectedContent);
-  }
-
   private void assertDiffForNewFile(
       PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
     DiffInfo diff =

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/BUILD

@@ -0,0 +1,7 @@
+load("//gerrit-acceptance-tests:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+    srcs = glob(["*IT.java"]),
+    group = "rest_revision",
+    labels = ["rest"],
+)

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/revision/RevisionIT.java

@@ -0,0 +1,78 @@
+// Copyright (C) 2017 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.gerrit.acceptance.rest.revision;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.PushOneCommit.FILE_CONTENT;
+import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import org.eclipse.jgit.util.Base64;
+import org.junit.Test;
+
+public class RevisionIT extends AbstractDaemonTest {
+  @Test
+  public void contentOfParent() throws Exception {
+    String parentContent = "parent content";
+    PushOneCommit.Result parent = createChange("Parent change", FILE_NAME, parentContent);
+    parent.assertOkStatus();
+
+    gApi.changes().id(parent.getChangeId()).current().review(ReviewInput.approve());
+    gApi.changes().id(parent.getChangeId()).current().submit();
+
+    PushOneCommit.Result child = createChange("Child change", FILE_NAME, FILE_CONTENT);
+    child.assertOkStatus();
+    assertContent(child, FILE_NAME, FILE_CONTENT);
+
+    RestResponse response =
+        adminRestSession.get(
+            "/changes/"
+                + child.getChangeId()
+                + "/revisions/current/files/"
+                + FILE_NAME
+                + "/content?parent=1");
+    response.assertOK();
+    assertThat(new String(Base64.decode(response.getEntityContent()), UTF_8))
+        .isEqualTo(parentContent);
+  }
+
+  @Test
+  public void contentOfInvalidParent() throws Exception {
+    String parentContent = "parent content";
+    PushOneCommit.Result parent = createChange("Parent change", FILE_NAME, parentContent);
+    parent.assertOkStatus();
+
+    gApi.changes().id(parent.getChangeId()).current().review(ReviewInput.approve());
+    gApi.changes().id(parent.getChangeId()).current().submit();
+
+    PushOneCommit.Result child = createChange("Child change", FILE_NAME, FILE_CONTENT);
+    child.assertOkStatus();
+    assertContent(child, FILE_NAME, FILE_CONTENT);
+
+    RestResponse response =
+        adminRestSession.get(
+            "/changes/"
+                + child.getChangeId()
+                + "/revisions/current/files/"
+                + FILE_NAME
+                + "/content?parent=10");
+    response.assertBadRequest();
+    assertThat(response.getEntityContent()).isEqualTo("invalid parent");
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java

@@ -397,8 +397,9 @@ public class ChangeEdits
                 base
                     ? ObjectId.fromString(edit.getBasePatchSet().getRevision().get())
                     : edit.getEditCommit(),
-                rsrc.getPath()));
+                rsrc.getPath(),
-      } catch (ResourceNotFoundException rnfe) {
+                null));
+      } catch (ResourceNotFoundException | BadRequestException e) {
         return Response.none();
       }
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java

@@ -22,6 +22,7 @@ import com.google.common.hash.Hashing;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.PatchScript.FileMode;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Patch;
@@ -67,9 +68,33 @@ public class FileContentUtil {
     this.registry = ftr;
   }
 
-  public BinaryResult getContent(ProjectState project, ObjectId revstr, String path)
+  /**
-      throws ResourceNotFoundException, IOException {
+   * Get the content of a file at a specific commit or one of it's parent commits.
-    try (Repository repo = openRepository(project)) {
+   *
+   * @param project A {@code Project} that this request refers to.
+   * @param revstr An {@code ObjectId} specifying the commit.
+   * @param path A string specifying the filepath.
+   * @param parent A 1-based parent index to get the content from instead. Null if the content
+   *     should be obtained from {@param revstr} instead.
+   * @return Content of the file as {@code BinaryResult}.
+   * @throws ResourceNotFoundException
+   * @throws IOException
+   */
+  public BinaryResult getContent(
+      ProjectState project, ObjectId revstr, String path, @Nullable Integer parent)
+      throws BadRequestException, ResourceNotFoundException, IOException {
+    try (Repository repo = openRepository(project);
+        RevWalk rw = new RevWalk(repo)) {
+      if (parent != null) {
+        RevCommit revCommit = rw.parseCommit(revstr);
+        if (revCommit == null) {
+          throw new ResourceNotFoundException("commit not found");
+        }
+        if (parent > revCommit.getParentCount()) {
+          throw new BadRequestException("invalid parent");
+        }
+        revstr = rw.parseCommit(revstr).getParent(Integer.max(0, parent - 1)).toObjectId();
+      }
       return getContent(repo, project, revstr, path);
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/GetContent.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
@@ -30,21 +31,23 @@ import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-import com.google.inject.Singleton;
 import java.io.IOException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.kohsuke.args4j.Option;
 
-@Singleton
 public class GetContent implements RestReadView<FileResource> {
   private final Provider<ReviewDb> db;
   private final GitRepositoryManager gitManager;
   private final PatchSetUtil psUtil;
   private final FileContentUtil fileContentUtil;
 
+  @Option(name = "--parent")
+  private Integer parent;
+
   @Inject
   GetContent(
       Provider<ReviewDb> db,
@@ -59,7 +62,7 @@ public class GetContent implements RestReadView<FileResource> {
 
   @Override
   public BinaryResult apply(FileResource rsrc)
-      throws ResourceNotFoundException, IOException, NoSuchChangeException, OrmException {
+      throws ResourceNotFoundException, IOException, BadRequestException, OrmException {
     String path = rsrc.getPatchKey().get();
     if (Patch.COMMIT_MSG.equals(path)) {
       String msg = getMessage(rsrc.getRevision().getChangeResource().getNotes());
@@ -75,7 +78,8 @@ public class GetContent implements RestReadView<FileResource> {
     return fileContentUtil.getContent(
         rsrc.getRevision().getControl().getProjectControl().getProjectState(),
         ObjectId.fromString(rsrc.getRevision().getPatchSet().getRevision().get()),
-        path);
+        path,
+        parent);
   }
 
   private String getMessage(ChangeNotes notes) throws OrmException, IOException {

gerrit-server/src/main/java/com/google/gerrit/server/project/GetContent.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.project;
 
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
@@ -32,8 +33,9 @@ public class GetContent implements RestReadView<FileResource> {
   }
 
   @Override
-  public BinaryResult apply(FileResource rsrc) throws ResourceNotFoundException, IOException {
+  public BinaryResult apply(FileResource rsrc)
+      throws ResourceNotFoundException, BadRequestException, IOException {
     return fileContentUtil.getContent(
-        rsrc.getProject().getProjectState(), rsrc.getRev(), rsrc.getPath());
+        rsrc.getProject().getProjectState(), rsrc.getRev(), rsrc.getPath(), null);
   }
 }