opendev/gerrit
Code Issues Proposed changes

Store ChangeControl instead of ChangeNotes in CodeReviewCommit

Browse Source 
We want to be able to use ApprovalCopier when generating the list of
approvals that goes in the cherry-pick commit message, which requires
the ChangeControl.

Exposed a couple of subtle bugs in merge code expecting one Change
object to be mutated when it was in fact another.

Change-Id: Iad025a0b7a54306bc3b636cc28f484ee0baf534b
This commit is contained in:
Dave Borowitz 2014-02-05 17:41:17 -08:00
parent 9aaec77200
commit c15bbc845f
10 changed files with 119 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
View File

@ -292,7 +292,7 @@ public class RebaseChange {
rebasedCommit = revWalk.parseCommit(newId);
final ChangeControl changeControl =
changeControlFactory.validateFor(change.getId(), uploader);
changeControlFactory.validateFor(change, uploader);
PatchSetInserter patchSetInserter = patchSetInserterFactory
.create(git, revWalk, changeControl, rebasedCommit)

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

@ -22,8 +22,10 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException;
@ -118,29 +120,30 @@ public class CherryPick extends SubmitStrategy {
}
}
} catch (IOException e) {
throw new MergeException("Cannot merge " + n.name(), e);
} catch (OrmException e) {
} catch (NoSuchChangeException | IOException | OrmException e) {
throw new MergeException("Cannot merge " + n.name(), e);
}
}
return newMergeTip;
}
private CodeReviewCommit writeCherryPickCommit(final CodeReviewCommit mergeTip, final CodeReviewCommit n)
throws IOException, OrmException {
private CodeReviewCommit writeCherryPickCommit(CodeReviewCommit mergeTip,
CodeReviewCommit n) throws IOException, OrmException,
NoSuchChangeException {
args.rw.parseBody(n);
final PatchSetApproval submitAudit = args.mergeUtil.getSubmitter(n);
IdentifiedUser cherryPickUser;
PersonIdent cherryPickCommitterIdent;
if (submitAudit != null) {
cherryPickCommitterIdent =
args.identifiedUserFactory.create(submitAudit.getAccountId())
.newCommitterIdent(submitAudit.getGranted(),
args.myIdent.getTimeZone());
cherryPickUser =
args.identifiedUserFactory.create(submitAudit.getAccountId());
cherryPickCommitterIdent = cherryPickUser.newCommitterIdent(
submitAudit.getGranted(), args.myIdent.getTimeZone());
} else {
cherryPickUser = args.identifiedUserFactory.create(n.change().getOwner());
cherryPickCommitterIdent = args.myIdent;
}
@ -156,7 +159,7 @@ public class CherryPick extends SubmitStrategy {
}
PatchSet.Id id =
ChangeUtil.nextPatchSetId(args.repo, n.getChange().currentPatchSetId());
ChangeUtil.nextPatchSetId(args.repo, n.change().currentPatchSetId());
final PatchSet ps = new PatchSet(id);
ps.setCreatedOn(TimeUtil.nowTs());
ps.setUploader(submitAudit.getAccountId());
@ -164,17 +167,17 @@ public class CherryPick extends SubmitStrategy {
final RefUpdate ru;
args.db.changes().beginTransaction(n.getChange().getId());
args.db.changes().beginTransaction(n.change().getId());
try {
insertAncestors(args.db, ps.getId(), newCommit);
args.db.patchSets().insert(Collections.singleton(ps));
n.getChange()
n.change()
.setCurrentPatchSet(patchSetInfoFactory.get(newCommit, ps.getId()));
args.db.changes().update(Collections.singletonList(n.getChange()));
args.db.changes().update(Collections.singletonList(n.change()));
final List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a
: args.approvalsUtil.byPatchSet(args.db, n.notes, n.patchsetId)) {
: args.approvalsUtil.byPatchSet(args.db, n.notes(), n.patchsetId)) {
approvals.add(new PatchSetApproval(ps.getId(), a));
}
// TODO(dborowitz): This doesn't copy labels in the notedb. We should
@ -188,7 +191,7 @@ public class CherryPick extends SubmitStrategy {
ru.disableRefLog();
if (ru.update(args.rw) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
"Failed to create ref %s in %s: %s", ps.getRefName(), n.getChange()
"Failed to create ref %s in %s: %s", ps.getRefName(), n.change()
.getDest().getParentKey().get(), ru.getResult()));
}
@ -197,10 +200,12 @@ public class CherryPick extends SubmitStrategy {
args.db.rollback();
}
gitRefUpdated.fire(n.getChange().getProject(), ru);
gitRefUpdated.fire(n.change().getProject(), ru);
newCommit.copyFrom(n);
newCommit.statusCode = CommitMergeStatus.CLEAN_PICK;
newCommit.control =
args.changeControlFactory.controlFor(n.change(), cherryPickUser);
newCommits.put(newCommit.patchsetId.getParentKey(), newCommit);
setRefLogIdent(submitAudit);
return newCommit;

15
gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.git;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
@ -40,8 +41,8 @@ public class CodeReviewCommit extends RevCommit {
*/
PatchSet.Id patchsetId;
/** Change info loaded from notes. */
public ChangeNotes notes;
/** Change control for the change owner. */
ChangeControl control;
/**
* Ordinal position of this commit within the submit queue.
@ -64,15 +65,19 @@ public class CodeReviewCommit extends RevCommit {
super(id);
}
public ChangeNotes notes() {
return control.getNotes();
}
void copyFrom(final CodeReviewCommit src) {
control = src.control;
patchsetId = src.patchsetId;
notes = src.notes;
originalOrder = src.originalOrder;
statusCode = src.statusCode;
missing = src.missing;
}
Change getChange() {
return notes.getChange();
Change change() {
return control.getChange();
}
}

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

@ -279,8 +279,8 @@ public class MergeOp {
for (final CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) {
final Capable capable = isSubmitStillPossible(commit);
if (capable != Capable.OK) {
sendMergeFail(commit.notes,
message(commit.getChange(), capable.getMessage()), false);
sendMergeFail(commit.notes(),
message(commit.change(), capable.getMessage()), false);
}
}
} catch (NoSuchProjectException noProject) {
@ -327,7 +327,12 @@ public class MergeOp {
}
for (CodeReviewCommit missingCommit : commit.missing) {
loadChangeInfo(missingCommit);
try {
loadChangeInfo(missingCommit);
} catch (NoSuchChangeException | OrmException e) {
log.error("Cannot check if missing commits can be submitted", e);
return false;
}
if (missingCommit.patchsetId == null) {
// The commit doesn't have a patch set, so it cannot be
@ -336,7 +341,7 @@ public class MergeOp {
return false;
}
if (!missingCommit.getChange().currentPatchSetId().equals(
if (!missingCommit.change().currentPatchSetId().equals(
missingCommit.patchsetId)) {
// If the missing commit is not the current patch set,
// the change must be rebased to use the proper parent.
@ -519,7 +524,12 @@ public class MergeOp {
continue;
}
commit.notes = notesFactory.create(chg);
try {
commit.control = changeControlFactory.controlFor(chg,
identifiedUserFactory.create(chg.getOwner()));
} catch (NoSuchChangeException e) {
throw new MergeException("Failed to validate changes", e);
}
commit.patchsetId = ps.getId();
commit.originalOrder = commitOrder++;
commits.put(changeId, commit);
@ -544,7 +554,7 @@ public class MergeOp {
}
}
final SubmitType submitType = getSubmitType(chg, ps);
final SubmitType submitType = getSubmitType(commit.control, ps);
if (submitType == null) {
commits.put(changeId,
CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE));
@ -559,21 +569,13 @@ public class MergeOp {
return toSubmit;
}
private SubmitType getSubmitType(final Change change, final PatchSet ps) {
try {
final SubmitTypeRecord r =
changeControlFactory.controlFor(change,
identifiedUserFactory.create(change.getOwner()))
.getSubmitTypeRecord(db, ps);
if (r.status != SubmitTypeRecord.Status.OK) {
log.error("Failed to get submit type for " + change.getKey());
return null;
}
return r.type;
} catch (NoSuchChangeException e) {
log.error("Failed to get submit type for " + change.getKey(), e);
private SubmitType getSubmitType(ChangeControl ctl, PatchSet ps) {
SubmitTypeRecord r = ctl.getSubmitTypeRecord(db, ps);
if (r.status != SubmitTypeRecord.Status.OK) {
log.error("Failed to get submit type for " + ctl.getChange().getKey());
return null;
}
return r.type;
}
private void updateBranch(final SubmitStrategy strategy,
@ -623,7 +625,7 @@ public class MergeOp {
Account account = null;
PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, mergeTip.notes, mergeTip.patchsetId);
db, mergeTip.notes(), mergeTip.patchsetId);
if (submitter != null) {
account = accountCache.get(submitter.getAccountId()).getAccount();
}
@ -723,7 +725,7 @@ public class MergeOp {
private Capable isSubmitStillPossible(final CodeReviewCommit commit) {
final Capable capable;
final Change c = commit.getChange();
final Change c = commit.change();
final boolean submitStillPossible = isSubmitForMissingCommitsStillPossible(commit);
final long now = TimeUtil.nowMs();
final long waitUntil = c.getLastUpdatedOn().getTime() + DEPENDENCY_DELAY;
@ -748,7 +750,7 @@ public class MergeOp {
m.append("\n");
for (CodeReviewCommit missingCommit : commit.missing) {
m.append("* ");
m.append(missingCommit.getChange().getKey().get());
m.append(missingCommit.change().getKey().get());
m.append("\n");
}
capable = new Capable(m.toString());
@ -767,10 +769,10 @@ public class MergeOp {
m.append("* Depends on patch set ");
m.append(missingCommit.patchsetId.get());
m.append(" of ");
m.append(missingCommit.getChange().getKey().abbreviate());
if (missingCommit.patchsetId.get() != missingCommit.getChange().currentPatchSetId().get()) {
m.append(missingCommit.change().getKey().abbreviate());
if (missingCommit.patchsetId.get() != missingCommit.change().currentPatchSetId().get()) {
m.append(", however the current patch set is ");
m.append(missingCommit.getChange().currentPatchSetId().get());
m.append(missingCommit.change().currentPatchSetId().get());
}
m.append(".\n");
@ -788,18 +790,16 @@ public class MergeOp {
return capable;
}
private void loadChangeInfo(final CodeReviewCommit commit) {
if (commit.notes == null) {
try {
List<PatchSet> matches =
db.patchSets().byRevision(new RevId(commit.name())).toList();
if (matches.size() == 1) {
final PatchSet ps = matches.get(0);
commit.patchsetId = ps.getId();
commit.notes =
notesFactory.create(db.changes().get(ps.getId().getParentKey()));
}
} catch (OrmException e) {
private void loadChangeInfo(final CodeReviewCommit commit)
throws NoSuchChangeException, OrmException {
if (commit.control == null) {
List<PatchSet> matches =
db.patchSets().byRevision(new RevId(commit.name())).toList();
if (matches.size() == 1) {
PatchSet ps = matches.get(0);
commit.patchsetId = ps.getId();
commit.control =
changeControl(db.changes().get(ps.getId().getParentKey()));
}
}
}
@ -825,10 +825,10 @@ public class MergeOp {
// We must pull the patchset out of commits, because the patchset ID is
// modified when using the cherry-pick merge strategy.
CodeReviewCommit commit = commits.get(c.getId());
PatchSet.Id merged = commit.getChange().currentPatchSetId();
PatchSet.Id merged = commit.change().currentPatchSetId();
c = setMergedPatchSet(c.getId(), merged);
PatchSetApproval submitter =
approvalsUtil.getSubmitter(db, commit.notes, merged);
approvalsUtil.getSubmitter(db, commit.notes(), merged);
addMergedMessage(submitter, msg);
db.commit();
@ -904,9 +904,7 @@ public class MergeOp {
}
try {
final ChangeControl control = changeControlFactory.controlFor(c,
identifiedUserFactory.create(c.getOwner()));
final MergedSender cm = mergedSenderFactory.create(control);
MergedSender cm = mergedSenderFactory.create(changeControl(c));
if (from != null) {
cm.setFrom(from.getAccountId());
}
@ -924,8 +922,13 @@ public class MergeOp {
}));
}
private ChangeControl changeControl(Change c) throws NoSuchChangeException {
return changeControlFactory.controlFor(
c, identifiedUserFactory.create(c.getOwner()));
}
private void setNew(CodeReviewCommit c, ChangeMessage msg) {
sendMergeFail(c.notes, msg, true);
sendMergeFail(c.notes(), msg, true);
}
private void setNew(Change c, ChangeMessage msg) throws OrmException {

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

@ -168,7 +168,7 @@ public class MergeUtil {
}
PatchSetApproval getSubmitter(CodeReviewCommit c) {
return approvalsUtil.getSubmitter(db.get(), c.notes, c.patchsetId);
return approvalsUtil.getSubmitter(db.get(), c.notes(), c.patchsetId);
}
public RevCommit createCherryPickFromCommit(Repository repo,
@ -217,10 +217,10 @@ public class MergeUtil {
msgbuf.append('\n');
}
if (!contains(footers, CHANGE_ID, n.getChange().getKey().get())) {
if (!contains(footers, CHANGE_ID, n.change().getKey().get())) {
msgbuf.append(CHANGE_ID.getName());
msgbuf.append(": ");
msgbuf.append(n.getChange().getKey().get());
msgbuf.append(n.change().getKey().get());
msgbuf.append('\n');
}
@ -313,7 +313,7 @@ public class MergeUtil {
private List<PatchSetApproval> safeGetApprovals(CodeReviewCommit n) {
try {
return approvalsUtil.byPatchSet(db.get(), n.notes, n.patchsetId);
return approvalsUtil.byPatchSet(db.get(), n.notes(), n.patchsetId);
} catch (OrmException e) {
log.error("Can't read approval records for " + n.patchsetId, e);
return Collections.emptyList();
@ -587,7 +587,10 @@ public class MergeUtil {
mergeCommit.setCommitter(myIdent);
mergeCommit.setMessage(msgbuf.toString());
return (CodeReviewCommit) rw.parseCommit(commit(inserter, mergeCommit));
CodeReviewCommit mergeResult =
(CodeReviewCommit) rw.parseCommit(commit(inserter, mergeCommit));
mergeResult.control = n.control;
return mergeResult;
}
private String summarize(RevWalk rw, List<CodeReviewCommit> merged)
@ -600,8 +603,8 @@ public class MergeUtil {
LinkedHashSet<String> topics = new LinkedHashSet<>(4);
for (CodeReviewCommit c : merged) {
if (!Strings.isNullOrEmpty(c.getChange().getTopic())) {
topics.add(c.getChange().getTopic());
if (!Strings.isNullOrEmpty(c.change().getTopic())) {
topics.add(c.change().getTopic());
}
}
@ -616,7 +619,7 @@ public class MergeUtil {
new Function<CodeReviewCommit, String>() {
@Override
public String apply(CodeReviewCommit in) {
return in.getChange().getKey().abbreviate();
return in.change().getKey().abbreviate();
}
})),
merged.size() > 5 ? ", ..." : "");

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

@ -22,6 +22,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.changedetail.PathConflictException;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
@ -35,14 +36,17 @@ import java.util.List;
import java.util.Map;
public class RebaseIfNecessary extends SubmitStrategy {
private final PatchSetInfoFactory patchSetInfoFactory;
private final RebaseChange rebaseChange;
private final Map<Change.Id, CodeReviewCommit> newCommits;
private final PersonIdent committerIdent;
RebaseIfNecessary(final SubmitStrategy.Arguments args,
final RebaseChange rebaseChange, PersonIdent committerIdent) {
RebaseIfNecessary(SubmitStrategy.Arguments args,
PatchSetInfoFactory patchSetInfoFactory,
RebaseChange rebaseChange,
PersonIdent committerIdent) {
super(args);
this.patchSetInfoFactory = patchSetInfoFactory;
this.rebaseChange = rebaseChange;
this.newCommits = new HashMap<Change.Id, CodeReviewCommit>();
this.committerIdent = committerIdent;
@ -82,7 +86,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
args.mergeUtil.getSubmitter(n).getAccountId());
final PatchSet newPatchSet =
rebaseChange.rebase(args.repo, args.rw, args.inserter,
n.patchsetId, n.getChange(), uploader,
n.patchsetId, n.change(), uploader,
newMergeTip, args.mergeUtil, committerIdent,
false, false, ValidatePolicy.NONE);
@ -91,7 +95,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
// describes the change being submitted.
List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a : args.approvalsUtil.byPatchSet(
args.db, n.notes, n.patchsetId)) {
args.db, n.notes(), n.patchsetId)) {
approvals.add(new PatchSetApproval(newPatchSet.getId(), a));
}
args.db.patchSetApprovals().insert(approvals);
@ -99,22 +103,19 @@ public class RebaseIfNecessary extends SubmitStrategy {
newMergeTip =
(CodeReviewCommit) args.rw.parseCommit(ObjectId
.fromString(newPatchSet.getRevision().get()));
n.change().setCurrentPatchSet(
patchSetInfoFactory.get(newMergeTip, newPatchSet.getId()));
newMergeTip.copyFrom(n);
newMergeTip.control =
args.changeControlFactory.controlFor(n.change(), uploader);
newMergeTip.patchsetId = newPatchSet.getId();
newMergeTip.notes = args.notesFactory.create(
args.db.changes().get(newPatchSet.getId().getParentKey()));
newMergeTip.statusCode = CommitMergeStatus.CLEAN_REBASE;
newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip);
setRefLogIdent(args.mergeUtil.getSubmitter(n));
} catch (PathConflictException e) {
n.statusCode = CommitMergeStatus.PATH_CONFLICT;
} catch (NoSuchChangeException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (OrmException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (IOException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
} catch (InvalidChangeOperationException e) {
} catch (NoSuchChangeException | OrmException | IOException
| InvalidChangeOperationException e) {
throw new MergeException("Cannot rebase " + n.name(), e);
}
}

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

@ -22,9 +22,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@ -52,8 +50,7 @@ public abstract class SubmitStrategy {
protected final IdentifiedUser.GenericFactory identifiedUserFactory;
protected final PersonIdent myIdent;
protected final ReviewDb db;
protected final ChangeNotes.Factory notesFactory;
protected final ChangeUpdate.Factory updateFactory;
protected final ChangeControl.GenericFactory changeControlFactory;
protected final Repository repo;
protected final RevWalk rw;
@ -68,17 +65,15 @@ public abstract class SubmitStrategy {
Arguments(final IdentifiedUser.GenericFactory identifiedUserFactory,
final PersonIdent myIdent, final ReviewDb db,
final ChangeNotes.Factory notesFactory,
final ChangeUpdate.Factory updateFactory, final Repository repo,
final RevWalk rw, final ObjectInserter inserter,
final ChangeControl.GenericFactory changeControlFactory,
final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch, final ApprovalsUtil approvalsUtil,
final MergeUtil mergeUtil, final ChangeIndexer indexer) {
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
this.db = db;
this.notesFactory = notesFactory;
this.updateFactory = updateFactory;
this.changeControlFactory = changeControlFactory;
this.repo = repo;
this.rw = rw;

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

@ -23,9 +23,8 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
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.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
@ -49,8 +48,7 @@ public class SubmitStrategyFactory {
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final PersonIdent myIdent;
private final ChangeNotes.Factory notesFactory;
private final ChangeUpdate.Factory updateFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final GitReferenceUpdated gitRefUpdated;
private final RebaseChange rebaseChange;
@ -63,8 +61,7 @@ public class SubmitStrategyFactory {
SubmitStrategyFactory(
final IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritPersonIdent final PersonIdent myIdent,
final ChangeNotes.Factory notesFactory,
final ChangeUpdate.Factory updateFactory,
final ChangeControl.GenericFactory changeControlFactory,
final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated gitRefUpdated, final RebaseChange rebaseChange,
final ProjectCache projectCache,
@ -73,8 +70,7 @@ public class SubmitStrategyFactory {
final ChangeIndexer indexer) {
this.identifiedUserFactory = identifiedUserFactory;
this.myIdent = myIdent;
this.notesFactory = notesFactory;
this.updateFactory = updateFactory;
this.changeControlFactory = changeControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.gitRefUpdated = gitRefUpdated;
this.rebaseChange = rebaseChange;
@ -92,7 +88,7 @@ public class SubmitStrategyFactory {
ProjectState project = getProject(destBranch);
final SubmitStrategy.Arguments args =
new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db,
notesFactory, updateFactory, repo, rw, inserter, canMergeFlag,
changeControlFactory, repo, rw, inserter, canMergeFlag,
alreadyAccepted, destBranch,approvalsUtil,
mergeUtilFactory.create(project), indexer);
switch (submitType) {
@ -105,7 +101,8 @@ public class SubmitStrategyFactory {
case MERGE_IF_NECESSARY:
return new MergeIfNecessary(args);
case REBASE_IF_NECESSARY:
return new RebaseIfNecessary(args, rebaseChange, myIdent);
return new RebaseIfNecessary(
args, patchSetInfoFactory, rebaseChange, myIdent);
default:
final String errorMsg = "No submit strategy for: " + submitType;
log.error(errorMsg);

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

@ -125,7 +125,7 @@ public class MergeValidators {
} else {
if (!oldParent.equals(newParent)) {
PatchSetApproval psa =
approvalsUtil.getSubmitter(db, commit.notes, patchSetId);
approvalsUtil.getSubmitter(db, commit.notes(), patchSetId);
if (psa == null) {
throw new MergeValidationException(CommitMergeStatus.
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);

10
gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
View File

@ -30,8 +30,6 @@ import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.SubmoduleSubscriptionAccess;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.ListResultSet;
@ -81,7 +79,6 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
private Provider<String> urlProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated gitRefUpdated;
private ChangeNotes.Factory notesFactory;
@SuppressWarnings("unchecked")
@Override
@ -95,7 +92,6 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
urlProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class);
gitRefUpdated = createStrictMock(GitReferenceUpdated.class);
notesFactory = new ChangeNotes.Factory(repoManager);
}
private void doReplay() {
@ -616,11 +612,10 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
final Change submittedChange = new Change(
new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1),
new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs());
codeReviewCommit.notes = notesFactory.create(submittedChange);
final Map<Change.Id, CodeReviewCommit> mergedCommits =
new HashMap<Change.Id, CodeReviewCommit>();
mergedCommits.put(codeReviewCommit.notes.getChangeId(), codeReviewCommit);
mergedCommits.put(submittedChange.getId(), codeReviewCommit);
final List<Change> submitted = new ArrayList<Change>();
submitted.add(submittedChange);
@ -720,11 +715,10 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
final Change submittedChange = new Change(
new Change.Key(sourceMergeTip.toObjectId().getName()), new Change.Id(1),
new Account.Id(1), sourceBranchNameKey, TimeUtil.nowTs());
codeReviewCommit.notes = notesFactory.create(submittedChange);
final Map<Change.Id, CodeReviewCommit> mergedCommits =
new HashMap<Change.Id, CodeReviewCommit>();
mergedCommits.put(codeReviewCommit.notes.getChangeId(), codeReviewCommit);
mergedCommits.put(submittedChange.getId(), codeReviewCommit);
final List<Change> submitted = new ArrayList<Change>();
submitted.add(submittedChange);