Merge changes from topic 'search-no-lazy-load' into stable-2.11

* changes:
  ChangeJson: Don't load all approvals on closed changes
  ChangeJson: Less eager loading of patch sets
  ChangeJson: Eliminate patch set loading in search results
  Remove StoredValues.PATCH_SET

gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java

@@ -50,7 +50,6 @@ import java.util.Map;
 public final class StoredValues {
   public static final StoredValue<ReviewDb> REVIEW_DB = create(ReviewDb.class);
   public static final StoredValue<ChangeData> CHANGE_DATA = create(ChangeData.class);
-  public static final StoredValue<PatchSet> PATCH_SET = create(PatchSet.class);
 
   // Note: no guarantees are made about the user passed in the ChangeControl; do
   // not depend on this directly. Either use .forUser(otherUser) to get a
@@ -68,11 +67,20 @@ public final class StoredValues {
     }
   }
 
+  public static PatchSet getPatchSet(Prolog engine) throws SystemException {
+    ChangeData cd = CHANGE_DATA.get(engine);
+    try {
+      return cd.currentPatchSet();
+    } catch (OrmException e) {
+      throw new SystemException(e.getMessage());
+    }
+  }
+
   public static final StoredValue<PatchSetInfo> PATCH_SET_INFO = new StoredValue<PatchSetInfo>() {
     @Override
     public PatchSetInfo createValue(Prolog engine) {
       Change change = getChange(engine);
-      PatchSet ps = StoredValues.PATCH_SET.get(engine);
+      PatchSet ps = getPatchSet(engine);
       PrologEnvironment env = (PrologEnvironment) engine.control;
       PatchSetInfoFactory patchInfoFactory =
               env.getArgs().getPatchSetInfoFactory();
@@ -88,7 +96,7 @@ public final class StoredValues {
     @Override
     public PatchList createValue(Prolog engine) {
       PrologEnvironment env = (PrologEnvironment) engine.control;
-      PatchSet ps = StoredValues.PATCH_SET.get(engine);
+      PatchSet ps = getPatchSet(engine);
       PatchListCache plCache = env.getArgs().getPatchListCache();
       Change change = getChange(engine);
       Project.NameKey projectKey = change.getProject();

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

@@ -264,7 +264,7 @@ public class ChangeJson {
     ChangeData.ensureChangeLoaded(all);
     if (has(ALL_REVISIONS)) {
       ChangeData.ensureAllPatchSetsLoaded(all);
-    } else {
+    } else if (has(CURRENT_REVISION) || has(MESSAGES)) {
       ChangeData.ensureCurrentPatchSetLoaded(all);
     }
     Set<Change.Id> reviewed = Sets.newHashSet();
@@ -396,15 +396,22 @@ public class ChangeJson {
       out.removableReviewers = removableReviewers(ctl, out.labels.values());
     }
 
-    Map<PatchSet.Id, PatchSet> src = loadPatchSets(cd, limitToPsId);
-    if (has(MESSAGES)) {
+    boolean needMessages = has(MESSAGES);
+    boolean needRevisions = has(ALL_REVISIONS)
+        || has(CURRENT_REVISION)
+        || limitToPsId.isPresent();
+    Map<PatchSet.Id, PatchSet> src;
+    if (needMessages || needRevisions) {
+      src = loadPatchSets(cd, limitToPsId);
+    } else {
+      src = null;
+    }
+    if (needMessages) {
       out.messages = messages(ctl, cd, src);
     }
     finish(out);
 
-    if (has(ALL_REVISIONS)
-        || has(CURRENT_REVISION)
-        || limitToPsId.isPresent()) {
+    if (needRevisions) {
       out.revisions = revisions(ctl, cd, src);
       if (out.revisions != null) {
         for (Map.Entry<String, RevisionInfo> entry : out.revisions.entrySet()) {
@@ -427,11 +434,7 @@ public class ChangeJson {
     if (cd.getSubmitRecords() != null) {
       return cd.getSubmitRecords();
     }
-    PatchSet ps = cd.currentPatchSet();
-    if (ps == null) {
-      return ImmutableList.of();
-    }
-    cd.setSubmitRecords(new SubmitRuleEvaluator(cd).setPatchSet(ps)
+    cd.setSubmitRecords(new SubmitRuleEvaluator(cd)
         .setFastEvalLabels(true)
         .setAllowDraft(true)
         .evaluate());
@@ -592,8 +595,14 @@ public class ChangeJson {
       LabelTypes labelTypes, boolean standard, boolean detailed)
       throws OrmException {
     Set<Account.Id> allUsers = Sets.newHashSet();
-    for (PatchSetApproval psa : cd.approvals().values()) {
-      allUsers.add(psa.getAccountId());
+    if (detailed) {
+      // Users expect to see all reviewers on closed changes, even if they
+      // didn't vote on the latest patch set. If we don't need detailed labels,
+      // we aren't including 0 votes for all users below, so we can just look at
+      // the latest patch set (in the next loop).
+      for (PatchSetApproval psa : cd.approvals().values()) {
+        allUsers.add(psa.getAccountId());
+      }
     }
 
     // We can only approximately reconstruct what the submit rule evaluator
@@ -601,6 +610,7 @@ public class ChangeJson {
     Set<String> labelNames = Sets.newHashSet();
     Multimap<Account.Id, PatchSetApproval> current = HashMultimap.create();
     for (PatchSetApproval a : cd.currentApprovals()) {
+      allUsers.add(a.getAccountId());
       LabelType type = labelTypes.byLabel(a.getLabelId());
       if (type != null) {
         labelNames.add(type.getName());

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

@@ -196,21 +196,25 @@ public class SubmitRuleEvaluator {
    *     rules, including any errors.
    */
   public List<SubmitRecord> evaluate() {
-    try {
-      initPatchSet();
-    } catch (OrmException e) {
-      return ruleError("Error looking up patch set "
-          + control.getChange().currentPatchSetId());
-    }
     Change c = control.getChange();
     if (!allowClosed && c.getStatus().isClosed()) {
       SubmitRecord rec = new SubmitRecord();
       rec.status = SubmitRecord.Status.CLOSED;
       return Collections.singletonList(rec);
     }
-    if ((c.getStatus() == Change.Status.DRAFT || patchSet.isDraft())
-        && !allowDraft) {
-      return cannotSubmitDraft();
+    if (!allowDraft) {
+      if (c.getStatus() == Change.Status.DRAFT) {
+        return cannotSubmitDraft();
+      }
+      try {
+        initPatchSet();
+      } catch (OrmException e) {
+        return ruleError("Error looking up patch set "
+            + control.getChange().currentPatchSetId());
+      }
+      if (patchSet.isDraft()) {
+        return cannotSubmitDraft();
+      }
     }
 
     List<Term> results;
@@ -500,8 +504,6 @@ public class SubmitRuleEvaluator {
 
   private PrologEnvironment getPrologEnvironment(CurrentUser user)
       throws RuleEvalException {
-    checkState(patchSet != null,
-        "getPrologEnvironment() called before initPatchSet()");
     ProjectState projectState = control.getProjectControl().getProjectState();
     PrologEnvironment env;
     try {
@@ -517,7 +519,6 @@ public class SubmitRuleEvaluator {
     }
     env.set(StoredValues.REVIEW_DB, cd.db());
     env.set(StoredValues.CHANGE_DATA, cd);
-    env.set(StoredValues.PATCH_SET, patchSet);
     env.set(StoredValues.CHANGE_CONTROL, control);
     if (user != null) {
       env.set(StoredValues.CURRENT_USER, user);