Add factory for ChangeResource

This will allow us to inject classes that we need for the ETag
computation.

Change-Id: I0411d53c085eb271b2704fc83f6236bf2a26cc5f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-02-16 09:33:10 +01:00
parent 6dfc1602e5
commit 398f7a5d0a
11 changed files with 50 additions and 13 deletions

View File

@@ -197,6 +197,9 @@ public abstract class AbstractDaemonTest {
@Inject @Inject
protected ChangeNoteUtil changeNoteUtil; protected ChangeNoteUtil changeNoteUtil;
@Inject
protected ChangeResource.Factory changeResourceFactory;
protected TestRepository<InMemoryRepository> testRepo; protected TestRepository<InMemoryRepository> testRepo;
protected GerritServer server; protected GerritServer server;
protected TestAccount admin; protected TestAccount admin;
@@ -741,6 +744,6 @@ public abstract class AbstractDaemonTest {
List<ChangeControl> ctls = changeFinder.find( List<ChangeControl> ctls = changeFinder.find(
changeId, atrScope.get().getUser()); changeId, atrScope.get().getUser());
assertThat(ctls).hasSize(1); assertThat(ctls).hasSize(1);
return new ChangeResource(ctls.get(0)); return changeResourceFactory.create(ctls.get(0));
} }
} }

View File

@@ -1174,6 +1174,6 @@ public class ChangeIT extends AbstractDaemonTest {
List<ChangeControl> ctls = changeFinder.find( List<ChangeControl> ctls = changeFinder.find(
r.getChangeId(), atrScope.get().getUser()); r.getChangeId(), atrScope.get().getUser());
assertThat(ctls).hasSize(1); assertThat(ctls).hasSize(1);
return new ChangeResource(ctls.get(0)); return changeResourceFactory.create(ctls.get(0));
} }
} }

View File

@@ -28,6 +28,7 @@ import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lifecycle.LifecycleManager;
@@ -38,6 +39,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
@@ -50,7 +52,6 @@ import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper; import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.AbstractModule;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Injector; import com.google.inject.Injector;
@@ -206,7 +207,7 @@ public class RebuildNoteDb extends SiteProgram {
} }
private Injector createSysInjector() { private Injector createSysInjector() {
return dbInjector.createChildInjector(new AbstractModule() { return dbInjector.createChildInjector(new FactoryModule() {
@Override @Override
public void configure() { public void configure() {
install(dbInjector.getInstance(BatchProgramModule.class)); install(dbInjector.getInstance(BatchProgramModule.class));
@@ -214,6 +215,7 @@ public class RebuildNoteDb extends SiteProgram {
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to( DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(
ReindexAfterUpdate.class); ReindexAfterUpdate.class);
install(new DummyIndexModule()); install(new DummyIndexModule());
factory(ChangeResource.Factory.class);
} }
}); });
} }

View File

@@ -18,11 +18,13 @@ import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER; import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
import com.google.gerrit.common.Die; import com.google.gerrit.common.Die;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.pgm.util.BatchProgramModule; import com.google.gerrit.pgm.util.BatchProgramModule;
import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.pgm.util.ThreadLimiter; import com.google.gerrit.pgm.util.ThreadLimiter;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.ScanningChangeCacheImpl; import com.google.gerrit.server.git.ScanningChangeCacheImpl;
import com.google.gerrit.server.index.Index; import com.google.gerrit.server.index.Index;
@@ -129,6 +131,12 @@ public class Reindex extends SiteProgram {
// will have just deleted the old (possibly corrupt) index. // will have just deleted the old (possibly corrupt) index.
modules.add(ScanningChangeCacheImpl.module()); modules.add(ScanningChangeCacheImpl.module());
modules.add(dbInjector.getInstance(BatchProgramModule.class)); modules.add(dbInjector.getInstance(BatchProgramModule.class));
modules.add(new FactoryModule() {
@Override
protected void configure() {
factory(ChangeResource.Factory.class);
}
});
return dbInjector.createChildInjector(modules); return dbInjector.createChildInjector(modules);
} }

View File

@@ -35,13 +35,16 @@ import java.util.Map;
@Singleton @Singleton
public class ActionJson { public class ActionJson {
private final Revisions revisions; private final Revisions revisions;
private final ChangeResource.Factory changeResourceFactory;
private final DynamicMap<RestView<ChangeResource>> changeViews; private final DynamicMap<RestView<ChangeResource>> changeViews;
@Inject @Inject
ActionJson( ActionJson(
Revisions revisions, Revisions revisions,
ChangeResource.Factory changeResourceFactory,
DynamicMap<RestView<ChangeResource>> changeViews) { DynamicMap<RestView<ChangeResource>> changeViews) {
this.revisions = revisions; this.revisions = revisions;
this.changeResourceFactory = changeResourceFactory;
this.changeViews = changeViews; this.changeViews = changeViews;
} }
@@ -69,7 +72,7 @@ public class ActionJson {
Provider<CurrentUser> userProvider = Providers.of(ctl.getUser()); Provider<CurrentUser> userProvider = Providers.of(ctl.getUser());
for (UiAction.Description d : UiActions.from( for (UiAction.Description d : UiActions.from(
changeViews, changeViews,
new ChangeResource(ctl), changeResourceFactory.create(ctl),
userProvider)) { userProvider)) {
out.put(d.getId(), new ActionInfo(d)); out.put(d.getId(), new ActionInfo(d));
} }

View File

@@ -161,6 +161,7 @@ public class ChangeJson {
private final ActionJson actionJson; private final ActionJson actionJson;
private final GpgApiAdapter gpgApi; private final GpgApiAdapter gpgApi;
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final ChangeResource.Factory changeResourceFactory;
private AccountLoader accountLoader; private AccountLoader accountLoader;
private Map<Change.Id, List<SubmitRecord>> submitRecords; private Map<Change.Id, List<SubmitRecord>> submitRecords;
@@ -187,6 +188,7 @@ public class ChangeJson {
ActionJson actionJson, ActionJson actionJson,
GpgApiAdapter gpgApi, GpgApiAdapter gpgApi,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ChangeResource.Factory changeResourceFactory,
@Assisted Set<ListChangesOption> options) { @Assisted Set<ListChangesOption> options) {
this.db = db; this.db = db;
this.labelNormalizer = ln; this.labelNormalizer = ln;
@@ -207,6 +209,7 @@ public class ChangeJson {
this.actionJson = actionJson; this.actionJson = actionJson;
this.gpgApi = gpgApi; this.gpgApi = gpgApi;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.changeResourceFactory = changeResourceFactory;
this.options = options.isEmpty() this.options = options.isEmpty()
? EnumSet.noneOf(ListChangesOption.class) ? EnumSet.noneOf(ListChangesOption.class)
: EnumSet.copyOf(options); : EnumSet.copyOf(options);
@@ -956,7 +959,7 @@ public class ChangeJson {
&& userProvider.get().isIdentifiedUser()) { && userProvider.get().isIdentifiedUser()) {
actionJson.addRevisionActions(out, actionJson.addRevisionActions(out,
new RevisionResource(new ChangeResource(ctl), in)); new RevisionResource(changeResourceFactory.create(ctl), in));
} }
if (has(PUSH_CERTIFICATES)) { if (has(PUSH_CERTIFICATES)) {

View File

@@ -30,6 +30,8 @@ import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@@ -45,9 +47,14 @@ public class ChangeResource implements RestResource, HasETag {
public static final TypeLiteral<RestView<ChangeResource>> CHANGE_KIND = public static final TypeLiteral<RestView<ChangeResource>> CHANGE_KIND =
new TypeLiteral<RestView<ChangeResource>>() {}; new TypeLiteral<RestView<ChangeResource>>() {};
public interface Factory {
ChangeResource create(ChangeControl ctl);
}
private final ChangeControl control; private final ChangeControl control;
public ChangeResource(ChangeControl control) { @AssistedInject
ChangeResource(@Assisted ChangeControl control) {
this.control = control; this.control = control;
} }

View File

@@ -45,6 +45,7 @@ public class ChangesCollection implements
private final DynamicMap<RestView<ChangeResource>> views; private final DynamicMap<RestView<ChangeResource>> views;
private final ChangeFinder changeFinder; private final ChangeFinder changeFinder;
private final CreateChange createChange; private final CreateChange createChange;
private final ChangeResource.Factory changeResourceFactory;
@Inject @Inject
ChangesCollection( ChangesCollection(
@@ -53,13 +54,15 @@ public class ChangesCollection implements
Provider<QueryChanges> queryFactory, Provider<QueryChanges> queryFactory,
DynamicMap<RestView<ChangeResource>> views, DynamicMap<RestView<ChangeResource>> views,
ChangeFinder changeFinder, ChangeFinder changeFinder,
CreateChange createChange) { CreateChange createChange,
ChangeResource.Factory changeResourceFactory) {
this.db = db; this.db = db;
this.user = user; this.user = user;
this.queryFactory = queryFactory; this.queryFactory = queryFactory;
this.views = views; this.views = views;
this.changeFinder = changeFinder; this.changeFinder = changeFinder;
this.createChange = createChange; this.createChange = createChange;
this.changeResourceFactory = changeResourceFactory;
} }
@Override @Override
@@ -86,7 +89,7 @@ public class ChangesCollection implements
if (!ctl.isVisible(db.get())) { if (!ctl.isVisible(db.get())) {
throw new ResourceNotFoundException(id); throw new ResourceNotFoundException(id);
} }
return new ChangeResource(ctl); return changeResourceFactory.create(ctl);
} }
public ChangeResource parse(Change.Id id) public ChangeResource parse(Change.Id id)
@@ -102,7 +105,7 @@ public class ChangesCollection implements
if (!ctl.isVisible(db.get())) { if (!ctl.isVisible(db.get())) {
throw new ResourceNotFoundException(toIdString(id)); throw new ResourceNotFoundException(toIdString(id));
} }
return new ChangeResource(ctl); return changeResourceFactory.create(ctl);
} }
private static IdString toIdString(Change.Id id) { private static IdString toIdString(Change.Id id) {
@@ -110,7 +113,7 @@ public class ChangesCollection implements
} }
public ChangeResource parse(ChangeControl control) { public ChangeResource parse(ChangeControl control) {
return new ChangeResource(control); return changeResourceFactory.create(control);
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")

View File

@@ -42,16 +42,19 @@ public class GetRevisionActions implements ETagView<RevisionResource> {
private final Config config; private final Config config;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final MergeSuperSet mergeSuperSet; private final MergeSuperSet mergeSuperSet;
private final ChangeResource.Factory changeResourceFactory;
@Inject @Inject
GetRevisionActions( GetRevisionActions(
ActionJson delegate, ActionJson delegate,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
MergeSuperSet mergeSuperSet, MergeSuperSet mergeSuperSet,
ChangeResource.Factory changeResourceFactory,
@GerritServerConfig Config config) { @GerritServerConfig Config config) {
this.delegate = delegate; this.delegate = delegate;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.mergeSuperSet = mergeSuperSet; this.mergeSuperSet = mergeSuperSet;
this.changeResourceFactory = changeResourceFactory;
this.config = config; this.config = config;
} }
@@ -71,7 +74,7 @@ public class GetRevisionActions implements ETagView<RevisionResource> {
ChangeSet cs = ChangeSet cs =
mergeSuperSet.completeChangeSet(db, rsrc.getChange(), user); mergeSuperSet.completeChangeSet(db, rsrc.getChange(), user);
for (ChangeData cd : cs.changes()) { for (ChangeData cd : cs.changes()) {
new ChangeResource(cd.changeControl()).prepareETag(h, user); changeResourceFactory.create(cd.changeControl()).prepareETag(h, user);
} }
} catch (IOException | OrmException e) { } catch (IOException | OrmException e) {
throw new OrmRuntimeException(e); throw new OrmRuntimeException(e);

View File

@@ -136,5 +136,6 @@ public class Module extends RestApiModule {
factory(RebaseChangeOp.Factory.class); factory(RebaseChangeOp.Factory.class);
factory(ReviewerResource.Factory.class); factory(ReviewerResource.Factory.class);
factory(SetHashtagsOp.Factory.class); factory(SetHashtagsOp.Factory.class);
factory(ChangeResource.Factory.class);
} }
} }

View File

@@ -56,6 +56,7 @@ public class RebaseChangeOp extends BatchUpdate.Op {
private final PatchSetInserter.Factory patchSetInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory;
private final MergeUtil.Factory mergeUtilFactory; private final MergeUtil.Factory mergeUtilFactory;
private final RebaseUtil rebaseUtil; private final RebaseUtil rebaseUtil;
private final ChangeResource.Factory changeResourceFactory;
private final ChangeControl ctl; private final ChangeControl ctl;
private final PatchSet originalPatchSet; private final PatchSet originalPatchSet;
@@ -77,12 +78,14 @@ public class RebaseChangeOp extends BatchUpdate.Op {
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
MergeUtil.Factory mergeUtilFactory, MergeUtil.Factory mergeUtilFactory,
RebaseUtil rebaseUtil, RebaseUtil rebaseUtil,
ChangeResource.Factory changeResourceFactory,
@Assisted ChangeControl ctl, @Assisted ChangeControl ctl,
@Assisted PatchSet originalPatchSet, @Assisted PatchSet originalPatchSet,
@Assisted @Nullable String baseCommitish) { @Assisted @Nullable String baseCommitish) {
this.patchSetInserterFactory = patchSetInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory;
this.mergeUtilFactory = mergeUtilFactory; this.mergeUtilFactory = mergeUtilFactory;
this.rebaseUtil = rebaseUtil; this.rebaseUtil = rebaseUtil;
this.changeResourceFactory = changeResourceFactory;
this.ctl = ctl; this.ctl = ctl;
this.originalPatchSet = originalPatchSet; this.originalPatchSet = originalPatchSet;
this.baseCommitish = baseCommitish; this.baseCommitish = baseCommitish;
@@ -138,7 +141,8 @@ public class RebaseChangeOp extends BatchUpdate.Op {
RevId baseRevId = new RevId((baseCommitish != null) ? baseCommitish RevId baseRevId = new RevId((baseCommitish != null) ? baseCommitish
: ObjectId.toString(baseCommit.getId())); : ObjectId.toString(baseCommit.getId()));
Base base = rebaseUtil.parseBase( Base base = rebaseUtil.parseBase(
new RevisionResource(new ChangeResource(ctl), originalPatchSet), new RevisionResource(
changeResourceFactory.create(ctl), originalPatchSet),
baseRevId.get()); baseRevId.get());
rebasedPatchSetId = ChangeUtil.nextPatchSetId( rebasedPatchSetId = ChangeUtil.nextPatchSetId(