Merge changes from topic 'change-inserter-batch-update'

* changes:
  Convert ChangeInserter to BatchUpdate.Op
  BatchUpdate: Create ChangeControl inside transaction
  Create change ref from ChangeInserter
  CherryPickChange: Write change message using ChangeInserter
  Validate new commits in ChangeInserter rather than callers
  Move PatchSetInserter.ValidatePolicy to CommitValidators
  Clean up ChangeInserter similar to PatchSetInserter
This commit is contained in:
Edwin Kempin
2015-10-12 14:01:23 +00:00
committed by Gerrit Code Review
24 changed files with 567 additions and 548 deletions

View File

@@ -37,8 +37,11 @@ import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
@@ -49,7 +52,9 @@ import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
import java.util.List;
@@ -70,6 +75,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
private final ProjectCache projectCache;
private final ChangesCollection changes;
private final ChangeInserter.Factory changeInserterFactory;
private final BatchUpdate.Factory updateFactory;
@Inject
ReviewProjectAccess(final ProjectControl.Factory projectControlFactory,
@@ -81,6 +87,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
AllProjectsNameProvider allProjects,
ChangesCollection changes,
ChangeInserter.Factory changeInserterFactory,
BatchUpdate.Factory updateFactory,
Provider<SetParent> setParent,
@Assisted("projectName") Project.NameKey projectName,
@@ -97,6 +104,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
this.projectCache = projectCache;
this.changes = changes;
this.changeInserterFactory = changeInserterFactory;
this.updateFactory = updateFactory;
}
@Override
@@ -120,9 +128,21 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
config.getProject().getNameKey(),
RefNames.REFS_CONFIG),
TimeUtil.nowTs());
ChangeInserter ins =
changeInserterFactory.create(ctl, change, commit);
ins.insert();
try (RevWalk rw = new RevWalk(md.getRepository());
ObjectInserter objInserter = md.getRepository().newObjectInserter();
BatchUpdate bu = updateFactory.create(
db, change.getProject(), ctl.getCurrentUser(),
change.getCreatedOn())) {
bu.setRepository(md.getRepository(), rw, objInserter);
bu.insertChange(
changeInserterFactory.create(
ctl.controlForRef(change.getDest().get()), change, commit)
.setValidatePolicy(CommitValidators.Policy.NONE)
.setUpdateRef(false)); // Created by commitToNewRef.
bu.execute();
} catch (UpdateException | RestApiException e) {
throw new IOException(e);
}
ChangeResource rsrc;
try {

View File

@@ -22,8 +22,8 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.client.Project;
@@ -32,21 +32,18 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.util.IdGenerator;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -194,7 +191,6 @@ public class ChangeUtil {
}
private final Provider<CurrentUser> userProvider;
private final CommitValidators.Factory commitValidatorsFactory;
private final Provider<ReviewDb> db;
private final Provider<InternalChangeQuery> queryProvider;
private final RevertedSender.Factory revertedSenderFactory;
@@ -202,19 +198,19 @@ public class ChangeUtil {
private final GitRepositoryManager gitManager;
private final GitReferenceUpdated gitRefUpdated;
private final ChangeIndexer indexer;
private final BatchUpdate.Factory updateFactory;
@Inject
ChangeUtil(Provider<CurrentUser> userProvider,
CommitValidators.Factory commitValidatorsFactory,
Provider<ReviewDb> db,
Provider<InternalChangeQuery> queryProvider,
RevertedSender.Factory revertedSenderFactory,
ChangeInserter.Factory changeInserterFactory,
GitRepositoryManager gitManager,
GitReferenceUpdated gitRefUpdated,
ChangeIndexer indexer) {
ChangeIndexer indexer,
BatchUpdate.Factory updateFactory) {
this.userProvider = userProvider;
this.commitValidatorsFactory = commitValidatorsFactory;
this.db = db;
this.queryProvider = queryProvider;
this.revertedSenderFactory = revertedSenderFactory;
@@ -222,13 +218,14 @@ public class ChangeUtil {
this.gitManager = gitManager;
this.gitRefUpdated = gitRefUpdated;
this.indexer = indexer;
this.updateFactory = updateFactory;
}
public Change.Id revert(ChangeControl ctl, PatchSet.Id patchSetId,
String message, PersonIdent myIdent, SshInfo sshInfo)
String message, PersonIdent myIdent)
throws NoSuchChangeException, OrmException,
MissingObjectException, IncorrectObjectTypeException, IOException,
InvalidChangeOperationException {
RestApiException, UpdateException {
Change.Id changeId = patchSetId.getParentKey();
PatchSet patch = db.get().patchSets().get(patchSetId);
if (patch == null) {
@@ -267,11 +264,11 @@ public class ChangeUtil {
ChangeIdUtil.insertId(message, computedChangeId, true));
RevCommit revertCommit;
ChangeInserter ins;
try (ObjectInserter oi = git.newObjectInserter()) {
ObjectId id = oi.insert(revertCommitBuilder);
oi.flush();
revertCommit = revWalk.parseCommit(id);
}
RefControl refControl = ctl.getRefControl();
Change change = new Change(
@@ -281,59 +278,35 @@ public class ChangeUtil {
changeToRevert.getDest(),
TimeUtil.nowTs());
change.setTopic(changeToRevert.getTopic());
ChangeInserter ins =
changeInserterFactory.create(refControl.getProjectControl(),
change, revertCommit);
PatchSet ps = ins.getPatchSet();
String ref = refControl.getRefName();
String cmdRef = MagicBranch.NEW_PUBLISH_CHANGE
+ ref.substring(ref.lastIndexOf('/') + 1);
CommitReceivedEvent commitReceivedEvent = new CommitReceivedEvent(
new ReceiveCommand(ObjectId.zeroId(), revertCommit.getId(), cmdRef),
refControl.getProjectControl().getProject(),
refControl.getRefName(), revertCommit, user());
try {
commitValidatorsFactory.create(refControl, sshInfo, git)
.validateForGerritCommits(commitReceivedEvent);
} catch (CommitValidationException e) {
throw new InvalidChangeOperationException(e.getMessage());
}
RefUpdate ru = git.updateRef(ps.getRefName());
ru.setExpectedOldObjectId(ObjectId.zeroId());
ru.setNewObjectId(revertCommit);
ru.disableRefLog();
if (ru.update(revWalk) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
"Failed to create ref %s in %s: %s", ps.getRefName(),
change.getDest().getParentKey().get(), ru.getResult()));
}
ChangeMessage cmsg = new ChangeMessage(
new ChangeMessage.Key(changeId, messageUUID(db.get())),
user().getAccountId(), TimeUtil.nowTs(), patchSetId);
ins = changeInserterFactory.create(
refControl, change, revertCommit)
.setValidatePolicy(CommitValidators.Policy.GERRIT);
StringBuilder msgBuf = new StringBuilder();
msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted");
msgBuf.append("\n\n");
msgBuf.append("This patchset was reverted in change: ")
.append(change.getKey().get());
cmsg.setMessage(msgBuf.toString());
ins.setMessage(cmsg).insert();
try {
RevertedSender cm = revertedSenderFactory.create(change.getId());
cm.setFrom(user().getAccountId());
cm.setChangeMessage(cmsg);
cm.send();
} catch (Exception err) {
log.error("Cannot send email for revert change " + change.getId(),
err);
ins.setMessage(msgBuf.toString());
try (BatchUpdate bu = updateFactory.create(
db.get(), change.getProject(), refControl.getCurrentUser(),
change.getCreatedOn())) {
bu.setRepository(git, revWalk, oi);
bu.insertChange(ins);
bu.execute();
}
}
return change.getId();
Change.Id id = ins.getChange().getId();
try {
RevertedSender cm = revertedSenderFactory.create(id);
cm.setFrom(user().getAccountId());
cm.setChangeMessage(ins.getChangeMessage());
cm.send();
} catch (Exception err) {
log.error("Cannot send email for revert change " + id, err);
}
return id;
} catch (RepositoryNotFoundException e) {
throw new NoSuchChangeException(changeId, e);
}

View File

@@ -195,7 +195,7 @@ class ChangeApiImpl implements ChangeApi {
public ChangeApi revert(RevertInput in) throws RestApiException {
try {
return changeApi.id(revert.apply(change, in)._number);
} catch (OrmException | EmailException | IOException e) {
} catch (OrmException | EmailException | IOException | UpdateException e) {
throw new RestApiException("Cannot revert change", e);
}
}

View File

@@ -32,6 +32,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.CreateChange;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmException;
@@ -95,7 +96,8 @@ class ChangesImpl implements Changes {
TopLevelResource.INSTANCE, in).value();
return api.create(changes.parse(TopLevelResource.INSTANCE,
IdString.fromUrl(out.changeId)));
} catch (OrmException | IOException | InvalidChangeOperationException e) {
} catch (OrmException | IOException | InvalidChangeOperationException
| UpdateException e) {
throw new RestApiException("Cannot create change", e);
}
}

View File

@@ -93,9 +93,10 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
final String msgTxt, final Account account)
throws RestApiException, UpdateException {
Op op = new Op(msgTxt, account);
Change c = control.getChange();
try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
control.getChange().getProject(), TimeUtil.nowTs())) {
u.addOp(control, op).execute();
c.getProject(), control.getCurrentUser(), TimeUtil.nowTs())) {
u.addOp(c.getId(), op).execute();
}
return op.change;
}
@@ -116,7 +117,7 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
@Override
public void updateChange(ChangeContext ctx) throws OrmException,
ResourceConflictException {
change = ctx.readChange();
change = ctx.getChange();
if (change == null || !change.getStatus().isOpen()) {
throw new ResourceConflictException("change is " + status(change));
} else if (change.getStatus() == Change.Status.DRAFT) {

View File

@@ -14,9 +14,11 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
import com.google.common.util.concurrent.CheckedFuture;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
@@ -31,25 +33,35 @@ 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.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
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.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.CreateChangeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -58,33 +70,34 @@ import java.util.Collections;
import java.util.Map;
import java.util.Set;
public class ChangeInserter {
public class ChangeInserter extends BatchUpdate.InsertChangeOp {
public static interface Factory {
ChangeInserter create(ProjectControl ctl, Change c, RevCommit rc);
ChangeInserter create(RefControl ctl, Change c, RevCommit rc);
}
private static final Logger log =
LoggerFactory.getLogger(ChangeInserter.class);
private final Provider<ReviewDb> dbProvider;
private final ChangeUpdate.Factory updateFactory;
private final GitReferenceUpdated gitRefUpdated;
private final ChangeHooks hooks;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final ChangeIndexer indexer;
private final CreateChangeSender.Factory createChangeSenderFactory;
private final HashtagsUtil hashtagsUtil;
private final AccountCache accountCache;
private final WorkQueue workQueue;
private final CommitValidators.Factory commitValidatorsFactory;
private final ProjectControl projectControl;
private final RefControl refControl;
private final IdentifiedUser user;
private final Change change;
private final PatchSet patchSet;
private final RevCommit commit;
private final PatchSetInfo patchSetInfo;
private ChangeMessage changeMessage;
// Fields exposed as setters.
private String message;
private CommitValidators.Policy validatePolicy =
CommitValidators.Policy.GERRIT;
private Set<Account.Id> reviewers;
private Set<Account.Id> extraCC;
private Map<String, Short> approvals;
@@ -92,35 +105,41 @@ public class ChangeInserter {
private RequestScopePropagator requestScopePropagator;
private boolean runHooks;
private boolean sendMail;
private boolean updateRef;
// Fields set during the insertion process.
private ChangeMessage changeMessage;
@Inject
ChangeInserter(Provider<ReviewDb> dbProvider,
ChangeUpdate.Factory updateFactory,
PatchSetInfoFactory patchSetInfoFactory,
GitReferenceUpdated gitRefUpdated,
ChangeInserter(PatchSetInfoFactory patchSetInfoFactory,
ChangeHooks hooks,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
ChangeIndexer indexer,
CreateChangeSender.Factory createChangeSenderFactory,
HashtagsUtil hashtagsUtil,
AccountCache accountCache,
WorkQueue workQueue,
@Assisted ProjectControl projectControl,
CommitValidators.Factory commitValidatorsFactory,
@Assisted RefControl refControl,
@Assisted Change change,
@Assisted RevCommit commit) {
this.dbProvider = dbProvider;
this.updateFactory = updateFactory;
this.gitRefUpdated = gitRefUpdated;
String projectName = refControl.getProjectControl().getProject().getName();
String refName = refControl.getRefName();
checkArgument(projectName.equals(change.getProject().get())
&& refName.equals(change.getDest().get()),
"RefControl for %s,%s does not match change destination %s",
projectName, refName, change.getDest());
this.hooks = hooks;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.indexer = indexer;
this.createChangeSenderFactory = createChangeSenderFactory;
this.hashtagsUtil = hashtagsUtil;
this.accountCache = accountCache;
this.workQueue = workQueue;
this.projectControl = projectControl;
this.commitValidatorsFactory = commitValidatorsFactory;
this.refControl = refControl;
this.change = change;
this.commit = commit;
this.reviewers = Collections.emptySet();
@@ -129,7 +148,9 @@ public class ChangeInserter {
this.hashtags = Collections.emptySet();
this.runHooks = true;
this.sendMail = true;
this.updateRef = true;
user = checkUser(refControl);
patchSet =
new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
patchSet.setCreatedOn(change.getCreatedOn());
@@ -139,12 +160,28 @@ public class ChangeInserter {
change.setCurrentPatchSet(patchSetInfo);
}
private static IdentifiedUser checkUser(RefControl ctl) {
checkArgument(ctl.getCurrentUser().isIdentifiedUser(),
"only IdentifiedUser may create change");
return (IdentifiedUser) ctl.getCurrentUser();
}
@Override
public Change getChange() {
return change;
}
public ChangeInserter setMessage(ChangeMessage changeMessage) {
this.changeMessage = changeMessage;
public IdentifiedUser getUser() {
return user;
}
public ChangeInserter setMessage(String message) {
this.message = message;
return this;
}
public ChangeInserter setValidatePolicy(CommitValidators.Policy validate) {
this.validatePolicy = checkNotNull(validate);
return this;
}
@@ -198,55 +235,76 @@ public class ChangeInserter {
return this;
}
public ChangeInserter setUpdateRef(boolean updateRef) {
this.updateRef = updateRef;
return this;
}
public PatchSetInfo getPatchSetInfo() {
return patchSetInfo;
}
public Change insert() throws OrmException, IOException {
ReviewDb db = dbProvider.get();
ChangeControl ctl = projectControl.controlFor(change);
ChangeUpdate update = updateFactory.create(
ctl,
change.getCreatedOn());
public ChangeMessage getChangeMessage() {
if (message == null) {
return null;
}
checkState(changeMessage != null,
"getChangeMessage() only valid after inserting change");
return changeMessage;
}
@Override
public void updateRepo(RepoContext ctx)
throws InvalidChangeOperationException, IOException {
validate(ctx);
if (!updateRef) {
return;
}
ctx.addRefUpdate(
new ReceiveCommand(ObjectId.zeroId(), commit, patchSet.getRefName()));
}
@Override
public void updateChange(ChangeContext ctx) throws OrmException, IOException {
ReviewDb db = ctx.getDb();
ChangeControl ctl = ctx.getChangeControl();
ChangeUpdate update = ctx.getChangeUpdate();
db.changes().beginTransaction(change.getId());
try {
ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
if (patchSet.getGroups() == null) {
patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet));
}
db.patchSets().insert(Collections.singleton(patchSet));
db.changes().insert(Collections.singleton(change));
LabelTypes labelTypes = projectControl.getLabelTypes();
LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes();
approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo,
ctl, approvals);
if (messageIsForChange()) {
ctx.getChangeControl(), approvals);
if (message != null) {
changeMessage =
new ChangeMessage(new ChangeMessage.Key(change.getId(),
ChangeUtil.messageUUID(db)), user.getAccountId(),
patchSet.getCreatedOn(), patchSet.getId());
changeMessage.setMessage(message);
cmUtil.addChangeMessage(db, update, changeMessage);
}
db.commit();
} finally {
db.rollback();
}
update.commit();
if (hashtags != null && hashtags.size() > 0) {
try {
HashtagsInput input = new HashtagsInput();
input.add = hashtags;
// TODO(dborowitz): Migrate HashtagsUtil so it doesn't create another
// ChangeUpdate.
hashtagsUtil.setHashtags(ctl, input, false, false);
} catch (ValidationException | AuthException e) {
log.error("Cannot add hashtags to change " + change.getId(), e);
}
}
CheckedFuture<?, IOException> f = indexer.indexAsync(change.getId());
if (!messageIsForChange()) {
commitMessageNotForChange();
}
f.checkedGet();
@Override
public void postUpdate(Context ctx) throws OrmException {
if (sendMail) {
Runnable sender = new Runnable() {
@Override
@@ -276,10 +334,8 @@ public class ChangeInserter {
}
}
gitRefUpdated.fire(change.getProject(), patchSet.getRefName(),
ObjectId.zeroId(), commit);
if (runHooks) {
ReviewDb db = ctx.getDb();
hooks.doPatchsetCreatedHook(change, patchSet, db);
if (hashtags != null && hashtags.size() > 0) {
hooks.doHashtagsChangedHook(change,
@@ -287,32 +343,42 @@ public class ChangeInserter {
hashtags, null, hashtags, db);
}
}
return change;
}
private void commitMessageNotForChange() throws OrmException,
IOException {
ReviewDb db = dbProvider.get();
if (changeMessage != null) {
Change otherChange =
db.changes().get(changeMessage.getPatchSetId().getParentKey());
ChangeUtil.bumpRowVersionNotLastUpdatedOn(
changeMessage.getKey().getParentKey(), db);
ChangeControl otherControl = projectControl.controlFor(otherChange);
ChangeUpdate updateForOtherChange =
updateFactory.create(otherControl, change.getLastUpdatedOn());
cmUtil.addChangeMessage(db, updateForOtherChange, changeMessage);
updateForOtherChange.commit();
}
private void validate(RepoContext ctx)
throws IOException, InvalidChangeOperationException {
if (validatePolicy == CommitValidators.Policy.NONE) {
return;
}
CommitValidators cv = commitValidatorsFactory.create(
refControl, new NoSshInfo(), ctx.getRepository());
private boolean messageIsForChange() {
if (changeMessage == null) {
return false;
String refName = patchSet.getId().toRefName();
CommitReceivedEvent event = new CommitReceivedEvent(
new ReceiveCommand(
ObjectId.zeroId(),
commit.getId(),
refName),
refControl.getProjectControl().getProject(),
change.getDest().get(),
commit,
user);
try {
switch (validatePolicy) {
case RECEIVE_COMMITS:
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(
ctx.getRepository(), ctx.getRevWalk());
cv.validateForReceiveCommits(event, rejectCommits);
break;
case GERRIT:
cv.validateForGerritCommits(event);
break;
case NONE:
break;
}
} catch (CommitValidationException e) {
throw new InvalidChangeOperationException(e.getMessage());
}
Change.Id id = change.getId();
Change.Id msgId = changeMessage.getKey().getParentKey();
return msgId.equals(id);
}
}

View File

@@ -30,7 +30,6 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -40,7 +39,6 @@ import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeIdenticalTreeException;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
@@ -50,7 +48,6 @@ import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -63,10 +60,8 @@ 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;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.util.ChangeIdUtil;
import java.io.IOException;
@@ -81,7 +76,6 @@ public class CherryPickChange {
private final GitRepositoryManager gitManager;
private final TimeZone serverTimeZone;
private final Provider<CurrentUser> currentUser;
private final CommitValidators.Factory commitValidatorsFactory;
private final ChangeInserter.Factory changeInserterFactory;
private final PatchSetInserter.Factory patchSetInserterFactory;
private final MergeUtil.Factory mergeUtilFactory;
@@ -95,7 +89,6 @@ public class CherryPickChange {
@GerritPersonIdent PersonIdent myIdent,
GitRepositoryManager gitManager,
Provider<CurrentUser> currentUser,
CommitValidators.Factory commitValidatorsFactory,
ChangeInserter.Factory changeInserterFactory,
PatchSetInserter.Factory patchSetInserterFactory,
MergeUtil.Factory mergeUtilFactory,
@@ -107,7 +100,6 @@ public class CherryPickChange {
this.gitManager = gitManager;
this.serverTimeZone = myIdent.getTimeZone();
this.currentUser = currentUser;
this.commitValidatorsFactory = commitValidatorsFactory;
this.changeInserterFactory = changeInserterFactory;
this.patchSetInserterFactory = patchSetInserterFactory;
this.mergeUtilFactory = mergeUtilFactory;
@@ -162,9 +154,6 @@ public class CherryPickChange {
cherryPickCommit =
mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip,
commitToCherryPick, committerIdent, commitMessage, revWalk);
} catch (MergeIdenticalTreeException | MergeConflictException e) {
throw new MergeException("Cherry pick failed: " + e.getMessage());
}
Change.Key changeKey;
final List<String> idList = cherryPickCommit.getFooterLines(
@@ -197,18 +186,18 @@ public class CherryPickChange {
if (!Strings.isNullOrEmpty(change.getTopic())) {
newTopic = change.getTopic() + "-" + newDest.getShortName();
}
Change newChange = createNewChange(git, revWalk, changeKey, project,
destRef, cherryPickCommit, refControl,
identifiedUser, newTopic);
Change newChange = createNewChange(git, revWalk, oi, changeKey,
project, destRef, cherryPickCommit, refControl, identifiedUser,
newTopic, change.getDest());
addMessageToSourceChange(change, patch.getId(), destinationBranch,
cherryPickCommit, identifiedUser, refControl);
addMessageToDestinationChange(newChange, change.getDest().getShortName(),
identifiedUser, refControl);
return newChange.getId();
}
} catch (MergeIdenticalTreeException | MergeConflictException e) {
throw new MergeException("Cherry pick failed: " + e.getMessage());
}
} catch (RepositoryNotFoundException e) {
throw new NoSuchChangeException(change.getId(), e);
}
@@ -226,8 +215,9 @@ public class CherryPickChange {
PatchSet current = db.get().patchSets().get(change.currentPatchSetId());
try (BatchUpdate bu = batchUpdateFactory.create(
db.get(), change.getDest().getParentKey(), TimeUtil.nowTs())) {
bu.addOp(changeControl, inserter
db.get(), change.getDest().getParentKey(), identifiedUser,
TimeUtil.nowTs())) {
bu.addOp(change.getId(), inserter
.setMessage("Uploaded patch set " + newPatchSetId.get() + ".")
.setDraft(current.isDraft())
.setUploader(identifiedUser.getAccountId())
@@ -238,47 +228,28 @@ public class CherryPickChange {
}
private Change createNewChange(Repository git, RevWalk revWalk,
Change.Key changeKey, Project.NameKey project,
ObjectInserter oi, Change.Key changeKey, Project.NameKey project,
Ref destRef, CodeReviewCommit cherryPickCommit, RefControl refControl,
IdentifiedUser identifiedUser, String topic)
throws OrmException, InvalidChangeOperationException, IOException {
IdentifiedUser identifiedUser, String topic, Branch.NameKey sourceBranch)
throws RestApiException, UpdateException, OrmException {
Change change =
new Change(changeKey, new Change.Id(db.get().nextChangeId()),
identifiedUser.getAccountId(), new Branch.NameKey(project,
destRef.getName()), TimeUtil.nowTs());
change.setTopic(topic);
ChangeInserter ins =
changeInserterFactory.create(refControl.getProjectControl(), change,
cherryPickCommit);
PatchSet newPatchSet = ins.getPatchSet();
ChangeInserter ins = changeInserterFactory.create(
refControl, change, cherryPickCommit)
.setValidatePolicy(CommitValidators.Policy.GERRIT);
CommitValidators commitValidators =
commitValidatorsFactory.create(refControl, new NoSshInfo(), git);
CommitReceivedEvent commitReceivedEvent =
new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
cherryPickCommit.getId(), newPatchSet.getRefName()), refControl
.getProjectControl().getProject(), refControl.getRefName(),
cherryPickCommit, identifiedUser);
try {
commitValidators.validateForGerritCommits(commitReceivedEvent);
} catch (CommitValidationException e) {
throw new InvalidChangeOperationException(e.getMessage());
ins.setMessage(
messageForDestinationChange(ins.getPatchSet().getId(), sourceBranch));
try (BatchUpdate bu = batchUpdateFactory.create(
db.get(), change.getProject(), identifiedUser, TimeUtil.nowTs())) {
bu.setRepository(git, revWalk, oi);
bu.insertChange(ins);
bu.execute();
}
final RefUpdate ru = git.updateRef(newPatchSet.getRefName());
ru.setExpectedOldObjectId(ObjectId.zeroId());
ru.setNewObjectId(cherryPickCommit);
ru.disableRefLog();
if (ru.update(revWalk) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
"Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
change.getDest().getParentKey().get(), ru.getResult()));
}
ins.insert();
return change;
return ins.getChange();
}
private void addMessageToSourceChange(Change change, PatchSet.Id patchSetId,
@@ -303,24 +274,13 @@ public class CherryPickChange {
changeMessagesUtil.addChangeMessage(db.get(), update, changeMessage);
}
private void addMessageToDestinationChange(Change change, String sourceBranch,
IdentifiedUser identifiedUser, RefControl refControl) throws OrmException {
PatchSet.Id patchSetId =
db.get().patchSets().get(change.currentPatchSetId()).getId();
ChangeMessage changeMessage = new ChangeMessage(
new ChangeMessage.Key(
patchSetId.getParentKey(), ChangeUtil.messageUUID(db.get())),
identifiedUser.getAccountId(), TimeUtil.nowTs(), patchSetId);
StringBuilder sb = new StringBuilder("Patch Set ")
private String messageForDestinationChange(PatchSet.Id patchSetId,
Branch.NameKey sourceBranch) {
return new StringBuilder("Patch Set ")
.append(patchSetId.get())
.append(": Cherry Picked from branch ")
.append(sourceBranch)
.append(".");
changeMessage.setMessage(sb.toString());
ChangeControl ctl = refControl.getProjectControl().controlFor(change);
ChangeUpdate update = updateFactory.create(ctl, change.getCreatedOn());
changeMessagesUtil.addChangeMessage(db.get(), update, changeMessage);
.append(sourceBranch.getShortName())
.append(".")
.toString();
}
}

View File

@@ -42,10 +42,10 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -466,8 +466,10 @@ public class ConsistencyChecker {
PatchSetInserter inserter =
patchSetInserterFactory.create(repo, rw, ctl, commit);
try (BatchUpdate bu = updateFactory.create(
db.get(), change.getDest().getParentKey(), TimeUtil.nowTs())) {
bu.addOp(ctl, inserter.setValidatePolicy(ValidatePolicy.NONE)
db.get(), change.getDest().getParentKey(), user.get(),
TimeUtil.nowTs())) {
bu.addOp(change.getId(), inserter
.setValidatePolicy(CommitValidators.Policy.NONE)
.setRunHooks(false)
.setSendMail(false)
.setAllowClosed(true)

View File

@@ -24,15 +24,13 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -42,15 +40,14 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectsCollection;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -62,11 +59,9 @@ 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;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.util.ChangeIdUtil;
import java.io.IOException;
@@ -84,10 +79,10 @@ public class CreateChange implements
private final TimeZone serverTimeZone;
private final Provider<CurrentUser> userProvider;
private final ProjectsCollection projectsCollection;
private final CommitValidators.Factory commitValidatorsFactory;
private final ChangeInserter.Factory changeInserterFactory;
private final ChangeJson.Factory jsonFactory;
private final ChangeUtil changeUtil;
private final BatchUpdate.Factory updateFactory;
private final boolean allowDrafts;
@Inject
@@ -96,29 +91,28 @@ public class CreateChange implements
@GerritPersonIdent PersonIdent myIdent,
Provider<CurrentUser> userProvider,
ProjectsCollection projectsCollection,
CommitValidators.Factory commitValidatorsFactory,
ChangeInserter.Factory changeInserterFactory,
ChangeJson.Factory json,
ChangeUtil changeUtil,
BatchUpdate.Factory updateFactory,
@GerritServerConfig Config config) {
this.db = db;
this.gitManager = gitManager;
this.serverTimeZone = myIdent.getTimeZone();
this.userProvider = userProvider;
this.projectsCollection = projectsCollection;
this.commitValidatorsFactory = commitValidatorsFactory;
this.changeInserterFactory = changeInserterFactory;
this.jsonFactory = json;
this.changeUtil = changeUtil;
this.updateFactory = updateFactory;
this.allowDrafts = config.getBoolean("change", "allowDrafts", true);
}
@Override
public Response<ChangeInfo> apply(TopLevelResource parent,
ChangeInfo input) throws AuthException, OrmException,
BadRequestException, UnprocessableEntityException, IOException,
InvalidChangeOperationException, ResourceNotFoundException,
MethodNotAllowedException, ResourceConflictException {
ChangeInfo input)
throws AuthException, OrmException, IOException,
InvalidChangeOperationException, RestApiException, UpdateException {
if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("project must be non-empty");
}
@@ -195,7 +189,8 @@ public class CreateChange implements
mergeTip, author, author, input.subject);
String commitMessage = ChangeIdUtil.insertId(input.subject, id);
RevCommit c = newCommit(git, rw, author, mergeTip, commitMessage);
try (ObjectInserter oi = git.newObjectInserter()) {
RevCommit c = newCommit(oi, rw, author, mergeTip, commitMessage);
Change change = new Change(
getChangeId(id, c),
@@ -204,22 +199,11 @@ public class CreateChange implements
new Branch.NameKey(project, refName),
now);
ChangeInserter ins =
changeInserterFactory.create(refControl.getProjectControl(),
change, c);
ChangeMessage msg = new ChangeMessage(new ChangeMessage.Key(change.getId(),
ChangeUtil.messageUUID(db.get())),
me.getAccountId(),
ins.getPatchSet().getCreatedOn(),
ins.getPatchSet().getId());
msg.setMessage(String.format("Uploaded patch set %s.",
ChangeInserter ins = changeInserterFactory
.create(refControl, change, c)
.setValidatePolicy(CommitValidators.Policy.GERRIT);
ins.setMessage(String.format("Uploaded patch set %s.",
ins.getPatchSet().getPatchSetId()));
ins.setMessage(msg);
validateCommit(git, refControl, c, me, ins);
updateRef(git, rw, c, change, ins.getPatchSet());
String topic = input.topic;
if (topic != null) {
topic = Strings.emptyToNull(topic.trim());
@@ -227,46 +211,16 @@ public class CreateChange implements
change.setTopic(topic);
ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT);
ins.setGroups(groups);
ins.insert();
try (BatchUpdate bu = updateFactory.create(
db.get(), change.getProject(), me, now)) {
bu.setRepository(git, rw, oi);
bu.insertChange(ins);
bu.execute();
}
ChangeJson json = jsonFactory.create(ChangeJson.NO_OPTIONS);
return Response.created(json.format(change.getId()));
}
}
private void validateCommit(Repository git, RefControl refControl,
RevCommit c, IdentifiedUser me, ChangeInserter ins)
throws ResourceConflictException {
PatchSet newPatchSet = ins.getPatchSet();
CommitValidators commitValidators =
commitValidatorsFactory.create(refControl, new NoSshInfo(), git);
CommitReceivedEvent commitReceivedEvent =
new CommitReceivedEvent(new ReceiveCommand(
ObjectId.zeroId(),
c.getId(),
newPatchSet.getRefName()),
refControl.getProjectControl().getProject(),
refControl.getRefName(),
c,
me);
try {
commitValidators.validateForGerritCommits(commitReceivedEvent);
} catch (CommitValidationException e) {
throw new ResourceConflictException(e.getMessage());
}
}
private static void updateRef(Repository git, RevWalk rw, RevCommit c,
Change change, PatchSet newPatchSet) throws IOException {
RefUpdate ru = git.updateRef(newPatchSet.getRefName());
ru.setExpectedOldObjectId(ObjectId.zeroId());
ru.setNewObjectId(c);
ru.disableRefLog();
if (ru.update(rw) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
"Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
change.getDest().getParentKey().get(), ru.getResult()));
}
}
@@ -279,20 +233,16 @@ public class CreateChange implements
return changeKey;
}
private static RevCommit newCommit(Repository git, RevWalk rw,
private static RevCommit newCommit(ObjectInserter oi, RevWalk rw,
PersonIdent authorIdent, RevCommit mergeTip, String commitMessage)
throws IOException {
RevCommit emptyCommit;
try (ObjectInserter oi = git.newObjectInserter()) {
CommitBuilder commit = new CommitBuilder();
commit.setTreeId(mergeTip.getTree().getId());
commit.setParentId(mergeTip);
commit.setAuthor(authorIdent);
commit.setCommitter(authorIdent);
commit.setMessage(commitMessage);
emptyCommit = rw.parseCommit(insert(oi, commit));
}
return emptyCommit;
return rw.parseCommit(insert(oi, commit));
}
private static ObjectId insert(ObjectInserter inserter,

View File

@@ -76,15 +76,6 @@ public class PatchSetInserter extends BatchUpdate.Op {
RevCommit commit);
}
/**
* Whether to use {@link CommitValidators#validateForGerritCommits},
* {@link CommitValidators#validateForReceiveCommits}, or no commit
* validation.
*/
public static enum ValidatePolicy {
GERRIT, RECEIVE_COMMITS, NONE
}
// Injected fields.
private final ChangeHooks hooks;
private final PatchSetInfoFactory patchSetInfoFactory;
@@ -108,7 +99,8 @@ public class PatchSetInserter extends BatchUpdate.Op {
// Fields exposed as setters.
private SshInfo sshInfo;
private String message;
private ValidatePolicy validatePolicy = ValidatePolicy.GERRIT;
private CommitValidators.Policy validatePolicy =
CommitValidators.Policy.GERRIT;
private boolean draft;
private Iterable<String> groups;
private boolean runHooks = true;
@@ -176,7 +168,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
return this;
}
public PatchSetInserter setValidatePolicy(ValidatePolicy validate) {
public PatchSetInserter setValidatePolicy(CommitValidators.Policy validate) {
this.validatePolicy = checkNotNull(validate);
return this;
}
@@ -228,7 +220,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
@Override
public void updateChange(ChangeContext ctx) throws OrmException,
InvalidChangeOperationException {
change = ctx.readChange();
change = ctx.getChange();
Change.Id id = change.getId();
final PatchSet.Id currentPatchSetId = change.currentPatchSetId();
if (!change.getStatus().isOpen() && !allowClosed) {

View File

@@ -78,8 +78,8 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
Op op = new Op(ctl, input != null ? input : new Input());
try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
req.getChange().getProject(), TimeUtil.nowTs())) {
u.addOp(ctl, op);
req.getChange().getProject(), ctl.getCurrentUser(), TimeUtil.nowTs())) {
u.addOp(req.getChange().getId(), op);
u.execute();
}
return Strings.isNullOrEmpty(op.newTopicName)
@@ -102,7 +102,7 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
@Override
public void updateChange(ChangeContext ctx) throws OrmException {
change = ctx.readChange();
change = ctx.getChange();
String newTopicName = Strings.nullToEmpty(input.topic);
String oldTopicName = Strings.nullToEmpty(change.getTopic());
if (oldTopicName.equals(newTopicName)) {

View File

@@ -26,12 +26,12 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -141,7 +141,7 @@ public class RebaseChange {
rebase(git, rw, inserter, change, patchSet.getId(),
uploader, baseCommit, mergeUtilFactory.create(
rsrc.getControl().getProjectControl().getProjectState(), true),
committerIdent, true, ValidatePolicy.GERRIT);
committerIdent, true, CommitValidators.Policy.GERRIT);
} catch (MergeConflictException e) {
throw new ResourceConflictException(e.getMessage());
}
@@ -259,7 +259,8 @@ public class RebaseChange {
public PatchSet rebase(Repository git, RevWalk rw,
ObjectInserter inserter, Change change, PatchSet.Id patchSetId,
IdentifiedUser uploader, RevCommit baseCommit, MergeUtil mergeUtil,
PersonIdent committerIdent, boolean runHooks, ValidatePolicy validate)
PersonIdent committerIdent, boolean runHooks,
CommitValidators.Policy validate)
throws NoSuchChangeException, OrmException, IOException,
InvalidChangeOperationException, MergeConflictException, UpdateException,
RestApiException {
@@ -287,8 +288,8 @@ public class RebaseChange {
.setRunHooks(runHooks);
try (BatchUpdate bu = updateFactory.create(
db.get(), change.getDest().getParentKey(), TimeUtil.nowTs())) {
bu.addOp(changeControl, patchSetInserter.setMessage(
db.get(), change.getProject(), uploader, TimeUtil.nowTs())) {
bu.addOp(change.getId(), patchSetInserter.setMessage(
"Patch Set " + patchSetInserter.getPatchSetId().get()
+ ": Patch Set " + patchSetId.get() + " was rebased"));
bu.execute();

View File

@@ -88,8 +88,8 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
Op op = new Op(input);
try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
req.getChange().getProject(), TimeUtil.nowTs())) {
u.addOp(ctl, op).execute();
req.getChange().getProject(), ctl.getCurrentUser(), TimeUtil.nowTs())) {
u.addOp(req.getChange().getId(), op).execute();
}
return json.create(ChangeJson.NO_OPTIONS).format(op.change);
}
@@ -110,7 +110,7 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
public void updateChange(ChangeContext ctx) throws OrmException,
ResourceConflictException {
caller = (IdentifiedUser) ctx.getUser();
change = ctx.readChange();
change = ctx.getChange();
if (change == null || change.getStatus() != Status.ABANDONED) {
throw new ResourceConflictException("change is " + status(change));
}

View File

@@ -20,19 +20,18 @@ import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -59,8 +58,8 @@ public class Revert implements RestModifyView<ChangeResource, RevertInput>,
@Override
public ChangeInfo apply(ChangeResource req, RevertInput input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, IOException, OrmException, EmailException {
throws IOException, OrmException, EmailException, RestApiException,
UpdateException {
ChangeControl control = req.getControl();
Change change = req.getChange();
if (!control.canAddPatchSet()) {
@@ -74,10 +73,7 @@ public class Revert implements RestModifyView<ChangeResource, RevertInput>,
revertedChangeId = changeUtil.revert(control,
change.currentPatchSetId(),
Strings.emptyToNull(input.message),
new PersonIdent(myIdent, TimeUtil.nowTs()),
new NoSshInfo());
} catch (InvalidChangeOperationException e) {
throw new BadRequestException(e.getMessage());
new PersonIdent(myIdent, TimeUtil.nowTs()));
} catch (NoSuchChangeException e) {
throw new ResourceNotFoundException(e.getMessage());
}

View File

@@ -239,8 +239,9 @@ public class ChangeEditUtil {
}
try (BatchUpdate bu = updateFactory.create(
db.get(), change.getDest().getParentKey(), TimeUtil.nowTs())) {
bu.addOp(ctl, inserter
db.get(), change.getProject(), ctl.getCurrentUser(),
TimeUtil.nowTs())) {
bu.addOp(change.getId(), inserter
.setDraft(change.getStatus() == Status.DRAFT ||
basePatchSet.isDraft())
.setMessage(message.toString()));

View File

@@ -31,7 +31,6 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -75,7 +74,7 @@ import java.util.Map;
public class BatchUpdate implements AutoCloseable {
public interface Factory {
public BatchUpdate create(ReviewDb db, Project.NameKey project,
Timestamp when);
CurrentUser user, Timestamp when);
}
public class Context {
@@ -118,18 +117,24 @@ public class BatchUpdate implements AutoCloseable {
}
public class ChangeContext extends Context {
private final ChangeControl ctl;
private final ChangeUpdate update;
private ChangeContext(ChangeUpdate update) {
this.update = update;
private ChangeContext(ChangeControl ctl) {
this.ctl = ctl;
this.update = changeUpdateFactory.create(ctl, when);
}
public ChangeUpdate getChangeUpdate() {
return update;
}
public Change readChange() throws OrmException {
return db.changes().get(update.getChange().getId());
public ChangeControl getChangeControl() {
return ctl;
}
public Change getChange() {
return update.getChange();
}
public CurrentUser getUser() {
@@ -152,17 +157,23 @@ public class BatchUpdate implements AutoCloseable {
}
}
public abstract static class InsertChangeOp extends Op {
public abstract Change getChange();
}
private final ReviewDb db;
private final GitRepositoryManager repoManager;
private final ChangeIndexer indexer;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeUpdate.Factory changeUpdateFactory;
private final GitReferenceUpdated gitRefUpdated;
private final Project.NameKey project;
private final CurrentUser user;
private final Timestamp when;
private final ListMultimap<Change.Id, Op> ops = ArrayListMultimap.create();
private final Map<Change.Id, ChangeControl> changeControls = new HashMap<>();
private final Map<Change.Id, Change> newChanges = new HashMap<>();
private final List<CheckedFuture<?, IOException>> indexFutures =
new ArrayList<>();
@@ -175,17 +186,21 @@ public class BatchUpdate implements AutoCloseable {
@AssistedInject
BatchUpdate(GitRepositoryManager repoManager,
ChangeIndexer indexer,
ChangeControl.GenericFactory changeControlFactory,
ChangeUpdate.Factory changeUpdateFactory,
GitReferenceUpdated gitRefUpdated,
@Assisted ReviewDb db,
@Assisted Project.NameKey project,
@Assisted CurrentUser user,
@Assisted Timestamp when) {
this.db = db;
this.repoManager = repoManager;
this.indexer = indexer;
this.changeControlFactory = changeControlFactory;
this.changeUpdateFactory = changeUpdateFactory;
this.gitRefUpdated = gitRefUpdated;
this.project = project;
this.user = user;
this.when = when;
}
@@ -232,14 +247,18 @@ public class BatchUpdate implements AutoCloseable {
return inserter;
}
public BatchUpdate addOp(ChangeControl ctl, Op op) {
Change.Id id = ctl.getChange().getId();
ChangeControl old = changeControls.get(id);
// TODO(dborowitz): Not sure this is guaranteed in general.
checkArgument(old == null || old == ctl,
"mismatched ChangeControls for change %s", id);
public BatchUpdate addOp(Change.Id id, Op op) {
checkArgument(!(op instanceof InsertChangeOp), "use insertChange");
ops.put(id, op);
changeControls.put(id, ctl);
return this;
}
public BatchUpdate insertChange(InsertChangeOp op) {
Change c = op.getChange();
checkArgument(!newChanges.containsKey(c.getId()),
"only one op allowed to create change %s", c.getId());
newChanges.put(c.getId(), c);
ops.get(c.getId()).add(0, op);
return this;
}
@@ -303,18 +322,18 @@ public class BatchUpdate implements AutoCloseable {
try {
for (Map.Entry<Change.Id, Collection<Op>> e : ops.asMap().entrySet()) {
Change.Id id = e.getKey();
ChangeUpdate update =
changeUpdateFactory.create(changeControls.get(id), when);
db.changes().beginTransaction(id);
ChangeContext ctx;
try {
ctx = newChangeContext(id);
for (Op op : e.getValue()) {
op.updateChange(new ChangeContext(update));
op.updateChange(ctx);
}
db.commit();
} finally {
db.rollback();
}
update.commit();
ctx.getChangeUpdate().commit();
indexFutures.add(indexer.indexAsync(id));
}
} catch (Exception e) {
@@ -323,6 +342,18 @@ public class BatchUpdate implements AutoCloseable {
}
}
private ChangeContext newChangeContext(Change.Id id) throws Exception {
Change c = newChanges.get(id);
if (c == null) {
c = db.changes().get(id);
}
// Pass in preloaded change to controlFor, to avoid:
// - reading from a db that does not belong to this update
// - attempting to read a change that doesn't exist yet
return new ChangeContext(
changeControlFactory.controlFor(c, user));
}
private void reindexChanges() throws IOException {
ChangeIndexer.allAsList(indexFutures).checkedGet();
}

View File

@@ -136,6 +136,7 @@ import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
@@ -305,6 +306,7 @@ public class ReceiveCommits {
private final AllProjectsName allProjectsName;
private final ReceiveConfig receiveConfig;
private final ChangeKindCache changeKindCache;
private final BatchUpdate.Factory batchUpdateFactory;
private final ProjectControl projectControl;
private final Project project;
@@ -381,7 +383,8 @@ public class ReceiveCommits {
final ChangeKindCache changeKindCache,
final DynamicMap<ProjectConfigEntry> pluginConfigEntries,
final NotesMigration notesMigration,
final ChangeEditUtil editUtil) throws IOException {
final ChangeEditUtil editUtil,
final BatchUpdate.Factory batchUpdateFactory) throws IOException {
this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
this.db = db;
this.queryProvider = queryProvider;
@@ -414,6 +417,7 @@ public class ReceiveCommits {
this.allProjectsName = allProjectsName;
this.receiveConfig = config;
this.changeKindCache = changeKindCache;
this.batchUpdateFactory = batchUpdateFactory;
this.projectControl = projectControl;
this.labelTypes = projectControl.getLabelTypes();
@@ -1717,8 +1721,10 @@ public class ReceiveCommits {
magicBranch.dest,
TimeUtil.nowTs());
change.setTopic(magicBranch.topic);
ins = changeInserterFactory.create(ctl.getProjectControl(), change, c)
.setDraft(magicBranch.draft);
ins = changeInserterFactory.create(ctl, change, c)
.setDraft(magicBranch.draft)
// Changes already validated in validateNewCommits.
.setValidatePolicy(CommitValidators.Policy.NONE);
cmd = new ReceiveCommand(ObjectId.zeroId(), c,
ins.getPatchSet().getRefName());
}
@@ -1726,19 +1732,12 @@ public class ReceiveCommits {
CheckedFuture<Void, RestApiException> insertChange() throws IOException {
rp.getRevWalk().parseBody(commit);
final Thread caller = Thread.currentThread();
ListenableFuture<Void> future = changeUpdateExector.submit(
requestScopePropagator.wrap(new Callable<Void>() {
@Override
public Void call() throws OrmException, IOException,
ResourceConflictException {
if (caller == Thread.currentThread()) {
insertChange(db);
} else {
try (ReviewDb db = schemaFactory.open()) {
insertChange(db);
}
}
public Void call()
throws OrmException, RestApiException, UpdateException {
insertChangeImpl();
synchronized (newProgress) {
newProgress.update(1);
}
@@ -1748,8 +1747,8 @@ public class ReceiveCommits {
return Futures.makeChecked(future, INSERT_EXCEPTION);
}
private void insertChange(ReviewDb db) throws OrmException, IOException,
ResourceConflictException {
private void insertChangeImpl()
throws OrmException, RestApiException, UpdateException {
final PatchSet ps = ins.setGroups(groups).getPatchSet();
final Account.Id me = currentUser.getAccountId();
final List<FooterLine> footerLines = commit.getFooterLines();
@@ -1762,20 +1761,20 @@ public class ReceiveCommits {
}
recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines));
recipients.remove(me);
ChangeMessage msg =
new ChangeMessage(new ChangeMessage.Key(change.getId(),
ChangeUtil.messageUUID(db)), me, ps.getCreatedOn(), ps.getId());
msg.setMessage("Uploaded patch set " + ps.getPatchSetId() + ".");
ins
try (ObjectInserter oi = repo.newObjectInserter();
BatchUpdate bu = batchUpdateFactory.create(
db, change.getProject(), currentUser, change.getCreatedOn())) {
bu.setRepository(repo, rp.getRevWalk(), oi);
bu.insertChange(ins
.setReviewers(recipients.getReviewers())
.setExtraCC(recipients.getCcOnly())
.setApprovals(approvals)
.setMessage(msg)
.setMessage("Uploaded patch set " + ps.getPatchSetId() + ".")
.setRequestScopePropagator(requestScopePropagator)
.setSendMail(true)
.insert();
.setUpdateRef(false));
bu.execute();
}
created = true;
if (magicBranch != null && magicBranch.submit) {

View File

@@ -72,14 +72,15 @@ public class CherryPick extends SubmitStrategy {
try (BatchUpdate u = args.newBatchUpdate(TimeUtil.nowTs())) {
while (!sorted.isEmpty()) {
CodeReviewCommit n = sorted.remove(0);
Change.Id cid = n.change().getId();
if (first && branchTip == null) {
u.addOp(n.getControl(), new CherryPickUnbornRootOp(mergeTip, n));
u.addOp(cid, new CherryPickUnbornRootOp(mergeTip, n));
} else if (n.getParentCount() == 0) {
u.addOp(n.getControl(), new CherryPickRootOp(n));
u.addOp(cid, new CherryPickRootOp(n));
} else if (n.getParentCount() == 1) {
u.addOp(n.getControl(), new CherryPickOneOp(mergeTip, n));
u.addOp(cid, new CherryPickOneOp(mergeTip, n));
} else {
u.addOp(n.getControl(), new CherryPickMultipleParentsOp(mergeTip, n));
u.addOp(cid, new CherryPickMultipleParentsOp(mergeTip, n));
}
first = false;
}

View File

@@ -19,7 +19,6 @@ 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.PatchSetApproval;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.change.RebaseChange;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
@@ -28,6 +27,7 @@ import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.RebaseSorter;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -90,7 +90,8 @@ public class RebaseIfNecessary extends SubmitStrategy {
rebaseChange.rebase(args.repo, args.rw, args.inserter,
n.change(), n.getPatchsetId(), args.caller,
mergeTip.getCurrentTip(), args.mergeUtil,
args.serverIdent.get(), false, ValidatePolicy.NONE);
args.serverIdent.get(), false,
CommitValidators.Policy.NONE);
List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db,
n.getControl(), n.getPatchsetId())) {

View File

@@ -100,7 +100,8 @@ public abstract class SubmitStrategy {
}
BatchUpdate newBatchUpdate(Timestamp when) {
return batchUpdateFactory.create(db, destBranch.getParentKey(), when)
return batchUpdateFactory
.create(db, destBranch.getParentKey(), caller, when)
.setRepository(repo, rw, inserter);
}
}

View File

@@ -60,6 +60,17 @@ public class CommitValidators {
private static final Logger log = LoggerFactory
.getLogger(CommitValidators.class);
public static enum Policy {
/** Use {@link #validateForGerritCommits}. */
GERRIT,
/** Use {@link #validateForReceiveCommits}. */
RECEIVE_COMMITS,
/** Do not validate commits. */
NONE
}
public interface Factory {
CommitValidators create(RefControl refControl, SshInfo sshInfo,
Repository repo);

View File

@@ -53,12 +53,13 @@ import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -212,8 +213,8 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byId() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
assertQuery("12345");
assertQuery(change1.getId().get(), change1);
@@ -223,7 +224,7 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byKey() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = newChange(repo, null, null, null, null).insert();
Change change = insert(newChange(repo, null, null, null, null));
String key = change.getKey().get();
assertQuery("I0000000000000000000000000000000000000000");
@@ -236,7 +237,7 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byTriplet() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = newChange(repo, null, null, null, "branch").insert();
Change change = insert(newChange(repo, null, null, null, "branch"));
String k = change.getKey().get();
assertQuery("repo~branch~" + k, change);
@@ -262,11 +263,11 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.getChange();
change1.setStatus(Change.Status.NEW);
ins1.insert();
insert(ins1);
ChangeInserter ins2 = newChange(repo, null, null, null, null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.MERGED);
ins2.insert();
insert(ins2);
assertQuery("status:new", change1);
assertQuery("status:NEW", change1);
@@ -281,15 +282,15 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.getChange();
change1.setStatus(Change.Status.NEW);
ins1.insert();
insert(ins1);
ChangeInserter ins2 = newChange(repo, null, null, null, null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.DRAFT);
ins2.insert();
insert(ins2);
ChangeInserter ins3 = newChange(repo, null, null, null, null);
Change change3 = ins3.getChange();
change3.setStatus(Change.Status.MERGED);
ins3.insert();
insert(ins3);
Change[] expected = new Change[] {change2, change1};
assertQuery("status:open", expected);
@@ -311,15 +312,15 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.getChange();
change1.setStatus(Change.Status.MERGED);
ins1.insert();
insert(ins1);
ChangeInserter ins2 = newChange(repo, null, null, null, null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.ABANDONED);
ins2.insert();
insert(ins2);
ChangeInserter ins3 = newChange(repo, null, null, null, null);
Change change3 = ins3.getChange();
change3.setStatus(Change.Status.NEW);
ins3.insert();
insert(ins3);
Change[] expected = new Change[] {change2, change1};
assertQuery("status:closed", expected);
@@ -339,11 +340,11 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.getChange();
change1.setStatus(Change.Status.NEW);
ins1.insert();
insert(ins1);
ChangeInserter ins2 = newChange(repo, null, null, null, null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.MERGED);
ins2.insert();
insert(ins2);
assertQuery("status:n", change1);
assertQuery("status:ne", change1);
@@ -359,7 +360,7 @@ public abstract class AbstractQueryChangesTest {
public void byCommit() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
ins.insert();
insert(ins);
String sha = ins.getPatchSet().getRevision().get();
assertQuery("0000000000000000000000000000000000000000");
@@ -372,10 +373,10 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byOwner() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
Change change2 = newChange(repo, null, null, user2, null).insert();
Change change2 = insert(newChange(repo, null, null, user2, null));
assertQuery("owner:" + userId.get(), change1);
assertQuery("owner:" + user2, change2);
@@ -384,7 +385,7 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byAuthor() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
// By exact email address
assertQuery("author:jauthor@example.com", change1);
@@ -407,7 +408,7 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byCommitter() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
// By exact email address
assertQuery("committer:jcommitter@example.com", change1);
@@ -430,10 +431,10 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byOwnerIn() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
Change change2 = newChange(repo, null, null, user2, null).insert();
Change change2 = insert(newChange(repo, null, null, user2, null));
assertQuery("ownerin:Administrators", change1);
assertQuery("ownerin:\"Registered Users\"", change2, change1);
@@ -443,8 +444,8 @@ public abstract class AbstractQueryChangesTest {
public void byProject() throws Exception {
TestRepository<Repo> repo1 = createProject("repo1");
TestRepository<Repo> repo2 = createProject("repo2");
Change change1 = newChange(repo1, null, null, null, null).insert();
Change change2 = newChange(repo2, null, null, null, null).insert();
Change change1 = insert(newChange(repo1, null, null, null, null));
Change change2 = insert(newChange(repo2, null, null, null, null));
assertQuery("project:foo");
assertQuery("project:repo");
@@ -456,8 +457,8 @@ public abstract class AbstractQueryChangesTest {
public void byProjectPrefix() throws Exception {
TestRepository<Repo> repo1 = createProject("repo1");
TestRepository<Repo> repo2 = createProject("repo2");
Change change1 = newChange(repo1, null, null, null, null).insert();
Change change2 = newChange(repo2, null, null, null, null).insert();
Change change1 = insert(newChange(repo1, null, null, null, null));
Change change2 = insert(newChange(repo2, null, null, null, null));
assertQuery("projects:foo");
assertQuery("projects:repo1", change1);
@@ -468,8 +469,8 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byBranchAndRef() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, "master").insert();
Change change2 = newChange(repo, null, null, null, "branch").insert();
Change change1 = insert(newChange(repo, null, null, null, "master"));
Change change2 = insert(newChange(repo, null, null, null, "branch"));
assertQuery("branch:foo");
assertQuery("branch:master", change1);
@@ -489,24 +490,24 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.getChange();
change1.setTopic("feature1");
ins1.insert();
insert(ins1);
ChangeInserter ins2 = newChange(repo, null, null, null, null);
Change change2 = ins2.getChange();
change2.setTopic("feature2");
ins2.insert();
insert(ins2);
ChangeInserter ins3 = newChange(repo, null, null, null, null);
Change change3 = ins3.getChange();
change3.setTopic("Cherrypick-feature2");
ins3.insert();
insert(ins3);
ChangeInserter ins4 = newChange(repo, null, null, null, null);
Change change4 = ins4.getChange();
change4.setTopic("feature2-fixup");
ins4.insert();
insert(ins4);
Change change5 = newChange(repo, null, null, null, null).insert();
Change change5 = insert(newChange(repo, null, null, null, null));
assertQuery("intopic:foo");
assertQuery("intopic:feature1", change1);
@@ -522,9 +523,9 @@ public abstract class AbstractQueryChangesTest {
public void byMessageExact() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().message("one").create());
Change change1 = newChange(repo, commit1, null, null, null).insert();
Change change1 = insert(newChange(repo, commit1, null, null, null));
RevCommit commit2 = repo.parseBody(repo.commit().message("two").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change2 = insert(newChange(repo, commit2, null, null, null));
assertQuery("message:foo");
assertQuery("message:one", change1);
@@ -536,10 +537,10 @@ public abstract class AbstractQueryChangesTest {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 =
repo.parseBody(repo.commit().message("12345 67890").create());
Change change1 = newChange(repo, commit1, null, null, null).insert();
Change change1 = insert(newChange(repo, commit1, null, null, null));
RevCommit commit2 =
repo.parseBody(repo.commit().message("12346 67891").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change2 = insert(newChange(repo, commit2, null, null, null));
assertQuery("message:1234");
assertQuery("message:12345", change1);
@@ -551,7 +552,7 @@ public abstract class AbstractQueryChangesTest {
accountManager.authenticate(AuthRequest.forUser("anotheruser"));
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
Change change = ins.insert();
Change change = insert(ins);
gApi.changes().id(change.getId().get()).current()
.review(new ReviewInput().label("Code-Review", 1));
@@ -593,7 +594,7 @@ public abstract class AbstractQueryChangesTest {
Change last = null;
int n = 5;
for (int i = 0; i < n; i++) {
last = newChange(repo, null, null, null, null).insert();
last = insert(newChange(repo, null, null, null, null));
}
for (int i = 1; i <= n + 2; i++) {
@@ -620,7 +621,7 @@ public abstract class AbstractQueryChangesTest {
TestRepository<Repo> repo = createProject("repo");
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 2; i++) {
changes.add(newChange(repo, null, null, null, null).insert());
changes.add(insert(newChange(repo, null, null, null, null)));
}
assertQuery("status:new", changes.get(1), changes.get(0));
@@ -634,7 +635,7 @@ public abstract class AbstractQueryChangesTest {
TestRepository<Repo> repo = createProject("repo");
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 3; i++) {
changes.add(newChange(repo, null, null, null, null).insert());
changes.add(insert(newChange(repo, null, null, null, null)));
}
assertQuery("status:new limit:2", changes.get(2), changes.get(1));
@@ -648,7 +649,7 @@ public abstract class AbstractQueryChangesTest {
@Test
public void maxPages() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = newChange(repo, null, null, null, null).insert();
Change change = insert(newChange(repo, null, null, null, null));
QueryRequest query = newQuery("status:new").withLimit(10);
assertQuery(query, change);
@@ -666,7 +667,7 @@ public abstract class AbstractQueryChangesTest {
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 5; i++) {
inserters.add(newChange(repo, null, null, null, null));
changes.add(inserters.get(i).insert());
changes.add(insert(inserters.get(i)));
}
for (int i : ImmutableList.of(2, 0, 1, 4, 3)) {
@@ -688,8 +689,8 @@ public abstract class AbstractQueryChangesTest {
clockStepMs = MILLISECONDS.convert(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(ins1);
Change change2 = insert(newChange(repo, null, null, null, null));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
assertQuery("status:new", change2, change1);
@@ -710,8 +711,8 @@ public abstract class AbstractQueryChangesTest {
public void updatedOrderWithSubMinuteResolution() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo, null, null, null, null);
Change change1 = ins1.insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(ins1);
Change change2 = insert(newChange(repo, null, null, null, null));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
@@ -732,11 +733,11 @@ public abstract class AbstractQueryChangesTest {
@Test
public void filterOutMoreThanOnePageOfResults() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = newChange(repo, null, null, userId.get(), null).insert();
Change change = insert(newChange(repo, null, null, userId.get(), null));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
for (int i = 0; i < 5; i++) {
newChange(repo, null, null, user2, null).insert();
insert(newChange(repo, null, null, user2, null));
}
assertQuery("status:new ownerin:Administrators", change);
@@ -749,7 +750,7 @@ public abstract class AbstractQueryChangesTest {
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
for (int i = 0; i < 5; i++) {
newChange(repo, null, null, user2, null).insert();
insert(newChange(repo, null, null, user2, null));
}
assertQuery("status:new ownerin:Administrators");
@@ -763,7 +764,7 @@ public abstract class AbstractQueryChangesTest {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
Change change = insert(newChange(repo, commit, null, null, null));
assertQuery("file:file");
assertQuery("file:dir", change);
@@ -780,7 +781,7 @@ public abstract class AbstractQueryChangesTest {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
Change change = insert(newChange(repo, commit, null, null, null));
assertQuery("file:.*file.*");
assertQuery("file:^file.*"); // Whole path only.
@@ -794,7 +795,7 @@ public abstract class AbstractQueryChangesTest {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
Change change = insert(newChange(repo, commit, null, null, null));
assertQuery("path:file");
assertQuery("path:dir");
@@ -811,7 +812,7 @@ public abstract class AbstractQueryChangesTest {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
Change change = insert(newChange(repo, commit, null, null, null));
assertQuery("path:.*file.*");
assertQuery("path:^dir.file.*", change);
@@ -821,7 +822,7 @@ public abstract class AbstractQueryChangesTest {
public void byComment() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo, null, null, null, null);
Change change = ins.insert();
Change change = insert(ins);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
@@ -842,8 +843,8 @@ public abstract class AbstractQueryChangesTest {
long thirtyHours = MILLISECONDS.convert(30, HOURS);
clockStepMs = thirtyHours;
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
clockStepMs = 0; // Queried by AgePredicate constructor.
long now = TimeUtil.nowMs();
assertThat(lastUpdatedMs(change2) - lastUpdatedMs(change1))
@@ -864,8 +865,8 @@ public abstract class AbstractQueryChangesTest {
public void byBefore() throws Exception {
clockStepMs = MILLISECONDS.convert(30, HOURS);
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
clockStepMs = 0;
assertQuery("before:2009-09-29");
@@ -884,8 +885,8 @@ public abstract class AbstractQueryChangesTest {
public void byAfter() throws Exception {
clockStepMs = MILLISECONDS.convert(30, HOURS);
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
clockStepMs = 0;
assertQuery("after:2009-10-03");
@@ -906,8 +907,8 @@ public abstract class AbstractQueryChangesTest {
RevCommit commit2 = repo.parseBody(
repo.commit().parent(commit1).add("file1", "foo").create());
Change change1 = newChange(repo, commit1, null, null, null).insert();
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change1 = insert(newChange(repo, commit1, null, null, null));
Change change2 = insert(newChange(repo, commit2, null, null, null));
assertQuery("added:>4");
assertQuery("-added:<=4");
@@ -956,8 +957,8 @@ public abstract class AbstractQueryChangesTest {
private List<Change> setUpHashtagChanges() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
HashtagsInput in = new HashtagsInput();
in.add = ImmutableSet.of("foo");
@@ -999,20 +1000,20 @@ public abstract class AbstractQueryChangesTest {
public void byDefault() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
RevCommit commit2 = repo.parseBody(
repo.commit().message("foosubject").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change2 = insert(newChange(repo, commit2, null, null, null));
RevCommit commit3 = repo.parseBody(
repo.commit()
.add("Foo.java", "foo contents")
.create());
Change change3 = newChange(repo, commit3, null, null, null).insert();
Change change3 = insert(newChange(repo, commit3, null, null, null));
ChangeInserter ins4 = newChange(repo, null, null, null, null);
Change change4 = ins4.insert();
Change change4 = insert(ins4);
ReviewInput ri4 = new ReviewInput();
ri4.message = "toplevel";
ri4.labels = ImmutableMap.<String, Short> of("Code-Review", (short) 1);
@@ -1021,9 +1022,9 @@ public abstract class AbstractQueryChangesTest {
ChangeInserter ins5 = newChange(repo, null, null, null, null);
Change change5 = ins5.getChange();
change5.setTopic("feature5");
ins5.insert();
insert(ins5);
Change change6 = newChange(repo, null, null, null, "branch6").insert();
Change change6 = insert(newChange(repo, null, null, null, "branch6"));
assertQuery(change1.getId().get(), change1);
assertQuery(ChangeTriplet.format(change1), change1);
@@ -1044,11 +1045,11 @@ public abstract class AbstractQueryChangesTest {
@Test
public void implicitVisibleTo() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
ChangeInserter ins2 = newChange(repo, null, null, userId.get(), null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.DRAFT);
ins2.insert();
insert(ins2);
String q = "project:repo";
assertQuery(q, change2, change1);
@@ -1062,11 +1063,11 @@ public abstract class AbstractQueryChangesTest {
@Test
public void explicitVisibleTo() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, userId.get(), null).insert();
Change change1 = insert(newChange(repo, null, null, userId.get(), null));
ChangeInserter ins2 = newChange(repo, null, null, userId.get(), null);
Change change2 = ins2.getChange();
change2.setStatus(Change.Status.DRAFT);
ins2.insert();
insert(ins2);
String q = "project:repo";
assertQuery(q, change2, change1);
@@ -1081,8 +1082,8 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byCommentBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
@@ -1107,12 +1108,12 @@ public abstract class AbstractQueryChangesTest {
@Test
public void byFrom() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
ChangeInserter ins2 = newChange(repo, null, null, user2, null);
Change change2 = ins2.insert();
Change change2 = insert(ins2);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
@@ -1148,10 +1149,10 @@ public abstract class AbstractQueryChangesTest {
repo.commit()
.add("file4", "contents4")
.create());
Change change1 = newChange(repo, commit1, null, null, null).insert();
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change3 = newChange(repo, commit3, null, null, null).insert();
Change change4 = newChange(repo, commit4, null, null, null).insert();
Change change1 = insert(newChange(repo, commit1, null, null, null));
Change change2 = insert(newChange(repo, commit2, null, null, null));
Change change3 = insert(newChange(repo, commit3, null, null, null));
Change change4 = insert(newChange(repo, commit4, null, null, null));
assertQuery("conflicts:" + change1.getId().get(), change3);
assertQuery("conflicts:" + change2.getId().get());
@@ -1163,9 +1164,9 @@ public abstract class AbstractQueryChangesTest {
public void reviewedBy() throws Exception {
clockStepMs = MILLISECONDS.convert(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change3 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
Change change3 = insert(newChange(repo, null, null, null, null));
gApi.changes()
.id(change1.getId().get())
@@ -1221,7 +1222,7 @@ public abstract class AbstractQueryChangesTest {
Branch.NameKey dest = null;
for (int i = 0; i < n; i++) {
ChangeInserter ins = newChange(repo, null, null, null, null);
ins.insert();
insert(ins);
if (dest == null) {
dest = ins.getChange().getDest();
}
@@ -1250,7 +1251,7 @@ public abstract class AbstractQueryChangesTest {
public void prepopulatedFields() throws Exception {
assume().that(notesMigration.enabled()).isFalse();
TestRepository<Repo> repo = createProject("repo");
Change change = newChange(repo, null, null, null, null).insert();
Change change = insert(newChange(repo, null, null, null, null));
db = new DisabledReviewDb();
requestContext.setContext(newRequestContext(userId));
@@ -1311,10 +1312,20 @@ public abstract class AbstractQueryChangesTest {
Change change = new Change(new Change.Key(key), id, ownerId,
new Branch.NameKey(project, branch), TimeUtil.nowTs());
IdentifiedUser user = userFactory.create(Providers.of(db), ownerId);
return changeFactory.create(
projectControlFactory.controlFor(project, user),
change,
commit);
RefControl refControl = projectControlFactory.controlFor(project, user)
.controlForRef(change.getDest());
return changeFactory.create(refControl, change, commit)
.setValidatePolicy(CommitValidators.Policy.NONE);
}
protected Change insert(ChangeInserter ins) throws Exception {
try (BatchUpdate bu = updateFactory.create(
db, ins.getChange().getProject(), ins.getUser(),
ins.getChange().getCreatedOn())) {
bu.insertChange(ins);
bu.execute();
return ins.getChange();
}
}
protected Change newPatchSet(TestRepository<Repo> repo, Change c)
@@ -1333,10 +1344,10 @@ public abstract class AbstractQueryChangesTest {
repo.getRepository(), repo.getRevWalk(), ctl, commit)
.setSendMail(false)
.setRunHooks(false)
.setValidatePolicy(ValidatePolicy.NONE);
.setValidatePolicy(CommitValidators.Policy.NONE);
try (BatchUpdate bu = updateFactory.create(
db, c.getDest().getParentKey(), TimeUtil.nowTs())) {
bu.addOp(ctl, inserter);
db, c.getDest().getParentKey(), user, TimeUtil.nowTs())) {
bu.addOp(c.getId(), inserter);
bu.execute();
}

View File

@@ -38,10 +38,10 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 =
repo.parseBody(repo.commit().message("foo_bar_foo").create());
Change change1 = newChange(repo, commit1, null, null, null).insert();
Change change1 = insert(newChange(repo, commit1, null, null, null));
RevCommit commit2 =
repo.parseBody(repo.commit().message("one.two.three").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
Change change2 = insert(newChange(repo, commit2, null, null, null));
assertQuery("message:foo_ba");
assertQuery("message:bar", change1);

View File

@@ -82,9 +82,9 @@ public class LuceneQueryChangesV14Test extends LuceneQueryChangesTest {
public void isReviewed() throws Exception {
clockStepMs = MILLISECONDS.convert(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
Change change2 = newChange(repo, null, null, null, null).insert();
Change change3 = newChange(repo, null, null, null, null).insert();
Change change1 = insert(newChange(repo, null, null, null, null));
Change change2 = insert(newChange(repo, null, null, null, null));
Change change3 = insert(newChange(repo, null, null, null, null));
gApi.changes()
.id(change1.getId().get())