Merge changes from topic 'commit-footers'

* changes:
  ChangeJson: Add option to include review footers
  ChangeJson: Refactor revision codepath

Documentation/rest-api-changes.txt

@@ -310,6 +310,12 @@ default. Optional fields are:
 * `CHECK`: include potential problems with the change.
 --
 
+[[commit-footers]]
+--
+* `COMMIT_FOOTERS`: include the full commit message with
+  Gerrit-specific commit footers in the
+  link:#revision-info[RevisionInfo].
+
 .Request
 ----
   GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -4215,6 +4221,12 @@ entities.
 |`reviewed`     |optional|
 Indicates whether the caller is authenticated and has commented on the
 current revision. Only set if link:#reviewed[REVIEWED] option is requested.
+|`messageWithFooter` |optional|
+If the link:#commit-footers[COMMIT_FOOTERS] option is requested and
+this is the current patch set, contains the full commit message with
+Gerrit-specific commit footers, as if this revision were submitted
+using the link:project-configuration.html#cherry_pick[Cherry Pick]
+submit type.
 |===========================
 
 [[rule-input]]

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

@@ -42,6 +42,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
@@ -131,6 +132,10 @@ public abstract class AbstractDaemonTest {
   @Inject
   protected Provider<InternalChangeQuery> queryProvider;
 
+  @Inject
+  @CanonicalWebUrl
+  protected Provider<String> canonicalWebUrl;
+
   @Inject
   @GerritServerConfig
   protected Config cfg;

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -15,6 +15,11 @@
 package com.google.gerrit.acceptance.api.change;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
+import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.project.Util.category;
+import static com.google.gerrit.server.project.Util.value;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -22,6 +27,8 @@ import com.google.common.collect.Sets;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.RebaseInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -33,8 +40,12 @@ import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.common.RevisionInfo;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.Util;
 
 import org.eclipse.jgit.lib.Constants;
 import org.junit.Test;
@@ -376,4 +387,60 @@ public class ChangeIT extends AbstractDaemonTest {
         .get(EnumSet.of(ListChangesOption.CHECK))
         .problems).isEmpty();
   }
+
+  @Test
+  public void commitFooters() throws Exception {
+    LabelType verified = category("Verified",
+        value(1, "Failed"), value(0, "No score"), value(-1, "Passes"));
+    LabelType custom1 = category("Custom1",
+        value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
+    LabelType custom2 = category("Custom2",
+        value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
+    ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+    cfg.getLabelSections().put(verified.getName(), verified);
+    cfg.getLabelSections().put(custom1.getName(), verified);
+    cfg.getLabelSections().put(custom2.getName(), verified);
+    String heads = "refs/heads/*";
+    AccountGroup.UUID anon =
+        SystemGroupBackend.getGroup(ANONYMOUS_USERS).getUUID();
+    Util.allow(cfg, Permission.forLabel("Verified"), -1, 1, anon, heads);
+    Util.allow(cfg, Permission.forLabel("Custom1"), -1, 1, anon, heads);
+    Util.allow(cfg, Permission.forLabel("Custom2"), -1, 1, anon, heads);
+    saveProjectConfig(project, cfg);
+
+    PushOneCommit.Result r1 = createChange();
+    r1.assertOkStatus();
+    PushOneCommit.Result r2 = pushFactory.create(
+          db, admin.getIdent(), testRepo, SUBJECT, FILE_NAME, "new content",
+          r1.getChangeId())
+        .to("refs/for/master");
+    r2.assertOkStatus();
+
+    ReviewInput in = new ReviewInput();
+    in.label("Code-Review", 1);
+    in.label("Verified", 1);
+    in.label("Custom1", -1);
+    in.label("Custom2", 1);
+    gApi.changes().id(r2.getChangeId()).current().review(in);
+
+    EnumSet<ListChangesOption> options = EnumSet.of(
+        ListChangesOption.ALL_REVISIONS, ListChangesOption.COMMIT_FOOTERS);
+    ChangeInfo actual = gApi.changes().id(r2.getChangeId()).get(options);
+    assertThat(actual.revisions).hasSize(2);
+
+    // No footers except on latest patch set.
+    assertThat(actual.revisions.get(r1.getCommit().getName()).commitWithFooters)
+        .isNull();
+
+    String expected = SUBJECT + "\n"
+        + "\n"
+        + "Change-Id: " + r2.getChangeId() + "\n"
+        + "Reviewed-on: "
+            + canonicalWebUrl.get() + r2.getChange().getId() + "\n"
+        + "Reviewed-by: Administrator <admin@example.com>\n"
+        + "Custom2: Administrator <admin@example.com>\n"
+        + "Tested-by: Administrator <admin@example.com>\n";
+    assertThat(actual.revisions.get(r2.getCommit().getName()).commitWithFooters)
+        .isEqualTo(expected);
+  }
 }

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListChangesOption.java

@@ -58,7 +58,10 @@ public enum ListChangesOption {
   CHECK(15),
 
   /** Include allowed change actions client could perform. */
-  CHANGE_ACTIONS(16);
+  CHANGE_ACTIONS(16),
+
+  /** Include a copy of commit messages including review footers. */
+  COMMIT_FOOTERS(17);
 
   private final int value;
 

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/RevisionInfo.java

@@ -29,4 +29,5 @@ public class RevisionInfo {
   public CommitInfo commit;
   public Map<String, FileInfo> files;
   public Map<String, ActionInfo> actions;
+  public String commitWithFooters;
 }

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

@@ -19,6 +19,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.ALL_FILES;
 import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
 import static com.google.gerrit.extensions.client.ListChangesOption.CHANGE_ACTIONS;
 import static com.google.gerrit.extensions.client.ListChangesOption.CHECK;
+import static com.google.gerrit.extensions.client.ListChangesOption.COMMIT_FOOTERS;
 import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_ACTIONS;
 import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
 import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_FILES;
@@ -31,12 +32,14 @@ import static com.google.gerrit.extensions.client.ListChangesOption.LABELS;
 import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
 import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
 import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
+import static com.google.gerrit.server.CommonConverters.toGitPerson;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Optional;
+import com.google.common.base.Throwables;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.HashMultimap;
@@ -65,7 +68,6 @@ import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
 import com.google.gerrit.extensions.common.CommitInfo;
 import com.google.gerrit.extensions.common.FetchInfo;
-import com.google.gerrit.extensions.common.GitPerson;
 import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.common.ProblemInfo;
 import com.google.gerrit.extensions.common.RevisionInfo;
@@ -80,10 +82,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.PatchSetInfo.ParentInfo;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.ChangeMessagesUtil;
@@ -92,12 +91,13 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchLineCommentsUtil;
 import com.google.gerrit.server.WebLinks;
 import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.LabelNormalizer;
+import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
@@ -106,10 +106,16 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
+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.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.sql.Timestamp;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -129,9 +135,11 @@ public class ChangeJson {
   private final LabelNormalizer labelNormalizer;
   private final Provider<CurrentUser> userProvider;
   private final AnonymousUser anonymous;
+  private final GitRepositoryManager repoManager;
+  private final ProjectCache projectCache;
+  private final MergeUtil.Factory mergeUtilFactory;
   private final IdentifiedUser.GenericFactory userFactory;
   private final ChangeData.Factory changeDataFactory;
-  private final PatchSetInfoFactory patchSetInfoFactory;
   private final FileInfoJson fileInfoJson;
   private final AccountLoader.Factory accountLoaderFactory;
   private final DynamicMap<DownloadScheme> downloadSchemes;
@@ -153,9 +161,11 @@ public class ChangeJson {
       LabelNormalizer ln,
       Provider<CurrentUser> user,
       AnonymousUser au,
+      GitRepositoryManager repoManager,
+      ProjectCache projectCache,
+      MergeUtil.Factory mergeUtilFactory,
       IdentifiedUser.GenericFactory uf,
       ChangeData.Factory cdf,
-      PatchSetInfoFactory psi,
       FileInfoJson fileInfoJson,
       AccountLoader.Factory ailf,
       DynamicMap<DownloadScheme> downloadSchemes,
@@ -170,9 +180,11 @@ public class ChangeJson {
     this.labelNormalizer = ln;
     this.userProvider = user;
     this.anonymous = au;
-    this.userFactory = uf;
     this.changeDataFactory = cdf;
-    this.patchSetInfoFactory = psi;
+    this.repoManager = repoManager;
+    this.userFactory = uf;
+    this.projectCache = projectCache;
+    this.mergeUtilFactory = mergeUtilFactory;
     this.fileInfoJson = fileInfoJson;
     this.accountLoaderFactory = ailf;
     this.downloadSchemes = downloadSchemes;
@@ -237,9 +249,11 @@ public class ChangeJson {
       ChangeInfo res = toChangeInfo(cd, reviewed, limitToPsId);
       accountLoader.fill();
       return res;
-    } catch (OrmException | RuntimeException e) {
+    } catch (PatchListNotAvailableException | OrmException | IOException
+        | RuntimeException e) {
       if (!has(CHECK)) {
-        throw e;
+        Throwables.propagateIfPossible(e, OrmException.class);
+        throw new OrmException(e);
       }
       return checkOnly(cd);
     }
@@ -297,7 +311,8 @@ public class ChangeJson {
       if (i == null) {
         try {
           i = toChangeInfo(cd, reviewed, Optional.<PatchSet.Id> absent());
-        } catch (OrmException | RuntimeException e) {
+        } catch (PatchListNotAvailableException | OrmException | IOException
+            | RuntimeException e) {
           if (has(CHECK)) {
             i = checkOnly(cd);
           } else {
@@ -340,7 +355,8 @@ public class ChangeJson {
   }
 
   private ChangeInfo toChangeInfo(ChangeData cd, Set<Change.Id> reviewed,
-      Optional<PatchSet.Id> limitToPsId) throws OrmException {
+      Optional<PatchSet.Id> limitToPsId)
+      throws PatchListNotAvailableException, OrmException, IOException {
     ChangeInfo out = new ChangeInfo();
 
     if (has(CHECK)) {
@@ -404,7 +420,7 @@ public class ChangeJson {
     if (has(ALL_REVISIONS)
         || has(CURRENT_REVISION)
         || limitToPsId.isPresent()) {
-      out.revisions = revisions(ctl, cd, src);
+      out.revisions = revisions(ctl, src);
       if (out.revisions != null) {
         for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
           if (entry.getValue().isCurrent) {
@@ -819,14 +835,15 @@ public class ChangeJson {
     return false;
   }
 
-  private Map<String, RevisionInfo> revisions(ChangeControl ctl, ChangeData cd,
+  private Map<String, RevisionInfo> revisions(ChangeControl ctl,
-      Map<PatchSet.Id, PatchSet> map) throws OrmException {
+      Map<PatchSet.Id, PatchSet> map)
+      throws PatchListNotAvailableException, OrmException, IOException {
     Map<String, RevisionInfo> res = Maps.newLinkedHashMap();
     for (PatchSet in : map.values()) {
       if ((has(ALL_REVISIONS)
-          || in.getId().equals(cd.change().currentPatchSetId()))
+          || in.getId().equals(ctl.getChange().currentPatchSetId()))
           && ctl.isPatchVisible(in, db.get())) {
-        res.put(in.getRevision().get(), toRevisionInfo(ctl, cd, in));
+        res.put(in.getRevision().get(), toRevisionInfo(ctl, in));
       }
     }
     return res;
@@ -860,10 +877,11 @@ public class ChangeJson {
     return map;
   }
 
-  private RevisionInfo toRevisionInfo(ChangeControl ctl, ChangeData cd,
+  private RevisionInfo toRevisionInfo(ChangeControl ctl, PatchSet in)
-      PatchSet in) throws OrmException {
+      throws PatchListNotAvailableException, OrmException, IOException {
+    Change c = ctl.getChange();
     RevisionInfo out = new RevisionInfo();
-    out.isCurrent = in.getId().equals(cd.change().currentPatchSetId());
+    out.isCurrent = in.getId().equals(c.currentPatchSetId());
     out._number = in.getId().get();
     out.ref = in.getRefName();
     out.created = in.getCreatedOn();
@@ -871,21 +889,30 @@ public class ChangeJson {
     out.draft = in.isDraft() ? true : null;
     out.fetch = makeFetchMap(ctl, in);
 
-    if (has(ALL_COMMITS) || (out.isCurrent && has(CURRENT_COMMIT))) {
+    boolean setCommit = has(ALL_COMMITS)
-      try {
+        || (out.isCurrent && has(CURRENT_COMMIT));
-        out.commit = toCommit(in, cd.change().getProject(), has(WEB_LINKS));
+    boolean addFooters = out.isCurrent && has(COMMIT_FOOTERS);
-      } catch (PatchSetInfoNotAvailableException e) {
+    if (setCommit || addFooters) {
-        throw new OrmException(e);
+      Project.NameKey project = c.getProject();
+      try (Repository repo = repoManager.openRepository(project);
+          RevWalk rw = new RevWalk(repo)) {
+        String rev = in.getRevision().get();
+        RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
+        rw.parseBody(commit);
+        if (setCommit) {
+          out.commit = toCommit(ctl, rw, commit, has(WEB_LINKS));
+        }
+        if (addFooters) {
+          out.commitWithFooters = mergeUtilFactory
+              .create(projectCache.get(project))
+              .createCherryPickCommitMessage(commit, ctl, in.getId());
+        }
       }
     }
 
     if (has(ALL_FILES) || (out.isCurrent && has(CURRENT_FILES))) {
-      try {
+      out.files = fileInfoJson.toFileInfoMap(c, in);
-        out.files = fileInfoJson.toFileInfoMap(cd.change(), in);
+      out.files.remove(Patch.COMMIT_MSG);
-        out.files.remove(Patch.COMMIT_MSG);
-      } catch (PatchListNotAvailableException e) {
-        throw new OrmException(e);
-      }
     }
 
     if ((out.isCurrent || (out.draft != null && out.draft))
@@ -908,34 +935,35 @@ public class ChangeJson {
     return out;
   }
 
-  CommitInfo toCommit(PatchSet in, Project.NameKey project, boolean addLinks)
+  CommitInfo toCommit(ChangeControl ctl, RevWalk rw, RevCommit commit,
-      throws PatchSetInfoNotAvailableException {
+      boolean addLinks) throws IOException {
-    PatchSetInfo info = patchSetInfoFactory.get(db.get(), in.getId());
+    Project.NameKey project = ctl.getChange().getProject();
-    CommitInfo commit = new CommitInfo();
+    CommitInfo info = new CommitInfo();
-    commit.parents = Lists.newArrayListWithCapacity(info.getParents().size());
+    info.parents = new ArrayList<>(commit.getParentCount());
-    commit.author = toGitPerson(info.getAuthor());
+    info.author = toGitPerson(commit.getAuthorIdent());
-    commit.committer = toGitPerson(info.getCommitter());
+    info.committer = toGitPerson(commit.getCommitterIdent());
-    commit.subject = info.getSubject();
+    info.subject = commit.getShortMessage();
-    commit.message = info.getMessage();
+    info.message = commit.getFullMessage();
 
     if (addLinks) {
       FluentIterable<WebLinkInfo> links =
-          webLinks.getPatchSetLinks(project, in.getRevision().get());
+          webLinks.getPatchSetLinks(project, commit.name());
-      commit.webLinks = links.isEmpty() ? null : links.toList();
+      info.webLinks = links.isEmpty() ? null : links.toList();
     }
 
-    for (ParentInfo parent : info.getParents()) {
+    for (RevCommit parent : commit.getParents()) {
+      rw.parseBody(parent);
       CommitInfo i = new CommitInfo();
-      i.commit = parent.id.get();
+      i.commit = parent.name();
-      i.subject = parent.shortMessage;
+      i.subject = parent.getShortMessage();
       if (addLinks) {
         FluentIterable<WebLinkInfo> parentLinks =
-            webLinks.getPatchSetLinks(project, parent.id.get());
+            webLinks.getPatchSetLinks(project, parent.name());
         i.webLinks = parentLinks.isEmpty() ? null : parentLinks.toList();
       }
-      commit.parents.add(i);
+      info.parents.add(i);
     }
-    return commit;
+    return info;
   }
 
   private Map<String, FetchInfo> makeFetchMap(ChangeControl ctl, PatchSet in)
@@ -991,15 +1019,6 @@ public class ChangeJson {
     fetchInfo.commands.put(commandName, c);
   }
 
-  private static GitPerson toGitPerson(UserIdentity committer) {
-    GitPerson p = new GitPerson();
-    p.name = committer.getName();
-    p.email = committer.getEmail();
-    p.date = committer.getDate();
-    p.tz = committer.getTimeZone();
-    return p;
-  }
-
   static void finish(ChangeInfo info) {
     info.id = Joiner.on('~').join(
         Url.encode(info.project),

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

@@ -18,38 +18,47 @@ import com.google.gerrit.extensions.common.CommitInfo;
 import com.google.gerrit.extensions.restapi.CacheControl;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.reviewdb.client.Project;
-import com.google.gwtorm.server.OrmException;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
 
+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;
 
+import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 
 public class GetCommit implements RestReadView<RevisionResource> {
+  private final GitRepositoryManager repoManager;
   private final ChangeJson json;
 
   @Option(name = "--links", usage = "Add weblinks")
   private boolean addLinks;
 
   @Inject
-  GetCommit(ChangeJson json) {
+  GetCommit(GitRepositoryManager repoManager,
+      ChangeJson json) {
+    this.repoManager = repoManager;
     this.json = json;
   }
 
   @Override
-  public Response<CommitInfo> apply(RevisionResource resource)
+  public Response<CommitInfo> apply(RevisionResource rsrc) throws IOException {
-      throws OrmException {
+    Project.NameKey p = rsrc.getChange().getProject();
-    try {
+    try (Repository repo = repoManager.openRepository(p);
-      Response<CommitInfo> r =
+        RevWalk rw = new RevWalk(repo)) {
-          Response.ok(json.toCommit(resource.getPatchSet(), resource
+      String rev = rsrc.getPatchSet().getRevision().get();
-              .getChange().getProject(), addLinks));
+      RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
-      if (resource.isCacheable()) {
+      rw.parseBody(commit);
+      Response<CommitInfo> r = Response.ok(
+          json.toCommit(rsrc.getControl(), rw, commit, addLinks));
+      if (rsrc.isCacheable()) {
         r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
       }
       return r;
-    } catch (PatchSetInfoNotAvailableException e) {
-      throw new OrmException(e);
     }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java

@@ -25,13 +25,16 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.LabelId;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Provider;
@@ -195,7 +198,9 @@ public class MergeUtil {
     }
   }
 
-  public String createCherryPickCommitMessage(final CodeReviewCommit n) {
+  public String createCherryPickCommitMessage(RevCommit n, ChangeControl ctl,
+      PatchSet.Id psId) {
+    Change c = ctl.getChange();
     final List<FooterLine> footers = n.getFooterLines();
     final StringBuilder msgbuf = new StringBuilder();
     msgbuf.append(n.getFullMessage());
@@ -215,16 +220,16 @@ public class MergeUtil {
       msgbuf.append('\n');
     }
 
-    if (!contains(footers, FooterConstants.CHANGE_ID, n.change().getKey().get())) {
+    if (!contains(footers, FooterConstants.CHANGE_ID, c.getKey().get())) {
       msgbuf.append(FooterConstants.CHANGE_ID.getName());
       msgbuf.append(": ");
-      msgbuf.append(n.change().getKey().get());
+      msgbuf.append(c.getKey().get());
       msgbuf.append('\n');
     }
 
     final String siteUrl = urlProvider.get();
     if (siteUrl != null) {
-      final String url = siteUrl + n.getPatchsetId().getParentKey().get();
+      final String url = siteUrl + c.getId().get();
       if (!contains(footers, FooterConstants.REVIEWED_ON, url)) {
         msgbuf.append(FooterConstants.REVIEWED_ON.getName());
         msgbuf.append(": ");
@@ -235,7 +240,7 @@ public class MergeUtil {
 
     PatchSetApproval submitAudit = null;
 
-    for (final PatchSetApproval a : safeGetApprovals(n)) {
+    for (final PatchSetApproval a : safeGetApprovals(ctl, psId)) {
       if (a.getValue() <= 0) {
         // Negative votes aren't counted.
         continue;
@@ -301,6 +306,10 @@ public class MergeUtil {
     return msgbuf.toString();
   }
 
+  public String createCherryPickCommitMessage(final CodeReviewCommit n) {
+    return createCherryPickCommitMessage(n, n.getControl(), n.getPatchsetId());
+  }
+
   private static boolean isCodeReview(LabelId id) {
     return "Code-Review".equalsIgnoreCase(id.get());
   }
@@ -309,11 +318,12 @@ public class MergeUtil {
     return "Verified".equalsIgnoreCase(id.get());
   }
 
-  private Iterable<PatchSetApproval> safeGetApprovals(CodeReviewCommit n) {
+  private Iterable<PatchSetApproval> safeGetApprovals(
+      ChangeControl ctl, PatchSet.Id psId) {
     try {
-      return approvalsUtil.byPatchSet(db.get(), n.getControl(), n.getPatchsetId());
+      return approvalsUtil.byPatchSet(db.get(), ctl, psId);
     } catch (OrmException e) {
-      log.error("Can't read approval records for " + n.getPatchsetId(), e);
+      log.error("Can't read approval records for " + psId, e);
       return Collections.emptyList();
     }
   }