opendev/gerrit
Code Issues Proposed changes

Merge changes I25c69e63,Ie1fd8770,I882422f4

Browse Source 
* changes:
  Allow ChangeControl.Factory#create(RefControl, Change) to throw OrmException
  Convert ChangeControl.AssistedFactory into a manual factory
  ChangeUtil#findChanges: Use provided user for change controls
This commit is contained in:
Dave Borowitz 2016-02-04 18:38:47 +00:00 committed by Gerrit Code Review
commit 256499a75f
13 changed files with 116 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
View File

@ -119,7 +119,7 @@ public class BatchProgramModule extends FactoryModule {
bind(new TypeLiteral<Set<AccountGroup.UUID>>() {})
.annotatedWith(GitReceivePackGroups.class)
.toInstance(Collections.<AccountGroup.UUID> emptySet());
factory(ChangeControl.AssistedFactory.class);
bind(ChangeControl.Factory.class);
factory(ProjectControl.AssistedFactory.class);
install(new BatchGitModule());

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

@ -351,7 +351,7 @@ public class ChangeUtil {
// Try isolated changeId
if (!id.contains("~")) {
return asChangeControls(query.byKeyPrefix(id));
return asChangeControls(query.byKeyPrefix(id), user);
}
// Try change triplet
@ -359,17 +359,18 @@ public class ChangeUtil {
if (triplet.isPresent()) {
return asChangeControls(query.byBranchKey(
triplet.get().branch(),
triplet.get().id()));
triplet.get().id()),
user);
}
return Collections.emptyList();
}
private List<ChangeControl> asChangeControls(List<ChangeData> cds)
throws OrmException {
private List<ChangeControl> asChangeControls(List<ChangeData> cds,
CurrentUser user) throws OrmException {
List<ChangeControl> ctls = new ArrayList<>(cds.size());
for (ChangeData cd : cds) {
ctls.add(cd.changeControl(user.get()));
ctls.add(cd.changeControl(user));
}
return ctls;
}

4
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
View File

@ -103,7 +103,7 @@ public class ChangeEdits implements
@Override
public ChangeEditResource parse(ChangeResource rsrc, IdString id)
throws ResourceNotFoundException, AuthException, IOException,
InvalidChangeOperationException {
InvalidChangeOperationException, OrmException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
if (!edit.isPresent()) {
throw new ResourceNotFoundException(id);
@ -556,7 +556,7 @@ public class ChangeEdits implements
@Override
public BinaryResult apply(ChangeResource rsrc) throws AuthException,
IOException, ResourceNotFoundException {
IOException, ResourceNotFoundException, OrmException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
if (edit.isPresent()) {
String msg = edit.get().getEditCommit().getFullMessage();

4
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.change.DeleteChangeEdit.Input;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@ -41,7 +42,8 @@ public class DeleteChangeEdit implements RestModifyView<ChangeResource, Input> {
@Override
public Response<?> apply(ChangeResource rsrc, Input input)
throws AuthException, ResourceNotFoundException, IOException {
throws AuthException, ResourceNotFoundException, IOException,
OrmException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
if (edit.isPresent()) {
editUtil.delete(edit.get());

2
gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
View File

@ -138,7 +138,7 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
}
private List<RevisionResource> loadEdit(ChangeResource change, RevId revid)
throws AuthException, IOException {
throws AuthException, IOException, OrmException {
Optional<ChangeEdit> edit = editUtil.byChange(change.getChange());
if (edit.isPresent()) {
PatchSet ps = new PatchSet(new PatchSet.Id(change.getId(), 0));

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

@ -114,9 +114,10 @@ public class ChangeEditUtil {
* @return edit for this change for this user, if present.
* @throws AuthException
* @throws IOException
* @throws OrmException
*/
public Optional<ChangeEdit> byChange(Change change)
throws AuthException, IOException {
throws AuthException, IOException, OrmException {
try {
return byChange(changeControlFactory.controlFor(change, user.get()));
} catch (NoSuchChangeException e) {

6
gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
View File

@ -32,6 +32,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@ -89,9 +90,10 @@ public class LabelNormalizer {
* included in the output, nor are approvals where the user has no
* permissions for that label.
* @throws NoSuchChangeException
* @throws OrmException
*/
public Result normalize(Change change,
Collection<PatchSetApproval> approvals) throws NoSuchChangeException {
public Result normalize(Change change, Collection<PatchSetApproval> approvals)
throws NoSuchChangeException, OrmException {
return normalize(
changeFactory.controlFor(change, userFactory.create(change.getOwner())),
approvals);

4
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
View File

@ -219,7 +219,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
}
private ObjectId toObjectId(PatchSet ps) throws NoSuchChangeException,
AuthException, NoSuchChangeException, IOException {
AuthException, NoSuchChangeException, IOException, OrmException {
if (ps.getId().get() == 0) {
return getEditRev();
}
@ -236,7 +236,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
}
private ObjectId getEditRev() throws AuthException,
NoSuchChangeException, IOException {
NoSuchChangeException, IOException, OrmException {
edit = editReader.byChange(change);
if (edit.isPresent()) {
return edit.get().getRef().getObjectId();

2
gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java
View File

@ -37,7 +37,7 @@ public class AccessControlModule extends FactoryModule {
.annotatedWith(GitReceivePackGroups.class) //
.toProvider(GitReceivePackGroupsProvider.class).in(SINGLETON);
factory(ChangeControl.AssistedFactory.class);
bind(ChangeControl.Factory.class);
factory(ProjectControl.AssistedFactory.class);
}
}

53
gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
View File

@ -35,8 +35,6 @@ import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
import java.util.Collection;
@ -65,7 +63,7 @@ public class ChangeControl {
}
public ChangeControl controlFor(Change change, CurrentUser user)
throws NoSuchChangeException {
throws NoSuchChangeException, OrmException {
final Project.NameKey projectKey = change.getProject();
try {
return projectControl.controlFor(projectKey, user).controlFor(change);
@ -106,34 +104,45 @@ public class ChangeControl {
}
}
public interface AssistedFactory {
ChangeControl create(RefControl refControl, Change change);
ChangeControl create(RefControl refControl, ChangeNotes notes);
public static class Factory {
private final ReviewDb db;
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
@Inject
Factory(ReviewDb db,
ChangeData.Factory changeDataFactory,
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil) {
this.db = db;
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
}
@SuppressWarnings("unused")
ChangeControl create(RefControl refControl, Change change)
throws OrmException {
return create(refControl, notesFactory.create(db, change));
}
ChangeControl create(RefControl refControl, ChangeNotes notes) {
return new ChangeControl(changeDataFactory, approvalsUtil, refControl,
notes);
}
}
private final ChangeData.Factory changeDataFactory;
private final ApprovalsUtil approvalsUtil;
private final RefControl refControl;
private final ChangeNotes notes;
private final ApprovalsUtil approvalsUtil;
@AssistedInject
ChangeControl(
ChangeData.Factory changeDataFactory,
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
ReviewDb db,
@Assisted RefControl refControl,
@Assisted Change change) {
this(changeDataFactory, approvalsUtil, refControl,
notesFactory.create(db, change));
}
@AssistedInject
ChangeControl(
ChangeData.Factory changeDataFactory,
ApprovalsUtil approvalsUtil,
@Assisted RefControl refControl,
@Assisted ChangeNotes notes) {
RefControl refControl,
ChangeNotes notes) {
this.changeDataFactory = changeDataFactory;
this.approvalsUtil = approvalsUtil;
this.refControl = refControl;

7
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
View File

@ -44,6 +44,7 @@ import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
@ -149,7 +150,7 @@ public class ProjectControl {
private final CurrentUser user;
private final ProjectState state;
private final GitRepositoryManager repoManager;
private final ChangeControl.AssistedFactory changeControlFactory;
private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final Collection<ContributorAgreement> contributorAgreements;
private final TagCache tagCache;
@ -167,7 +168,7 @@ public class ProjectControl {
ProjectCache pc,
PermissionCollection.Factory permissionFilter,
GitRepositoryManager repoManager,
ChangeControl.AssistedFactory changeControlFactory,
ChangeControl.Factory changeControlFactory,
TagCache tagCache,
ChangeCache changeCache,
@CanonicalWebUrl @Nullable String canonicalWebUrl,
@ -193,7 +194,7 @@ public class ProjectControl {
return r;
}
public ChangeControl controlFor(final Change change) {
public ChangeControl controlFor(Change change) throws OrmException {
return changeControlFactory.create(controlForRef(change.getDest()), change);
}

18
gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
View File

@ -25,17 +25,21 @@ import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gerrit.testutil.InMemoryDatabase;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Provider;
import com.google.inject.util.Providers;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@ -55,6 +59,7 @@ public class ProjectControlTest {
@Inject private InMemoryRepositoryManager repoManager;
@Inject private ProjectControl.GenericFactory projectControlFactory;
@Inject private SchemaCreator schemaCreator;
@Inject private ThreadLocalRequestContext requestContext;
private LifecycleManager lifecycle;
private ReviewDb db;
@ -81,6 +86,18 @@ public class ProjectControlTest {
project = new ProjectConfig(name);
project.load(inMemoryRepo);
repo = new TestRepository<>(inMemoryRepo);
requestContext.setContext(new RequestContext() {
@Override
public CurrentUser getUser() {
return user;
}
@Override
public Provider<ReviewDb> getReviewDbProvider() {
return Providers.of(db);
}
});
}
@After
@ -91,6 +108,7 @@ public class ProjectControlTest {
if (lifecycle != null) {
lifecycle.stop();
}
requestContext.setContext(null);
if (db != null) {
db.close();
}

101
gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
View File

@ -40,7 +40,6 @@ import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
@ -49,43 +48,29 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.PrologEnvironment;
import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.FakeRealm;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DisableReverseDnsLookup;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.FakeAccountCache;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gerrit.testutil.InMemoryDatabase;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.gerrit.testutil.InMemoryRepositoryManager;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Provider;
import com.google.inject.util.Providers;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@ -245,8 +230,13 @@ public class RefControlTest {
private InMemoryRepositoryManager repoManager;
private ProjectCache projectCache;
private PermissionCollection.Factory sectionSorter;
private CapabilityControl.Factory capabilityControlFactory;
private ChangeControl.AssistedFactory changeControlFactory;
private ChangeControl.Factory changeControlFactory;
private ReviewDb db;
@Inject private CapabilityControl.Factory capabilityControlFactory;
@Inject private SchemaCreator schemaCreator;
@Inject private InMemoryDatabase schemaFactory;
@Inject private ThreadLocalRequestContext requestContext;
@Before
public void setUp() throws Exception {
@ -317,43 +307,11 @@ public class RefControlTest {
}
};
Injector injector = Guice.createInjector(new FactoryModule() {
@SuppressWarnings({"rawtypes", "unchecked"})
@Override
protected void configure() {
Provider nullProvider = Providers.of(null);
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(
new Config());
bind(ReviewDb.class).toProvider(nullProvider);
bind(GitRepositoryManager.class).toInstance(repoManager);
bind(PatchListCache.class).toProvider(nullProvider);
bind(Realm.class).to(FakeRealm.class);
Injector injector = Guice.createInjector(new InMemoryModule());
injector.injectMembers(this);
factory(CapabilityControl.Factory.class);
factory(ChangeControl.AssistedFactory.class);
factory(ChangeData.Factory.class);
factory(MergeUtil.Factory.class);
bind(ProjectCache.class).toInstance(projectCache);
bind(AccountCache.class).toInstance(new FakeAccountCache());
bind(GroupBackend.class).to(SystemGroupBackend.class);
bind(String.class).annotatedWith(CanonicalWebUrl.class)
.toProvider(CanonicalWebUrlProvider.class);
bind(Boolean.class).annotatedWith(DisableReverseDnsLookup.class)
.toInstance(Boolean.FALSE);
bind(String.class).annotatedWith(AnonymousCowardName.class)
.toProvider(AnonymousCowardNameProvider.class);
bind(ChangeKindCache.class).to(ChangeKindCacheImpl.NoCache.class);
bind(MergeabilityCache.class)
.to(MergeabilityCache.NotImplemented.class);
bind(StarredChangesUtil.class)
.toProvider(Providers.<StarredChangesUtil> of(null));
}
});
capabilityControlFactory =
injector.getInstance(CapabilityControl.Factory.class);
changeControlFactory =
injector.getInstance(ChangeControl.AssistedFactory.class);
db = schemaFactory.open();
schemaCreator.create(db);
Cache<SectionSortCache.EntryKey, SectionSortCache.EntryVal> c =
CacheBuilder.newBuilder().build();
@ -367,6 +325,29 @@ public class RefControlTest {
local.load(newRepository(localKey));
add(local);
local.getProject().setParentName(parentKey);
requestContext.setContext(new RequestContext() {
@Override
public CurrentUser getUser() {
return null;
}
@Override
public Provider<ReviewDb> getReviewDbProvider() {
return Providers.of(db);
}
});
changeControlFactory = injector.getInstance(ChangeControl.Factory.class);
}
@After
public void tearDown() {
requestContext.setContext(null);
if (db != null) {
db.close();
}
InMemoryDatabase.drop(schemaFactory);
}
@Test