diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 5175f9765d..0a421f8732 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -118,7 +118,7 @@ public class Abandon implements RestModifyView, } @Override - public void updateChange(ChangeContext ctx) throws OrmException, + public boolean updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException { change = ctx.getChange(); PatchSet.Id psId = change.currentPatchSetId(); @@ -137,6 +137,7 @@ public class Abandon implements RestModifyView, update.setStatus(change.getStatus()); message = newMessage(ctx.getDb()); cmUtil.addChangeMessage(ctx.getDb(), update, message); + return true; } private ChangeMessage newMessage(ReviewDb db) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index ef35045fa8..31c88e049f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -292,7 +292,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { } @Override - public void updateChange(ChangeContext ctx) throws OrmException, IOException { + public boolean updateChange(ChangeContext ctx) throws OrmException, IOException { change = ctx.getChange(); // Use defensive copy created by ChangeControl. ReviewDb db = ctx.getDb(); ChangeControl ctl = ctx.getControl(); @@ -335,6 +335,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { changeMessage.setMessage(message); cmUtil.addChangeMessage(db, update, changeMessage); } + return true; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java index 6461303de7..6b8d8db58d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java @@ -104,7 +104,7 @@ public class CreateDraftComment implements RestModifyView maybeComment = plcUtil.get(ctx.getDb(), ctx.getNotes(), key); if (!maybeComment.isPresent()) { - return; // Nothing to do. + return false; // Nothing to do. } PatchSet.Id psId = key.getParentKey().getParentKey(); PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId); @@ -102,6 +102,7 @@ public class DeleteDraftComment setCommentRevId(c, patchListCache, ctx.getChange(), ps); plcUtil.deleteComments( ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c)); + return true; } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java index 9d4b91745f..34c25e57e7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java @@ -100,11 +100,11 @@ public class DeleteDraftPatchSet implements RestModifyView { } @Override - public void updateChange(ChangeContext ctx) + public boolean updateChange(ChangeContext ctx) throws OrmException, AuthException, ResourceNotFoundException { IdentifiedUser user = ctx.getUser().asIdentifiedUser(); Change change = ctx.getChange(); @@ -135,6 +135,7 @@ public class DeleteVote implements RestModifyView { cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), changeMessage); } + return true; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index e278a9219b..51aaedbfe0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -204,7 +204,7 @@ public class PatchSetInserter extends BatchUpdate.Op { } @Override - public void updateChange(ChangeContext ctx) throws OrmException, + public boolean updateChange(ChangeContext ctx) throws OrmException, InvalidChangeOperationException, IOException { ChangeControl ctl = ctx.getControl(); @@ -249,6 +249,7 @@ public class PatchSetInserter extends BatchUpdate.Op { if (changeMessage != null) { cmUtil.addChangeMessage(db, update, changeMessage); } + return true; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index a4381a596a..ab795f4a97 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -354,7 +354,7 @@ public class PostReview implements RestModifyView } @Override - public void updateChange(ChangeContext ctx) + public boolean updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException { user = ctx.getUser().asIdentifiedUser(); change = ctx.getChange(); @@ -369,6 +369,7 @@ public class PostReview implements RestModifyView if (dirty) { ctx.saveChange(); } + return dirty; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java index e93a9d825c..cdc6daabce 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java @@ -168,7 +168,7 @@ public class PublishDraftPatchSet implements RestModifyView maybeComment = plcUtil.get(ctx.getDb(), ctx.getNotes(), key); @@ -152,6 +152,7 @@ public class PutDraftComment implements RestModifyView, } @Override - public void updateChange(ChangeContext ctx) throws OrmException { + public boolean updateChange(ChangeContext ctx) throws OrmException { change = ctx.getChange(); ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId()); newTopicName = Strings.nullToEmpty(input.topic); oldTopicName = Strings.nullToEmpty(change.getTopic()); if (oldTopicName.equals(newTopicName)) { - return; + return false; } String summary; if (oldTopicName.isEmpty()) { @@ -128,6 +128,7 @@ public class PutTopic implements RestModifyView, change.currentPatchSetId()); cmsg.setMessage(summary); cmUtil.addChangeMessage(ctx.getDb(), update, cmsg); + return true; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java index 346d9492ae..b4ad49f6cf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -152,10 +152,11 @@ public class RebaseChangeOp extends BatchUpdate.Op { } @Override - public void updateChange(ChangeContext ctx) + public boolean updateChange(ChangeContext ctx) throws OrmException, InvalidChangeOperationException, IOException { - patchSetInserter.updateChange(ctx); + boolean ret = patchSetInserter.updateChange(ctx); rebasedPatchSet = patchSetInserter.getPatchSet(); + return ret; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java index be2d9af03a..a195e462fe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java @@ -108,7 +108,7 @@ public class Restore implements RestModifyView, } @Override - public void updateChange(ChangeContext ctx) throws OrmException, + public boolean updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException { caller = ctx.getUser().asIdentifiedUser(); change = ctx.getChange(); @@ -125,6 +125,7 @@ public class Restore implements RestModifyView, message = newMessage(ctx.getDb()); cmUtil.addChangeMessage(ctx.getDb(), update, message); + return true; } private ChangeMessage newMessage(ReviewDb db) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java index 48a17b4277..5e75f96bc2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java @@ -73,12 +73,12 @@ public class SetHashtagsOp extends BatchUpdate.Op { } @Override - public void updateChange(ChangeContext ctx) + public boolean updateChange(ChangeContext ctx) throws AuthException, BadRequestException, OrmException, IOException { if (input == null || (input.add == null && input.remove == null)) { updatedHashtags = ImmutableSortedSet.of(); - return; + return false; } if (!ctx.getControl().canEditHashtags()) { throw new AuthException("Editing hashtags not permitted"); @@ -112,6 +112,7 @@ public class SetHashtagsOp extends BatchUpdate.Op { } updatedHashtags = ImmutableSortedSet.copyOf(updated); + return true; } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java index 3e0c67cd81..909a453663 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java @@ -221,7 +221,8 @@ public class BatchUpdate implements AutoCloseable { } @SuppressWarnings("unused") - public void updateChange(ChangeContext ctx) throws Exception { + public boolean updateChange(ChangeContext ctx) throws Exception { + return false; } // TODO(dborowitz): Support async operations? @@ -542,10 +543,11 @@ public class BatchUpdate implements AutoCloseable { Change.Id id = e.getKey(); db.changes().beginTransaction(id); ChangeContext ctx; + boolean dirty = false; try { ctx = newChangeContext(id); for (Op op : e.getValue()) { - op.updateChange(ctx); + dirty |= op.updateChange(ctx); } Iterable changes = Collections.singleton(ctx.getChange()); if (newChanges.containsKey(id)) { @@ -555,26 +557,30 @@ public class BatchUpdate implements AutoCloseable { } else if (ctx.deleted) { db.changes().delete(changes); } - db.commit(); + if (dirty) { + db.commit(); + } } finally { db.rollback(); } - BatchMetaDataUpdate bmdu = null; - for (ChangeUpdate u : ctx.updates.values()) { - if (bmdu == null) { - bmdu = u.openUpdate(); + if (dirty) { + BatchMetaDataUpdate bmdu = null; + for (ChangeUpdate u : ctx.updates.values()) { + if (bmdu == null) { + bmdu = u.openUpdate(); + } + u.writeCommit(bmdu); + } + if (bmdu != null) { + bmdu.commit(); } - u.writeCommit(bmdu); - } - if (bmdu != null) { - bmdu.commit(); - } - if (ctx.deleted) { - indexFutures.add(indexer.deleteAsync(id)); - } else { - indexFutures.add(indexer.indexAsync(id)); + if (ctx.deleted) { + indexFutures.add(indexer.deleteAsync(id)); + } else { + indexFutures.add(indexer.indexAsync(id)); + } } } } catch (Exception e) { 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 f809f75062..037367d6b8 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 @@ -1810,8 +1810,9 @@ public class ReceiveCommits { changeId, new BatchUpdate.Op() { @Override - public void updateChange(ChangeContext ctx) throws Exception { + public boolean updateChange(ChangeContext ctx) { ctx.getUpdate(psId).setTopic(magicBranch.topic); + return true; } }); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java index e3d98437f3..3d527a513a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java @@ -162,11 +162,11 @@ public class CherryPick extends SubmitStrategy { } @Override - public void updateChange(ChangeContext ctx) throws OrmException, + public boolean updateChange(ChangeContext ctx) throws OrmException, NoSuchChangeException { if (newCommit == null) { // Merge conflict; don't update change. - return; + return false; } PatchSet prevPs = args.psUtil.current(ctx.getDb(), ctx.getNotes()); @@ -185,6 +185,7 @@ public class CherryPick extends SubmitStrategy { newCommit.setControl( args.changeControlFactory.controlFor(toMerge.change(), args.caller)); + return true; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index ec6e1f258b..a3b7c11643 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -170,11 +170,11 @@ public class RebaseIfNecessary extends SubmitStrategy { } @Override - public void updateChange(ChangeContext ctx) throws NoSuchChangeException, + public boolean updateChange(ChangeContext ctx) throws NoSuchChangeException, InvalidChangeOperationException, OrmException, IOException { if (rebaseOp == null) { // Took the fast-forward option, nothing to do. - return; + return false; } rebaseOp.updateChange(ctx); @@ -191,6 +191,7 @@ public class RebaseIfNecessary extends SubmitStrategy { newPatchSetId)); newCommit.setControl( args.changeControlFactory.controlFor(toMerge.change(), args.caller)); + return true; } @Override