diff --git a/gerrit-reviewdb/BUCK b/gerrit-reviewdb/BUCK index 7bed0f38c5..82e01351c7 100644 --- a/gerrit-reviewdb/BUCK +++ b/gerrit-reviewdb/BUCK @@ -19,6 +19,7 @@ java_library( resources = glob(['src/main/resources/**/*']), deps = [ '//gerrit-extension-api:api', + '//lib:guava', '//lib:gwtorm', ], visibility = ['PUBLIC'], diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java new file mode 100644 index 0000000000..7cec109db2 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java @@ -0,0 +1,41 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb.server; + +import com.google.common.base.Function; +import com.google.common.collect.Ordering; +import com.google.gwtorm.client.IntKey; + +/** Static utilities for ReviewDb types. */ +public class ReviewDbUtil { + private static final Function, Integer> INT_KEY_FUNCTION = + new Function, Integer>() { + @Override + public Integer apply(IntKey in) { + return in.get(); + } + }; + + private static final Ordering> INT_KEY_ORDERING = + Ordering.natural().onResultOf(INT_KEY_FUNCTION); + + @SuppressWarnings("unchecked") + public static > Ordering intKeyOrdering() { + return (Ordering) INT_KEY_ORDERING; + } + + private ReviewDbUtil() { + } +} 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 2c934d66b0..85516cf772 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 @@ -119,14 +119,15 @@ public class Abandon implements RestModifyView, public void updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException { change = ctx.getChange(); - ChangeUpdate update = ctx.getUpdate(); + PatchSet.Id psId = change.currentPatchSetId(); + ChangeUpdate update = ctx.getUpdate(psId); if (change == null || !change.getStatus().isOpen()) { throw new ResourceConflictException("change is " + status(change)); } else if (change.getStatus() == Change.Status.DRAFT) { throw new ResourceConflictException( "draft changes cannot be abandoned"); } - patchSet = ctx.getDb().patchSets().get(change.currentPatchSetId()); + patchSet = ctx.getDb().patchSets().get(psId); change.setStatus(Change.Status.ABANDONED); change.setLastUpdatedOn(ctx.getWhen()); ctx.getDb().changes().update(Collections.singleton(change)); 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 5e7d1c5884..1535422ccd 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 @@ -242,10 +242,10 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { change = ctx.getChange(); // Use defensive copy created by ChangeControl. ReviewDb db = ctx.getDb(); ChangeControl ctl = ctx.getControl(); - ChangeUpdate update = ctx.getUpdate(); patchSetInfo = patchSetInfoFactory.get( ctx.getRevWalk(), commit, patchSet.getId()); ctx.getChange().setCurrentPatchSet(patchSetInfo); + ChangeUpdate update = ctx.getUpdate(patchSet.getId()); if (patchSet.getGroups() == null) { patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet)); 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 fe1bd59867..2143604dd6 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 @@ -121,7 +121,7 @@ public class CreateDraftComment implements RestModifyView { .append("\n"); psa = a; a.setValue((short)0); - ctx.getUpdate().setPatchSetId(psId); - ctx.getUpdate().removeApprovalFor(a.getAccountId(), label); + ctx.getUpdate(psId).removeApprovalFor(a.getAccountId(), label); break; } } else { @@ -133,7 +132,7 @@ public class DeleteVote implements RestModifyView { ctx.getWhen(), change.currentPatchSetId()); changeMessage.setMessage(msg.toString()); - cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(), + cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), changeMessage); } } 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 daca350cf2..1810dbdded 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 @@ -209,7 +209,7 @@ public class PatchSetInserter extends BatchUpdate.Op { ChangeControl ctl = ctx.getControl(); change = ctx.getChange(); - ChangeUpdate update = ctx.getUpdate(); + ChangeUpdate update = ctx.getUpdate(psId); if (!change.getStatus().isOpen() && !allowClosed) { throw new InvalidChangeOperationException(String.format( @@ -222,8 +222,6 @@ public class PatchSetInserter extends BatchUpdate.Op { patchSet.setRevision(new RevId(commit.name())); patchSet.setDraft(draft); - update.setPatchSetId(patchSet.getId()); - if (groups != null) { patchSet.setGroups(groups); } else { @@ -251,7 +249,7 @@ public class PatchSetInserter extends BatchUpdate.Op { db.changes().update(Collections.singleton(change)); approvalCopier.copy(db, ctl, patchSet); if (changeMessage != null) { - cmUtil.addChangeMessage(db, ctx.getUpdate(), changeMessage); + cmUtil.addChangeMessage(db, update, changeMessage); } } 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 1730e2f3cd..99e27d6d14 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 @@ -358,7 +358,6 @@ public class PostReview implements RestModifyView change.setLastUpdatedOn(ctx.getWhen()); } ps = ctx.getDb().patchSets().get(psId); - ctx.getUpdate().setPatchSetId(psId); boolean dirty = false; dirty |= insertComments(ctx); dirty |= updateLabels(ctx); @@ -466,8 +465,11 @@ public class PostReview implements RestModifyView } break; } - plcUtil.deleteComments(ctx.getDb(), ctx.getUpdate(), del); - plcUtil.upsertComments(ctx.getDb(), ctx.getUpdate(), ups); + ChangeUpdate u = ctx.getUpdate(psId); + // TODO(dborowitz): Currently doesn't work for PUBLISH_ALL_REVISIONS with + // notedb. + plcUtil.deleteComments(ctx.getDb(), u, del); + plcUtil.upsertComments(ctx.getDb(), u, ups); comments.addAll(ups); return !del.isEmpty() || !ups.isEmpty(); } @@ -513,7 +515,7 @@ public class PostReview implements RestModifyView List ups = Lists.newArrayList(); Map current = scanLabels(ctx, del); - ChangeUpdate update = ctx.getUpdate(); + ChangeUpdate update = ctx.getUpdate(psId); LabelTypes labelTypes = ctx.getControl().getLabelTypes(); for (Map.Entry ent : labels.entrySet()) { String name = ent.getKey(); @@ -644,7 +646,7 @@ public class PostReview implements RestModifyView "Patch Set %d:%s", psId.get(), buf.toString())); - cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(), message); + cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), message); return true; } 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 5f5c4cd7a4..68a05ee9e0 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 @@ -182,7 +182,7 @@ public class PublishDraftPatchSet implements RestModifyView, @Override public void 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)) { @@ -116,7 +118,7 @@ public class PutTopic implements RestModifyView, oldTopicName, newTopicName); } change.setTopic(Strings.emptyToNull(newTopicName)); - ctx.getUpdate().setTopic(change.getTopic()); + update.setTopic(change.getTopic()); ChangeUtil.updated(change); ctx.getDb().changes().update(Collections.singleton(change)); @@ -127,7 +129,7 @@ public class PutTopic implements RestModifyView, caller.getAccountId(), ctx.getWhen(), change.currentPatchSetId()); cmsg.setMessage(summary); - cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(), cmsg); + cmUtil.addChangeMessage(ctx.getDb(), update, cmsg); } @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 bc3301af5c..0da733c807 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 @@ -110,11 +110,12 @@ public class Restore implements RestModifyView, ResourceConflictException { caller = ctx.getUser().asIdentifiedUser(); change = ctx.getChange(); - ChangeUpdate update = ctx.getUpdate(); + PatchSet.Id psId = change.currentPatchSetId(); + ChangeUpdate update = ctx.getUpdate(psId); if (change == null || change.getStatus() != Status.ABANDONED) { throw new ResourceConflictException("change is " + status(change)); } - patchSet = ctx.getDb().patchSets().get(change.currentPatchSetId()); + patchSet = ctx.getDb().patchSets().get(psId); change.setStatus(Status.NEW); change.setLastUpdatedOn(ctx.getWhen()); ctx.getDb().changes().update(Collections.singleton(change)); 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 c590e03b63..48a17b4277 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 @@ -83,7 +83,8 @@ public class SetHashtagsOp extends BatchUpdate.Op { if (!ctx.getControl().canEditHashtags()) { throw new AuthException("Editing hashtags not permitted"); } - ChangeUpdate update = ctx.getUpdate(); + change = ctx.getChange(); + ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId()); ChangeNotes notes = update.getChangeNotes().load(); Set existingHashtags = notes.getHashtags(); @@ -110,7 +111,6 @@ public class SetHashtagsOp extends BatchUpdate.Op { update.setHashtags(updated); } - change = update.getChange(); updatedHashtags = ImmutableSortedSet.copyOf(updated); } 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 77026b1f22..8755ffa839 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 @@ -25,11 +25,14 @@ import com.google.common.collect.ListMultimap; import com.google.common.util.concurrent.CheckedFuture; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate; import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; @@ -54,6 +57,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.TimeZone; +import java.util.TreeMap; /** * Context for a set of updates that should be applied for a site. @@ -159,20 +163,27 @@ public class BatchUpdate implements AutoCloseable { public class ChangeContext extends Context { private final ChangeControl ctl; - private final ChangeUpdate update; + private final Map updates; + private boolean deleted; private ChangeContext(ChangeControl ctl) { this.ctl = ctl; - this.update = changeUpdateFactory.create(ctl, when); + updates = new TreeMap<>(ReviewDbUtil.intKeyOrdering()); } - public ChangeUpdate getUpdate() { - return update; + public ChangeUpdate getUpdate(PatchSet.Id psId) { + ChangeUpdate u = updates.get(psId); + if (u == null) { + u = changeUpdateFactory.create(ctl, when); + u.setPatchSetId(psId); + updates.put(psId, u); + } + return u; } public ChangeNotes getNotes() { - return update.getChangeNotes(); + return ctl.getNotes(); } public ChangeControl getControl() { @@ -180,7 +191,7 @@ public class BatchUpdate implements AutoCloseable { } public Change getChange() { - return update.getChange(); + return ctl.getChange(); } public void markDeleted() { @@ -523,7 +534,18 @@ public class BatchUpdate implements AutoCloseable { } finally { db.rollback(); } - ctx.getUpdate().commit(); + + BatchMetaDataUpdate bmdu = null; + for (ChangeUpdate u : ctx.updates.values()) { + if (bmdu == null) { + bmdu = u.openUpdate(); + } + u.writeCommit(bmdu); + } + if (bmdu != null) { + bmdu.commit(); + } + if (ctx.deleted) { indexFutures.add(indexer.deleteAsync(id)); } else { 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 c628c45229..8defdb01dc 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 @@ -1807,7 +1807,7 @@ public class ReceiveCommits { new BatchUpdate.Op() { @Override public void updateChange(ChangeContext ctx) throws Exception { - ctx.getUpdate().setTopic(magicBranch.topic); + ctx.getUpdate(ps.getId()).setTopic(magicBranch.topic); } }); } 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 e27bd5f799..7926eac6b0 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 @@ -167,7 +167,6 @@ public class CherryPick extends SubmitStrategy { // Merge conflict; don't update change. return; } - ctx.getUpdate().setPatchSetId(psId); PatchSet ps = new PatchSet(psId); ps.setCreatedOn(ctx.getWhen()); ps.setUploader(args.caller.getAccountId()); @@ -183,7 +182,7 @@ public class CherryPick extends SubmitStrategy { for (PatchSetApproval a : args.approvalsUtil.byPatchSet( args.db, toMerge.getControl(), toMerge.getPatchsetId())) { approvals.add(new PatchSetApproval(ps.getId(), a)); - ctx.getUpdate().putApproval(a.getLabel(), a.getValue()); + ctx.getUpdate(psId).putApproval(a.getLabel(), a.getValue()); } args.db.patchSetApprovals().insert(approvals); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index f896d89e89..d948a150c4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -386,9 +386,6 @@ public class ChangeUpdate extends AbstractChangeUpdate { BatchMetaDataUpdate batch = openUpdate(); try { writeCommit(batch); - if (draftUpdate != null) { - draftUpdate.commit(); - } RevCommit c = batch.commit(); return c; } catch (OrmException e) { @@ -409,6 +406,9 @@ public class ChangeUpdate extends AbstractChangeUpdate { } } batch.write(this, builder); + if (draftUpdate != null) { + draftUpdate.commit(); + } } @Override