Adjust more classes to inject UrlFormatter via DynamicItem

Since change I375245647 the UrlFormatter is bound as a DynamicItem,
so it should also be injected as such.

Adjust most remaining classes to do that. The only one remaining is
the ChangeCleanupConfig which requires slightly more invasive changes
and will be done in a separate commit.

Bug: Issue 10500
Change-Id: Ie9eeef11bd5ce63168bb339e22ee3062482ddd25
This commit is contained in:
David Pursehouse
2019-02-20 15:52:43 +09:00
parent a193aee064
commit 27daa2c652
10 changed files with 47 additions and 28 deletions

View File

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.common.io.BaseEncoding; import com.google.common.io.BaseEncoding;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalId;
@@ -58,7 +59,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@Singleton @Singleton
public static class Factory { public static class Factory {
private final Provider<InternalAccountQuery> accountQueryProvider; private final Provider<InternalAccountQuery> accountQueryProvider;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final int maxTrustDepth; private final int maxTrustDepth;
private final ImmutableMap<Long, Fingerprint> trusted; private final ImmutableMap<Long, Fingerprint> trusted;
@@ -68,7 +69,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
Provider<InternalAccountQuery> accountQueryProvider, Provider<InternalAccountQuery> accountQueryProvider,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
UrlFormatter urlFormatter) { DynamicItem<UrlFormatter> urlFormatter) {
this.accountQueryProvider = accountQueryProvider; this.accountQueryProvider = accountQueryProvider;
this.urlFormatter = urlFormatter; this.urlFormatter = urlFormatter;
this.userFactory = userFactory; this.userFactory = userFactory;
@@ -101,7 +102,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
} }
private final Provider<InternalAccountQuery> accountQueryProvider; private final Provider<InternalAccountQuery> accountQueryProvider;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private IdentifiedUser expectedUser; private IdentifiedUser expectedUser;
@@ -144,7 +145,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
private CheckResult checkIdsForExpectedUser(PGPPublicKey key) throws PGPException { private CheckResult checkIdsForExpectedUser(PGPPublicKey key) throws PGPException {
Set<String> allowedUserIds = getAllowedUserIds(expectedUser); Set<String> allowedUserIds = getAllowedUserIds(expectedUser);
if (allowedUserIds.isEmpty()) { if (allowedUserIds.isEmpty()) {
Optional<String> settings = urlFormatter.getSettingsUrl("Identities"); Optional<String> settings = urlFormatter.get().getSettingsUrl("Identities");
return CheckResult.bad( return CheckResult.bad(
"No identities found for user" "No identities found for user"
+ (settings.isPresent() ? "; check " + settings.get() : "")); + (settings.isPresent() ? "; check " + settings.get() : ""));

View File

@@ -24,6 +24,7 @@ import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement; import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
@@ -85,7 +86,7 @@ public class EventFactory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AccountCache accountCache; private final AccountCache accountCache;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
private final Emails emails; private final Emails emails;
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final Provider<PersonIdent> myIdent; private final Provider<PersonIdent> myIdent;
@@ -100,7 +101,7 @@ public class EventFactory {
EventFactory( EventFactory(
AccountCache accountCache, AccountCache accountCache,
Emails emails, Emails emails,
UrlFormatter urlFormatter, DynamicItem<UrlFormatter> urlFormatter,
PatchListCache patchListCache, PatchListCache patchListCache,
@GerritPersonIdent Provider<PersonIdent> myIdent, @GerritPersonIdent Provider<PersonIdent> myIdent,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
@@ -678,7 +679,7 @@ public class EventFactory {
/** Get a link to the change; null if the server doesn't know its own address. */ /** Get a link to the change; null if the server doesn't know its own address. */
private String getChangeUrl(Change change) { private String getChangeUrl(Change change) {
if (change != null) { if (change != null) {
return urlFormatter.getChangeViewUrl(change.getProject(), change.getId()).orElse(null); return urlFormatter.get().getChangeViewUrl(change.getProject(), change.getId()).orElse(null);
} }
return null; return null;
} }

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.config.UrlFormatter; import com.google.gerrit.server.config.UrlFormatter;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -27,10 +28,10 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
private static final String SUBJECT_CROP_APPENDIX = "..."; private static final String SUBJECT_CROP_APPENDIX = "...";
private static final int SUBJECT_CROP_RANGE = 10; private static final int SUBJECT_CROP_RANGE = 10;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
@Inject @Inject
DefaultChangeReportFormatter(UrlFormatter urlFormatter) { DefaultChangeReportFormatter(DynamicItem<UrlFormatter> urlFormatter) {
this.urlFormatter = urlFormatter; this.urlFormatter = urlFormatter;
} }
@@ -49,7 +50,10 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
Change c = input.change(); Change c = input.change();
return String.format( return String.format(
"change %s closed", "change %s closed",
urlFormatter.getChangeViewUrl(c.getProject(), c.getId()).orElse(c.getId().toString())); urlFormatter
.get()
.getChangeViewUrl(c.getProject(), c.getId())
.orElse(c.getId().toString()));
} }
protected String cropSubject(String subject) { protected String cropSubject(String subject) {
@@ -69,7 +73,7 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
protected String formatChangeUrl(Input input) { protected String formatChangeUrl(Input input) {
Change c = input.change(); Change c = input.change();
Optional<String> changeUrl = urlFormatter.getChangeViewUrl(c.getProject(), c.getId()); Optional<String> changeUrl = urlFormatter.get().getChangeViewUrl(c.getProject(), c.getId());
checkState(changeUrl.isPresent()); checkState(changeUrl.isPresent());
StringBuilder m = StringBuilder m =

View File

@@ -30,6 +30,7 @@ import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException; import com.google.gerrit.extensions.restapi.MergeConflictException;
@@ -164,7 +165,7 @@ public class MergeUtil {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final IdentifiedUser.GenericFactory identifiedUserFactory; private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final ProjectState project; private final ProjectState project;
private final boolean useContentMerge; private final boolean useContentMerge;
@@ -176,7 +177,7 @@ public class MergeUtil {
@GerritServerConfig Config serverConfig, @GerritServerConfig Config serverConfig,
Provider<ReviewDb> db, Provider<ReviewDb> db,
IdentifiedUser.GenericFactory identifiedUserFactory, IdentifiedUser.GenericFactory identifiedUserFactory,
UrlFormatter urlFormatter, DynamicItem<UrlFormatter> urlFormatter,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
PluggableCommitMessageGenerator commitMessageGenerator, PluggableCommitMessageGenerator commitMessageGenerator,
@Assisted ProjectState project) { @Assisted ProjectState project) {
@@ -196,7 +197,7 @@ public class MergeUtil {
@GerritServerConfig Config serverConfig, @GerritServerConfig Config serverConfig,
Provider<ReviewDb> db, Provider<ReviewDb> db,
IdentifiedUser.GenericFactory identifiedUserFactory, IdentifiedUser.GenericFactory identifiedUserFactory,
UrlFormatter urlFormatter, DynamicItem<UrlFormatter> urlFormatter,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
@Assisted ProjectState project, @Assisted ProjectState project,
PluggableCommitMessageGenerator commitMessageGenerator, PluggableCommitMessageGenerator commitMessageGenerator,
@@ -491,7 +492,7 @@ public class MergeUtil {
msgbuf.append('\n'); msgbuf.append('\n');
} }
Optional<String> url = urlFormatter.getChangeViewUrl(null, c.getId()); Optional<String> url = urlFormatter.get().getChangeViewUrl(null, c.getId());
if (url.isPresent()) { if (url.isPresent()) {
if (!contains(footers, FooterConstants.REVIEWED_ON, url.get())) { if (!contains(footers, FooterConstants.REVIEWED_ON, url.get())) {
msgbuf msgbuf

View File

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.Side; import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.Extension; import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
@@ -98,7 +99,7 @@ public class MailProcessor {
private final CommentAdded commentAdded; private final CommentAdded commentAdded;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final AccountCache accountCache; private final AccountCache accountCache;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
@Inject @Inject
public MailProcessor( public MailProcessor(
@@ -116,7 +117,7 @@ public class MailProcessor {
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
CommentAdded commentAdded, CommentAdded commentAdded,
AccountCache accountCache, AccountCache accountCache,
UrlFormatter urlFormatter) { DynamicItem<UrlFormatter> urlFormatter) {
this.emails = emails; this.emails = emails;
this.emailRejectionSender = emailRejectionSender; this.emailRejectionSender = emailRejectionSender;
this.retryHelper = retryHelper; this.retryHelper = retryHelper;
@@ -242,7 +243,10 @@ public class MailProcessor {
// If URL is not defined, we won't be able to parse line comments. We still attempt to get the // If URL is not defined, we won't be able to parse line comments. We still attempt to get the
// other ones. // other ones.
String changeUrl = String changeUrl =
urlFormatter.getChangeViewUrl(cd.project(), cd.getId()).orElse("http://gerrit.invalid/"); urlFormatter
.get()
.getChangeViewUrl(cd.project(), cd.getId())
.orElse("http://gerrit.invalid/");
List<MailComment> parsedComments; List<MailComment> parsedComments;
if (useHtmlParser(message)) { if (useHtmlParser(message)) {

View File

@@ -220,7 +220,10 @@ public abstract class ChangeEmail extends NotificationEmail {
/** Get a link to the change; null if the server doesn't know its own address. */ /** Get a link to the change; null if the server doesn't know its own address. */
@Nullable @Nullable
public String getChangeUrl() { public String getChangeUrl() {
return args.urlFormatter.getChangeViewUrl(change.getProject(), change.getId()).orElse(null); return args.urlFormatter
.get()
.getChangeViewUrl(change.getProject(), change.getId())
.orElse(null);
} }
public String getChangeMessageThreadId() { public String getChangeMessageThreadId() {

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.AnonymousUser;
@@ -68,7 +69,7 @@ public class EmailArguments {
final AnonymousUser anonymousUser; final AnonymousUser anonymousUser;
final String anonymousCowardName; final String anonymousCowardName;
final PersonIdent gerritPersonIdent; final PersonIdent gerritPersonIdent;
final UrlFormatter urlFormatter; final DynamicItem<UrlFormatter> urlFormatter;
final AllProjectsName allProjectsName; final AllProjectsName allProjectsName;
final List<String> sshAddresses; final List<String> sshAddresses;
final SitePaths site; final SitePaths site;
@@ -102,7 +103,7 @@ public class EmailArguments {
AnonymousUser anonymousUser, AnonymousUser anonymousUser,
@AnonymousCowardName String anonymousCowardName, @AnonymousCowardName String anonymousCowardName,
GerritPersonIdentProvider gerritPersonIdentProvider, GerritPersonIdentProvider gerritPersonIdentProvider,
UrlFormatter urlFormatter, DynamicItem<UrlFormatter> urlFormatter,
AllProjectsName allProjectsName, AllProjectsName allProjectsName,
ChangeQueryBuilder queryBuilder, ChangeQueryBuilder queryBuilder,
Provider<ReviewDb> db, Provider<ReviewDb> db,

View File

@@ -299,7 +299,7 @@ public abstract class OutgoingEmail {
} }
public String getGerritUrl() { public String getGerritUrl() {
return args.urlFormatter.getWebUrl().orElse(null); return args.urlFormatter.get().getWebUrl().orElse(null);
} }
/** Set a header in the outgoing message. */ /** Set a header in the outgoing message. */

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.project;
import com.google.gerrit.common.data.ContributorAgreement; import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action; import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.metrics.Counter0; import com.google.gerrit.metrics.Counter0;
import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Description;
@@ -38,7 +39,7 @@ import java.util.List;
@Singleton @Singleton
public class ContributorAgreementsChecker { public class ContributorAgreementsChecker {
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final Metrics metrics; private final Metrics metrics;
@@ -57,7 +58,7 @@ public class ContributorAgreementsChecker {
@Inject @Inject
ContributorAgreementsChecker( ContributorAgreementsChecker(
UrlFormatter urlFormatter, ProjectCache projectCache, Metrics metrics) { DynamicItem<UrlFormatter> urlFormatter, ProjectCache projectCache, Metrics metrics) {
this.urlFormatter = urlFormatter; this.urlFormatter = urlFormatter;
this.projectCache = projectCache; this.projectCache = projectCache;
this.metrics = metrics; this.metrics = metrics;
@@ -110,7 +111,7 @@ public class ContributorAgreementsChecker {
.append(iUser.getAccountId()) .append(iUser.getAccountId())
.append(")"); .append(")");
msg.append(urlFormatter.getSettingsUrl("Agreements").orElse("")); msg.append(urlFormatter.get().getSettingsUrl("Agreements").orElse(""));
throw new AuthException(msg.toString()); throw new AuthException(msg.toString());
} }
} }

View File

@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.gerrit.common.data.GarbageCollectionResult; import com.google.gerrit.common.data.GarbageCollectionResult;
import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability; import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -55,14 +56,14 @@ public class GarbageCollect
private final boolean canGC; private final boolean canGC;
private final GarbageCollection.Factory garbageCollectionFactory; private final GarbageCollection.Factory garbageCollectionFactory;
private final WorkQueue workQueue; private final WorkQueue workQueue;
private final UrlFormatter urlFormatter; private final DynamicItem<UrlFormatter> urlFormatter;
@Inject @Inject
GarbageCollect( GarbageCollect(
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
GarbageCollection.Factory garbageCollectionFactory, GarbageCollection.Factory garbageCollectionFactory,
WorkQueue workQueue, WorkQueue workQueue,
UrlFormatter urlFormatter) { DynamicItem<UrlFormatter> urlFormatter) {
this.workQueue = workQueue; this.workQueue = workQueue;
this.urlFormatter = urlFormatter; this.urlFormatter = urlFormatter;
this.canGC = repoManager instanceof LocalDiskRepositoryManager; this.canGC = repoManager instanceof LocalDiskRepositoryManager;
@@ -99,7 +100,9 @@ public class GarbageCollect
WorkQueue.Task<Void> task = (WorkQueue.Task<Void>) workQueue.getDefaultQueue().submit(job); WorkQueue.Task<Void> task = (WorkQueue.Task<Void>) workQueue.getDefaultQueue().submit(job);
Optional<String> url = Optional<String> url =
urlFormatter.getRestUrl("a/config/server/tasks/" + HexFormat.fromInt(task.getTaskId())); urlFormatter
.get()
.getRestUrl("a/config/server/tasks/" + HexFormat.fromInt(task.getTaskId()));
// We're in a HTTP handler, so must be present. // We're in a HTTP handler, so must be present.
checkState(url.isPresent()); checkState(url.isPresent());
return Response.accepted(url.get()); return Response.accepted(url.get());