PatchSetParser: Load change via ChangeNotes.Factory

Changes should not be loaded directly from the database but via
ChangeNotes.Factory, so that changes can be loaded from notedb when it
is enabled.

A project control is only available when a project name was specified
by the user. In case it is not available we must look up the change
from the index to find out the project name, which is required for
loading the change from notedb.

Change-Id: I32fb5486acc02aa36ef52f9f5ed40bdff3fd601f
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java

@@ -20,6 +20,7 @@ import com.google.common.primitives.Ints;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.change.ChangeTriplet;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
@@ -82,6 +83,15 @@ public class ChangeFinder {
     return Collections.emptyList();
   }
 
+  public ChangeControl findOne(Change.Id id, CurrentUser user)
+      throws OrmException, NoSuchChangeException {
+    List<ChangeControl> ctls = find(id, user);
+    if (ctls.size() != 1) {
+      throw new NoSuchChangeException(id);
+    }
+    return ctls.get(0);
+  }
+
   public List<ChangeControl> find(Change.Id id, CurrentUser user)
       throws OrmException {
     // Use the index to search for changes, but don't return any stored fields,

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

@@ -92,11 +92,8 @@ public class ChangeControl {
 
     public ChangeControl validateFor(Change.Id changeId, CurrentUser user)
         throws NoSuchChangeException, OrmException {
-      List<ChangeControl> ctls = changeFinder.find(changeId, user);
-      if (ctls.size() != 1) {
-        throw new NoSuchChangeException(changeId);
-      }
-      return validateFor(ctls.get(0).getChange(), user);
+      ChangeControl ctl = changeFinder.findOne(changeId, user);
+      return validateFor(ctl.getChange(), user);
     }
 
     public ChangeControl validateFor(Change change, CurrentUser user)

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

@@ -14,13 +14,18 @@
 
 package com.google.gerrit.sshd.commands;
 
+import com.google.gerrit.common.Nullable;
 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.ChangeFinder;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -39,16 +44,22 @@ public class PatchSetParser {
   private final Provider<InternalChangeQuery> queryProvider;
   private final ChangeNotes.Factory notesFactory;
   private final PatchSetUtil psUtil;
+  private final ChangeFinder changeFinder;
+  private final Provider<CurrentUser> self;
 
   @Inject
   PatchSetParser(Provider<ReviewDb> db,
       Provider<InternalChangeQuery> queryProvider,
       ChangeNotes.Factory notesFactory,
-      PatchSetUtil psUtil) {
+      PatchSetUtil psUtil,
+      ChangeFinder changeFinder,
+      Provider<CurrentUser> self) {
     this.db = db;
     this.queryProvider = queryProvider;
     this.notesFactory = notesFactory;
     this.psUtil = psUtil;
+    this.changeFinder = changeFinder;
+    this.self = self;
   }
 
   public PatchSet parsePatchSet(String token, ProjectControl projectControl,
@@ -100,7 +111,7 @@ public class PatchSetParser {
       } catch (IllegalArgumentException e) {
         throw error("\"" + token + "\" is not a valid patch set");
       }
-      ChangeNotes notes = getNotes(patchSetId.getParentKey());
+      ChangeNotes notes = getNotes(projectControl, patchSetId.getParentKey());
       PatchSet patchSet = psUtil.get(db.get(), notes, patchSetId);
       if (patchSet == null) {
         throw error("\"" + token + "\" no such patch set");
@@ -121,13 +132,20 @@ public class PatchSetParser {
     throw error("\"" + token + "\" is not a valid patch set");
   }
 
-  private ChangeNotes getNotes(Change.Id changeId)
-      throws OrmException, UnloggedFailure {
-    Change c = db.get().changes().get(changeId);
-    if (c == null) {
-      throw error("\"" + changeId + "\" no such change");
+  private ChangeNotes getNotes(@Nullable ProjectControl projectControl,
+      Change.Id changeId) throws OrmException, UnloggedFailure {
+    if (projectControl != null) {
+      return notesFactory.create(db.get(), projectControl.getProject().getNameKey(),
+          changeId);
+    } else {
+      try {
+        ChangeControl ctl = changeFinder.findOne(changeId, self.get());
+        return notesFactory.create(db.get(), ctl.getProject().getNameKey(),
+            changeId);
+      } catch (NoSuchChangeException e) {
+        throw error("\"" + changeId + "\" no such change");
+      }
     }
-    return notesFactory.create(db.get(), c.getProject(), changeId);
   }
 
   private static boolean inProject(Change change,