Add defensive checks around status changes

Task-number: QTQAINFRA-3029
Change-Id: I447e56c6ed0654731c17094c495a95b93ad6c31d
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Jukka Jokiniva <jukka.jokiniva@qt.io>
Reviewed-by: Kari Oikarinen <kari.oikarinen@qt.io>

10 files changed, 17 insertions, 10 deletions

diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtAbandon.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtAbandon.java
index 0d37198..9df4a9a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtAbandon.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtAbandon.java
@@ -79,6 +79,7 @@ public class QtAbandon extends RetryingRestModifyView<ChangeResource, AbandonInp
         }
 
         QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.ABANDONED,
+                                                     null,
                                                      "Abandoned",
                                                      input.message,
                                                      ChangeMessagesUtil.TAG_ABANDON,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtChangeUpdateOp.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtChangeUpdateOp.java
index 6db1074..7301484 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtChangeUpdateOp.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtChangeUpdateOp.java
@@ -32,7 +32,8 @@ import java.util.Map;
 public class QtChangeUpdateOp implements BatchUpdateOp {
 
     public interface Factory {
-        QtChangeUpdateOp create(Change.Status newStatus,
+        QtChangeUpdateOp create(@Assisted("newStatus") Change.Status newStatus,
+                                @Assisted("oldStatus") Change.Status oldStatus,
                                 @Assisted("defaultMessage") String defaultMessage,
                                 @Assisted("inputMessage") String inputMessage,
                                 @Assisted("tag") String tag,
@@ -42,6 +43,7 @@ public class QtChangeUpdateOp implements BatchUpdateOp {
     private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
     private final Change.Status newStatus;
+    private final Change.Status oldStatus;
     private final String defaultMessage;
     private final String inputMessage;
     private final String tag;
@@ -58,7 +60,8 @@ public class QtChangeUpdateOp implements BatchUpdateOp {
     QtChangeUpdateOp(ChangeMessagesUtil cmUtil,
                      ApprovalsUtil approvalsUtil,
                      LabelNormalizer labelNormalizer,
-                     @Nullable @Assisted Change.Status newStatus,
+                     @Nullable @Assisted("newStatus") Change.Status newStatus,
+                     @Nullable @Assisted("oldStatus") Change.Status oldStatus,
                      @Nullable @Assisted("defaultMessage") String defaultMessage,
                      @Nullable @Assisted("inputMessage") String inputMessage,
                      @Nullable @Assisted("tag") String tag,
@@ -67,6 +70,7 @@ public class QtChangeUpdateOp implements BatchUpdateOp {
         this.approvalsUtil = approvalsUtil;
         this.labelNormalizer = labelNormalizer;
         this.newStatus = newStatus;
+        this.oldStatus = oldStatus;
         this.defaultMessage = defaultMessage;
         this.inputMessage = inputMessage;
         this.tag = tag;
@@ -84,7 +88,7 @@ public class QtChangeUpdateOp implements BatchUpdateOp {
         PatchSet.Id psId = change.currentPatchSetId();
         ChangeUpdate update = ctx.getUpdate(psId);
 
-        if (newStatus != null) {
+        if (newStatus != null && (oldStatus == null || change.getStatus() == oldStatus)) {
             change.setStatus(newStatus);
             update.fixStatus(newStatus);
             updated = true;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java
index a7ccba1..0169885 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java
@@ -159,6 +159,7 @@ public class QtCherryPickPatch {
             if (!patchSetNotChanged && !mergeCommit) {
                 Change.Id changeId = insertPatchSet(bu, git, changeData.notes(), cherryPickCommit);
                 bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus,
+                                                                    null,
                                                                     defaultMessage,
                                                                     inputMessage,
                                                                     tag,
@@ -166,6 +167,7 @@ public class QtCherryPickPatch {
                 logger.atInfo().log("qtcodereview: cherrypick new patch %s for %s", cherryPickCommit.toObjectId(), changeId);
             } else {
                 bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus,
+                                                                    null,
                                                                     defaultMessage,
                                                                     inputMessage,
                                                                     tag,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandAdminChangeStatus.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandAdminChangeStatus.java
index 255e14e..008b173 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandAdminChangeStatus.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandAdminChangeStatus.java
@@ -83,7 +83,7 @@ class QtCommandAdminChangeStatus extends SshCommand {
             ChangeData change = list.get(0);
 
             Project.NameKey projectKey = QtUtil.getProjectKey(project);
-            QtChangeUpdateOp op = qtUpdateFactory.create(to, null, null, null, null);
+            QtChangeUpdateOp op = qtUpdateFactory.create(to, null, null, null, null, null);
             try (BatchUpdate u =  updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
                 Change c = change.change();
                 if (c.getStatus() == from) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
index fb8058f..872c59c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java
@@ -247,7 +247,7 @@ class QtCommandBuildApprove extends SshCommand {
                                Boolean passed)
                                throws UpdateException, RestApiException, OrmException {
         // do the db update
-        QtChangeUpdateOp op = qtUpdateFactory.create(status, changeMessage, null, tag, null);
+        QtChangeUpdateOp op = qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, null);
         try (BatchUpdate u =  updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
             for (Entry<ChangeData,RevCommit> item : list) {
                 Change change = item.getKey().change();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java
index d6cba28..9068155 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java
@@ -128,7 +128,7 @@ class QtCommandNewBuild extends SshCommand {
                     throw die("No changes in staging branch. Not creating a build reference");
                 }
 
-                QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, message, null, null, null);
+                QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, Change.Status.STAGED, message, null, null, null);
                 try (BatchUpdate u =  updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
                     for (Entry<ChangeData, RevCommit> item: openChanges) {
                         Change change = item.getKey().change();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java
index afd1771..52c87ad 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java
@@ -90,7 +90,7 @@ class QtDefer extends RetryingRestModifyView<ChangeResource, AbandonInput, Chang
             throw new ResourceConflictException("change is " + ChangeUtil.status(change));
         }
 
-        QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, "Deferred", input.message, null, null);
+        QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, null, "Deferred", input.message, null, null);
         try (BatchUpdate u =  updateFactory.create(dbProvider.get(), change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
             u.addOp(rsrc.getId(), op).execute();
         }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java
index fcc1e38..f699df0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java
@@ -84,7 +84,7 @@ class QtReOpen extends RetryingRestModifyView<ChangeResource, RestoreInput, Chan
             throw new ResourceConflictException("change is " + ChangeUtil.status(change));
         }
 
-        QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, "Reopened", input.message, null, null);
+        QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.DEFERRED, "Reopened", input.message, null, null);
         try (BatchUpdate u =  updateFactory.create(dbProvider.get(), change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
             u.addOp(rsrc.getId(), op).execute();
         }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java
index 0b5d167..c8ebbf5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java
@@ -142,7 +142,7 @@ class QtUnStage implements RestModifyView<RevisionResource, SubmitInput>, UiActi
                 throw new ResourceConflictException("Invalid Revision: " + patchSet);
             }
 
-            QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, "Unstaged", null, null, null);
+            QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, "Unstaged", null, null, null);
             BatchUpdate u =  updateFactory.create(dbProvider.get(), projectKey, submitter, TimeUtil.nowTs());
             u.addOp(rsrc.getChange().getId(), op).execute();
 
diff --git a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
index b94190b..3a11d88 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java
@@ -417,7 +417,7 @@ public class QtUtil {
                  newObjId = null;
                  logger.atInfo().log("qtcodereview: rebuild staging ref %s merge conflict", stagingBranchKey);
                  String message = "Merge conflict in staging branch. Status changed back to new. Please stage again.";
-                 QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, message, null, null, null);
+                 QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, message, null, null, null);
                  try (BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
                      for (ChangeData item: changes_staged) {
                          Change change = item.change();