Fix wrong Guice wiring
BanCommit and SuggestParentCandidates Guice beans were using assisted inject pattern. The factories were bound in request scope module even though both beans have nothing to do with request scope. Fix it by dropping unnecessary assisted inject bindings and move the beans into singleton scope. Change-Id: Ia792fb0db60e594456034c1517468a309393f5b5
This commit is contained in:

committed by
David Pursehouse

parent
17aa3fe3ee
commit
e6d550e559
@@ -18,13 +18,11 @@ import static com.google.inject.Scopes.SINGLETON;
|
||||
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.RequestCleanup;
|
||||
import com.google.gerrit.server.git.BanCommit;
|
||||
import com.google.gerrit.server.git.MergeOp;
|
||||
import com.google.gerrit.server.git.SubmoduleOp;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.project.PerRequestProjectControlCache;
|
||||
import com.google.gerrit.server.project.ProjectControl;
|
||||
import com.google.gerrit.server.project.SuggestParentCandidates;
|
||||
import com.google.inject.servlet.RequestScoped;
|
||||
|
||||
/** Bindings for {@link RequestScoped} entities. */
|
||||
@@ -41,11 +39,5 @@ public class GerritRequestModule extends FactoryModule {
|
||||
|
||||
factory(SubmoduleOp.Factory.class);
|
||||
factory(MergeOp.Factory.class);
|
||||
|
||||
// Not really per-request, but dammit, I don't know where else to
|
||||
// easily park this stuff.
|
||||
//
|
||||
factory(SuggestParentCandidates.Factory.class);
|
||||
factory(BanCommit.Factory.class);
|
||||
}
|
||||
}
|
||||
|
@@ -24,6 +24,7 @@ import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.project.ProjectControl;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
|
||||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||
@@ -45,38 +46,36 @@ import java.util.Date;
|
||||
import java.util.List;
|
||||
import java.util.TimeZone;
|
||||
|
||||
@Singleton
|
||||
public class BanCommit {
|
||||
public interface Factory {
|
||||
BanCommit create();
|
||||
}
|
||||
|
||||
/**
|
||||
* Loads a list of commits to reject from {@code refs/meta/reject-commits}.
|
||||
*
|
||||
* @param repo repository from which the rejected commits should be loaded
|
||||
* @return NoteMap of commits to be rejected, null if there are none.
|
||||
* @throws IOException the map cannot be loaded.
|
||||
*/
|
||||
* Loads a list of commits to reject from {@code refs/meta/reject-commits}.
|
||||
*
|
||||
* @param repo repository from which the rejected commits should be loaded
|
||||
* @return NoteMap of commits to be rejected, null if there are none.
|
||||
* @throws IOException the map cannot be loaded.
|
||||
*/
|
||||
public static NoteMap loadRejectCommitsMap(Repository repo)
|
||||
throws IOException {
|
||||
try {
|
||||
Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
|
||||
if (ref == null) {
|
||||
return NoteMap.newEmptyMap();
|
||||
}
|
||||
try {
|
||||
Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
|
||||
if (ref == null) {
|
||||
return NoteMap.newEmptyMap();
|
||||
}
|
||||
|
||||
RevWalk rw = new RevWalk(repo);
|
||||
try {
|
||||
RevCommit map = rw.parseCommit(ref.getObjectId());
|
||||
return NoteMap.read(rw.getObjectReader(), map);
|
||||
} finally {
|
||||
rw.release();
|
||||
}
|
||||
} catch (IOException badMap) {
|
||||
throw new IOException("Cannot load "
|
||||
+ RefNames.REFS_REJECT_COMMITS, badMap);
|
||||
}
|
||||
}
|
||||
RevWalk rw = new RevWalk(repo);
|
||||
try {
|
||||
RevCommit map = rw.parseCommit(ref.getObjectId());
|
||||
return NoteMap.read(rw.getObjectReader(), map);
|
||||
} finally {
|
||||
rw.release();
|
||||
}
|
||||
} catch (IOException badMap) {
|
||||
throw new IOException("Cannot load " + RefNames.REFS_REJECT_COMMITS,
|
||||
badMap);
|
||||
}
|
||||
}
|
||||
|
||||
private final Provider<IdentifiedUser> currentUser;
|
||||
private final GitRepositoryManager repoManager;
|
||||
@@ -96,8 +95,8 @@ public class BanCommit {
|
||||
|
||||
public BanCommitResult ban(final ProjectControl projectControl,
|
||||
final List<ObjectId> commitsToBan, final String reason)
|
||||
throws PermissionDeniedException, IOException,
|
||||
InterruptedException, MergeException, ConcurrentRefUpdateException {
|
||||
throws PermissionDeniedException, IOException, InterruptedException,
|
||||
MergeException, ConcurrentRefUpdateException {
|
||||
if (!projectControl.isOwner()) {
|
||||
throw new PermissionDeniedException(
|
||||
"No project owner: not permitted to ban commits");
|
||||
@@ -124,8 +123,8 @@ public class BanCommit {
|
||||
banCommitNotes.set(commitToBan, createNoteContent(reason, inserter));
|
||||
}
|
||||
inserter.flush();
|
||||
NotesBranchUtil notesBranchUtil = notesBranchUtilFactory.create(project,
|
||||
repo, inserter);
|
||||
NotesBranchUtil notesBranchUtil =
|
||||
notesBranchUtilFactory.create(project, repo, inserter);
|
||||
NoteMap newlyCreated =
|
||||
notesBranchUtil.commitNewNotes(banCommitNotes, REFS_REJECT_COMMITS,
|
||||
createPersonIdent(), buildCommitMessage(commitsToBan, reason));
|
||||
|
@@ -18,6 +18,7 @@ import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.server.config.AllProjectsName;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Comparator;
|
||||
@@ -25,11 +26,8 @@ import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.TreeSet;
|
||||
|
||||
@Singleton
|
||||
public class SuggestParentCandidates {
|
||||
public interface Factory {
|
||||
SuggestParentCandidates create();
|
||||
}
|
||||
|
||||
private final ProjectControl.Factory projectControlFactory;
|
||||
private final ProjectCache projectCache;
|
||||
private final AllProjectsName allProject;
|
||||
|
Reference in New Issue
Block a user