Pass only ref name into ChangeInserter, not ref control

ChangeInserter only needs the ref control when validation is done and
it can just instantiate it for this purpose.

The project and the user are provided by the Context and a provided
ref control may have been created for another project or user. By not
passing in the ref control we avoid this possible inconsistency.

Also remove the getProject() and getUser() methods from
ChangeInserter. They were only used by tests and were needed only
because the API required to pass in the project and user twice, once
through ref control to the ChangeInserter and once to the BatchUpdate.

Change-Id: I2c4faf27e3c1183a474fecfa06dbbbc14a3c143f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-01-18 10:25:13 +01:00
parent 4d1f3c3da9
commit 0242ea54d3
10 changed files with 174 additions and 186 deletions

View File

@@ -21,7 +21,7 @@ import com.google.gerrit.common.data.ProjectAccess;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -54,7 +54,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
private final ChangeHooks hooks;
private final GitReferenceUpdated gitRefUpdated;
private final IdentifiedUser user;
private final ProjectAccessFactory.Factory projectAccessFactory;
private final ProjectCache projectCache;
@@ -67,7 +66,6 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
Provider<SetParent> setParent,
ChangeHooks hooks,
GitReferenceUpdated gitRefUpdated,
IdentifiedUser user,
@Assisted("projectName") Project.NameKey projectName,
@Nullable @Assisted ObjectId base,
@Assisted List<AccessSection> sectionList,
@@ -80,11 +78,10 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
this.projectCache = projectCache;
this.hooks = hooks;
this.gitRefUpdated = gitRefUpdated;
this.user = user;
}
@Override
protected ProjectAccess updateProjectConfig(ProjectControl ctl,
protected ProjectAccess updateProjectConfig(CurrentUser user,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException {
RevCommit commit = config.commit(md);
@@ -93,7 +90,7 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
base, commit.getId());
hooks.doRefUpdatedHook(
new Branch.NameKey(config.getProject().getNameKey(), RefNames.REFS_CONFIG),
base, commit.getId(), user.getAccount());
base, commit.getId(), user.asIdentifiedUser().getAccount());
projectCache.evict(config.getProject());
return projectAccessFactory.create(projectName).call();

View File

@@ -29,6 +29,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupBackends;
import com.google.gerrit.server.config.AllProjectsNameProvider;
@@ -155,14 +156,14 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
md.setMessage("Modify access rules\n");
}
return updateProjectConfig(projectControl, config, md,
return updateProjectConfig(projectControl.getUser(), config, md,
parentProjectUpdate);
} catch (RepositoryNotFoundException notFound) {
throw new NoSuchProjectException(projectName);
}
}
protected abstract T updateProjectConfig(ProjectControl ctl,
protected abstract T updateProjectConfig(CurrentUser user,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException,
OrmException;

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeInserter;
@@ -105,7 +106,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
}
@Override
protected Change.Id updateProjectConfig(ProjectControl ctl,
protected Change.Id updateProjectConfig(CurrentUser user,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, OrmException {
md.setInsertChangeId(true);
@@ -120,12 +121,11 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
try (RevWalk rw = new RevWalk(md.getRepository());
ObjectInserter objInserter = md.getRepository().newObjectInserter();
BatchUpdate bu = updateFactory.create(
db, config.getProject().getNameKey(), ctl.getUser(),
db, config.getProject().getNameKey(), user,
TimeUtil.nowTs())) {
bu.setRepository(md.getRepository(), rw, objInserter);
bu.insertChange(
changeInserterFactory.create(
ctl.controlForRef(RefNames.REFS_CONFIG), changeId, commit)
changeInserterFactory.create(changeId, commit, RefNames.REFS_CONFIG)
.setValidatePolicy(CommitValidators.Policy.NONE)
.setUpdateRef(false)); // Created by commitToNewRef.
bu.execute();

View File

@@ -39,7 +39,6 @@ import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
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.util.IdGenerator;
@@ -252,9 +251,8 @@ public class ChangeUtil {
oi.flush();
revertCommit = revWalk.parseCommit(id);
RefControl refControl = ctl.getRefControl();
ins = changeInserterFactory.create(
refControl, changeId, revertCommit)
changeId, revertCommit, ctl.getChange().getDest().get())
.setValidatePolicy(CommitValidators.Policy.GERRIT)
.setTopic(changeToRevert.getTopic());
@@ -274,7 +272,7 @@ public class ChangeUtil {
ins.setMessage("Uploaded patch set 1.");
try (BatchUpdate bu = updateFactory.create(
db.get(), project, refControl.getUser(),
db.get(), project, ctl.getUser(),
TimeUtil.nowTs())) {
bu.setRepository(git, revWalk, oi);
bu.insertChange(ins);

View File

@@ -29,13 +29,11 @@ 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.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
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.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.BatchUpdate;
@@ -50,6 +48,8 @@ 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.NoSuchProjectException;
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;
@@ -73,12 +73,13 @@ import java.util.Set;
public class ChangeInserter extends BatchUpdate.InsertChangeOp {
public static interface Factory {
ChangeInserter create(RefControl ctl, Change.Id cid, RevCommit rc);
ChangeInserter create(Change.Id cid, RevCommit rc, String refName);
}
private static final Logger log =
LoggerFactory.getLogger(ChangeInserter.class);
private final ProjectControl.GenericFactory projectControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final ChangeHooks hooks;
private final ApprovalsUtil approvalsUtil;
@@ -87,11 +88,10 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private final WorkQueue workQueue;
private final CommitValidators.Factory commitValidatorsFactory;
private final RefControl refControl;
private final IdentifiedUser user;
private final PatchSet patchSet;
private final Change.Id changeId;
private final RevCommit commit;
private final String refName;
// Fields exposed as setters.
private Change.Status status;
@@ -113,16 +113,18 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private PatchSetInfo patchSetInfo;
@Inject
ChangeInserter(PatchSetInfoFactory patchSetInfoFactory,
ChangeInserter(ProjectControl.GenericFactory projectControlFactory,
PatchSetInfoFactory patchSetInfoFactory,
ChangeHooks hooks,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
CreateChangeSender.Factory createChangeSenderFactory,
WorkQueue workQueue,
CommitValidators.Factory commitValidatorsFactory,
@Assisted RefControl refControl,
@Assisted Change.Id changeId,
@Assisted RevCommit commit) {
@Assisted RevCommit commit,
@Assisted String refName) {
this.projectControlFactory = projectControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.hooks = hooks;
this.approvalsUtil = approvalsUtil;
@@ -131,9 +133,9 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
this.workQueue = workQueue;
this.commitValidatorsFactory = commitValidatorsFactory;
this.refControl = refControl;
this.changeId = changeId;
this.commit = commit;
this.refName = refName;
this.reviewers = Collections.emptySet();
this.extraCC = Collections.emptySet();
this.approvals = Collections.emptyMap();
@@ -141,7 +143,6 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
this.sendMail = true;
this.updateRef = true;
user = refControl.getUser().asIdentifiedUser();
patchSet =
new PatchSet(new PatchSet.Id(changeId, INITIAL_PATCH_SET_ID));
patchSet.setRevision(new RevId(commit.name()));
@@ -153,9 +154,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
getChangeKey(commit),
changeId,
ctx.getUser().getAccountId(),
new Branch.NameKey(
refControl.getProjectControl().getProject().getNameKey(),
refControl.getRefName()),
new Branch.NameKey(ctx.getProject(), refName),
ctx.getWhen());
change.setStatus(MoreObjects.firstNonNull(status, Change.Status.NEW));
change.setTopic(topic);
@@ -183,14 +182,6 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
return change;
}
public IdentifiedUser getUser() {
return user;
}
public Project.NameKey getProject() {
return refControl.getProjectControl().getProject().getNameKey();
}
public ChangeInserter setTopic(String topic) {
checkState(change == null, "setTopic(String) only valid before creating change");
this.topic = topic;
@@ -323,7 +314,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
if (message != null) {
changeMessage =
new ChangeMessage(new ChangeMessage.Key(change.getId(),
ChangeUtil.messageUUID(db)), user.getAccountId(),
ChangeUtil.messageUUID(db)), ctx.getUser().getAccountId(),
patchSet.getCreatedOn(), patchSet.getId());
changeMessage.setMessage(message);
cmUtil.addChangeMessage(db, update, changeMessage);
@@ -365,8 +356,9 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
ReviewDb db = ctx.getDb();
hooks.doPatchsetCreatedHook(change, patchSet, db);
if (approvals != null && !approvals.isEmpty()) {
hooks.doCommentAddedHook(
change, user.getAccount(), patchSet, null, approvals, db);
hooks.doCommentAddedHook(change,
ctx.getUser().asIdentifiedUser().getAccount(), patchSet, null,
approvals, db);
}
}
}
@@ -376,6 +368,10 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
if (validatePolicy == CommitValidators.Policy.NONE) {
return;
}
try {
RefControl refControl = projectControlFactory
.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName);
CommitValidators cv = commitValidatorsFactory.create(
refControl, new NoSshInfo(), ctx.getRepository());
@@ -388,9 +384,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
refControl.getProjectControl().getProject(),
change.getDest().get(),
commit,
user);
ctx.getUser().asIdentifiedUser());
try {
switch (validatePolicy) {
case RECEIVE_COMMITS:
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(
@@ -405,6 +400,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
}
} catch (CommitValidationException e) {
throw new ResourceConflictException(e.getFullMessage());
} catch (NoSuchProjectException e) {
throw new ResourceConflictException(e.getMessage());
}
}
}

View File

@@ -191,7 +191,8 @@ public class CherryPickChange {
}
Change.Id newChangeId =
createNewChange(git, revWalk, oi, project, cherryPickCommit,
refControl, identifiedUser, newTopic, change.getDest());
refControl.getRefName(), identifiedUser, newTopic,
change.getDest());
addMessageToSourceChange(change, patch.getId(), destinationBranch,
cherryPickCommit, identifiedUser, refControl);
@@ -233,12 +234,12 @@ public class CherryPickChange {
private Change.Id createNewChange(Repository git, RevWalk revWalk,
ObjectInserter oi, Project.NameKey project,
CodeReviewCommit cherryPickCommit, RefControl refControl,
CodeReviewCommit cherryPickCommit, String refName,
IdentifiedUser identifiedUser, String topic, Branch.NameKey sourceBranch)
throws RestApiException, UpdateException, OrmException, IOException {
Change.Id changeId = new Change.Id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory.create(
refControl, changeId, cherryPickCommit)
changeId, cherryPickCommit, refName)
.setValidatePolicy(CommitValidators.Policy.GERRIT)
.setTopic(topic);

View File

@@ -195,8 +195,7 @@ public class CreateChange implements
RevCommit c = newCommit(oi, rw, author, mergeTip, commitMessage);
Change.Id changeId = new Change.Id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory
.create(refControl, changeId, c)
ChangeInserter ins = changeInserterFactory.create(changeId, c, refName)
.setValidatePolicy(CommitValidators.Policy.GERRIT);
ins.setMessage(String.format("Uploaded patch set %s.",
ins.getPatchSet().getPatchSetId()));

View File

@@ -1585,7 +1585,7 @@ public class ReceiveCommits {
final List<String> idList = c.getFooterLines(CHANGE_ID);
if (idList.isEmpty()) {
newChanges.add(new CreateRequest(magicBranch.ctl, c));
newChanges.add(new CreateRequest(c, magicBranch.dest.get()));
continue;
}
@@ -1648,7 +1648,7 @@ public class ReceiveCommits {
newChangeIds.add(p.changeKey);
}
newChanges.add(new CreateRequest(magicBranch.ctl, p.commit));
newChanges.add(new CreateRequest(p.commit, magicBranch.dest.get()));
}
} catch (IOException e) {
// Should never happen, the core receive process would have
@@ -1731,11 +1731,11 @@ public class ReceiveCommits {
Change change;
Collection<String> groups;
CreateRequest(RefControl ctl, RevCommit c)
CreateRequest(RevCommit c, String refName)
throws OrmException {
commit = c;
changeId = new Change.Id(seq.nextChangeId());
ins = changeInserterFactory.create(ctl, changeId, c)
ins = changeInserterFactory.create(changeId, c, refName)
.setDraft(magicBranch.draft)
.setTopic(magicBranch.topic)
// Changes already validated in validateNewCommits.

View File

@@ -194,8 +194,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byId() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
assertQuery("12345");
assertQuery(change1.getId().get(), change1);
@@ -205,7 +205,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byKey() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChange(repo));
Change change = insert(repo, newChange(repo));
String key = change.getKey().get();
assertQuery("I0000000000000000000000000000000000000000");
@@ -218,7 +218,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byTriplet() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChangeForBranch(repo, "branch"));
Change change = insert(repo, newChangeForBranch(repo, "branch"));
String k = change.getKey().get();
assertQuery("repo~branch~" + k, change);
@@ -242,9 +242,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byStatus() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
Change change1 = insert(ins1);
Change change1 = insert(repo, ins1);
ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.MERGED);
Change change2 = insert(ins2);
Change change2 = insert(repo, ins2);
assertQuery("status:new", change1);
assertQuery("status:NEW", change1);
@@ -257,10 +257,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byStatusOpen() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
Change change1 = insert(ins1);
Change change1 = insert(repo, ins1);
ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT);
Change change2 = insert(ins2);
insert(newChangeWithStatus(repo, Change.Status.MERGED));
Change change2 = insert(repo, ins2);
insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
Change[] expected = new Change[] {change2, change1};
assertQuery("status:open", expected);
@@ -279,9 +279,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byStatusDraft() throws Exception {
TestRepository<Repo> repo = createProject("repo");
insert(newChangeWithStatus(repo, Change.Status.NEW));
insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT);
Change change2 = insert(ins2);
Change change2 = insert(repo, ins2);
Change[] expected = new Change[] {change2};
assertQuery("status:draft", expected);
@@ -297,10 +297,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byStatusClosed() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.MERGED);
Change change1 = insert(ins1);
Change change1 = insert(repo, ins1);
ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.ABANDONED);
Change change2 = insert(ins2);
insert(newChangeWithStatus(repo, Change.Status.NEW));
Change change2 = insert(repo, ins2);
insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
Change[] expected = new Change[] {change2, change1};
assertQuery("status:closed", expected);
@@ -318,8 +318,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byStatusPrefix() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
Change change1 = insert(ins1);
insert(newChangeWithStatus(repo, Change.Status.MERGED));
Change change1 = insert(repo, ins1);
insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
assertQuery("status:n", change1);
assertQuery("status:ne", change1);
@@ -335,7 +335,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byCommit() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo);
insert(ins);
insert(repo, ins);
String sha = ins.getPatchSet().getRevision().get();
assertQuery("0000000000000000000000000000000000000000");
@@ -348,10 +348,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byOwner() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
Change change1 = insert(repo, newChange(repo), userId);
Account.Id user2 = accountManager.authenticate(
AuthRequest.forUser("anotheruser")).getAccountId();
Change change2 = insert(newChangeForUser(repo, user2));
Change change2 = insert(repo, newChange(repo), user2);
assertQuery("owner:" + userId.get(), change1);
assertQuery("owner:" + user2, change2);
@@ -360,7 +360,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byAuthor() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
Change change1 = insert(repo, newChange(repo), userId);
// By exact email address
assertQuery("author:jauthor@example.com", change1);
@@ -387,7 +387,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byCommitter() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
Change change1 = insert(repo, newChange(repo), userId);
// By exact email address
assertQuery("committer:jcommitter@example.com", change1);
@@ -414,10 +414,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byOwnerIn() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
Change change1 = insert(repo, newChange(repo), userId);
Account.Id user2 = accountManager.authenticate(
AuthRequest.forUser("anotheruser")).getAccountId();
Change change2 = insert(newChangeForUser(repo, user2));
Change change2 = insert(repo, newChange(repo), user2);
assertQuery("ownerin:Administrators", change1);
assertQuery("ownerin:\"Registered Users\"", change2, change1);
@@ -427,8 +427,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byProject() throws Exception {
TestRepository<Repo> repo1 = createProject("repo1");
TestRepository<Repo> repo2 = createProject("repo2");
Change change1 = insert(newChange(repo1));
Change change2 = insert(newChange(repo2));
Change change1 = insert(repo1, newChange(repo1));
Change change2 = insert(repo2, newChange(repo2));
assertQuery("project:foo");
assertQuery("project:repo");
@@ -440,8 +440,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byProjectPrefix() throws Exception {
TestRepository<Repo> repo1 = createProject("repo1");
TestRepository<Repo> repo2 = createProject("repo2");
Change change1 = insert(newChange(repo1));
Change change2 = insert(newChange(repo2));
Change change1 = insert(repo1, newChange(repo1));
Change change2 = insert(repo2, newChange(repo2));
assertQuery("projects:foo");
assertQuery("projects:repo1", change1);
@@ -452,8 +452,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byBranchAndRef() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForBranch(repo, "master"));
Change change2 = insert(newChangeForBranch(repo, "branch"));
Change change1 = insert(repo, newChangeForBranch(repo, "master"));
Change change2 = insert(repo, newChangeForBranch(repo, "branch"));
assertQuery("branch:foo");
assertQuery("branch:master", change1);
@@ -471,18 +471,18 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byTopic() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithTopic(repo, "feature1");
Change change1 = insert(ins1);
Change change1 = insert(repo, ins1);
ChangeInserter ins2 = newChangeWithTopic(repo, "feature2");
Change change2 = insert(ins2);
Change change2 = insert(repo, ins2);
ChangeInserter ins3 = newChangeWithTopic(repo, "Cherrypick-feature2");
Change change3 = insert(ins3);
Change change3 = insert(repo, ins3);
ChangeInserter ins4 = newChangeWithTopic(repo, "feature2-fixup");
Change change4 = insert(ins4);
Change change4 = insert(repo, ins4);
Change change5 = insert(newChange(repo));
Change change5 = insert(repo, newChange(repo));
assertQuery("intopic:foo");
assertQuery("intopic:feature1", change1);
@@ -498,9 +498,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byMessageExact() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().message("one").create());
Change change1 = insert(newChangeForCommit(repo, commit1));
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
RevCommit commit2 = repo.parseBody(repo.commit().message("two").create());
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
assertQuery("message:foo");
assertQuery("message:one", change1);
@@ -512,10 +512,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 =
repo.parseBody(repo.commit().message("12345 67890").create());
Change change1 = insert(newChangeForCommit(repo, commit1));
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
RevCommit commit2 =
repo.parseBody(repo.commit().message("12346 67891").create());
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
assertQuery("message:1234");
assertQuery("message:12345", change1);
@@ -527,7 +527,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
accountManager.authenticate(AuthRequest.forUser("anotheruser"));
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo);
Change change = insert(ins);
Change change = insert(repo, ins);
gApi.changes().id(change.getId().get()).current()
.review(new ReviewInput().label("Code-Review", 1));
@@ -589,8 +589,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
gApi.groups().id(g2).addMembers("user2");
// create a change
ChangeInserter ins = newChangeForUser(repo, user1);
Change change1 = insert(ins);
Change change1 = insert(repo, newChange(repo), user1);
// post a review with user1
requestContext.setContext(newRequestContext(user1));
@@ -612,7 +611,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Change last = null;
int n = 5;
for (int i = 0; i < n; i++) {
last = insert(newChange(repo));
last = insert(repo, newChange(repo));
}
for (int i = 1; i <= n + 2; i++) {
@@ -639,7 +638,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
TestRepository<Repo> repo = createProject("repo");
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 2; i++) {
changes.add(insert(newChange(repo)));
changes.add(insert(repo, newChange(repo)));
}
assertQuery("status:new", changes.get(1), changes.get(0));
@@ -653,7 +652,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
TestRepository<Repo> repo = createProject("repo");
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 3; i++) {
changes.add(insert(newChange(repo)));
changes.add(insert(repo, newChange(repo)));
}
assertQuery("status:new limit:2", changes.get(2), changes.get(1));
@@ -667,7 +666,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void maxPages() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChange(repo));
Change change = insert(repo, newChange(repo));
QueryRequest query = newQuery("status:new").withLimit(10);
assertQuery(query, change);
@@ -685,7 +684,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
List<Change> changes = Lists.newArrayList();
for (int i = 0; i < 5; i++) {
inserters.add(newChange(repo));
changes.add(insert(inserters.get(i)));
changes.add(insert(repo, inserters.get(i)));
}
for (int i : ImmutableList.of(2, 0, 1, 4, 3)) {
@@ -707,8 +706,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
TestTimeUtil.resetWithClockStep(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo);
Change change1 = insert(ins1);
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, ins1);
Change change2 = insert(repo, newChange(repo));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
assertQuery("status:new", change2, change1);
@@ -729,8 +728,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void updatedOrderWithSubMinuteResolution() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo);
Change change1 = insert(ins1);
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, ins1);
Change change2 = insert(repo, newChange(repo));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
@@ -751,11 +750,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void filterOutMoreThanOnePageOfResults() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChangeForUser(repo, userId));
Change change = insert(repo, newChange(repo), userId);
Account.Id user2 = accountManager.authenticate(
AuthRequest.forUser("anotheruser")).getAccountId();
for (int i = 0; i < 5; i++) {
insert(newChangeForUser(repo, user2));
insert(repo, newChange(repo), user2);
}
assertQuery("status:new ownerin:Administrators", change);
@@ -768,7 +767,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Account.Id user2 = accountManager.authenticate(
AuthRequest.forUser("anotheruser")).getAccountId();
for (int i = 0; i < 5; i++) {
insert(newChangeForUser(repo, user2));
insert(repo, newChange(repo), user2);
}
assertQuery("status:new ownerin:Administrators");
@@ -782,7 +781,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = insert(newChangeForCommit(repo, commit));
Change change = insert(repo, newChangeForCommit(repo, commit));
assertQuery("file:file");
assertQuery("file:dir", change);
@@ -799,7 +798,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = insert(newChangeForCommit(repo, commit));
Change change = insert(repo, newChangeForCommit(repo, commit));
assertQuery("file:.*file.*");
assertQuery("file:^file.*"); // Whole path only.
@@ -813,7 +812,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = insert(newChangeForCommit(repo, commit));
Change change = insert(repo, newChangeForCommit(repo, commit));
assertQuery("path:file");
assertQuery("path:dir");
@@ -830,7 +829,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = insert(newChangeForCommit(repo, commit));
Change change = insert(repo, newChangeForCommit(repo, commit));
assertQuery("path:.*file.*");
assertQuery("path:^dir.file.*", change);
@@ -840,7 +839,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byComment() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo);
Change change = insert(ins);
Change change = insert(repo, ins);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
@@ -861,8 +860,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
TestTimeUtil.resetWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
// Queried by AgePredicate constructor.
TestTimeUtil.setClockStep(0, MILLISECONDS);
long now = TimeUtil.nowMs();
@@ -884,8 +883,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byBefore() throws Exception {
TestTimeUtil.resetWithClockStep(30, HOURS);
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
TestTimeUtil.setClockStep(0, MILLISECONDS);
assertQuery("before:2009-09-29");
@@ -904,8 +903,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byAfter() throws Exception {
TestTimeUtil.resetWithClockStep(30, HOURS);
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
TestTimeUtil.setClockStep(0, MILLISECONDS);
assertQuery("after:2009-10-03");
@@ -926,8 +925,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
RevCommit commit2 = repo.parseBody(
repo.commit().parent(commit1).add("file1", "foo").create());
Change change1 = insert(newChangeForCommit(repo, commit1));
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
assertQuery("added:>4");
assertQuery("-added:<=4");
@@ -976,8 +975,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
private List<Change> setUpHashtagChanges() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
HashtagsInput in = new HashtagsInput();
in.add = ImmutableSet.of("foo");
@@ -1019,29 +1018,29 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void byDefault() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
RevCommit commit2 = repo.parseBody(
repo.commit().message("foosubject").create());
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
RevCommit commit3 = repo.parseBody(
repo.commit()
.add("Foo.java", "foo contents")
.create());
Change change3 = insert(newChangeForCommit(repo, commit3));
Change change3 = insert(repo, newChangeForCommit(repo, commit3));
ChangeInserter ins4 = newChange(repo);
Change change4 = insert(ins4);
Change change4 = insert(repo, ins4);
ReviewInput ri4 = new ReviewInput();
ri4.message = "toplevel";
ri4.labels = ImmutableMap.<String, Short> of("Code-Review", (short) 1);
gApi.changes().id(change4.getId().get()).current().review(ri4);
ChangeInserter ins5 = newChangeWithTopic(repo, "feature5");
Change change5 = insert(ins5);
Change change5 = insert(repo, ins5);
Change change6 = insert(newChangeForBranch(repo, "branch6"));
Change change6 = insert(repo, newChangeForBranch(repo, "branch6"));
assertQuery(change1.getId().get(), change1);
assertQuery(ChangeTriplet.format(change1), change1);
@@ -1062,10 +1061,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void implicitVisibleTo() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
ChangeInserter ins2 =
newChange(repo, null, userId, null, Change.Status.DRAFT, null);
Change change2 = insert(ins2);
Change change1 = insert(repo, newChange(repo), userId);
Change change2 =
insert(repo, newChangeWithStatus(repo, Change.Status.DRAFT), userId);
String q = "project:repo";
assertQuery(q, change2, change1);
@@ -1079,10 +1077,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void explicitVisibleTo() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChangeForUser(repo, userId));
ChangeInserter ins2 =
newChange(repo, null, userId, null, Change.Status.DRAFT, null);
Change change2 = insert(ins2);
Change change1 = insert(repo, newChange(repo), userId);
Change change2 =
insert(repo, newChangeWithStatus(repo, Change.Status.DRAFT), userId);
String q = "project:repo";
assertQuery(q, change2, change1);
@@ -1097,8 +1094,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byCommentBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
.getAccountId().get();
@@ -1123,8 +1120,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byDraftBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
DraftInput in = new DraftInput();
in.line = 1;
@@ -1148,12 +1145,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byFrom() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Account.Id user2 = accountManager.authenticate(
AuthRequest.forUser("anotheruser")).getAccountId();
ChangeInserter ins2 = newChangeForUser(repo, user2);
Change change2 = insert(ins2);
Change change2 = insert(repo, newChange(repo), user2);
ReviewInput input = new ReviewInput();
input.message = "toplevel";
@@ -1189,10 +1185,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
repo.commit()
.add("file4", "contents4")
.create());
Change change1 = insert(newChangeForCommit(repo, commit1));
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change3 = insert(newChangeForCommit(repo, commit3));
Change change4 = insert(newChangeForCommit(repo, commit4));
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
Change change3 = insert(repo, newChangeForCommit(repo, commit3));
Change change4 = insert(repo, newChangeForCommit(repo, commit4));
assertQuery("conflicts:" + change1.getId().get(), change3);
assertQuery("conflicts:" + change2.getId().get());
@@ -1204,9 +1200,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void reviewedBy() throws Exception {
TestTimeUtil.resetWithClockStep(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(newChange(repo));
Change change2 = insert(newChange(repo));
Change change3 = insert(newChange(repo));
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
Change change3 = insert(repo, newChange(repo));
gApi.changes()
.id(change1.getId().get())
@@ -1262,7 +1258,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Branch.NameKey dest = null;
for (int i = 0; i < n; i++) {
ChangeInserter ins = newChange(repo);
insert(ins);
insert(repo, ins);
if (dest == null) {
dest = ins.getChange().getDest();
}
@@ -1291,7 +1287,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void prepopulatedFields() throws Exception {
assume().that(notesMigration.enabled()).isFalse();
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChange(repo));
Change change = insert(repo, newChange(repo));
db = new DisabledReviewDb();
requestContext.setContext(newRequestContext(userId));
@@ -1320,7 +1316,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void prepopulateOnlyRequestedFields() throws Exception {
assume().that(notesMigration.enabled()).isFalse();
TestRepository<Repo> repo = createProject("repo");
Change change = insert(newChange(repo));
Change change = insert(repo, newChange(repo));
db = new DisabledReviewDb();
requestContext.setContext(newRequestContext(userId));
@@ -1343,63 +1339,62 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
protected ChangeInserter newChange(TestRepository<Repo> repo)
throws Exception {
return newChange(repo, null, null, null, null, null);
return newChange(repo, null, null, null, null);
}
protected ChangeInserter newChangeForCommit(TestRepository<Repo> repo,
RevCommit commit) throws Exception {
return newChange(repo, commit, null, null, null, null);
}
protected ChangeInserter newChangeForUser(TestRepository<Repo> repo,
Account.Id owner) throws Exception {
return newChange(repo, null, owner, null, null, null);
return newChange(repo, commit, null, null, null);
}
protected ChangeInserter newChangeForBranch(TestRepository<Repo> repo,
String branch) throws Exception {
return newChange(repo, null, null, branch, null, null);
return newChange(repo, null, branch, null, null);
}
protected ChangeInserter newChangeWithStatus(TestRepository<Repo> repo,
Change.Status status) throws Exception {
return newChange(repo, null, null, null, status, null);
return newChange(repo, null, null, status, null);
}
protected ChangeInserter newChangeWithTopic(TestRepository<Repo> repo,
String topic) throws Exception {
return newChange(repo, null, null, null, null, topic);
return newChange(repo, null, null, null, topic);
}
protected ChangeInserter newChange(TestRepository<Repo> repo,
@Nullable RevCommit commit, @Nullable Account.Id owner,
@Nullable String branch, @Nullable Change.Status status,
@Nullable String topic) throws Exception {
@Nullable RevCommit commit, @Nullable String branch,
@Nullable Change.Status status, @Nullable String topic) throws Exception {
if (commit == null) {
commit = repo.parseBody(repo.commit().message("message").create());
}
Account.Id ownerId = owner != null ? owner : userId;
branch = MoreObjects.firstNonNull(branch, "refs/heads/master");
if (!branch.startsWith("refs/heads/")) {
branch = "refs/heads/" + branch;
}
Project.NameKey project = new Project.NameKey(
repo.getRepository().getDescription().getRepositoryName());
Change.Id id = new Change.Id(seq.nextChangeId());
IdentifiedUser user = userFactory.create(Providers.of(db), ownerId);
RefControl refControl = projectControlFactory.controlFor(project, user)
.controlForRef(new Branch.NameKey(project, branch));
ChangeInserter ins = changeFactory.create(refControl, id, commit)
ChangeInserter ins = changeFactory.create(
id, commit, branch)
.setValidatePolicy(CommitValidators.Policy.NONE)
.setStatus(status)
.setTopic(topic);
return ins;
}
protected Change insert(ChangeInserter ins) throws Exception {
try (BatchUpdate bu = updateFactory.create(db, ins.getProject(),
ins.getUser(), TimeUtil.nowTs())) {
protected Change insert(TestRepository<Repo> repo, ChangeInserter ins) throws Exception {
return insert(repo, ins, null);
}
protected Change insert(TestRepository<Repo> repo, ChangeInserter ins,
@Nullable Account.Id owner) throws Exception {
Project.NameKey project = new Project.NameKey(
repo.getRepository().getDescription().getRepositoryName());
Account.Id ownerId = owner != null ? owner : userId;
IdentifiedUser user = userFactory.create(Providers.of(db), ownerId);
try (BatchUpdate bu =
updateFactory.create(db, project, user, TimeUtil.nowTs())) {
bu.insertChange(ins);
bu.execute();
return ins.getChange();

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 = insert(newChangeForCommit(repo, commit1));
Change change1 = insert(repo, newChangeForCommit(repo, commit1));
RevCommit commit2 =
repo.parseBody(repo.commit().message("one.two.three").create());
Change change2 = insert(newChangeForCommit(repo, commit2));
Change change2 = insert(repo, newChangeForCommit(repo, commit2));
assertQuery("message:foo_ba");
assertQuery("message:bar", change1);