MergeOp: Give helpful error when all changes fail validation

If all changes fail validation, we call continue on each iteration of
the loop and might not be able to choose a submit type. The AutoValue
type wasn't allowing null, causing an NPE that obscured the actual
error.

Allow null for this field and explicitly checkNotNull only after
calling failFast.

Also move the submit type check earlier in the loop so it's more
likely to succeed in the first place.

Bug: Issue 3741
Change-Id: I1353643ba3400b2679c29c564b2538882e19de4b

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

@@ -35,6 +35,7 @@ import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.common.hash.Hasher;
 import com.google.common.hash.Hashing;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.common.data.SubmitTypeRecord;
@@ -620,6 +621,9 @@ public class MergeOp implements AutoCloseable {
         OpenRepo or = getRepo(branch.getParentKey());
         OpenBranch ob = or.getBranch(branch);
         BranchBatch submitting = toSubmit.get(branch);
+        checkNotNull(submitting.submitType(),
+            "null submit type for %s; expected to previously fail fast",
+            submitting);
         Set<CodeReviewCommit> commitsToSubmit = commits(submitting.changes());
         ob.mergeTip = new MergeTip(ob.oldTip, commitsToSubmit);
         SubmitStrategy strategy = createStrategy(or, ob.mergeTip, branch,
@@ -699,7 +703,7 @@ public class MergeOp implements AutoCloseable {
 
   @AutoValue
   static abstract class BranchBatch {
-    abstract SubmitType submitType();
+    @Nullable abstract SubmitType submitType();
     abstract List<ChangeData> changes();
   }
 
@@ -722,6 +726,22 @@ public class MergeOp implements AutoCloseable {
         commits.logProblem(changeId, e);
         continue;
       }
+
+      SubmitType st = getSubmitType(cd);
+      if (st == null) {
+        commits.logProblem(changeId, "No submit type for change");
+        continue;
+      }
+      if (submitType == null) {
+        submitType = st;
+        choseSubmitTypeFrom = cd;
+      } else if (st != submitType) {
+        commits.problem(changeId, String.format(
+            "Change has submit type %s, but previously chose submit type %s "
+            + "from change %s in the same batch",
+            st, submitType, choseSubmitTypeFrom.getId()));
+        continue;
+      }
       if (chg.currentPatchSetId() == null) {
         String msg = "Missing current patch set on change";
         logError(msg + " " + changeId);
@@ -784,22 +804,6 @@ public class MergeOp implements AutoCloseable {
         commits.problem(changeId, mve.getMessage());
         continue;
       }
-
-      SubmitType st = getSubmitType(cd);
-      if (st == null) {
-        commits.logProblem(changeId, "No submit type for change");
-        continue;
-      }
-      if (submitType == null) {
-        submitType = st;
-        choseSubmitTypeFrom = cd;
-      } else if (st != submitType) {
-        commits.problem(changeId, String.format(
-            "Change has submit type %s, but previously chose submit type %s "
-            + "from change %s in the same batch",
-            st, submitType, choseSubmitTypeFrom.getId()));
-        continue;
-      }
       commit.add(or.canMergeFlag);
       toSubmit.add(cd);
     }