Merge branch 'stable-3.3'

* stable-3.3:
  Polygerrit: Wipe out license headers in minified gr-app.js
  ReceiveCommits: Avoid use of secondary index in readChangesForReplace
  Fix 'is:submittable' query on multiple submit records
  Fix 'is:submittable' query on multiple submit records
  Update git submodules

Change-Id: I32167e21189b488c9d88d00ca33b3c5c1fd4eda1

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -2692,12 +2692,10 @@ class ReceiveCommits {
 
   private void readChangesForReplace() {
     try (TraceTimer traceTimer = newTimer("readChangesForReplace")) {
-      Collection<ChangeNotes> allNotes =
-          notesFactory.createUsingIndexLookup(
-              replaceByChange.values().stream().map(r -> r.ontoChange).collect(toList()));
-      for (ChangeNotes notes : allNotes) {
-        replaceByChange.get(notes.getChangeId()).notes = notes;
-      }
+      replaceByChange.values().stream()
+          .map(r -> r.ontoChange)
+          .map(id -> notesFactory.create(project.getNameKey(), id))
+          .forEach(notes -> replaceByChange.get(notes.getChangeId()).notes = notes);
     }
   }
 

java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -660,7 +660,15 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
     }
 
     if ("submittable".equalsIgnoreCase(value)) {
-      return new SubmittablePredicate(SubmitRecord.Status.OK);
+      // SubmittablePredicate will match if *any* of the submit records are OK,
+      // but we need to check that they're *all* OK, so check that none of the
+      // submit records match any of the negative cases. To avoid checking yet
+      // more negative cases for CLOSED and FORCED, instead make sure at least
+      // one submit record is OK.
+      return Predicate.and(
+          new SubmittablePredicate(SubmitRecord.Status.OK),
+          Predicate.not(new SubmittablePredicate(SubmitRecord.Status.NOT_READY)),
+          Predicate.not(new SubmittablePredicate(SubmitRecord.Status.RULE_ERROR)));
     }
 
     if ("ignored".equalsIgnoreCase(value)) {

javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java

@@ -22,6 +22,8 @@ import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.entities.SubmitRecord;
 import com.google.gerrit.entities.SubmitRequirement;
 import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.SubmitRequirementInfo;
 import com.google.gerrit.extensions.config.FactoryModule;
@@ -33,7 +35,6 @@ import com.google.inject.Singleton;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.Test;
 
 public class ChangeSubmitRequirementIT extends AbstractDaemonTest {
@@ -93,20 +94,113 @@ public class ChangeSubmitRequirementIT extends AbstractDaemonTest {
     assertThat(result.get(0).requirements).containsExactly(reqInfo);
   }
 
+  @Test
+  public void submittableQueryRuleNotReady() throws Exception {
+    ChangeApi change = newChangeApi();
+
+    // Satisfy the default rule.
+    approveChange(change);
+
+    // The custom rule is NOT_READY.
+    rule.block(true);
+    change.index();
+
+    assertThat(queryIsSubmittable()).isEmpty();
+  }
+
+  @Test
+  public void submittableQueryRuleError() throws Exception {
+    ChangeApi change = newChangeApi();
+
+    // Satisfy the default rule.
+    approveChange(change);
+
+    rule.status(Optional.of(SubmitRecord.Status.RULE_ERROR));
+    change.index();
+
+    assertThat(queryIsSubmittable()).isEmpty();
+  }
+
+  @Test
+  public void submittableQueryDefaultRejected() throws Exception {
+    ChangeApi change = newChangeApi();
+
+    // CodeReview:-2 the change, causing the default rule to fail.
+    rejectChange(change);
+
+    rule.status(Optional.of(SubmitRecord.Status.OK));
+    change.index();
+
+    assertThat(queryIsSubmittable()).isEmpty();
+  }
+
+  @Test
+  public void submittableQueryRuleOk() throws Exception {
+    ChangeApi change = newChangeApi();
+
+    // Satisfy the default rule.
+    approveChange(change);
+
+    rule.status(Optional.of(SubmitRecord.Status.OK));
+    change.index();
+
+    List<ChangeInfo> result = queryIsSubmittable();
+    assertThat(result).hasSize(1);
+    assertThat(result.get(0).changeId).isEqualTo(change.info().changeId);
+  }
+
+  @Test
+  public void submittableQueryRuleNoRecord() throws Exception {
+    ChangeApi change = newChangeApi();
+
+    // Satisfy the default rule.
+    approveChange(change);
+
+    // Our custom rule isn't providing any submit records.
+    rule.status(Optional.empty());
+    change.index();
+
+    // is:submittable should return the change, since it was approved and the custom rule is not
+    // blocking it.
+    List<ChangeInfo> result = queryIsSubmittable();
+    assertThat(result).hasSize(1);
+    assertThat(result.get(0).changeId).isEqualTo(change.info().changeId);
+  }
+
+  private List<ChangeInfo> queryIsSubmittable() throws Exception {
+    return gApi.changes().query("is:submittable project:" + project.get()).get();
+  }
+
+  private ChangeApi newChangeApi() throws Exception {
+    return gApi.changes().id(createChange().getChangeId());
+  }
+
+  private void approveChange(ChangeApi changeApi) throws Exception {
+    changeApi.current().review(ReviewInput.approve());
+  }
+
+  private void rejectChange(ChangeApi changeApi) throws Exception {
+    changeApi.current().review(ReviewInput.reject());
+  }
+
   @Singleton
   private static class CustomSubmitRule implements SubmitRule {
-    private final AtomicBoolean block = new AtomicBoolean(true);
+    private Optional<SubmitRecord.Status> recordStatus = Optional.empty();
 
     public void block(boolean block) {
-      this.block.set(block);
+      this.status(block ? Optional.of(SubmitRecord.Status.NOT_READY) : Optional.empty());
+    }
+
+    public void status(Optional<SubmitRecord.Status> status) {
+      this.recordStatus = status;
     }
 
     @Override
     public Optional<SubmitRecord> evaluate(ChangeData changeData) {
-      if (block.get()) {
+      if (this.recordStatus.isPresent()) {
         SubmitRecord record = new SubmitRecord();
         record.labels = new ArrayList<>();
-        record.status = SubmitRecord.Status.NOT_READY;
+        record.status = this.recordStatus.get();
         record.requirements = ImmutableList.of(req);
         return Optional.of(record);
       }

polygerrit-ui/app/rollup.config.js

@@ -69,7 +69,13 @@ export default {
   output: {
     format: 'iife',
     compact: true,
-    plugins: [terser()]
+    plugins: [
+      terser({
+        output: {
+          comments: false
+        }
+      })
+    ]
   },
   //Context must be set to window to correctly processing global variables
   context: 'window',