Do not expect ApprovalType of Category SUBM

Pushing a new, rebased Patch Set for a Change failed with a
NullPointer if the first Patch Set could not be merged due
to a patch conflict.

The reason was that there is no ApprovalType of Category SUBM
anymore, but it is still stored in the database.

Change-Id: I058ba99b835813245283c04fdb08199c94957645
Signed-off-by: Stefan Lay <stefan.lay@sap.com>

5 files changed, 29 insertions, 17 deletions

diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
index a76d0dfa3d..c81c2e9ba8 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
@@ -187,11 +187,13 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
             for (final PatchSetApproval ca : db.patchSetApprovals()
                 .byPatchSetUser(ps_id, aid)) {
               final ApprovalCategory.Id category = ca.getCategoryId();
+              if (ApprovalCategory.SUBMIT.equals(category)) {
+                continue;
+              }
               if (change.getStatus().isOpen()) {
                 fs.normalize(approvalTypes.byId(category), ca);
               }
-              if (ca.getValue() == 0
-                  || ApprovalCategory.SUBMIT.equals(category)) {
+              if (ca.getValue() == 0) {
                 continue;
               }
               psas.put(category, ca);
@@ -231,11 +233,13 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
 
             for (PatchSetApproval ca : db.patchSetApprovals().byPatchSet(ps_id)) {
               final ApprovalCategory.Id category = ca.getCategoryId();
+              if (ApprovalCategory.SUBMIT.equals(category)) {
+                continue;
+              }
               if (change.getStatus().isOpen()) {
                 fs.normalize(approvalTypes.byId(category), ca);
               }
-              if (ca.getValue() == 0
-                  || ApprovalCategory.SUBMIT.equals(category)) {
+              if (ca.getValue() == 0) {
                 continue;
               }
               boolean keep = true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
index c46a114c72..8205946ec8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
@@ -433,8 +433,10 @@ public class ChangeHookRunner {
             Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval) {
         ApprovalAttribute a = new ApprovalAttribute();
         a.type = approval.getKey().get();
-        final ApprovalType at = approvalTypes.byId(approval.getKey());
-        a.description = at.getCategory().getName();
+        ApprovalType at = approvalTypes.byId(approval.getKey());
+        if (at != null) {
+          a.description = at.getCategory().getName();
+        }
         a.value = Short.toString(approval.getValue().get());
         return a;
     }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java
index 112ed4d9fb..a7f4d98740 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.git;
 
+import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.reviewdb.ApprovalCategory;
 import com.google.gerrit.reviewdb.PatchSetApproval;
@@ -212,10 +213,10 @@ public class CreateCodeReviewNotes {
         if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
           submit = a;
         } else {
-          ApprovalCategory type = approvalTypes.byId(a.getCategoryId()).getCategory();
+          ApprovalType type = approvalTypes.byId(a.getCategoryId());
           if (type != null) {
             formatter.appendApproval(
-                type,
+                type.getCategory(),
                 a.getValue(),
                 accountCache.get(a.getAccountId()).getAccount());
           }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 84f3422223..94c9d3c67d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -1358,15 +1358,18 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
         oldCC.add(a.getAccountId());
       }
 
-      final ApprovalType type =
+      // ApprovalCategory.SUBMIT is still in db but not relevant in git-store
+      if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
+        final ApprovalType type =
           approvalTypes.byId(a.getCategoryId());
-      if (a.getPatchSetId().equals(priorPatchSet)
-          && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) {
-        // If there was a negative vote on the prior patch set, carry it
-        // into this patch set.
-        //
-        db.patchSetApprovals().insert(
-            Collections.singleton(new PatchSetApproval(ps.getId(), a)));
+        if (a.getPatchSetId().equals(priorPatchSet)
+            && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) {
+          // If there was a negative vote on the prior patch set, carry it
+          // into this patch set.
+          //
+          db.patchSetApprovals().insert(
+              Collections.singleton(new PatchSetApproval(ps.getId(), a)));
+        }
       }
 
       if (!haveAuthor && authorId != null && a.getAccountId().equals(authorId)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java
index 7d25ee8d3b..db297aec17 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java
@@ -171,7 +171,9 @@ public class PublishComments implements Callable<VoidResult> {
       final short o = a.getValue();
       a.setValue(want.get());
       a.cache(change);
-      functionState.normalize(types.byId(a.getCategoryId()), a);
+      if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
+        functionState.normalize(types.byId(a.getCategoryId()), a);
+      }
       if (o != a.getValue()) {
         // Value changed, ensure we update the database.
         //