Remove PatchSetAccess#byRevision(Range)

These cannot be implemented efficiently in notedb, and we can get what
we need from the secondary index in most cases, or else (in the case
of ConsistencyChecker) directly from the repo.

This change intends to provide the same behavior, and does not correct
for some sketchy usages of patch sets, like failing to check
visibility.

Change-Id: I2d961f8d2436b44308116c19597f0ed74b9d1337

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java

@@ -564,7 +564,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
     ProblemInfo p = problems.get(0);
     assertThat(p.message).isEqualTo(
         "Multiple patch sets for expected merged commit " + commit.name()
-        + ": [" + ps2 + ", " + ps3 + "]");
+        + ": [" + ps2.getId() + ", " + ps3.getId() + "]");
   }
 
   private Change insertChange() throws Exception {

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RevId.java

@@ -69,4 +69,8 @@ public final class RevId {
   public String toString() {
     return getClass().getSimpleName() + "{" + id + "}";
   }
+
+  public boolean matches(String str) {
+    return id.startsWith(str.toLowerCase());
+  }
 }

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/PatchSetAccess.java

@@ -16,7 +16,6 @@ package com.google.gerrit.reviewdb.server;
 
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gwtorm.server.Access;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.PrimaryKey;
@@ -30,11 +29,4 @@ public interface PatchSetAccess extends Access<PatchSet, PatchSet.Id> {
 
   @Query("WHERE id.changeId = ? ORDER BY id.patchSetId")
   ResultSet<PatchSet> byChange(Change.Id id) throws OrmException;
-
-  @Query("WHERE revision = ?")
-  ResultSet<PatchSet> byRevision(RevId rev) throws OrmException;
-
-  @Query("WHERE revision >= ? AND revision <= ?")
-  ResultSet<PatchSet> byRevisionRange(RevId reva, RevId revb)
-      throws OrmException;
 }

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

@@ -14,14 +14,14 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
+import static com.google.gerrit.reviewdb.server.ReviewDbUtil.intKeyOrdering;
 import static com.google.gerrit.server.ChangeUtil.PS_ID_ORDER;
 import static com.google.gerrit.server.ChangeUtil.TO_PS_ID;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Function;
-import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
-import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
@@ -36,7 +36,6 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.CurrentUser;
@@ -406,22 +405,27 @@ public class ConsistencyChecker {
         return;
       }
 
-      RevId revId = new RevId(commit.name());
+      List<PatchSet.Id> psIds = new ArrayList<>();
-      List<PatchSet> patchSets = FluentIterable
+      for (Ref ref : repo.getRefDatabase().getRefs(REFS_CHANGES).values()) {
-          .from(db.get().patchSets().byRevision(revId))
+        if (!ref.getObjectId().equals(commit)) {
-          .filter(new Predicate<PatchSet>() {
+          continue;
-            @Override
+        }
-            public boolean apply(PatchSet ps) {
+        PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
-              try {
+        if (psId == null) {
-                Change c = db.get().changes().get(ps.getId().getParentKey());
+          continue;
-                return c != null && c.getDest().equals(change.getDest());
+        }
-              } catch (OrmException e) {
+        try {
-                warn(e);
+          Change c = db.get().changes().get(psId.getParentKey());
-                return true; // Should cause an error below, that's good.
+          if (c == null || !c.getDest().equals(change.getDest())) {
-              }
+            continue;
-            }
+          }
-          }).toSortedList(ChangeUtil.PS_ID_ORDER);
+        } catch (OrmException e) {
-      switch (patchSets.size()) {
+          warn(e);
+          // Include this patch set; should cause an error below, which is good.
+        }
+        psIds.add(psId);
+      }
+      switch (psIds.size()) {
         case 0:
           // No patch set for this commit; insert one.
           rw.parseBody(commit);
@@ -445,7 +449,7 @@ public class ConsistencyChecker {
           // patch set.
           // TODO(dborowitz): This could be fixed if it's an older patch set of
           // the current change.
-          PatchSet.Id id = patchSets.get(0).getId();
+          PatchSet.Id id = psIds.get(0);
           if (!id.equals(change.currentPatchSetId())) {
             problem(String.format("Expected merged commit %s corresponds to"
                   + " patch set %s, which is not the current patch set %s",
@@ -456,10 +460,10 @@ public class ConsistencyChecker {
         default:
           problem(String.format(
                 "Multiple patch sets for expected merged commit %s: %s",
-                commit.name(), patchSets));
+                commit.name(), intKeyOrdering().sortedCopy(psIds)));
           break;
       }
-    } catch (OrmException | IOException e) {
+    } catch (IOException e) {
       error("Error looking up expected merged commit " + fix.expectMergedAs,
           e);
     }

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

@@ -29,13 +29,15 @@ import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -66,6 +68,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
   private final RebaseUtil rebaseUtil;
   private final ChangeJson.Factory json;
   private final Provider<ReviewDb> dbProvider;
+  private final Provider<InternalChangeQuery> queryProvider;
 
   @Inject
   public Rebase(BatchUpdate.Factory updateFactory,
@@ -73,13 +76,15 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
       RebaseChangeOp.Factory rebaseFactory,
       RebaseUtil rebaseUtil,
       ChangeJson.Factory json,
-      Provider<ReviewDb> dbProvider) {
+      Provider<ReviewDb> dbProvider,
+      Provider<InternalChangeQuery> queryProvider) {
     this.updateFactory = updateFactory;
     this.repoManager = repoManager;
     this.rebaseFactory = rebaseFactory;
     this.rebaseUtil = rebaseUtil;
     this.json = json;
     this.dbProvider = dbProvider;
+    this.queryProvider = queryProvider;
   }
 
   @Override
@@ -130,7 +135,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
 
     @SuppressWarnings("resource")
     ReviewDb db = dbProvider.get();
-    PatchSet basePatchSet = parseBase(base);
+    PatchSet basePatchSet = parseBase(change.getProject(), base);
     if (basePatchSet == null) {
       throw new ResourceConflictException("base revision is missing: " + base);
     } else if (!rsrc.getControl().isPatchVisible(basePatchSet, db)) {
@@ -168,7 +173,8 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
     return rw.isMergedInto(rw.parseCommit(baseId), rw.parseCommit(tipId));
   }
 
-  private PatchSet parseBase(String base) throws OrmException {
+  private PatchSet parseBase(Project.NameKey project, String base)
+      throws OrmException {
     ReviewDb db = dbProvider.get();
 
     PatchSet.Id basePatchSetId = PatchSet.Id.fromRef(base);
@@ -193,10 +199,15 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
     }
 
     // Try parsing as SHA-1.
-    for (PatchSet ps : db.patchSets().byRevision(new RevId(base))) {
+    for (ChangeData cd : queryProvider.get().byProjectCommit(project, base)) {
-      if (basePatchSet == null
+      for (PatchSet ps : cd.patchSets()) {
-          || basePatchSet.getId().get() < ps.getId().get()) {
+        if (!ps.getRevision().matches(base)) {
-        basePatchSet = ps;
+          continue;
+        }
+        if (basePatchSet == null
+            || basePatchSet.getId().get() < ps.getId().get()) {
+          basePatchSet = ps;
+        }
       }
     }
     return basePatchSet;

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

@@ -22,7 +22,8 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RevId;
-import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -41,11 +42,11 @@ import java.io.IOException;
 public class RebaseUtil {
   private static final Logger log = LoggerFactory.getLogger(RebaseUtil.class);
 
-  private final Provider<ReviewDb> db;
+  private final Provider<InternalChangeQuery> queryProvider;
 
   @Inject
-  RebaseUtil(Provider<ReviewDb> db) {
+  RebaseUtil(Provider<InternalChangeQuery> queryProvider) {
-    this.db = db;
+    this.queryProvider = queryProvider;
   }
 
   public boolean canRebase(PatchSet patchSet, Branch.NameKey dest,
@@ -97,30 +98,29 @@ public class RebaseUtil {
 
     RevId parentRev = new RevId(commit.getParent(0).name());
 
-    for (PatchSet depPatchSet : db.get().patchSets().byRevision(parentRev)) {
+    CHANGES: for (ChangeData cd : queryProvider.get()
-      Change.Id depChangeId = depPatchSet.getId().getParentKey();
+        .byBranchCommit(destBranch, parentRev.get())) {
-      Change depChange = db.get().changes().get(depChangeId);
+      for (PatchSet depPatchSet : cd.patchSets()) {
-      if (!depChange.getDest().equals(destBranch)) {
+        if (!depPatchSet.getRevision().equals(parentRev)) {
-        continue;
+          continue;
-      }
-
-      if (depChange.getStatus() == Status.ABANDONED) {
-        throw new ResourceConflictException(
-            "Cannot rebase a change with an abandoned parent: "
-            + depChange.getKey());
-      }
-
-      if (depChange.getStatus().isOpen()) {
-        if (depPatchSet.getId().equals(depChange.currentPatchSetId())) {
-          throw new ResourceConflictException(
-              "Change is already based on the latest patch set of the"
-              + " dependent change.");
         }
-        PatchSet latestDepPatchSet =
+        Change depChange = cd.change();
-            db.get().patchSets().get(depChange.currentPatchSetId());
+        if (depChange.getStatus() == Status.ABANDONED) {
-        baseRev = latestDepPatchSet.getRevision().get();
+          throw new ResourceConflictException(
+              "Cannot rebase a change with an abandoned parent: "
+              + depChange.getKey());
+        }
+
+        if (depChange.getStatus().isOpen()) {
+          if (depPatchSet.getId().equals(depChange.currentPatchSetId())) {
+            throw new ResourceConflictException(
+                "Change is already based on the latest patch set of the"
+                + " dependent change.");
+          }
+          baseRev = cd.currentPatchSet().getRevision().get();
+        }
+        break CHANGES;
       }
-      break;
     }
 
     if (baseRev == null) {

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

@@ -14,11 +14,8 @@
 
 package com.google.gerrit.server.change;
 
-import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
-import com.google.common.base.Predicate;
-import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -26,14 +23,12 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.edit.ChangeEdit;
 import com.google.gerrit.server.edit.ChangeEditUtil;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -114,19 +109,7 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
       // Require a minimum of 4 digits.
       // Impossibly long identifier will never match.
       return Collections.emptyList();
-    } else if (id.length() >= 8) {
-      // Commit names are rather unique. Query for the commit and later
-      // match to the change. This is most likely going to identify 1 or
-      // at most 2 patch sets to consider, which is smaller than looking
-      // for all patch sets in the change.
-      RevId revid = new RevId(id);
-      if (revid.isComplete()) {
-        List<RevisionResource> m = toResources(change, findExactMatch(revid));
-        return m.isEmpty() ? loadEdit(change, revid)  : m;
-      }
-      return toResources(change, findByPrefix(revid));
     } else {
-      // Chance of collision rises; look at all patch sets on the change.
       List<RevisionResource> out = Lists.newArrayList();
       for (PatchSet ps : dbProvider.get().patchSets()
           .byChange(change.getId())) {
@@ -149,14 +132,6 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
     return Collections.emptyList();
   }
 
-  private ResultSet<PatchSet> findExactMatch(RevId revid) throws OrmException {
-    return dbProvider.get().patchSets().byRevision(revid);
-  }
-
-  private ResultSet<PatchSet> findByPrefix(RevId revid) throws OrmException {
-    return dbProvider.get().patchSets().byRevisionRange(revid, revid.max());
-  }
-
   private List<RevisionResource> loadEdit(ChangeResource change, RevId revid)
       throws AuthException, IOException {
     Optional<ChangeEdit> edit = editUtil.byChange(change.getChange());
@@ -170,21 +145,4 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
     }
     return Collections.emptyList();
   }
-
-  private static List<RevisionResource> toResources(final ChangeResource change,
-      Iterable<PatchSet> patchSets) {
-    final Change.Id changeId = change.getId();
-    return FluentIterable.from(patchSets)
-        .filter(new Predicate<PatchSet>() {
-          @Override
-          public boolean apply(PatchSet in) {
-            return changeId.equals(in.getId().getParentKey());
-          }
-        }).transform(new Function<PatchSet, RevisionResource>() {
-          @Override
-          public RevisionResource apply(PatchSet in) {
-            return new RevisionResource(change, in);
-          }
-        }).toList();
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java

@@ -218,7 +218,16 @@ public class InternalChangeQuery {
   }
 
   public List<ChangeData> byCommit(ObjectId id) throws OrmException {
-    return query(commit(schema(indexes), id.name()));
+    return byCommit(id.name());
+  }
+
+  public List<ChangeData> byCommit(String hash) throws OrmException {
+    return query(commit(schema(indexes), hash));
+  }
+
+  public List<ChangeData> byProjectCommit(Project.NameKey project,
+      String hash) throws OrmException {
+    return query(and(project(project), commit(schema(indexes), hash)));
   }
 
   public List<ChangeData> byProjectCommits(Project.NameKey project,
@@ -228,6 +237,19 @@ public class InternalChangeQuery {
     return query(and(project(project), or(commits(schema(indexes), hashes))));
   }
 
+  public List<ChangeData> byBranchCommit(String project, String branch,
+      String hash) throws OrmException {
+    return query(and(
+        new ProjectPredicate(project),
+        new RefPredicate(branch),
+        commit(schema(indexes), hash)));
+  }
+
+  public List<ChangeData> byBranchCommit(Branch.NameKey branch, String hash)
+      throws OrmException {
+    return byBranchCommit(branch.getParentKey().get(), branch.get(), hash);
+  }
+
   public List<ChangeData> bySubmissionId(String cs) throws OrmException {
     if (Strings.isNullOrEmpty(cs) || !schema(indexes).hasField(SUBMISSIONID)) {
       return Collections.emptyList();

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -32,7 +32,7 @@ import java.util.List;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_117> C = Schema_117.class;
+  public static final Class<Schema_118> C = Schema_118.class;
 
   public static int getBinaryVersion() {
     return guessVersion(C);

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_118.java

@@ -0,0 +1,25 @@
+// Copyright (C) 2016 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.server.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class Schema_118 extends SchemaVersion {
+  @Inject
+  Schema_118(Provider<Schema_117> prior) {
+    super(prior);
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CommandUtils.java

@@ -16,36 +16,46 @@ package com.google.gerrit.sshd.commands;
 
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 
-import java.util.HashSet;
+import java.util.ArrayList;
-import java.util.Set;
+import java.util.List;
 
 public class CommandUtils {
-  public static PatchSet parsePatchSet(final String patchIdentity, ReviewDb db,
+  public static PatchSet parsePatchSet(String patchIdentity, ReviewDb db,
-      ProjectControl projectControl, String branch)
+      InternalChangeQuery query, ProjectControl projectControl, String branch)
       throws UnloggedFailure, OrmException {
     // By commit?
     //
     if (patchIdentity.matches("^([0-9a-fA-F]{4," + RevId.LEN + "})$")) {
-      final RevId id = new RevId(patchIdentity);
+      List<ChangeData> cds;
-      final ResultSet<PatchSet> patches;
+      if (projectControl != null) {
-      if (id.isComplete()) {
+        Project.NameKey p = projectControl.getProject().getNameKey();
-        patches = db.patchSets().byRevision(id);
+        if (branch != null) {
+          cds = query.byBranchCommit(p.get(), branch, patchIdentity);
+        } else {
+          cds = query.byProjectCommit(p, patchIdentity);
+        }
       } else {
-        patches = db.patchSets().byRevisionRange(id, id.max());
+        cds = query.byCommit(patchIdentity);
       }
-
+      List<PatchSet> matches = new ArrayList<>(cds.size());
-      final Set<PatchSet> matches = new HashSet<>();
+      for (ChangeData cd : cds) {
-      for (final PatchSet ps : patches) {
+        Change c = cd.change();
-        final Change change = db.changes().get(ps.getId().getParentKey());
+        if (!(inProject(c, projectControl) && inBranch(c, branch))) {
-        if (inProject(change, projectControl) && inBranch(change, branch)) {
+          continue;
-          matches.add(ps);
+        }
+        for (PatchSet ps : cd.patchSets()) {
+          if (ps.getRevision().matches(patchIdentity)) {
+            matches.add(ps);
+          }
         }
       }
 
@@ -62,18 +72,18 @@ public class CommandUtils {
     // By older style change,patchset?
     //
     if (patchIdentity.matches("^[1-9][0-9]*,[1-9][0-9]*$")) {
-      final PatchSet.Id patchSetId;
+      PatchSet.Id patchSetId;
       try {
         patchSetId = PatchSet.Id.parse(patchIdentity);
       } catch (IllegalArgumentException e) {
         throw error("\"" + patchIdentity + "\" is not a valid patch set");
       }
-      final PatchSet patchSet = db.patchSets().get(patchSetId);
+      PatchSet patchSet = db.patchSets().get(patchSetId);
       if (patchSet == null) {
         throw error("\"" + patchIdentity + "\" no such patch set");
       }
       if (projectControl != null || branch != null) {
-        final Change change = db.changes().get(patchSetId.getParentKey());
+        Change change = db.changes().get(patchSetId.getParentKey());
         if (!inProject(change, projectControl)) {
           throw error("change " + change.getId() + " not in project "
               + projectControl.getProject().getName());
@@ -89,7 +99,7 @@ public class CommandUtils {
     throw error("\"" + patchIdentity + "\" is not a valid patch set");
   }
 
-  private static boolean inProject(final Change change,
+  private static boolean inProject(Change change,
       ProjectControl projectControl) {
     if (projectControl == null) {
       // No --project option, so they want every project.
@@ -98,7 +108,7 @@ public class CommandUtils {
     return projectControl.getProject().getNameKey().equals(change.getProject());
   }
 
-  private static boolean inBranch(final Change change, String branch) {
+  private static boolean inBranch(Change change, String branch) {
     if (branch == null) {
       // No --branch option, so they want every branch.
       return true;
@@ -106,7 +116,7 @@ public class CommandUtils {
     return change.getDest().get().equals(branch);
   }
 
-  public static UnloggedFailure error(final String msg) {
+  public static UnloggedFailure error(String msg) {
     return new UnloggedFailure(1, msg);
   }
 }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java

@@ -36,6 +36,7 @@ import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
@@ -79,8 +80,8 @@ public class ReviewCommand extends SshCommand {
       usage = "list of commits or patch sets to review")
   void addPatchSetId(final String token) {
     try {
-      PatchSet ps = CommandUtils.parsePatchSet(token, db, projectControl,
+      PatchSet ps = CommandUtils.parsePatchSet(token, db, queryProvider.get(),
-          branch);
+          projectControl, branch);
       patchSets.add(ps);
     } catch (UnloggedFailure e) {
       throw new IllegalArgumentException(e.getMessage(), e);
@@ -145,6 +146,9 @@ public class ReviewCommand extends SshCommand {
   @Inject
   private Provider<GerritApi> gApi;
 
+  @Inject
+  private Provider<InternalChangeQuery> queryProvider;
+
   private List<ApproveOption> optionList;
   private Map<String, Short> customLabels;