Skip to content

Commit 81c7aa5

Browse files
jujokinijujokini
committed
Add defensive checks around status changes
Task-number: QTQAINFRA-3029 Change-Id: I447e56c6ed0654731c17094c495a95b93ad6c31d Reviewed-by: Paul Wicking <[email protected]> Reviewed-by: Jukka Jokiniva <[email protected]> Reviewed-by: Kari Oikarinen <[email protected]>
1 parent b63c0de commit 81c7aa5

File tree

10 files changed

+17
-10
lines changed

10 files changed

+17
-10
lines changed

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtAbandon.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ protected ChangeInfo applyImpl(BatchUpdate.Factory updateFactory,
7979
}
8080

8181
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.ABANDONED,
82+
null,
8283
"Abandoned",
8384
input.message,
8485
ChangeMessagesUtil.TAG_ABANDON,

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtChangeUpdateOp.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@
3232
public class QtChangeUpdateOp implements BatchUpdateOp {
3333

3434
public interface Factory {
35-
QtChangeUpdateOp create(Change.Status newStatus,
35+
QtChangeUpdateOp create(@Assisted("newStatus") Change.Status newStatus,
36+
@Assisted("oldStatus") Change.Status oldStatus,
3637
@Assisted("defaultMessage") String defaultMessage,
3738
@Assisted("inputMessage") String inputMessage,
3839
@Assisted("tag") String tag,
@@ -42,6 +43,7 @@ QtChangeUpdateOp create(Change.Status newStatus,
4243
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
4344

4445
private final Change.Status newStatus;
46+
private final Change.Status oldStatus;
4547
private final String defaultMessage;
4648
private final String inputMessage;
4749
private final String tag;
@@ -58,7 +60,8 @@ QtChangeUpdateOp create(Change.Status newStatus,
5860
QtChangeUpdateOp(ChangeMessagesUtil cmUtil,
5961
ApprovalsUtil approvalsUtil,
6062
LabelNormalizer labelNormalizer,
61-
@Nullable @Assisted Change.Status newStatus,
63+
@Nullable @Assisted("newStatus") Change.Status newStatus,
64+
@Nullable @Assisted("oldStatus") Change.Status oldStatus,
6265
@Nullable @Assisted("defaultMessage") String defaultMessage,
6366
@Nullable @Assisted("inputMessage") String inputMessage,
6467
@Nullable @Assisted("tag") String tag,
@@ -67,6 +70,7 @@ QtChangeUpdateOp create(Change.Status newStatus,
6770
this.approvalsUtil = approvalsUtil;
6871
this.labelNormalizer = labelNormalizer;
6972
this.newStatus = newStatus;
73+
this.oldStatus = oldStatus;
7074
this.defaultMessage = defaultMessage;
7175
this.inputMessage = inputMessage;
7276
this.tag = tag;
@@ -84,7 +88,7 @@ public boolean updateChange(ChangeContext ctx) throws OrmException, IOException,
8488
PatchSet.Id psId = change.currentPatchSetId();
8589
ChangeUpdate update = ctx.getUpdate(psId);
8690

87-
if (newStatus != null) {
91+
if (newStatus != null && (oldStatus == null || change.getStatus() == oldStatus)) {
8892
change.setStatus(newStatus);
8993
update.fixStatus(newStatus);
9094
updated = true;

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,15 @@ public CodeReviewCommit cherryPickPatch(ChangeData changeData,
159159
if (!patchSetNotChanged && !mergeCommit) {
160160
Change.Id changeId = insertPatchSet(bu, git, changeData.notes(), cherryPickCommit);
161161
bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus,
162+
null,
162163
defaultMessage,
163164
inputMessage,
164165
tag,
165166
commitToCherryPick));
166167
logger.atInfo().log("qtcodereview: cherrypick new patch %s for %s", cherryPickCommit.toObjectId(), changeId);
167168
} else {
168169
bu.addOp(changeData.getId(), qtUpdateFactory.create(newStatus,
170+
null,
169171
defaultMessage,
170172
inputMessage,
171173
tag,

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandAdminChangeStatus.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ protected void run() throws UnloggedFailure {
8383
ChangeData change = list.get(0);
8484

8585
Project.NameKey projectKey = QtUtil.getProjectKey(project);
86-
QtChangeUpdateOp op = qtUpdateFactory.create(to, null, null, null, null);
86+
QtChangeUpdateOp op = qtUpdateFactory.create(to, null, null, null, null, null);
8787
try (BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
8888
Change c = change.change();
8989
if (c.getStatus() == from) {

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ private void updateChanges(List<Entry<ChangeData,RevCommit>> list,
247247
Boolean passed)
248248
throws UpdateException, RestApiException, OrmException {
249249
// do the db update
250-
QtChangeUpdateOp op = qtUpdateFactory.create(status, changeMessage, null, tag, null);
250+
QtChangeUpdateOp op = qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, null);
251251
try (BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
252252
for (Entry<ChangeData,RevCommit> item : list) {
253253
Change change = item.getKey().change();

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ protected void run() throws UnloggedFailure {
128128
throw die("No changes in staging branch. Not creating a build reference");
129129
}
130130

131-
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, message, null, null, null);
131+
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.INTEGRATING, Change.Status.STAGED, message, null, null, null);
132132
try (BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
133133
for (Entry<ChangeData, RevCommit> item: openChanges) {
134134
Change change = item.getKey().change();

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtDefer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ protected ChangeInfo applyImpl(BatchUpdate.Factory updateFactory,
9090
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
9191
}
9292

93-
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, "Deferred", input.message, null, null);
93+
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.DEFERRED, null, "Deferred", input.message, null, null);
9494
try (BatchUpdate u = updateFactory.create(dbProvider.get(), change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
9595
u.addOp(rsrc.getId(), op).execute();
9696
}

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtReOpen.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected ChangeInfo applyImpl(BatchUpdate.Factory updateFactory,
8484
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
8585
}
8686

87-
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, "Reopened", input.message, null, null);
87+
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.DEFERRED, "Reopened", input.message, null, null);
8888
try (BatchUpdate u = updateFactory.create(dbProvider.get(), change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
8989
u.addOp(rsrc.getId(), op).execute();
9090
}

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUnStage.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ private Change removeChangeFromStaging(RevisionResource rsrc, IdentifiedUser sub
142142
throw new ResourceConflictException("Invalid Revision: " + patchSet);
143143
}
144144

145-
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, "Unstaged", null, null, null);
145+
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, "Unstaged", null, null, null);
146146
BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, submitter, TimeUtil.nowTs());
147147
u.addOp(rsrc.getChange().getId(), op).execute();
148148

‎src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ public void rebuildStagingBranch(Repository git,
417417
newObjId = null;
418418
logger.atInfo().log("qtcodereview: rebuild staging ref %s merge conflict", stagingBranchKey);
419419
String message = "Merge conflict in staging branch. Status changed back to new. Please stage again.";
420-
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, message, null, null, null);
420+
QtChangeUpdateOp op = qtUpdateFactory.create(Change.Status.NEW, Change.Status.STAGED, message, null, null, null);
421421
try (BatchUpdate u = updateFactory.create(dbProvider.get(), projectKey, user, TimeUtil.nowTs())) {
422422
for (ChangeData item: changes_staged) {
423423
Change change = item.change();

0 commit comments

Comments
 (0)