opendev/gerrit
Code Issues Proposed changes

Use ChangeInserter in ReviewProjectAccess

Browse Source 
Change-Id: Id8a182ed2e3f374b54405d1bc336ae4dbd7d29e8
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2015-02-17 17:17:15 +01:00
parent cf8b73d9ca
commit a287645912
9 changed files with 39 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
View File

@@ -79,9 +79,9 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
}
@Override
protected ProjectAccess updateProjectConfig(ProjectConfig config,
MetaDataUpdate md, boolean parentProjectUpdate) throws IOException,
NoSuchProjectException, ConfigInvalidException {
protected ProjectAccess updateProjectConfig(ProjectControl ctl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException {
RevCommit commit = config.commit(md);
hooks.doRefUpdatedHook(

10
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
View File

@@ -163,15 +163,17 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
md.setMessage("Modify access rules\n");
}
return updateProjectConfig(config, md, parentProjectUpdate);
return updateProjectConfig(projectControl, config, md,
parentProjectUpdate);
} finally {
md.close();
}
}
protected abstract T updateProjectConfig(ProjectConfig config,
MetaDataUpdate md, boolean parentProjectUpdate) throws IOException,
NoSuchProjectException, ConfigInvalidException, OrmException;
protected abstract T updateProjectConfig(ProjectControl ctl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException,
OrmException;
private void replace(ProjectConfig config, Set<String> toDelete,
AccessSection section) throws NoSuchGroupException {

89
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.httpd.rpc.project;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.AccessSection;
@@ -25,15 +24,12 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
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.account.GroupBackend;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReviewers;
@@ -41,9 +37,6 @@ import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.CreateChangeSender;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.SetParent;
@@ -54,18 +47,11 @@ import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
private static final Logger log =
LoggerFactory.getLogger(ReviewProjectAccess.class);
interface Factory {
ReviewProjectAccess create(
@Assisted("projectName") Project.NameKey projectName,
@@ -77,26 +63,21 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
private final ReviewDb db;
private final IdentifiedUser user;
private final PatchSetInfoFactory patchSetInfoFactory;
private final Provider<PostReviewers> reviewersProvider;
private final ChangeIndexer indexer;
private final ChangeHooks hooks;
private final CreateChangeSender.Factory createChangeSenderFactory;
private final ProjectCache projectCache;
private final ChangesCollection changes;
private final ChangeInserter.Factory changeInserterFactory;
@Inject
ReviewProjectAccess(final ProjectControl.Factory projectControlFactory,
GroupBackend groupBackend,
MetaDataUpdate.User metaDataUpdateFactory, ReviewDb db,
IdentifiedUser user, PatchSetInfoFactory patchSetInfoFactory,
IdentifiedUser user,
Provider<PostReviewers> reviewersProvider,
ChangeIndexer indexer,
ChangeHooks hooks,
CreateChangeSender.Factory createChangeSenderFactory,
ProjectCache projectCache,
AllProjectsNameProvider allProjects,
ChangesCollection changes,
ChangeInserter.Factory changeInserterFactory,
Provider<SetParent> setParent,
@Assisted("projectName") Project.NameKey projectName,
@@ -109,23 +90,20 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
parentProjectName, message, false);
this.db = db;
this.user = user;
this.patchSetInfoFactory = patchSetInfoFactory;
this.reviewersProvider = reviewersProvider;
this.indexer = indexer;
this.hooks = hooks;
this.createChangeSenderFactory = createChangeSenderFactory;
this.projectCache = projectCache;
this.changes = changes;
this.changeInserterFactory = changeInserterFactory;
}
@Override
protected Change.Id updateProjectConfig(ProjectConfig config,
MetaDataUpdate md, boolean parentProjectUpdate) throws IOException,
OrmException {
protected Change.Id updateProjectConfig(ProjectControl ctl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, OrmException {
Change.Id changeId = new Change.Id(db.nextChangeId());
PatchSet ps =
new PatchSet(new PatchSet.Id(changeId, Change.INITIAL_PATCH_SET_ID));
RevCommit commit = config.commitToNewRef(md, ps.getRefName());
RevCommit commit =
config.commitToNewRef(md, new PatchSet.Id(changeId,
Change.INITIAL_PATCH_SET_ID).toRefName());
if (commit.getId().equals(base)) {
return null;
}
@@ -138,35 +116,10 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
config.getProject().getNameKey(),
RefNames.REFS_CONFIG),
TimeUtil.nowTs());
ChangeInserter ins =
changeInserterFactory.create(ctl, change, commit);
ins.insert();
ps.setCreatedOn(change.getCreatedOn());
ps.setUploader(change.getOwner());
ps.setRevision(new RevId(commit.name()));
PatchSetInfo info = patchSetInfoFactory.get(commit, ps.getId());
change.setCurrentPatchSet(info);
ChangeUtil.updated(change);
db.changes().beginTransaction(changeId);
try {
insertAncestors(ps.getId(), commit);
db.patchSets().insert(Collections.singleton(ps));
db.changes().insert(Collections.singleton(change));
db.commit();
} finally {
db.rollback();
}
indexer.index(db, change);
hooks.doPatchsetCreatedHook(change, ps, db);
try {
CreateChangeSender cm =
createChangeSenderFactory.create(change);
cm.setFrom(change.getOwner());
cm.setPatchSet(ps, info);
cm.send();
} catch (Exception err) {
log.error("Cannot send email for new change " + change.getId(), err);
}
ChangeResource rsrc;
try {
rsrc = changes.parse(changeId);
@@ -180,20 +133,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
return changeId;
}
private void insertAncestors(PatchSet.Id id, RevCommit src)
throws OrmException {
final int cnt = src.getParentCount();
List<PatchSetAncestor> toInsert = new ArrayList<>(cnt);
for (int p = 0; p < cnt; p++) {
PatchSetAncestor a;
a = new PatchSetAncestor(new PatchSetAncestor.Id(id, p + 1));
a.setAncestorRevision(new RevId(src.getParent(p).name()));
toInsert.add(a);
}
db.patchSetAncestors().insert(toInsert);
}
private void addProjectOwnersAsReviewers(ChangeResource rsrc) {
final String projectOwners =
groupBackend.get(SystemGroupBackend.PROJECT_OWNERS).getName();

3
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -282,7 +282,8 @@ public class ChangeUtil {
TimeUtil.nowTs());
change.setTopic(changeToRevert.getTopic());
ChangeInserter ins =
changeInserterFactory.create(refControl, change, revertCommit);
changeInserterFactory.create(refControl.getProjectControl(),
change, revertCommit);
PatchSet ps = ins.getPatchSet();
String ref = refControl.getRefName();

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

@@ -39,7 +39,7 @@ 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.RefControl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
@@ -59,7 +59,7 @@ import java.util.Set;
public class ChangeInserter {
public static interface Factory {
ChangeInserter create(RefControl ctl, Change c, RevCommit rc);
ChangeInserter create(ProjectControl ctl, Change c, RevCommit rc);
}
private static final Logger log =
@@ -77,7 +77,7 @@ public class ChangeInserter {
private final AccountCache accountCache;
private final WorkQueue workQueue;
private final RefControl refControl;
private final ProjectControl projectControl;
private final Change change;
private final PatchSet patchSet;
private final RevCommit commit;
@@ -105,7 +105,7 @@ public class ChangeInserter {
HashtagsUtil hashtagsUtil,
AccountCache accountCache,
WorkQueue workQueue,
@Assisted RefControl refControl,
@Assisted ProjectControl projectControl,
@Assisted Change change,
@Assisted RevCommit commit) {
this.dbProvider = dbProvider;
@@ -119,7 +119,7 @@ public class ChangeInserter {
this.hashtagsUtil = hashtagsUtil;
this.accountCache = accountCache;
this.workQueue = workQueue;
this.refControl = refControl;
this.projectControl = projectControl;
this.change = change;
this.commit = commit;
this.reviewers = Collections.emptySet();
@@ -198,7 +198,7 @@ public class ChangeInserter {
public Change insert() throws OrmException, IOException {
ReviewDb db = dbProvider.get();
ChangeControl ctl = refControl.getProjectControl().controlFor(change);
ChangeControl ctl = projectControl.controlFor(change);
ChangeUpdate update = updateFactory.create(
ctl,
change.getCreatedOn());
@@ -207,7 +207,7 @@ public class ChangeInserter {
ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
db.patchSets().insert(Collections.singleton(patchSet));
db.changes().insert(Collections.singleton(change));
LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes();
LabelTypes labelTypes = projectControl.getLabelTypes();
approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo,
@@ -291,8 +291,7 @@ public class ChangeInserter {
db.changes().get(changeMessage.getPatchSetId().getParentKey());
ChangeUtil.bumpRowVersionNotLastUpdatedOn(
changeMessage.getKey().getParentKey(), db);
ChangeControl otherControl =
refControl.getProjectControl().controlFor(otherChange);
ChangeControl otherControl = projectControl.controlFor(otherChange);
ChangeUpdate updateForOtherChange =
updateFactory.create(otherControl, change.getLastUpdatedOn());
cmUtil.addChangeMessage(db, updateForOtherChange, changeMessage);

3
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
View File

@@ -244,7 +244,8 @@ public class CherryPickChange {
destRef.getName()), TimeUtil.nowTs());
change.setTopic(topic);
ChangeInserter ins =
changeInserterFactory.create(refControl, change, cherryPickCommit);
changeInserterFactory.create(refControl.getProjectControl(), change,
cherryPickCommit);
PatchSet newPatchSet = ins.getPatchSet();
CommitValidators commitValidators =

3
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
View File

@@ -207,7 +207,8 @@ public class CreateChange implements
now);
ChangeInserter ins =
changeInserterFactory.create(refControl, change, c);
changeInserterFactory.create(refControl.getProjectControl(),
change, c);
validateCommit(git, refControl, c, me, ins);
updateRef(git, rw, c, change, ins.getPatchSet());

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

@@ -1654,7 +1654,7 @@ public class ReceiveCommits {
magicBranch.dest,
TimeUtil.nowTs());
change.setTopic(magicBranch.topic);
ins = changeInserterFactory.create(ctl, change, c)
ins = changeInserterFactory.create(ctl.getProjectControl(), change, c)
.setDraft(magicBranch.draft);
cmd = new ReceiveCommand(ObjectId.zeroId(), c,
ins.getPatchSet().getRefName());

3
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -1121,8 +1121,7 @@ public abstract class AbstractQueryChangesTest {
Change change = new Change(new Change.Key(key), id, ownerId,
new Branch.NameKey(project, branch), TimeUtil.nowTs());
return changeFactory.create(
projectControlFactory.controlFor(project,
userFactory.create(ownerId)).controlFor(change).getRefControl(),
projectControlFactory.controlFor(project, userFactory.create(ownerId)),
change,
commit);
}