opendev
/
gerrit
Code Issues Proposed changes

Merge changes from topic 'mergeop-prep' 

* changes:
  MergeOp: Pre-open all OpenRepos
  MergeOp: Store caller in an instance field
  MergeOp: Hold onto timestamp from submissionId
  MergeUtil: Don't step on existing status in markCleanMerges
  MergeTip: Expose initial tip
  ChangeSet: Expose changes keyed by ID
  ChangeNotesTest: Fix batch writing examples
  BatchUpdate: Key ChangeUpdates by patch set
  BatchUpdate: Shorten ChangeContext method names
  RebaseChangeOp: Expose new commit object in a getter
  AbstractSubmit: Include error message when submit fails
  BatchUpdate: Support executing multiple updates at once
  MergeOp: Encapsulate maps keyed by Change.Id
  BatchUpdate: Support chaining ReceiveCommands on the same ref
  MergeOp: Simplify stamping labels at submit time
  AbstractSubmit: Add extra isNotNull assertion
  RebaseIfNecessary: Don't overwrite committer ident
This commit is contained in:
Dave Borowitz 2016-01-14 14:14:36 +00:00 committed by Gerrit Code Review
parent 8afc549ada c5b1712ae8
commit e4f06b3d95
31 changed files with 540 additions and 371 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@ -211,7 +211,9 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
SubmitInput subm = new SubmitInput();
RestResponse r =
adminSession.post("/changes/" + changeId + "/submit", subm);
assertThat(r.getStatusCode()).isEqualTo(expectedStatus);
assertThat(r.getStatusCode())
.named("Status code [" + r.getEntityContent() + "]")
.isEqualTo(expectedStatus);
if (expectedStatus == HttpStatus.SC_OK) {
checkArgument(msg == null, "msg must be null for successful submits");
ChangeInfo change =
@ -289,6 +291,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change());
PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, cn, new PatchSet.Id(cn.getChangeId(), psId));
assertThat(submitter).isNotNull();
assertThat(submitter.isSubmit()).isTrue();
assertThat(submitter.getAccountId()).isEqualTo(admin.getId());
}

1
gerrit-reviewdb/BUCK
View File

@ -19,6 +19,7 @@ java_library(
resources = glob(['src/main/resources/**/*']),
deps = [
'//gerrit-extension-api:api',
'//lib:guava',
'//lib:gwtorm',
],
visibility = ['PUBLIC'],

41
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java Normal file
View File

@ -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<IntKey<?>, Integer> INT_KEY_FUNCTION =
new Function<IntKey<?>, Integer>() {
@Override
public Integer apply(IntKey<?> in) {
return in.get();
}
};
private static final Ordering<? extends IntKey<?>> INT_KEY_ORDERING =
Ordering.natural().onResultOf(INT_KEY_FUNCTION);
@SuppressWarnings("unchecked")
public static <K extends IntKey<?>> Ordering<K> intKeyOrdering() {
return (Ordering<K>) INT_KEY_ORDERING;
}
private ReviewDbUtil() {
}
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
View File

@ -119,14 +119,15 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
public void updateChange(ChangeContext ctx) throws OrmException,
ResourceConflictException {
change = ctx.getChange();
ChangeUpdate update = ctx.getChangeUpdate();
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));

6
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
View File

@ -241,11 +241,11 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
public void updateChange(ChangeContext ctx) throws OrmException, IOException {
change = ctx.getChange(); // Use defensive copy created by ChangeControl.
ReviewDb db = ctx.getDb();
ChangeControl ctl = ctx.getChangeControl();
ChangeUpdate update = ctx.getChangeUpdate();
ChangeControl ctl = ctx.getControl();
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));
@ -268,7 +268,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
approvalsUtil.addApprovals(db, update, labelTypes, patchSet,
ctx.getChangeControl(), approvals);
ctx.getControl(), approvals);
if (message != null) {
changeMessage =
new ChangeMessage(new ChangeMessage.Key(change.getId(),

2
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
View File

@ -121,7 +121,7 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
setCommentRevId(
comment, patchListCache, ctx.getChange(), ps);
plcUtil.insertComments(
ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(comment));
ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(comment));
}
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
View File

@ -74,7 +74,7 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
if (!allowDrafts) {
throw new MethodNotAllowedException("Draft workflow is disabled");
}
if (!ctx.getChangeControl().canDeleteDraft(ctx.getDb())) {
if (!ctx.getControl().canDeleteDraft(ctx.getDb())) {
throw new AuthException("Not permitted to delete this draft change");
}
List<PatchSet> patchSets = ctx.getDb().patchSets().byChange(id).toList();

4
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
View File

@ -85,7 +85,7 @@ public class DeleteDraftComment
public void updateChange(ChangeContext ctx)
throws ResourceNotFoundException, OrmException {
Optional<PatchLineComment> maybeComment =
plcUtil.get(ctx.getDb(), ctx.getChangeNotes(), key);
plcUtil.get(ctx.getDb(), ctx.getNotes(), key);
if (!maybeComment.isPresent()) {
return; // Nothing to do.
}
@ -97,7 +97,7 @@ public class DeleteDraftComment
PatchLineComment c = maybeComment.get();
setCommentRevId(c, patchListCache, ctx.getChange(), ps);
plcUtil.deleteComments(
ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(c));
ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c));
}
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
View File

@ -108,7 +108,7 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
if (!allowDrafts) {
throw new MethodNotAllowedException("Draft workflow is disabled");
}
if (!ctx.getChangeControl().canDeleteDraft(ctx.getDb())) {
if (!ctx.getControl().canDeleteDraft(ctx.getDb())) {
throw new AuthException("Not permitted to delete this draft patch set");
}

7
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
View File

@ -95,7 +95,7 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
throws OrmException, AuthException, ResourceNotFoundException {
IdentifiedUser user = ctx.getUser().asIdentifiedUser();
Change change = ctx.getChange();
ChangeControl ctl = ctx.getChangeControl();
ChangeControl ctl = ctx.getControl();
PatchSet.Id psId = change.currentPatchSetId();
PatchSetApproval psa = null;
@ -111,8 +111,7 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
.append("\n");
psa = a;
a.setValue((short)0);
ctx.getChangeUpdate().setPatchSetId(psId);
ctx.getChangeUpdate().removeApprovalFor(a.getAccountId(), label);
ctx.getUpdate(psId).removeApprovalFor(a.getAccountId(), label);
break;
}
} else {
@ -133,7 +132,7 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
ctx.getWhen(),
change.currentPatchSetId());
changeMessage.setMessage(msg.toString());
cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(),
cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId),
changeMessage);
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
View File

@ -206,10 +206,10 @@ public class PatchSetInserter extends BatchUpdate.Op {
@Override
public void updateChange(ChangeContext ctx) throws OrmException,
InvalidChangeOperationException, IOException {
ChangeControl ctl = ctx.getChangeControl();
ChangeControl ctl = ctx.getControl();
change = ctx.getChange();
ChangeUpdate update = ctx.getChangeUpdate();
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.getChangeUpdate(), changeMessage);
cmUtil.addChangeMessage(db, update, changeMessage);
}
}

26
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@ -358,7 +358,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
change.setLastUpdatedOn(ctx.getWhen());
}
ps = ctx.getDb().patchSets().get(psId);
ctx.getChangeUpdate().setPatchSetId(psId);
boolean dirty = false;
dirty |= insertComments(ctx);
dirty |= updateLabels(ctx);
@ -466,8 +465,11 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
}
break;
}
plcUtil.deleteComments(ctx.getDb(), ctx.getChangeUpdate(), del);
plcUtil.upsertComments(ctx.getDb(), ctx.getChangeUpdate(), 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();
}
@ -476,7 +478,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
throws OrmException {
Set<CommentSetEntry> r = new HashSet<>();
for (PatchLineComment c : plcUtil.publishedByChange(ctx.getDb(),
ctx.getChangeNotes())) {
ctx.getNotes())) {
r.add(CommentSetEntry.create(c));
}
return r;
@ -486,7 +488,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
throws OrmException {
Map<String, PatchLineComment> drafts = Maps.newHashMap();
for (PatchLineComment c : plcUtil.draftByChangeAuthor(
ctx.getDb(), ctx.getChangeNotes(), user.getAccountId())) {
ctx.getDb(), ctx.getNotes(), user.getAccountId())) {
drafts.put(c.getKey().get(), c);
}
return drafts;
@ -496,7 +498,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
throws OrmException {
Map<String, PatchLineComment> drafts = Maps.newHashMap();
for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(ctx.getDb(),
psId, user.getAccountId(), ctx.getChangeNotes())) {
psId, user.getAccountId(), ctx.getNotes())) {
drafts.put(c.getKey().get(), c);
}
return drafts;
@ -513,8 +515,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
List<PatchSetApproval> ups = Lists.newArrayList();
Map<String, PatchSetApproval> current = scanLabels(ctx, del);
ChangeUpdate update = ctx.getChangeUpdate();
LabelTypes labelTypes = ctx.getChangeControl().getLabelTypes();
ChangeUpdate update = ctx.getUpdate(psId);
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
for (Map.Entry<String, Short> ent : labels.entrySet()) {
String name = ent.getKey();
LabelType lt = checkNotNull(labelTypes.byLabel(name), name);
@ -576,7 +578,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
PatchSetApproval c = new PatchSetApproval(new PatchSetApproval.Key(
psId,
user.getAccountId(),
ctx.getChangeControl().getLabelTypes().getLabelTypes().get(0)
ctx.getControl().getLabelTypes().getLabelTypes().get(0)
.getLabelId()),
(short) 0, TimeUtil.nowTs());
c.setGranted(ctx.getWhen());
@ -595,11 +597,11 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx,
List<PatchSetApproval> del) throws OrmException {
LabelTypes labelTypes = ctx.getChangeControl().getLabelTypes();
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
Map<String, PatchSetApproval> current = Maps.newHashMap();
for (PatchSetApproval a : approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getChangeControl(), psId, user.getAccountId())) {
ctx.getDb(), ctx.getControl(), psId, user.getAccountId())) {
if (a.isSubmit()) {
continue;
}
@ -644,7 +646,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
"Patch Set %d:%s",
psId.get(),
buf.toString()));
cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(), message);
cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), message);
return true;
}

10
gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
View File

@ -166,7 +166,7 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
@Override
public void updateChange(ChangeContext ctx)
throws RestApiException, OrmException, IOException {
if (!ctx.getChangeControl().canPublish(ctx.getDb())) {
if (!ctx.getControl().canPublish(ctx.getDb())) {
throw new AuthException("Cannot publish this draft patch set");
}
if (patchSet == null) {
@ -182,7 +182,7 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
private void saveChange(ChangeContext ctx) throws OrmException {
change = ctx.getChange();
ChangeUpdate update = ctx.getChangeUpdate();
ChangeUpdate update = ctx.getUpdate(psId);
wasDraftChange = change.getStatus() == Change.Status.DRAFT;
if (wasDraftChange) {
change.setStatus(Change.Status.NEW);
@ -208,9 +208,9 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
private void addReviewers(ChangeContext ctx)
throws OrmException, IOException {
LabelTypes labelTypes = ctx.getChangeControl().getLabelTypes();
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
Collection<Account.Id> oldReviewers = approvalsUtil.getReviewers(
ctx.getDb(), ctx.getChangeNotes()).values();
ctx.getDb(), ctx.getNotes()).values();
RevCommit commit = ctx.getRevWalk().parseCommit(
ObjectId.fromString(patchSet.getRevision().get()));
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
@ -219,7 +219,7 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
recipients =
getRecipientsFromFooters(accountResolver, patchSet, footerLines);
recipients.remove(ctx.getUser().getAccountId());
approvalsUtil.addReviewers(ctx.getDb(), ctx.getChangeUpdate(), labelTypes,
approvalsUtil.addReviewers(ctx.getDb(), ctx.getUpdate(psId), labelTypes,
change, patchSet, patchSetInfo, recipients.getReviewers(),
oldReviewers);
}

10
gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
View File

@ -35,6 +35,7 @@ import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -107,7 +108,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
public void updateChange(ChangeContext ctx)
throws ResourceNotFoundException, OrmException {
Optional<PatchLineComment> maybeComment =
plcUtil.get(ctx.getDb(), ctx.getChangeNotes(), key);
plcUtil.get(ctx.getDb(), ctx.getNotes(), key);
if (!maybeComment.isPresent()) {
// Disappeared out from under us. Can't easily fall back to insert,
// because the input might be missing required fields. Just give up.
@ -116,6 +117,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
comment = maybeComment.get();
PatchSet.Id psId = comment.getKey().getParentKey().getParentKey();
ChangeUpdate update = ctx.getUpdate(psId);
PatchSet ps = ctx.getDb().patchSets().get(psId);
if (ps == null) {
throw new ResourceNotFoundException("patch set not found: " + psId);
@ -126,7 +128,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
// Delete then recreate the comment instead of an update.
plcUtil.deleteComments(
ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(comment));
ctx.getDb(), update, Collections.singleton(comment));
comment = new PatchLineComment(
new PatchLineComment.Key(
new Patch.Key(psId, in.path),
@ -135,14 +137,14 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
ctx.getUser().getAccountId(),
comment.getParentUuid(), ctx.getWhen());
setCommentRevId(comment, patchListCache, ctx.getChange(), ps);
plcUtil.insertComments(ctx.getDb(), ctx.getChangeUpdate(),
plcUtil.insertComments(ctx.getDb(), update,
Collections.singleton(update(comment, in)));
} else {
if (comment.getRevId() == null) {
setCommentRevId(
comment, patchListCache, ctx.getChange(), ps);
}
plcUtil.updateComments(ctx.getDb(), ctx.getChangeUpdate(),
plcUtil.updateComments(ctx.getDb(), update,
Collections.singleton(update(comment, in)));
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
View File

@ -34,6 +34,7 @@ import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.Context;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -101,6 +102,7 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
@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<ChangeResource, Input>,
oldTopicName, newTopicName);
}
change.setTopic(Strings.emptyToNull(newTopicName));
ctx.getChangeUpdate().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<ChangeResource, Input>,
caller.getAccountId(), ctx.getWhen(),
change.currentPatchSetId());
cmsg.setMessage(summary);
cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(), cmsg);
cmUtil.addChangeMessage(ctx.getDb(), update, cmsg);
}
@Override

6
gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
View File

@ -154,6 +154,12 @@ public class RebaseChangeOp extends BatchUpdate.Op {
patchSetInserter.postUpdate(ctx);
}
public RevCommit getRebasedCommit() {
checkState(rebasedCommit != null,
"getRebasedCommit() only valid after updateRepo");
return rebasedCommit;
}
public PatchSet getPatchSet() {
checkState(rebasedPatchSet != null,
"getPatchSet() only valid after executing update");

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
View File

@ -110,11 +110,12 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
ResourceConflictException {
caller = ctx.getUser().asIdentifiedUser();
change = ctx.getChange();
ChangeUpdate update = ctx.getChangeUpdate();
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));

6
gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
View File

@ -80,10 +80,11 @@ public class SetHashtagsOp extends BatchUpdate.Op {
updatedHashtags = ImmutableSortedSet.of();
return;
}
if (!ctx.getChangeControl().canEditHashtags()) {
if (!ctx.getControl().canEditHashtags()) {
throw new AuthException("Editing hashtags not permitted");
}
ChangeUpdate update = ctx.getChangeUpdate();
change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
ChangeNotes notes = update.getChangeNotes().load();
Set<String> existingHashtags = notes.getHashtags();
@ -110,7 +111,6 @@ public class SetHashtagsOp extends BatchUpdate.Op {
update.setHashtags(updated);
}
change = update.getChange();
updatedHashtags = ImmutableSortedSet.copyOf(updated);
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@ -186,8 +186,6 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
ReviewDb db = dbProvider.get();
op.merge(db, change, caller, true);
change = db.changes().get(change.getId());
} catch (NoSuchChangeException e) {
throw new OrmException("Submission failed", e);
}
if (change == null) {

254
gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
View File

@ -20,15 +20,19 @@ import static com.google.common.base.Preconditions.checkState;
import com.google.common.base.Throwables;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
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;
@ -49,9 +53,11 @@ import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
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.
@ -146,16 +152,8 @@ public class BatchUpdate implements AutoCloseable {
return BatchUpdate.this.getObjectInserter();
}
private BatchRefUpdate getBatchRefUpdate() throws IOException {
initRepository();
if (batchRefUpdate == null) {
batchRefUpdate = repo.getRefDatabase().newBatchUpdate();
}
return batchRefUpdate;
}
public void addRefUpdate(ReceiveCommand cmd) throws IOException {
getBatchRefUpdate().addCommand(cmd);
public void addRefUpdate(ReceiveCommand cmd) {
commands.add(cmd);
}
public TimeZone getTimeZone() {
@ -165,28 +163,35 @@ public class BatchUpdate implements AutoCloseable {
public class ChangeContext extends Context {
private final ChangeControl ctl;
private final ChangeUpdate update;
private final Map<PatchSet.Id, ChangeUpdate> updates;
private boolean deleted;
private ChangeContext(ChangeControl ctl) {
this.ctl = ctl;
this.update = changeUpdateFactory.create(ctl, when);
updates = new TreeMap<>(ReviewDbUtil.intKeyOrdering());
}
public ChangeUpdate getChangeUpdate() {
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 getChangeNotes() {
return update.getChangeNotes();
public ChangeNotes getNotes() {
return ctl.getNotes();
}
public ChangeControl getChangeControl() {
public ChangeControl getControl() {
return ctl;
}
public Change getChange() {
return update.getChange();
return ctl.getChange();
}
public void markDeleted() {
@ -213,6 +218,146 @@ public class BatchUpdate implements AutoCloseable {
public abstract Change getChange();
}
private static class ChainedReceiveCommands {
private final Map<String, ReceiveCommand> commands = new LinkedHashMap<>();
private boolean isEmpty() {
return commands.isEmpty();
}
private void add(ReceiveCommand cmd) {
checkArgument(!cmd.getOldId().equals(cmd.getNewId()),
"ref update is a no-op: %s", cmd);
ReceiveCommand old = commands.get(cmd.getRefName());
if (old == null) {
commands.put(cmd.getRefName(), cmd);
return;
}
checkArgument(old.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED,
"cannot chain ref update %s after update %s with result %s",
cmd, old, old.getResult());
checkArgument(cmd.getOldId().equals(old.getNewId()),
"cannot chain ref update %s after update %s with different new ID",
cmd, old);
commands.put(cmd.getRefName(), new ReceiveCommand(
old.getOldId(), cmd.getNewId(), cmd.getRefName()));
}
private void addTo(BatchRefUpdate bru) {
checkState(!isEmpty(), "no commands to add");
for (ReceiveCommand cmd : commands.values()) {
bru.addCommand(cmd);
}
}
}
/**
* Interface for listening during batch update execution.
* <p>
* When used during execution of multiple batch updates, the {@code after*}
* methods are called after that phase has been completed for <em>all</em> updates.
*/
public static class Listener {
private static final Listener NONE = new Listener();
/**
* Called after updating all repositories and flushing objects but before
* updating any refs.
*/
public void afterUpdateRepos() throws Exception {
}
/** Called after updating all refs. */
public void afterRefUpdates() throws Exception {
}
/** Called after updating all changes. */
public void afterUpdateChanges() throws Exception {
}
}
private static Order getOrder(Collection<BatchUpdate> updates) {
Order o = null;
for (BatchUpdate u : updates) {
if (o == null) {
o = u.order;
} else if (u.order != o) {
throw new IllegalArgumentException("cannot mix execution orders");
}
}
return o;
}
static void execute(Collection<BatchUpdate> updates, Listener listener)
throws UpdateException, RestApiException {
if (updates.isEmpty()) {
return;
}
try {
Order order = getOrder(updates);
switch (order) {
case REPO_BEFORE_DB:
for (BatchUpdate u : updates) {
u.executeUpdateRepo();
}
listener.afterUpdateRepos();
for (BatchUpdate u : updates) {
u.executeRefUpdates();
}
listener.afterRefUpdates();
for (BatchUpdate u : updates) {
u.executeChangeOps();
}
listener.afterUpdateChanges();
break;
case DB_BEFORE_REPO:
for (BatchUpdate u : updates) {
u.executeChangeOps();
}
listener.afterUpdateChanges();
for (BatchUpdate u : updates) {
u.executeUpdateRepo();
}
listener.afterUpdateRepos();
for (BatchUpdate u : updates) {
u.executeRefUpdates();
}
listener.afterRefUpdates();
break;
default:
throw new IllegalStateException("invalid execution order: " + order);
}
List<CheckedFuture<?, IOException>> indexFutures = new ArrayList<>();
for (BatchUpdate u : updates) {
indexFutures.addAll(u.indexFutures);
}
ChangeIndexer.allAsList(indexFutures).get();
for (BatchUpdate u : updates) {
if (u.batchRefUpdate != null) {
// Fire ref update events only after all mutations are finished, since
// callers may assume a patch set ref being created means the change
// was created, or a branch advancing meaning some changes were
// closed.
u.gitRefUpdated.fire(u.project, u.batchRefUpdate);
}
}
for (BatchUpdate u : updates) {
u.executePostOps();
}
} catch (UpdateException | RestApiException e) {
// Propagate REST API exceptions thrown by operations; they commonly throw
// exceptions like ResourceConflictException to indicate an atomic update
// failure.
throw e;
} catch (Exception e) {
Throwables.propagateIfPossible(e);
throw new UpdateException(e);
}
}
private final ReviewDb db;
private final GitRepositoryManager repoManager;
private final ChangeIndexer indexer;
@ -233,6 +378,7 @@ public class BatchUpdate implements AutoCloseable {
private Repository repo;
private ObjectInserter inserter;
private RevWalk revWalk;
private ChainedReceiveCommands commands = new ChainedReceiveCommands();
private BatchRefUpdate batchRefUpdate;
private boolean closeRepo;
private Order order;
@ -329,58 +475,37 @@ public class BatchUpdate implements AutoCloseable {
}
public void execute() throws UpdateException, RestApiException {
try {
switch (order) {
case REPO_BEFORE_DB:
executeRefUpdates();
executeChangeOps();
break;
case DB_BEFORE_REPO:
executeChangeOps();
executeRefUpdates();
break;
default:
throw new IllegalStateException("invalid execution order: " + order);
}
reindexChanges();
if (batchRefUpdate != null) {
// Fire ref update events only after all mutations are finished, since
// callers may assume a patch set ref being created means the change was
// created, or a branch advancing meaning some changes were closed.
gitRefUpdated.fire(project, batchRefUpdate);
}
executePostOps();
} catch (UpdateException | RestApiException e) {
// Propagate REST API exceptions thrown by operations; they commonly throw
// exceptions like ResourceConflictException to indicate an atomic update
// failure.
throw e;
} catch (Exception e) {
Throwables.propagateIfPossible(e);
throw new UpdateException(e);
}
execute(Listener.NONE);
}
private void executeRefUpdates()
throws IOException, UpdateException, RestApiException {
public void execute(Listener listener)
throws UpdateException, RestApiException {
execute(ImmutableList.of(this), listener);
}
private void executeUpdateRepo() throws UpdateException, RestApiException {
try {
RepoContext ctx = new RepoContext();
for (Op op : ops.values()) {
op.updateRepo(ctx);
}
if (inserter != null) {
inserter.flush();
}
} catch (Exception e) {
Throwables.propagateIfPossible(e, RestApiException.class);
throw new UpdateException(e);
}
}
if (repo == null || batchRefUpdate == null
|| batchRefUpdate.getCommands().isEmpty()) {
private void executeRefUpdates() throws IOException, UpdateException {
if (commands.isEmpty()) {
return;
}
inserter.flush();
// May not be opened if the caller added ref updates but no new objects.
initRepository();
batchRefUpdate = repo.getRefDatabase().newBatchUpdate();
commands.addTo(batchRefUpdate);
batchRefUpdate.execute(revWalk, NullProgressMonitor.INSTANCE);
boolean ok = true;
for (ReceiveCommand cmd : batchRefUpdate.getCommands()) {
@ -409,7 +534,18 @@ public class BatchUpdate implements AutoCloseable {
} finally {
db.rollback();
}
ctx.getChangeUpdate().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 {
@ -434,10 +570,6 @@ public class BatchUpdate implements AutoCloseable {
changeControlFactory.controlFor(c, user));
}
private void reindexChanges() throws IOException {
ChangeIndexer.allAsList(indexFutures).checkedGet();
}
private void executePostOps() throws Exception {
Context ctx = new Context();
for (Op op : ops.values()) {

34
gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeSet.java
View File

@ -18,6 +18,7 @@ import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Multimap;
@ -30,6 +31,8 @@ import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
/**
@ -38,17 +41,16 @@ import java.util.Set;
* This class is not thread safe.
*/
public class ChangeSet {
private final ImmutableCollection<ChangeData> changeData;
private final ImmutableMap<Change.Id, ChangeData> changeData;
public ChangeSet(Iterable<ChangeData> changes) {
Set<Change.Id> ids = new HashSet<>();
ImmutableSet.Builder<ChangeData> cdb = ImmutableSet.builder();
Map<Change.Id, ChangeData> cds = new LinkedHashMap<>();
for (ChangeData cd : changes) {
if (ids.add(cd.getId())) {
cdb.add(cd);
if (!cds.containsKey(cd.getId())) {
cds.put(cd.getId(), cd);
}
}
changeData = cdb.build();
changeData = ImmutableMap.copyOf(cds);
}
public ChangeSet(ChangeData change) {
@ -56,16 +58,16 @@ public class ChangeSet {
}
public ImmutableSet<Change.Id> ids() {
ImmutableSet.Builder<Change.Id> ret = ImmutableSet.builder();
for (ChangeData cd : changeData) {
ret.add(cd.getId());
}
return ret.build();
return changeData.keySet();
}
public ImmutableMap<Change.Id, ChangeData> changesById() {
return changeData;
}
public Set<PatchSet.Id> patchIds() throws OrmException {
Set<PatchSet.Id> ret = new HashSet<>();
for (ChangeData cd : changeData) {
for (ChangeData cd : changeData.values()) {
ret.add(cd.change().currentPatchSetId());
}
return ret;
@ -75,7 +77,7 @@ public class ChangeSet {
throws OrmException {
SetMultimap<Project.NameKey, Branch.NameKey> ret =
HashMultimap.create();
for (ChangeData cd : changeData) {
for (ChangeData cd : changeData.values()) {
ret.put(cd.change().getProject(), cd.change().getDest());
}
return ret;
@ -85,7 +87,7 @@ public class ChangeSet {
throws OrmException {
ListMultimap<Project.NameKey, Change.Id> ret =
ArrayListMultimap.create();
for (ChangeData cd : changeData) {
for (ChangeData cd : changeData.values()) {
ret.put(cd.change().getProject(), cd.getId());
}
return ret;
@ -95,14 +97,14 @@ public class ChangeSet {
throws OrmException {
ListMultimap<Branch.NameKey, ChangeData> ret =
ArrayListMultimap.create();
for (ChangeData cd : changeData) {
for (ChangeData cd : changeData.values()) {
ret.put(cd.change().getDest(), cd);
}
return ret;
}
public ImmutableCollection<ChangeData> changes() {
return changeData;
return changeData.values();
}
public int size() {

352
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -24,16 +24,15 @@ import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.ChangeHooks;
@ -55,7 +54,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@ -89,7 +87,6 @@ import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
@ -200,6 +197,49 @@ public class MergeOp implements AutoCloseable {
}
}
public static class CommitStatus {
private final Map<Change.Id, CodeReviewCommit> commits = new HashMap<>();
private final Multimap<Change.Id, String> problems =
MultimapBuilder.treeKeys(
Ordering.natural().onResultOf(new Function<Change.Id, Integer>() {
@Override
public Integer apply(Change.Id in) {
return in.get();
}
})).arrayListValues(1).build();
public CodeReviewCommit get(Change.Id changeId) {
return commits.get(changeId);
}
public void put(CodeReviewCommit c) {
commits.put(c.change().getId(), c);
}
public void problem(Change.Id id, String problem) {
problems.put(id, problem);
}
public void logProblem(Change.Id id, Throwable t) {
String msg = "Error reading change";
log.error(msg + " " + id, t);
problems.put(id, msg);
}
public void logProblem(Change.Id id, String msg) {
log.error(msg + " " + id);
problems.put(id, msg);
}
boolean isOk() {
return problems.isEmpty();
}
ImmutableMultimap<Change.Id, String> getProblems() {
return ImmutableMultimap.copyOf(problems);
}
}
private final AccountCache accountCache;
private final ApprovalsUtil approvalsUtil;
private final ChangeControl.GenericFactory changeControlFactory;
@ -217,14 +257,12 @@ public class MergeOp implements AutoCloseable {
private final PatchSetInfoFactory patchSetInfoFactory;
private final ProjectCache projectCache;
private final InternalChangeQuery internalChangeQuery;
private final PersonIdent serverIdent;
private final SubmitStrategyFactory submitStrategyFactory;
private final Provider<SubmoduleOp> subOpProvider;
private final TagCache tagCache;
private final CommitStatus commits;
private final Map<Project.NameKey, OpenRepo> openRepos;
private final Map<Change.Id, CodeReviewCommit> commits;
private final Multimap<Change.Id, String> problems;
private static final String MACHINE_ID;
static {
@ -236,8 +274,9 @@ public class MergeOp implements AutoCloseable {
}
MACHINE_ID = id;
}
private String staticSubmissionId;
private Timestamp ts;
private String submissionId;
private IdentifiedUser caller;
private ReviewDb db;
@ -259,7 +298,6 @@ public class MergeOp implements AutoCloseable {
PatchSetInfoFactory patchSetInfoFactory,
ProjectCache projectCache,
InternalChangeQuery internalChangeQuery,
@GerritPersonIdent PersonIdent serverIdent,
SubmitStrategyFactory submitStrategyFactory,
Provider<SubmoduleOp> subOpProvider,
TagCache tagCache) {
@ -280,38 +318,35 @@ public class MergeOp implements AutoCloseable {
this.patchSetInfoFactory = patchSetInfoFactory;
this.projectCache = projectCache;
this.internalChangeQuery = internalChangeQuery;
this.serverIdent = serverIdent;
this.submitStrategyFactory = submitStrategyFactory;
this.subOpProvider = subOpProvider;
this.tagCache = tagCache;
commits = new HashMap<>();
openRepos = new HashMap<>();
problems = MultimapBuilder.treeKeys(
Ordering.natural().onResultOf(new Function<Change.Id, Integer>() {
@Override
public Integer apply(Change.Id in) {
return in.get();
}
})).arrayListValues(1).build();
commits = new CommitStatus();
}
private OpenRepo openRepo(Project.NameKey project)
private OpenRepo getRepo(Project.NameKey project) {
OpenRepo or = openRepos.get(project);
checkState(or != null, "repo not yet opened: %s", project);
return or;
}
private void openRepo(Project.NameKey project)
throws NoSuchProjectException, IOException {
OpenRepo repo = openRepos.get(project);
if (repo == null) {
ProjectState projectState = projectCache.get(project);
if (projectState == null) {
throw new NoSuchProjectException(project);
}
try {
repo = new OpenRepo(repoManager.openRepository(project), projectState);
} catch (RepositoryNotFoundException e) {
throw new NoSuchProjectException(project);
}
openRepos.put(project, repo);
checkState(!openRepos.containsKey(project),
"repo already opened: %s", project);
ProjectState projectState = projectCache.get(project);
if (projectState == null) {
throw new NoSuchProjectException(project);
}
try {
OpenRepo or =
new OpenRepo(repoManager.openRepository(project), projectState);
openRepos.put(project, or);
} catch (RepositoryNotFoundException e) {
throw new NoSuchProjectException(project);
}
return repo;
}
@Override
@ -419,17 +454,17 @@ public class MergeOp implements AutoCloseable {
for (ChangeData cd : cs.changes()) {
try {
if (cd.change().getStatus() != Change.Status.NEW) {
problems.put(cd.getId(), "Change " + cd.getId() + " is "
commits.problem(cd.getId(), "Change " + cd.getId() + " is "
+ cd.change().getStatus().toString().toLowerCase());
} else {
checkSubmitRule(cd);
}
} catch (ResourceConflictException e) {
problems.put(cd.getId(), e.getMessage());
commits.problem(cd.getId(), e.getMessage());
} catch (OrmException e) {
String msg = "Error checking submit rules for change";
log.warn(msg + " " + cd.getId(), e);
problems.put(cd.getId(), msg);
commits.problem(cd.getId(), msg);
}
}
}
@ -438,14 +473,14 @@ public class MergeOp implements AutoCloseable {
Hasher h = Hashing.sha1().newHasher();
h.putLong(Thread.currentThread().getId())
.putUnencodedChars(MACHINE_ID);
staticSubmissionId = h.hash().toString().substring(0, 8);
submissionId = change.getId().get() + "-" + TimeUtil.nowMs() +
"-" + staticSubmissionId;
ts = TimeUtil.nowTs();
submissionId = change.getId().get() + "-" + ts.getTime() +
"-" + h.hash().toString().substring(0, 8);
}
public void merge(ReviewDb db, Change change, IdentifiedUser caller,
boolean checkSubmitRules) throws NoSuchChangeException,
OrmException, ResourceConflictException {
boolean checkSubmitRules) throws OrmException, ResourceConflictException {
this.caller = caller;
updateSubmissionId(change);
this.db = db;
logDebug("Beginning integration of {}", change);
@ -459,7 +494,7 @@ public class MergeOp implements AutoCloseable {
failFast(cs); // Done checks that don't involve opening repo.
}
try {
integrateIntoHistory(cs, caller);
integrateIntoHistory(cs);
} catch (IntegrationException e) {
logError("Merge Conflict", e);
throw new ResourceConflictException("Merge Conflict", e);
@ -478,11 +513,12 @@ public class MergeOp implements AutoCloseable {
}
private void failFast(ChangeSet cs) throws ResourceConflictException {
if (problems.isEmpty()) {
if (commits.isOk()) {
return;
}
String msg = "Failed to submit " + cs.size() + " change"
+ (cs.size() > 1 ? "s" : "") + " due to the following problems:\n";
Multimap<Change.Id, String> problems = commits.getProblems();
List<String> ps = new ArrayList<>(problems.keySet().size());
for (Change.Id id : problems.keySet()) {
ps.add("Change " + id + ": " + Joiner.on("; ").join(problems.get(id)));
@ -490,32 +526,32 @@ public class MergeOp implements AutoCloseable {
throw new ResourceConflictException(msg + Joiner.on('\n').join(ps));
}
private void integrateIntoHistory(ChangeSet cs, IdentifiedUser caller)
throws IntegrationException, NoSuchChangeException,
ResourceConflictException {
private void integrateIntoHistory(ChangeSet cs)
throws IntegrationException, ResourceConflictException {
logDebug("Beginning merge attempt on {}", cs);
Map<Branch.NameKey, BranchBatch> toSubmit = new HashMap<>();
logDebug("Perform the merges");
try {
Multimap<Project.NameKey, Branch.NameKey> br = cs.branchesByProject();
Multimap<Branch.NameKey, ChangeData> cbb = cs.changesByBranch();
openRepos(br.keySet());
for (Branch.NameKey branch : cbb.keySet()) {
OpenRepo or = openRepo(branch.getParentKey());
toSubmit.put(branch, validateChangeList(or, cbb.get(branch), caller));
OpenRepo or = getRepo(branch.getParentKey());
toSubmit.put(branch, validateChangeList(or, cbb.get(branch)));
}
failFast(cs); // Done checks that don't involve running submit strategies.
for (Branch.NameKey branch : cbb.keySet()) {
OpenRepo or = openRepo(branch.getParentKey());
OpenRepo or = getRepo(branch.getParentKey());
OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch);
SubmitStrategy strategy = createStrategy(or, branch,
submitting.submitType(), ob.oldTip, caller);
submitting.submitType(), ob.oldTip);
ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip);
}
checkMergeStrategyResults(cs, toSubmit.values());
for (Project.NameKey project : br.keySet()) {
openRepo(project).ins.flush();
getRepo(project).ins.flush();
}
Set<Branch.NameKey> done =
@ -523,13 +559,13 @@ public class MergeOp implements AutoCloseable {
logDebug("Write out the new branch tips");
SubmoduleOp subOp = subOpProvider.get();
for (Project.NameKey project : br.keySet()) {
OpenRepo or = openRepo(project);
OpenRepo or = getRepo(project);
for (Branch.NameKey branch : br.get(project)) {
OpenBranch ob = or.getBranch(branch);
boolean updated = updateBranch(or, branch, caller);
boolean updated = updateBranch(or, branch);
BranchBatch submitting = toSubmit.get(branch);
updateChangeStatus(ob, submitting.changes(), caller);
updateChangeStatus(ob, submitting.changes());
updateSubmoduleSubscriptions(ob, subOp);
if (updated) {
fireRefUpdated(ob);
@ -542,10 +578,6 @@ public class MergeOp implements AutoCloseable {
checkState(done.equals(cbb.keySet()), "programmer error: did not process"
+ " all branches in input set.\nExpected: %s\nActual: %s",
done, cbb.keySet());
} catch (NoSuchProjectException noProject) {
logWarn("Project " + noProject.project() + " no longer exists, "
+ "abandoning open changes");
abandonAllOpenChanges(noProject.project());
} catch (OrmException e) {
throw new IntegrationException("Cannot query the database", e);
} catch (IOException e) {
@ -560,23 +592,20 @@ public class MergeOp implements AutoCloseable {
strategy.getClass().getSimpleName(), submitted.size(), submitted);
List<CodeReviewCommit> toMerge = new ArrayList<>(submitted.size());
for (ChangeData cd : submitted) {
CodeReviewCommit commit = commits.get(cd.change().getId());
CodeReviewCommit commit = commits.get(cd.getId());
checkState(commit != null,
"commit for %s not found by validateChangeList", cd.change().getId());
toMerge.add(commit);
}
MergeTip mergeTip = strategy.run(branchTip, toMerge);
logDebug("Produced {} new commits", strategy.getNewCommits().size());
commits.putAll(strategy.getNewCommits());
return mergeTip;
return strategy.run(branchTip, toMerge);
}
private SubmitStrategy createStrategy(OpenRepo or,
Branch.NameKey destBranch, SubmitType submitType,
CodeReviewCommit branchTip, IdentifiedUser caller)
throws IntegrationException, NoSuchProjectException {
CodeReviewCommit branchTip) throws IntegrationException {
return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins,
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller);
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller,
commits);
}
private Set<RevCommit> getAlreadyAccepted(OpenRepo or,
@ -611,20 +640,8 @@ public class MergeOp implements AutoCloseable {
abstract List<ChangeData> changes();
}
private void logProblem(Change.Id id, Throwable t) {
String msg = "Error reading change";
log.error(msg + " " + id, t);
problems.put(id, msg);
}
private void logProblem(Change.Id id, String msg) {
log.error(msg + " " + id);
problems.put(id, msg);
}
private BranchBatch validateChangeList(OpenRepo or,
Collection<ChangeData> submitted, IdentifiedUser caller)
throws IntegrationException {
Collection<ChangeData> submitted) throws IntegrationException {
logDebug("Validating {} changes", submitted.size());
List<ChangeData> toSubmit = new ArrayList<>(submitted.size());
Multimap<ObjectId, PatchSet.Id> revisions = getRevisions(or, submitted);
@ -639,13 +656,13 @@ public class MergeOp implements AutoCloseable {
ctl = cd.changeControl();
chg = cd.change();
} catch (OrmException e) {
logProblem(changeId, e);
commits.logProblem(changeId, e);
continue;
}
if (chg.currentPatchSetId() == null) {
String msg = "Missing current patch set on change";
logError(msg + " " + changeId);
problems.put(changeId, msg);
commits.problem(changeId, msg);
continue;
}
@ -654,12 +671,12 @@ public class MergeOp implements AutoCloseable {
try {
ps = cd.currentPatchSet();
} catch (OrmException e) {
logProblem(changeId, e);
commits.logProblem(changeId, e);
continue;
}
if (ps == null || ps.getRevision() == null
|| ps.getRevision().get() == null) {
logProblem(changeId, "Missing patch set or revision on change");
commits.logProblem(changeId, "Missing patch set or revision on change");
continue;
}
@ -668,7 +685,7 @@ public class MergeOp implements AutoCloseable {
try {
id = ObjectId.fromString(idstr);
} catch (IllegalArgumentException e) {
logProblem(changeId, e);
commits.logProblem(changeId, e);
continue;
}
@ -677,7 +694,7 @@ public class MergeOp implements AutoCloseable {
// want to merge the issue. We can't safely do that if the
// tip is not reachable.
//
logProblem(changeId, "Revision " + idstr + " of patch set "
commits.logProblem(changeId, "Revision " + idstr + " of patch set "
+ ps.getPatchSetId() + " does not match " + ps.getId().toRefName()
+ " for change");
continue;
@ -687,34 +704,34 @@ public class MergeOp implements AutoCloseable {
try {
commit = or.rw.parseCommit(id);
} catch (IOException e) {
logProblem(changeId, e);
commits.logProblem(changeId, e);
continue;
}
// TODO(dborowitz): Consider putting ChangeData in CodeReviewCommit.
commit.setControl(ctl);
commit.setPatchsetId(ps.getId());
commits.put(changeId, commit);
commits.put(commit);
MergeValidators mergeValidators = mergeValidatorsFactory.create();
try {
mergeValidators.validatePreMerge(
or.repo, commit, or.project, destBranch, ps.getId(), caller);
} catch (MergeValidationException mve) {
problems.put(changeId, mve.getMessage());
commits.problem(changeId, mve.getMessage());
continue;
}
SubmitType st = getSubmitType(cd);
if (st == null) {
logProblem(changeId, "No submit type for change");
commits.logProblem(changeId, "No submit type for change");
continue;
}
if (submitType == null) {
submitType = st;
choseSubmitTypeFrom = cd;
} else if (st != submitType) {
problems.put(changeId, String.format(
commits.problem(changeId, String.format(
"Change has submit type %s, but previously chose submit type %s "
+ "from change %s in the same batch",
st, submitType, choseSubmitTypeFrom.getId()));
@ -760,8 +777,8 @@ public class MergeOp implements AutoCloseable {
}
}
private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch,
IdentifiedUser caller) throws IntegrationException {
private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch)
throws IntegrationException {
OpenBranch ob = or.getBranch(destBranch);
CodeReviewCommit currentTip = ob.getCurrentTip();
if (Objects.equals(ob.oldTip, currentTip)) {
@ -873,7 +890,7 @@ public class MergeOp implements AutoCloseable {
CodeReviewCommit commit = commits.get(id);
CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
if (s == null) {
problems.put(id,
commits.problem(id,
"internal error: change not processed by merge strategy");
continue;
}
@ -891,24 +908,24 @@ public class MergeOp implements AutoCloseable {
case NOT_FAST_FORWARD:
// TODO(dborowitz): Reformat these messages to be more appropriate for
// short problem descriptions.
problems.put(id,
commits.problem(id,
CharMatcher.is('\n').collapseFrom(s.getMessage(), ' '));
break;
case MISSING_DEPENDENCY:
problems.put(id, "depends on change that was not submitted");
commits.problem(id, "depends on change that was not submitted");
break;
default:
problems.put(id, "unspecified merge failure: " + s);
commits.problem(id, "unspecified merge failure: " + s);
break;
}
}
failFast(cs);
}
private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted,
IdentifiedUser caller) throws ResourceConflictException {
private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted)
throws ResourceConflictException {
List<Change.Id> problemChanges = new ArrayList<>(submitted.size());
logDebug("Updating change status for {} changes", submitted.size());
@ -923,7 +940,7 @@ public class MergeOp implements AutoCloseable {
checkState(s != null,
"status not set for change %s; expected to previously fail fast",
id);
setApproval(cd, caller);
setApproval(cd);
ObjectId mergeResultRev = ob.mergeTip != null
? ob.mergeTip.getMergeResults().get(commit) : null;
@ -1076,38 +1093,38 @@ public class MergeOp implements AutoCloseable {
});
}
private void setApproval(ChangeData cd, IdentifiedUser user)
throws OrmException, IOException {
private void setApproval(ChangeData cd) throws OrmException, IOException {
Timestamp timestamp = TimeUtil.nowTs();
ChangeControl control = cd.changeControl();
PatchSet.Id psId = cd.currentPatchSet().getId();
PatchSet.Id psIdNewRev = commits.get(cd.change().getId())
.change().currentPatchSetId();
logDebug("Add approval for " + cd + " from user " + user);
logDebug("Add approval for " + cd);
ChangeUpdate update = updateFactory.create(control, timestamp);
update.putReviewer(user.getAccountId(), REVIEWER);
update.setPatchSetId(psId);
update.putReviewer(caller.getAccountId(), REVIEWER);
Optional<SubmitRecord> okRecord = findOkRecord(cd.getSubmitRecords());
if (okRecord.isPresent()) {
update.merge(ImmutableList.of(okRecord.get()));
}
db.changes().beginTransaction(cd.change().getId());
try {
BatchMetaDataUpdate batch = approve(control, psId, user,
update, timestamp);
BatchMetaDataUpdate batch = update.openUpdate();
LabelNormalizer.Result normalized =
approve(control, psId, caller, update, timestamp);
batch.write(update, new CommitBuilder());
// If the submit strategy created a new revision (rebase, cherry-pick)
// approve that as well
if (!psIdNewRev.equals(psId)) {
update.setPatchSetId(psId);
update.commit();
// Create a new ChangeUpdate instance because we need to store meta data
// on another patch set (psIdNewRev).
update = updateFactory.create(control, timestamp);
batch = approve(control, psIdNewRev, user,
update, timestamp);
// Write update commit after all normalized label commits.
update.setPatchSetId(psIdNewRev);
saveApprovals(normalized, update);
batch = update.openUpdate();
batch.write(update, new CommitBuilder());
}
db.commit();
@ -1118,9 +1135,9 @@ public class MergeOp implements AutoCloseable {
indexer.index(db, cd.change());
}
private BatchMetaDataUpdate approve(ChangeControl control, PatchSet.Id psId,
IdentifiedUser user, ChangeUpdate update, Timestamp timestamp)
throws OrmException {
private LabelNormalizer.Result approve(ChangeControl control,
PatchSet.Id psId, IdentifiedUser user, ChangeUpdate update,
Timestamp timestamp) throws OrmException {
Map<PatchSetApproval.Key, PatchSetApproval> byKey = Maps.newHashMap();
for (PatchSetApproval psa :
approvalsUtil.byPatchSet(db, control, psId)) {
@ -1146,77 +1163,78 @@ public class MergeOp implements AutoCloseable {
// permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized =
labelNormalizer.normalize(control, byKey.values());
update.putApproval(submit.getLabel(), submit.getValue());
saveApprovals(normalized, update);
return normalized;
}
private void saveApprovals(LabelNormalizer.Result normalized,
ChangeUpdate update) throws OrmException {
PatchSet.Id psId = update.getPatchSetId();
db.patchSetApprovals().upsert(
convertPatchSet(normalized.getNormalized(), psId));
db.patchSetApprovals().delete(convertPatchSet(normalized.deleted(), psId));
for (PatchSetApproval psa : normalized.updated()) {
update.putApprovalFor(psa.getAccountId(), psa.getLabel(), psa.getValue());
}
for (PatchSetApproval psa : normalized.deleted()) {
update.removeApprovalFor(psa.getAccountId(), psa.getLabel());
}
// TODO(dborowitz): Don't use a label in notedb; just check when status
// change happened.
update.putApproval(submit.getLabel(), submit.getValue());
logDebug("Adding submit label " + submit);
db.patchSetApprovals().upsert(normalized.getNormalized());
db.patchSetApprovals().delete(normalized.deleted());
try {
return saveToBatch(control, update, normalized, timestamp);
} catch (IOException e) {
throw new OrmException(e);
}
}
private BatchMetaDataUpdate saveToBatch(ChangeControl ctl,
ChangeUpdate callerUpdate, LabelNormalizer.Result normalized,
Timestamp timestamp) throws IOException {
Table<Account.Id, String, Optional<Short>> byUser = HashBasedTable.create();
for (PatchSetApproval psa : normalized.updated()) {
byUser.put(psa.getAccountId(), psa.getLabel(),
Optional.of(psa.getValue()));
}
for (PatchSetApproval psa : normalized.deleted()) {
byUser.put(psa.getAccountId(), psa.getLabel(), Optional.<Short> absent());
}
BatchMetaDataUpdate batch = callerUpdate.openUpdate();
for (Account.Id accountId : byUser.rowKeySet()) {
if (!accountId.equals(callerUpdate.getUser().getAccountId())) {
ChangeUpdate update = updateFactory.create(
ctl.forUser(identifiedUserFactory.create(accountId)), timestamp);
update.setSubject("Finalize approvals at submit");
putApprovals(update, byUser.row(accountId));
CommitBuilder commit = new CommitBuilder();
commit.setCommitter(new PersonIdent(serverIdent, timestamp));
batch.write(update, commit);
}
}
putApprovals(callerUpdate,
byUser.row(callerUpdate.getUser().getAccountId()));
return batch;
}
private static void putApprovals(ChangeUpdate update,
Map<String, Optional<Short>> approvals) {
for (Map.Entry<String, Optional<Short>> e : approvals.entrySet()) {
if (e.getValue().isPresent()) {
update.putApproval(e.getKey(), e.getValue().get());
} else {
update.removeApproval(e.getKey());
for (PatchSetApproval psa : normalized.unchanged()) {
if (psa.isSubmit()) {
logDebug("Adding submit label " + psa);
update.putApprovalFor(
psa.getAccountId(), psa.getLabel(), psa.getValue());
break;
}
}
}
private void abandonAllOpenChanges(Project.NameKey destProject)
throws NoSuchChangeException {
private static Iterable<PatchSetApproval> convertPatchSet(
Iterable<PatchSetApproval> approvals, final PatchSet.Id psId) {
return Iterables.transform(approvals,
new Function<PatchSetApproval, PatchSetApproval>() {
@Override
public PatchSetApproval apply(PatchSetApproval in) {
if (in.getPatchSetId().equals(psId)) {
return in;
} else {
return new PatchSetApproval(psId, in);
}
}
});
}
private void openRepos(Collection<Project.NameKey> projects)
throws IntegrationException {
for (Project.NameKey project : projects) {
try {
openRepo(project);
} catch (NoSuchProjectException noProject) {
logWarn("Project " + noProject.project() + " no longer exists, "
+ "abandoning open changes");
abandonAllOpenChanges(noProject.project());
} catch (IOException e) {
throw new IntegrationException("Error opening project " + project, e);
}
}
}
private void abandonAllOpenChanges(Project.NameKey destProject) {
try {
for (ChangeData cd : internalChangeQuery.byProjectOpen(destProject)) {
abandonOneChange(cd.change());
}
} catch (IOException | OrmException e) {
logWarn("Cannot abandon changes for deleted project ", e);
} catch (NoSuchChangeException | IOException | OrmException e) {
logWarn("Cannot abandon changes for deleted project " + destProject, e);
}
}
private void abandonOneChange(Change change) throws OrmException,
NoSuchChangeException, IOException {
NoSuchChangeException, IOException {
db.changes().beginTransaction(change.getId());
//TODO(dborowitz): support InternalUser in ChangeUpdate

22
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeTip.java
View File

@ -34,27 +34,37 @@ import java.util.Map;
* merge failed or another error state.
*/
public class MergeTip {
private CodeReviewCommit initialTip;
private CodeReviewCommit branchTip;
private Map<ObjectId, ObjectId> mergeResults;
/**
* @param initial Tip before the merge operation; may be null, indicating an
* unborn branch.
* @param toMerge List of CodeReview commits to be merged in merge operation;
* may not be null or empty.
* @param initialTip tip before the merge operation; may be null, indicating
* an unborn branch.
* @param toMerge list of commits to be merged in merge operation; may not be
* null or empty.
*/
public MergeTip(@Nullable CodeReviewCommit initial,
public MergeTip(@Nullable CodeReviewCommit initialTip,
Collection<CodeReviewCommit> toMerge) {
checkNotNull(toMerge, "toMerge may not be null");
checkArgument(!toMerge.isEmpty(), "toMerge may not be empty");
this.initialTip = initialTip;
this.branchTip = initialTip;
this.mergeResults = Maps.newHashMap();
this.branchTip = initial;
// Assume fast-forward merge until opposite is proven.
for (CodeReviewCommit commit : toMerge) {
mergeResults.put(commit.copy(), commit.copy());
}
}
/**
* @return the initial tip of the branch before the merge operation started;
* may be null, indicating a previously unborn branch.
*/
public CodeReviewCommit getInitialTip() {
return initialTip;
}
/**
* Moves this MergeTip to newTip and appends mergeResult.
*

2
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@ -663,7 +663,7 @@ public class MergeUtil {
CodeReviewCommit c;
while ((c = (CodeReviewCommit) rw.next()) != null) {
if (c.getPatchsetId() != null) {
if (c.getPatchsetId() != null && c.getStatusCode() == null) {
c.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -1807,7 +1807,7 @@ public class ReceiveCommits {
new BatchUpdate.Op() {
@Override
public void updateChange(ChangeContext ctx) throws Exception {
ctx.getChangeUpdate().setTopic(magicBranch.topic);
ctx.getUpdate(ps.getId()).setTopic(magicBranch.topic);
}
});
}
@ -1830,8 +1830,6 @@ public class ReceiveCommits {
try (MergeOp op = mergeOpProvider.get()) {
op.merge(db, rsrc.getChange(),
changeCtl.getUser().asIdentifiedUser(), false);
} catch (NoSuchChangeException e) {
throw new OrmException(e);
}
addMessage("");
Change c = db.changes().get(rsrc.getChange().getId());

14
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@ -43,16 +43,12 @@ import org.eclipse.jgit.transport.ReceiveCommand;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class CherryPick extends SubmitStrategy {
private final Map<Change.Id, CodeReviewCommit> newCommits;
CherryPick(SubmitStrategy.Arguments args) {
super(args);
this.newCommits = new HashMap<>();
}
@Override
@ -171,7 +167,6 @@ public class CherryPick extends SubmitStrategy {
// Merge conflict; don't update change.
return;
}
ctx.getChangeUpdate().setPatchSetId(psId);
PatchSet ps = new PatchSet(psId);
ps.setCreatedOn(ctx.getWhen());
ps.setUploader(args.caller.getAccountId());
@ -187,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.getChangeUpdate().putApproval(a.getLabel(), a.getValue());
ctx.getUpdate(psId).putApproval(a.getLabel(), a.getValue());
}
args.db.patchSetApprovals().insert(approvals);
@ -196,7 +191,7 @@ public class CherryPick extends SubmitStrategy {
newCommit.setControl(
args.changeControlFactory.controlFor(toMerge.change(), args.caller));
mergeTip.moveTipTo(newCommit, newCommit);
newCommits.put(c.getId(), newCommit);
args.commits.put(newCommit);
}
}
@ -238,11 +233,6 @@ public class CherryPick extends SubmitStrategy {
}
}
@Override
public Map<Change.Id, CodeReviewCommit> getNewCommits() {
return newCommits;
}
static boolean dryRun(SubmitDryRun.Arguments args,
CodeReviewCommit mergeTip, CodeReviewCommit toMerge)
throws IntegrationException {

20
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@ -37,27 +37,16 @@ import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class RebaseIfNecessary extends SubmitStrategy {
private final Map<Change.Id, CodeReviewCommit> newCommits;
RebaseIfNecessary(SubmitStrategy.Arguments args) {
super(args);
this.newCommits = new HashMap<>();
}
private PersonIdent getSubmitterIdent() {
return args.caller.newCommitterIdent(
args.serverIdent.getWhen(),
args.serverIdent.getTimeZone());
}
@Override
@ -163,7 +152,6 @@ public class RebaseIfNecessary extends SubmitStrategy {
// Racy read of patch set is ok; see comments in RebaseChangeOp.
args.db.patchSets().get(toMerge.getPatchsetId()),
mergeTip.getCurrentTip().name())
.setCommitterIdent(getSubmitterIdent())
.setRunHooks(false)
.setValidatePolicy(CommitValidators.Policy.NONE);
try {
@ -207,8 +195,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
mergeTip.getCurrentTip().setPatchsetId(newPatchSet.getId());
mergeTip.getCurrentTip().setStatusCode(
CommitMergeStatus.CLEAN_REBASE);
newCommits.put(newPatchSet.getId().getParentKey(),
mergeTip.getCurrentTip());
args.commits.put(mergeTip.getCurrentTip());
acceptMergeTip(mergeTip);
}
@ -271,11 +258,6 @@ public class RebaseIfNecessary extends SubmitStrategy {
}
}
@Override
public Map<Change.Id, CodeReviewCommit> getNewCommits() {
return newCommits;
}
static boolean dryRun(SubmitDryRun.Arguments args,
CodeReviewCommit mergeTip, CodeReviewCommit toMerge)
throws IntegrationException {

27
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
View File

@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.GerritPersonIdent;
@ -29,6 +28,7 @@ import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.MergeOp.CommitStatus;
import com.google.gerrit.server.git.MergeSorter;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.MergeUtil;
@ -48,8 +48,6 @@ import org.eclipse.jgit.revwalk.RevFlag;
import java.sql.Timestamp;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
/**
@ -72,6 +70,7 @@ public abstract class SubmitStrategy {
interface Factory {
Arguments create(
Branch.NameKey destBranch,
CommitStatus commits,
CodeReviewRevWalk rw,
IdentifiedUser caller,
ObjectInserter inserter,
@ -91,6 +90,7 @@ public abstract class SubmitStrategy {
final Branch.NameKey destBranch;
final CodeReviewRevWalk rw;
final CommitStatus commits;
final IdentifiedUser caller;
final ObjectInserter inserter;
final Repository repo;
@ -113,6 +113,7 @@ public abstract class SubmitStrategy {
ProjectCache projectCache,
RebaseChangeOp.Factory rebaseFactory,
@Assisted Branch.NameKey destBranch,
@Assisted CommitStatus commits,
@Assisted CodeReviewRevWalk rw,
@Assisted IdentifiedUser caller,
@Assisted ObjectInserter inserter,
@ -129,6 +130,7 @@ public abstract class SubmitStrategy {
this.serverIdent = serverIdent;
this.destBranch = destBranch;
this.commits = commits;
this.rw = rw;
this.caller = caller;
this.inserter = inserter;
@ -137,7 +139,8 @@ public abstract class SubmitStrategy {
this.db = db;
this.alreadyAccepted = alreadyAccepted;
this.project = projectCache.get(destBranch.getParentKey());
this.project = checkNotNull(projectCache.get(destBranch.getParentKey()),
"project not found: %s", destBranch.getParentKey());
this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag);
this.mergeUtil = mergeUtilFactory.create(project);
}
@ -169,20 +172,4 @@ public abstract class SubmitStrategy {
*/
public abstract MergeTip run(CodeReviewCommit currentTip,
Collection<CodeReviewCommit> toMerge) throws IntegrationException;
/**
* Returns all commits that have been newly created for the changes that are
* getting merged.
* <p>
* By default this method returns an empty map, but subclasses may override
* this method to provide any newly created commits.
* <p>
* This method may only be called after {@link #run(CodeReviewCommit,
* Collection)}.
*
* @return new commits created for changes that were merged.
*/
public Map<Change.Id, CodeReviewCommit> getNewCommits() {
return Collections.emptyMap();
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java
View File

@ -20,7 +20,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.git.MergeOp.CommitStatus;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@ -49,13 +49,10 @@ public class SubmitStrategyFactory {
public SubmitStrategy create(SubmitType submitType, ReviewDb db,
Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter,
RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted,
Branch.NameKey destBranch, IdentifiedUser caller)
throws IntegrationException, NoSuchProjectException {
SubmitStrategy.Arguments args = argsFactory.create(destBranch, rw, caller,
inserter, repo, canMergeFlag, db, alreadyAccepted);
if (args.project == null) {
throw new NoSuchProjectException(destBranch.getParentKey());
}
Branch.NameKey destBranch, IdentifiedUser caller, CommitStatus commits)
throws IntegrationException {
SubmitStrategy.Arguments args = argsFactory.create(destBranch, commits, rw,
caller, inserter, repo, canMergeFlag, db, alreadyAccepted);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args);

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@ -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

9
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@ -43,7 +43,6 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
@ -494,8 +493,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
BatchMetaDataUpdate batch = update1.openUpdate();
try {
batch.write(update1, new CommitBuilder());
batch.write(update2, new CommitBuilder());
update1.writeCommit(batch);
update2.writeCommit(batch);
batch.commit();
} finally {
batch.close();
@ -587,12 +586,12 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
try {
batch1 = update1.openUpdateInBatch(bru);
batch1.write(update1, new CommitBuilder());
update1.writeCommit(batch1);
batch1.commit();
assertThat(repo.exactRef(update1.getRefName())).isNull();
batch2 = update2.openUpdateInBatch(bru);
batch2.write(update2, new CommitBuilder());
update2.writeCommit(batch2);
batch2.commit();
assertThat(repo.exactRef(update2.getRefName())).isNull();
} finally {