diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java index 0a7262efad..29cb0d7881 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java @@ -37,11 +37,12 @@ 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.InvalidChangeOperationException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.SetParent; @@ -51,6 +52,7 @@ 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; @@ -73,6 +75,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { private final ProjectCache projectCache; private final ChangesCollection changes; private final ChangeInserter.Factory changeInserterFactory; + private final BatchUpdate.Factory updateFactory; @Inject ReviewProjectAccess(final ProjectControl.Factory projectControlFactory, @@ -84,6 +87,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { AllProjectsNameProvider allProjects, ChangesCollection changes, ChangeInserter.Factory changeInserterFactory, + BatchUpdate.Factory updateFactory, Provider setParent, @Assisted("projectName") Project.NameKey projectName, @@ -100,6 +104,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { this.projectCache = projectCache; this.changes = changes; this.changeInserterFactory = changeInserterFactory; + this.updateFactory = updateFactory; } @Override @@ -123,12 +128,19 @@ public class ReviewProjectAccess extends ProjectAccessHandler { config.getProject().getNameKey(), RefNames.REFS_CONFIG), TimeUtil.nowTs()); - try (RevWalk rw = new RevWalk(md.getRepository())) { - changeInserterFactory.create(md.getRepository(), rw, ctl, change, commit) - .setValidatePolicy(CommitValidators.Policy.NONE) - .setUpdateRef(false) // Created by commitToNewRef. - .insert(); - } catch (InvalidChangeOperationException e) { + 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); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index fb6b629d13..9f69c440db 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -22,6 +22,7 @@ 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.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetAncestor; @@ -32,12 +33,13 @@ 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.extensions.events.GitReferenceUpdated; +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.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; @@ -196,6 +198,7 @@ public class ChangeUtil { private final GitRepositoryManager gitManager; private final GitReferenceUpdated gitRefUpdated; private final ChangeIndexer indexer; + private final BatchUpdate.Factory updateFactory; @Inject ChangeUtil(Provider userProvider, @@ -205,7 +208,8 @@ public class ChangeUtil { ChangeInserter.Factory changeInserterFactory, GitRepositoryManager gitManager, GitReferenceUpdated gitRefUpdated, - ChangeIndexer indexer) { + ChangeIndexer indexer, + BatchUpdate.Factory updateFactory) { this.userProvider = userProvider; this.db = db; this.queryProvider = queryProvider; @@ -214,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) throws NoSuchChangeException, OrmException, MissingObjectException, IncorrectObjectTypeException, IOException, - InvalidChangeOperationException { + RestApiException, UpdateException { Change.Id changeId = patchSetId.getParentKey(); PatchSet patch = db.get().patchSets().get(patchSetId); if (patch == null) { @@ -259,42 +264,49 @@ 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( + new Change.Key("I" + computedChangeId.name()), + new Change.Id(db.get().nextChangeId()), + user().getAccountId(), + changeToRevert.getDest(), + TimeUtil.nowTs()); + change.setTopic(changeToRevert.getTopic()); + 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()); + 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(); + } } - RefControl refControl = ctl.getRefControl(); - Change change = new Change( - new Change.Key("I" + computedChangeId.name()), - new Change.Id(db.get().nextChangeId()), - user().getAccountId(), - changeToRevert.getDest(), - TimeUtil.nowTs()); - change.setTopic(changeToRevert.getTopic()); - ChangeInserter ins = changeInserterFactory.create( - git, revWalk, refControl.getProjectControl(), 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()); - ins.setMessage(msgBuf.toString()) - .insert(); - + Change.Id id = ins.getChange().getId(); try { - RevertedSender cm = revertedSenderFactory.create(change.getId()); + 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 " + change.getId(), - err); + log.error("Cannot send email for revert change " + id, err); } - return change.getId(); + return id; } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(changeId, e); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index 4d485898da..5aa5519ba7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -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); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java index f7705a7dfd..41e34dd5d0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java @@ -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); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 9c43257ff4..b546c4b74e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -36,34 +36,31 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.events.CommitReceivedEvent; -import com.google.gerrit.server.extensions.events.GitReferenceUpdated; 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.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidators; -import com.google.gerrit.server.index.ChangeIndexer; 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.InvalidChangeOperationException; -import com.google.gerrit.server.project.ProjectControl; 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.lib.RefUpdate; -import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.notes.NoteMap; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,30 +70,23 @@ 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(Repository git, RevWalk revWalk, 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 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 Repository git; - private final RevWalk revWalk; private final RefControl refControl; private final IdentifiedUser user; private final Change change; @@ -121,40 +111,35 @@ public class ChangeInserter { private ChangeMessage changeMessage; @Inject - ChangeInserter(Provider 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, CommitValidators.Factory commitValidatorsFactory, - @Assisted Repository git, - @Assisted RevWalk revWalk, - @Assisted ProjectControl projectControl, + @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.commitValidatorsFactory = commitValidatorsFactory; - this.git = git; - this.revWalk = revWalk; - this.refControl = projectControl.controlForRef(change.getDest()); + this.refControl = refControl; this.change = change; this.commit = commit; this.reviewers = Collections.emptySet(); @@ -165,7 +150,7 @@ public class ChangeInserter { this.sendMail = true; this.updateRef = true; - user = checkUser(projectControl); + user = checkUser(refControl); patchSet = new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID)); patchSet.setCreatedOn(change.getCreatedOn()); @@ -175,16 +160,21 @@ public class ChangeInserter { change.setCurrentPatchSet(patchSetInfo); } - private static IdentifiedUser checkUser(ProjectControl ctl) { + 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 IdentifiedUser getUser() { + return user; + } + public ChangeInserter setMessage(String message) { this.message = message; return this; @@ -263,58 +253,58 @@ public class ChangeInserter { return changeMessage; } - public Change insert() - throws OrmException, IOException, InvalidChangeOperationException { - validate(); - - updateRef(); - - ReviewDb db = dbProvider.get(); - ProjectControl projectControl = refControl.getProjectControl(); - ChangeControl ctl = projectControl.controlFor(change); - ChangeUpdate update = updateFactory.create( - ctl, - change.getCreatedOn()); - 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(); - approvalsUtil.addReviewers(db, update, labelTypes, change, - patchSet, patchSetInfo, reviewers, Collections. emptySet()); - approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo, - ctl, 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(); + @Override + public void updateRepo(RepoContext ctx) + throws InvalidChangeOperationException, IOException { + validate(ctx); + if (!updateRef) { + return; } + ctx.addRefUpdate( + new ReceiveCommand(ObjectId.zeroId(), commit, patchSet.getRefName())); + } - update.commit(); + @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()); + 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 = ctl.getProjectControl().getLabelTypes(); + approvalsUtil.addReviewers(db, update, labelTypes, change, + patchSet, patchSetInfo, reviewers, Collections. emptySet()); + approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo, + 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); + } 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); } } + } - indexer.index(db, change); - + @Override + public void postUpdate(Context ctx) throws OrmException { if (sendMail) { Runnable sender = new Runnable() { @Override @@ -344,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, @@ -355,31 +343,15 @@ public class ChangeInserter { hashtags, null, hashtags, db); } } - - return change; } - private void updateRef() throws IOException { - if (!updateRef) { - return; - } - RefUpdate ru = git.updateRef(patchSet.getRefName()); - ru.setExpectedOldObjectId(ObjectId.zeroId()); - ru.setNewObjectId(commit); - ru.disableRefLog(); - if (ru.update(revWalk) != RefUpdate.Result.NEW) { - throw new IOException(String.format( - "Failed to create ref %s in %s: %s", ru.getRef().getName(), - change.getDest().getParentKey().get(), ru.getResult())); - } - } - - private void validate() throws IOException, InvalidChangeOperationException { + private void validate(RepoContext ctx) + throws IOException, InvalidChangeOperationException { if (validatePolicy == CommitValidators.Policy.NONE) { return; } - CommitValidators cv = - commitValidatorsFactory.create(refControl, new NoSshInfo(), git); + CommitValidators cv = commitValidatorsFactory.create( + refControl, new NoSshInfo(), ctx.getRepository()); String refName = patchSet.getId().toRefName(); CommitReceivedEvent event = new CommitReceivedEvent( @@ -388,14 +360,15 @@ public class ChangeInserter { commit.getId(), refName), refControl.getProjectControl().getProject(), - refControl.getRefName(), + change.getDest().get(), commit, user); try { switch (validatePolicy) { case RECEIVE_COMMITS: - NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(git, revWalk); + NoteMap rejectCommits = BanCommit.loadRejectCommitsMap( + ctx.getRepository(), ctx.getRevWalk()); cv.validateForReceiveCommits(event, rejectCommits); break; case GERRIT: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index 8c6cb47274..8fbe447c1d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -154,50 +154,50 @@ public class CherryPickChange { cherryPickCommit = mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip, commitToCherryPick, committerIdent, commitMessage, revWalk); + + Change.Key changeKey; + final List idList = cherryPickCommit.getFooterLines( + FooterConstants.CHANGE_ID); + if (!idList.isEmpty()) { + final String idStr = idList.get(idList.size() - 1).trim(); + changeKey = new Change.Key(idStr); + } else { + changeKey = new Change.Key("I" + computedChangeId.name()); + } + + Branch.NameKey newDest = + new Branch.NameKey(change.getProject(), destRef.getName()); + List destChanges = queryProvider.get() + .setLimit(2) + .byBranchKey(newDest, changeKey); + if (destChanges.size() > 1) { + throw new InvalidChangeOperationException("Several changes with key " + + changeKey + " reside on the same branch. " + + "Cannot create a new patch set."); + } else if (destChanges.size() == 1) { + // The change key exists on the destination branch. The cherry pick + // will be added as a new patch set. + return insertPatchSet(git, revWalk, destChanges.get(0).change(), + cherryPickCommit, refControl, identifiedUser); + } else { + // Change key not found on destination branch. We can create a new + // change. + String newTopic = null; + if (!Strings.isNullOrEmpty(change.getTopic())) { + newTopic = change.getTopic() + "-" + newDest.getShortName(); + } + Change newChange = createNewChange(git, revWalk, oi, changeKey, + project, destRef, cherryPickCommit, refControl, identifiedUser, + newTopic, change.getDest()); + + addMessageToSourceChange(change, patch.getId(), destinationBranch, + cherryPickCommit, identifiedUser, refControl); + + return newChange.getId(); + } } catch (MergeIdenticalTreeException | MergeConflictException e) { throw new MergeException("Cherry pick failed: " + e.getMessage()); } - - Change.Key changeKey; - final List idList = cherryPickCommit.getFooterLines( - FooterConstants.CHANGE_ID); - if (!idList.isEmpty()) { - final String idStr = idList.get(idList.size() - 1).trim(); - changeKey = new Change.Key(idStr); - } else { - changeKey = new Change.Key("I" + computedChangeId.name()); - } - - Branch.NameKey newDest = - new Branch.NameKey(change.getProject(), destRef.getName()); - List destChanges = queryProvider.get() - .setLimit(2) - .byBranchKey(newDest, changeKey); - if (destChanges.size() > 1) { - throw new InvalidChangeOperationException("Several changes with key " - + changeKey + " reside on the same branch. " - + "Cannot create a new patch set."); - } else if (destChanges.size() == 1) { - // The change key exists on the destination branch. The cherry pick - // will be added as a new patch set. - return insertPatchSet(git, revWalk, destChanges.get(0).change(), - cherryPickCommit, refControl, identifiedUser); - } else { - // Change key not found on destination branch. We can create a new - // change. - String newTopic = null; - if (!Strings.isNullOrEmpty(change.getTopic())) { - newTopic = change.getTopic() + "-" + newDest.getShortName(); - } - Change newChange = createNewChange(git, revWalk, changeKey, project, - destRef, cherryPickCommit, refControl, - identifiedUser, newTopic, change.getDest()); - - addMessageToSourceChange(change, patch.getId(), destinationBranch, - cherryPickCommit, identifiedUser, refControl); - - return newChange.getId(); - } } catch (RepositoryNotFoundException e) { throw new NoSuchChangeException(change.getId(), e); } @@ -228,22 +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, Branch.NameKey sourceBranch) - throws OrmException, InvalidChangeOperationException, IOException { + 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( - git, revWalk, refControl.getProjectControl(), change, - cherryPickCommit) + refControl, change, cherryPickCommit) .setValidatePolicy(CommitValidators.Policy.GERRIT); - return ins.setMessage( - messageForDestinationChange(ins.getPatchSet().getId(), sourceBranch)) - .insert(); + + 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(); + } + return ins.getChange(); } private void addMessageToSourceChange(Change change, PatchSet.Id patchSetId, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java index 702ae03098..03c0176e22 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java @@ -24,9 +24,8 @@ 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; @@ -41,7 +40,9 @@ 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.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.project.InvalidChangeOperationException; import com.google.gerrit.server.project.ProjectResource; @@ -81,6 +82,7 @@ public class CreateChange implements private final ChangeInserter.Factory changeInserterFactory; private final ChangeJson.Factory jsonFactory; private final ChangeUtil changeUtil; + private final BatchUpdate.Factory updateFactory; private final boolean allowDrafts; @Inject @@ -92,6 +94,7 @@ public class CreateChange implements ChangeInserter.Factory changeInserterFactory, ChangeJson.Factory json, ChangeUtil changeUtil, + BatchUpdate.Factory updateFactory, @GerritServerConfig Config config) { this.db = db; this.gitManager = gitManager; @@ -101,15 +104,15 @@ public class CreateChange implements this.changeInserterFactory = changeInserterFactory; this.jsonFactory = json; this.changeUtil = changeUtil; + this.updateFactory = updateFactory; this.allowDrafts = config.getBoolean("change", "allowDrafts", true); } @Override public Response 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"); } @@ -186,31 +189,38 @@ 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), - new Change.Id(db.get().nextChangeId()), - me.getAccountId(), - new Branch.NameKey(project, refName), - now); + Change change = new Change( + getChangeId(id, c), + new Change.Id(db.get().nextChangeId()), + me.getAccountId(), + new Branch.NameKey(project, refName), + now); - ChangeInserter ins = changeInserterFactory.create( - git, rw, refControl.getProjectControl(), change, c) - .setValidatePolicy(CommitValidators.Policy.GERRIT); - ins.setMessage(String.format("Uploaded patch set %s.", - ins.getPatchSet().getPatchSetId())); - String topic = input.topic; - if (topic != null) { - topic = Strings.emptyToNull(topic.trim()); + ChangeInserter ins = changeInserterFactory + .create(refControl, change, c) + .setValidatePolicy(CommitValidators.Policy.GERRIT); + ins.setMessage(String.format("Uploaded patch set %s.", + ins.getPatchSet().getPatchSetId())); + String topic = input.topic; + if (topic != null) { + topic = Strings.emptyToNull(topic.trim()); + } + change.setTopic(topic); + ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT); + ins.setGroups(groups); + 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())); } - change.setTopic(topic); - ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT); - ins.setGroups(groups); - ins.insert(); - ChangeJson json = jsonFactory.create(ChangeJson.NO_OPTIONS); - return Response.created(json.format(change.getId())); } } @@ -223,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; + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(mergeTip.getTree().getId()); + commit.setParentId(mergeTip); + commit.setAuthor(authorIdent); + commit.setCommitter(authorIdent); + commit.setMessage(commitMessage); + return rw.parseCommit(insert(oi, commit)); } private static ObjectId insert(ObjectInserter inserter, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java index af3668248f..ef0231ea42 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java @@ -20,17 +20,17 @@ 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.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -58,8 +58,8 @@ public class Revert implements RestModifyView, @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,8 +74,6 @@ public class Revert implements RestModifyView, change.currentPatchSetId(), Strings.emptyToNull(input.message), new PersonIdent(myIdent, TimeUtil.nowTs())); - } catch (InvalidChangeOperationException e) { - throw new BadRequestException(e.getMessage()); } catch (NoSuchChangeException e) { throw new ResourceNotFoundException(e.getMessage()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java index 0bf19715f4..647314c5e5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.git; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -44,6 +45,7 @@ import java.io.IOException; import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -115,16 +117,22 @@ 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 ChangeControl getChangeControl() { + return ctl; + } + public Change getChange() { return update.getChange(); } @@ -149,6 +157,10 @@ 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; @@ -161,6 +173,7 @@ public class BatchUpdate implements AutoCloseable { private final Timestamp when; private final ListMultimap ops = ArrayListMultimap.create(); + private final Map newChanges = new HashMap<>(); private final List> indexFutures = new ArrayList<>(); @@ -235,10 +248,20 @@ public class BatchUpdate implements AutoCloseable { } public BatchUpdate addOp(Change.Id id, Op op) { + checkArgument(!(op instanceof InsertChangeOp), "use insertChange"); ops.put(id, op); 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; + } + public void execute() throws UpdateException, RestApiException { try { executeRefUpdates(); @@ -300,18 +323,17 @@ public class BatchUpdate implements AutoCloseable { for (Map.Entry> e : ops.asMap().entrySet()) { Change.Id id = e.getKey(); db.changes().beginTransaction(id); - Change change = db.changes().get(id); - ChangeControl ctl = changeControlFactory.controlFor(change, user); - ChangeUpdate update = changeUpdateFactory.create(ctl, when); + 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) { @@ -320,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(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index d814dfd747..1a3b61bc43 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -111,7 +111,6 @@ import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.PatchSetInfoFactory; 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.ProjectCache; import com.google.gerrit.server.project.ProjectControl; @@ -137,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; @@ -306,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; @@ -382,7 +383,8 @@ public class ReceiveCommits { final ChangeKindCache changeKindCache, final DynamicMap 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; @@ -415,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(); @@ -1718,8 +1721,7 @@ public class ReceiveCommits { magicBranch.dest, TimeUtil.nowTs()); change.setTopic(magicBranch.topic); - ins = changeInserterFactory.create( - repo, rp.getRevWalk(), ctl.getProjectControl(), change, c) + ins = changeInserterFactory.create(ctl, change, c) .setDraft(magicBranch.draft) // Changes already validated in validateNewCommits. .setValidatePolicy(CommitValidators.Policy.NONE); @@ -1733,8 +1735,8 @@ public class ReceiveCommits { ListenableFuture future = changeUpdateExector.submit( requestScopePropagator.wrap(new Callable() { @Override - public Void call() throws OrmException, IOException, - ResourceConflictException, InvalidChangeOperationException { + public Void call() + throws OrmException, RestApiException, UpdateException { insertChangeImpl(); synchronized (newProgress) { newProgress.update(1); @@ -1745,8 +1747,8 @@ public class ReceiveCommits { return Futures.makeChecked(future, INSERT_EXCEPTION); } - private void insertChangeImpl() throws OrmException, IOException, - ResourceConflictException, InvalidChangeOperationException { + private void insertChangeImpl() + throws OrmException, RestApiException, UpdateException { final PatchSet ps = ins.setGroups(groups).getPatchSet(); final Account.Id me = currentUser.getAccountId(); final List footerLines = commit.getFooterLines(); @@ -1759,15 +1761,20 @@ public class ReceiveCommits { } recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); recipients.remove(me); - ins - .setReviewers(recipients.getReviewers()) - .setExtraCC(recipients.getCcOnly()) - .setApprovals(approvals) - .setMessage("Uploaded patch set " + ps.getPatchSetId() + ".") - .setRequestScopePropagator(requestScopePropagator) - .setSendMail(true) - .setUpdateRef(false) - .insert(); + 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("Uploaded patch set " + ps.getPatchSetId() + ".") + .setRequestScopePropagator(requestScopePropagator) + .setSendMail(true) + .setUpdateRef(false)); + bu.execute(); + } created = true; if (magicBranch != null && magicBranch.submit) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index ad6f0a5d5b..c598cec1de 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -59,6 +59,7 @@ 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 = 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 = 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 = 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 = 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 = 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 = 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 = 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 = 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 repo1 = createProject("repo1"); TestRepository 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 repo1 = createProject("repo1"); TestRepository 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 = 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 = 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 = 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 = 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 = createProject("repo"); List 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 = createProject("repo"); List 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 = 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 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 = 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 = 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 = 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 = 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 = 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 = 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 = 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 setUpHashtagChanges() throws Exception { TestRepository 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 = 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. 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 = 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 = 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 = 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 = 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 = 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 = 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,13 +1312,22 @@ 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( - repo.getRepository(), repo.getRevWalk(), - 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, Change c) throws Exception { // Add a new file so the patch set is not a trivial rebase, to avoid default diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java index 6122d65e83..b503a13b9e 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java @@ -38,10 +38,10 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest { TestRepository 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); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesV14Test.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesV14Test.java index 9bdd795778..7e7899b50c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesV14Test.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesV14Test.java @@ -82,9 +82,9 @@ public class LuceneQueryChangesV14Test extends LuceneQueryChangesTest { public void isReviewed() throws Exception { clockStepMs = MILLISECONDS.convert(2, MINUTES); TestRepository 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())