opendev/gerrit
Code Issues Proposed changes

UrlFormatter: add new interface

Browse Source 
BrowseUrls centralizes formatting of URLs that are displayed to users
in commit messages, the git command-line and out-going emails.

Change-Id: I043f6e1f7f38e4645a34a60a0138bcb7bdc4c75e
This commit is contained in:
Han-Wen Nienhuys 2018-09-19 13:12:00 +02:00
parent 26f524df55
commit 3d8323078e
21 changed files with 239 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
View File

@ -22,12 +22,11 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -38,6 +37,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKey;
@ -58,7 +58,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@Singleton
public static class Factory {
private final Provider<InternalAccountQuery> accountQueryProvider;
private final String webUrl;
private final UrlFormatter urlFormatter;
private final IdentifiedUser.GenericFactory userFactory;
private final int maxTrustDepth;
private final ImmutableMap<Long, Fingerprint> trusted;
@ -68,9 +68,9 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@GerritServerConfig Config cfg,
Provider<InternalAccountQuery> accountQueryProvider,
IdentifiedUser.GenericFactory userFactory,
@CanonicalWebUrl String webUrl) {
UrlFormatter urlFormatter) {
this.accountQueryProvider = accountQueryProvider;
this.webUrl = webUrl;
this.urlFormatter = urlFormatter;
this.userFactory = userFactory;
this.maxTrustDepth = cfg.getInt("receive", null, "maxTrustDepth", 0);
@ -101,14 +101,14 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
}
private final Provider<InternalAccountQuery> accountQueryProvider;
private final String webUrl;
private final UrlFormatter urlFormatter;
private final IdentifiedUser.GenericFactory userFactory;
private IdentifiedUser expectedUser;
private GerritPublicKeyChecker(Factory factory) {
this.accountQueryProvider = factory.accountQueryProvider;
this.webUrl = factory.webUrl;
this.urlFormatter = factory.urlFormatter;
this.userFactory = factory.userFactory;
if (factory.trusted != null) {
enableTrust(factory.maxTrustDepth, factory.trusted);
@ -144,8 +144,10 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
private CheckResult checkIdsForExpectedUser(PGPPublicKey key) throws PGPException {
Set<String> allowedUserIds = getAllowedUserIds(expectedUser);
if (allowedUserIds.isEmpty()) {
Optional<String> settings = urlFormatter.getSettingsUrl("Identities");
return CheckResult.bad(
"No identities found for user; check " + webUrl + "#" + PageLinks.SETTINGS_WEBIDENT);
"No identities found for user"
+ (settings.isPresent() ? "; check " + settings.get() : ""));
}
if (hasAllowedUserId(key, allowedUserIds)) {
return CheckResult.trusted();

3
java/com/google/gerrit/pgm/util/BatchProgramModule.java
View File

@ -47,12 +47,14 @@ import com.google.gerrit.server.change.RebaseChangeOp;
import com.google.gerrit.server.config.AdministrateServerGroups;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultBrowseUrls;
import com.google.gerrit.server.config.DisableReverseDnsLookup;
import com.google.gerrit.server.config.DisableReverseDnsLookupProvider;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.config.SysExecutorModule;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.EventUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.extensions.events.RevisionCreated;
@ -138,6 +140,7 @@ public class BatchProgramModule extends FactoryModule {
bind(String.class)
.annotatedWith(CanonicalWebUrl.class)
.toProvider(CanonicalWebUrlProvider.class);
bind(UrlFormatter.class).to(DefaultBrowseUrls.class);
bind(Boolean.class)
.annotatedWith(DisableReverseDnsLookup.class)
.toProvider(DisableReverseDnsLookupProvider.class)

1
java/com/google/gerrit/server/config/CanonicalWebUrlModule.java
View File

@ -30,6 +30,7 @@ public abstract class CanonicalWebUrlModule extends AbstractModule {
//
final Class<? extends Provider<String>> provider = provider();
bind(String.class).annotatedWith(CanonicalWebUrl.class).toProvider(provider);
bind(UrlFormatter.class).to(DefaultBrowseUrls.class);
}
protected abstract Class<? extends Provider<String>> provider();

17
java/com/google/gerrit/server/config/ChangeCleanupConfig.java
View File

@ -15,7 +15,6 @@
package com.google.gerrit.server.config;
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.config.ScheduleConfig.Schedule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@ -31,7 +30,7 @@ public class ChangeCleanupConfig {
private static String KEY_ABANDON_MESSAGE = "abandonMessage";
private static String DEFAULT_ABANDON_MESSAGE =
"Auto-Abandoned due to inactivity, see "
+ "${URL}Documentation/user-change-cleanup.html#auto-abandon\n"
+ "${URL}\n"
+ "\n"
+ "If this change is still wanted it should be restored.";
@ -41,12 +40,11 @@ public class ChangeCleanupConfig {
private final String abandonMessage;
@Inject
ChangeCleanupConfig(
@GerritServerConfig Config cfg, @CanonicalWebUrl @Nullable String canonicalWebUrl) {
ChangeCleanupConfig(@GerritServerConfig Config cfg, UrlFormatter urlFormatter) {
schedule = ScheduleConfig.createSchedule(cfg, SECTION);
abandonAfter = readAbandonAfter(cfg);
abandonIfMergeable = cfg.getBoolean(SECTION, null, KEY_ABANDON_IF_MERGEABLE, true);
abandonMessage = readAbandonMessage(cfg, canonicalWebUrl);
abandonMessage = readAbandonMessage(cfg, urlFormatter);
}
private long readAbandonAfter(Config cfg) {
@ -55,14 +53,17 @@ public class ChangeCleanupConfig {
return abandonAfter >= 0 ? abandonAfter : 0;
}
private String readAbandonMessage(Config cfg, String webUrl) {
private String readAbandonMessage(Config cfg, UrlFormatter urlFormatter) {
String abandonMessage = cfg.getString(SECTION, null, KEY_ABANDON_MESSAGE);
if (Strings.isNullOrEmpty(abandonMessage)) {
abandonMessage = DEFAULT_ABANDON_MESSAGE;
}
if (!Strings.isNullOrEmpty(webUrl)) {
abandonMessage = abandonMessage.replaceAll("\\$\\{URL\\}", webUrl);
String docUrl = urlFormatter.getDocUrl("user-change-cleanup.html", "auto-abandon").orElse("");
if (!docUrl.isEmpty()) {
abandonMessage = abandonMessage.replaceAll("\\$\\{URL\\}", docUrl);
}
return abandonMessage;
}

35
java/com/google/gerrit/server/config/DefaultBrowseUrls.java Normal file
View File

@ -0,0 +1,35 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.config;
import java.util.Optional;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
@Singleton
public class DefaultBrowseUrls implements UrlFormatter {
private final Provider<String> canonicalWebUrlProvider;
@Inject
DefaultBrowseUrls(@CanonicalWebUrl Provider<String> canonicalWebUrlProvider) {
this.canonicalWebUrlProvider = canonicalWebUrlProvider;
}
@Override
public Optional<String> getWebUrl() {
return Optional.ofNullable(canonicalWebUrlProvider.get());
}
}

66
java/com/google/gerrit/server/config/UrlFormatter.java Normal file
View File

@ -0,0 +1,66 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.config;
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import java.util.Optional;
/**
* Formats URLs to different parts of the Gerrit API and UI.
*
* <p>By default, these gerrit URLs are formed by adding suffixes to the web URL. The interface
* centralizes these conventions, and also allows introducing different, custom URL schemes.
*
* <p>Unfortunately, Gerrit operates in modes for which there is no canonical URL. This can be in
* standalone utilities that have no HTTP server (eg. index upgrade commands), in servers that run
* SSH only, or in a HTTP/SSH server that is accessed over SSH without canonical web URL configured.
*/
public interface UrlFormatter {
/**
* The canonical base URL where this Gerrit installation can be reached.
*
* <p>For the default implementations below to work, it must end in "/".
*/
Optional<String> getWebUrl();
/** Returns the URL for viewing a change. */
default Optional<String> getChangeViewUrl(@Nullable Project.NameKey project, Change.Id id) {
// In the PolyGerrit URL (contrary to REST URLs) there is no need to URL-escape strings, since
// the /+/ separator unambiguously defines how to parse the path.
return getWebUrl()
.map(url -> url + "c/" + (project != null ? project.get() + "/+/" : "") + id.get());
}
/** Returns a URL pointing to a section of the settings page. */
default Optional<String> getSettingsUrl(String section) {
return getWebUrl()
.map(url -> url + "settings" + (Strings.isNullOrEmpty(section) ? "" : "#" + section));
}
/** Returns a URL pointing to a documentation page, at a given named anchor. */
default Optional<String> getDocUrl(String page, String anchor) {
return getWebUrl().map(url -> url + "Documentation/" + page + "#" + anchor);
}
/** Returns a REST API URL for a given suffix (eg. "accounts/self/details") */
default Optional<String> getRestUrl(String suffix) {
return getWebUrl().map(url -> url + suffix);
}
}

16
java/com/google/gerrit/server/events/EventFactory.java
View File

@ -20,7 +20,6 @@ import static java.util.Comparator.comparing;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
@ -42,7 +41,7 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ApprovalAttribute;
import com.google.gerrit.server.data.ChangeAttribute;
@ -86,8 +85,8 @@ public class EventFactory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AccountCache accountCache;
private final UrlFormatter urlFormatter;
private final Emails emails;
private final Provider<String> urlProvider;
private final PatchListCache patchListCache;
private final Provider<PersonIdent> myIdent;
private final ChangeData.Factory changeDataFactory;
@ -101,7 +100,7 @@ public class EventFactory {
EventFactory(
AccountCache accountCache,
Emails emails,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
UrlFormatter urlFormatter,
PatchListCache patchListCache,
@GerritPersonIdent Provider<PersonIdent> myIdent,
ChangeData.Factory changeDataFactory,
@ -111,8 +110,8 @@ public class EventFactory {
SchemaFactory<ReviewDb> schema,
IndexConfig indexConfig) {
this.accountCache = accountCache;
this.urlFormatter = urlFormatter;
this.emails = emails;
this.urlProvider = urlProvider;
this.patchListCache = patchListCache;
this.myIdent = myIdent;
this.changeDataFactory = changeDataFactory;
@ -678,11 +677,8 @@ public class EventFactory {
/** Get a link to the change; null if the server doesn't know its own address. */
private String getChangeUrl(Change change) {
if (change != null && urlProvider.get() != null) {
StringBuilder r = new StringBuilder();
r.append(urlProvider.get());
r.append(change.getChangeId());
return r.toString();
if (change != null) {
return urlFormatter.getChangeViewUrl(change.getProject(), change.getId()).orElse(null);
}
return null;
}

31
java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
View File

@ -14,9 +14,11 @@
package com.google.gerrit.server.git;
import com.google.common.base.Preconditions;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.inject.Inject;
import java.util.Optional;
/** Print a change description for use in git command-line progress. */
public class DefaultChangeReportFormatter implements ChangeReportFormatter {
@ -24,30 +26,29 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
private static final String SUBJECT_CROP_APPENDIX = "...";
private static final int SUBJECT_CROP_RANGE = 10;
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
@Inject
DefaultChangeReportFormatter(@CanonicalWebUrl String canonicalWebUrl) {
this.canonicalWebUrl = canonicalWebUrl;
DefaultChangeReportFormatter(UrlFormatter urlFormatter) {
this.urlFormatter = urlFormatter;
}
@Override
public String newChange(ChangeReportFormatter.Input input) {
return formatChangeUrl(canonicalWebUrl, input);
return formatChangeUrl(input);
}
@Override
public String changeUpdated(ChangeReportFormatter.Input input) {
return formatChangeUrl(canonicalWebUrl, input);
}
public static String formatChangeUrl(String canonicalWebUrl, Change change) {
return canonicalWebUrl + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
return formatChangeUrl(input);
}
@Override
public String changeClosed(ChangeReportFormatter.Input input) {
return String.format("change %s closed", formatChangeUrl(canonicalWebUrl, input.change()));
Change c = input.change();
return String.format(
"change %s closed",
urlFormatter.getChangeViewUrl(c.getProject(), c.getId()).orElse(c.getId().toString()));
}
protected String cropSubject(String subject) {
@ -65,11 +66,15 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
return subject;
}
protected String formatChangeUrl(String url, Input input) {
protected String formatChangeUrl(Input input) {
Change c = input.change();
Optional<String> changeUrl = urlFormatter.getChangeViewUrl(c.getProject(), c.getId());
Preconditions.checkState(changeUrl.isPresent());
StringBuilder m =
new StringBuilder()
.append(" ")
.append(formatChangeUrl(url, input.change()))
.append(changeUrl.get())
.append(" ")
.append(cropSubject(input.subject()));
if (input.isEdit()) {

31
java/com/google/gerrit/server/git/MergeUtil.java
View File

@ -25,7 +25,6 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
@ -42,8 +41,8 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectState;
@ -65,6 +64,7 @@ import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.AmbiguousObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@ -147,7 +147,7 @@ public class MergeUtil {
private final Provider<ReviewDb> db;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Provider<String> urlProvider;
private final UrlFormatter urlFormatter;
private final ApprovalsUtil approvalsUtil;
private final ProjectState project;
private final boolean useContentMerge;
@ -159,7 +159,7 @@ public class MergeUtil {
@GerritServerConfig Config serverConfig,
Provider<ReviewDb> db,
IdentifiedUser.GenericFactory identifiedUserFactory,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
UrlFormatter urlFormatter,
ApprovalsUtil approvalsUtil,
PluggableCommitMessageGenerator commitMessageGenerator,
@Assisted ProjectState project) {
@ -167,7 +167,7 @@ public class MergeUtil {
serverConfig,
db,
identifiedUserFactory,
urlProvider,
urlFormatter,
approvalsUtil,
project,
commitMessageGenerator,
@ -179,14 +179,14 @@ public class MergeUtil {
@GerritServerConfig Config serverConfig,
Provider<ReviewDb> db,
IdentifiedUser.GenericFactory identifiedUserFactory,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
UrlFormatter urlFormatter,
ApprovalsUtil approvalsUtil,
@Assisted ProjectState project,
PluggableCommitMessageGenerator commitMessageGenerator,
@Assisted boolean useContentMerge) {
this.db = db;
this.identifiedUserFactory = identifiedUserFactory;
this.urlProvider = urlProvider;
this.urlFormatter = urlFormatter;
this.approvalsUtil = approvalsUtil;
this.project = project;
this.useContentMerge = useContentMerge;
@ -345,17 +345,16 @@ public class MergeUtil {
msgbuf.append('\n');
}
final String siteUrl = urlProvider.get();
if (siteUrl != null) {
final String url = siteUrl + c.getId().get();
if (!contains(footers, FooterConstants.REVIEWED_ON, url)) {
msgbuf.append(FooterConstants.REVIEWED_ON.getName());
msgbuf.append(": ");
msgbuf.append(url);
msgbuf.append('\n');
Optional<String> url = urlFormatter.getChangeViewUrl(null, c.getId());
if (url.isPresent()) {
if (!contains(footers, FooterConstants.REVIEWED_ON, url.get())) {
msgbuf
.append(FooterConstants.REVIEWED_ON.getName())
.append(": ")
.append(url.get())
.append('\n');
}
}
PatchSetApproval submitAudit = null;
for (PatchSetApproval a : safeGetApprovals(notes, psId)) {

74
java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@ -20,13 +20,13 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
import static java.util.stream.Collectors.toList;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
@ -41,8 +41,8 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError;
@ -63,6 +63,7 @@ import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
@ -87,7 +88,7 @@ public class CommitValidators {
@Singleton
public static class Factory {
private final PersonIdent gerritIdent;
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
private final DynamicSet<CommitValidationListener> pluginValidators;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
@ -100,7 +101,7 @@ public class CommitValidators {
@Inject
Factory(
@GerritPersonIdent PersonIdent gerritIdent,
@CanonicalWebUrl @Nullable String canonicalWebUrl,
UrlFormatter urlFormatter,
@GerritServerConfig Config cfg,
DynamicSet<CommitValidationListener> pluginValidators,
GitRepositoryManager repoManager,
@ -110,7 +111,7 @@ public class CommitValidators {
AccountValidator accountValidator,
ProjectCache projectCache) {
this.gerritIdent = gerritIdent;
this.canonicalWebUrl = canonicalWebUrl;
this.urlFormatter = urlFormatter;
this.pluginValidators = pluginValidators;
this.repoManager = repoManager;
this.allUsers = allUsers;
@ -138,16 +139,11 @@ public class CommitValidators {
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl),
new AuthorUploaderValidator(user, perm, urlFormatter),
new CommitterUploaderValidator(user, perm, urlFormatter),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
projectState,
user,
canonicalWebUrl,
installCommitMsgHookCommand,
sshInfo,
change),
projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
new ConfigValidator(branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
@ -171,15 +167,10 @@ public class CommitValidators {
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new AuthorUploaderValidator(user, perm, urlFormatter),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())),
new ChangeIdValidator(
projectState,
user,
canonicalWebUrl,
installCommitMsgHookCommand,
sshInfo,
change),
projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
new ConfigValidator(branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@ -208,8 +199,8 @@ public class CommitValidators {
ImmutableList.of(
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectCache.checkedGet(branch.getParentKey())),
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new CommitterUploaderValidator(user, perm, canonicalWebUrl)));
new AuthorUploaderValidator(user, perm, urlFormatter),
new CommitterUploaderValidator(user, perm, urlFormatter)));
}
}
@ -253,7 +244,7 @@ public class CommitValidators {
private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN);
private final ProjectState projectState;
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
private final String installCommitMsgHookCommand;
private final SshInfo sshInfo;
private final IdentifiedUser user;
@ -262,12 +253,12 @@ public class CommitValidators {
public ChangeIdValidator(
ProjectState projectState,
IdentifiedUser user,
String canonicalWebUrl,
UrlFormatter urlFormatter,
String installCommitMsgHookCommand,
SshInfo sshInfo,
Change change) {
this.projectState = projectState;
this.canonicalWebUrl = canonicalWebUrl;
this.urlFormatter = urlFormatter;
this.installCommitMsgHookCommand = installCommitMsgHookCommand;
this.sshInfo = sshInfo;
this.user = user;
@ -352,10 +343,12 @@ public class CommitValidators {
// If there are no SSH keys, the commit-msg hook must be installed via
// HTTP(S)
Optional<String> webUrl = urlFormatter.getWebUrl();
if (hostKeys.isEmpty()) {
Preconditions.checkState(webUrl.isPresent());
return String.format(
" f=\"$(git rev-parse --git-dir)/hooks/commit-msg\"; curl -o \"$f\" %stools/hooks/commit-msg ; chmod +x \"$f\"",
canonicalWebUrl);
webUrl.get());
}
// SSH keys exist, so the hook can be installed with scp.
@ -365,7 +358,8 @@ public class CommitValidators {
int c = host.lastIndexOf(':');
if (0 <= c) {
if (host.startsWith("*:")) {
sshHost = getGerritHost(canonicalWebUrl);
Preconditions.checkState(webUrl.isPresent());
sshHost = getGerritHost(webUrl.get());
} else {
sshHost = host.substring(0, c);
}
@ -547,13 +541,13 @@ public class CommitValidators {
public static class AuthorUploaderValidator implements CommitValidationListener {
private final IdentifiedUser user;
private final PermissionBackend.ForRef perm;
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
public AuthorUploaderValidator(
IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) {
IdentifiedUser user, PermissionBackend.ForRef perm, UrlFormatter urlFormatter) {
this.user = user;
this.perm = perm;
this.canonicalWebUrl = canonicalWebUrl;
this.urlFormatter = urlFormatter;
}
@Override
@ -568,7 +562,7 @@ public class CommitValidators {
return Collections.emptyList();
} catch (AuthException e) {
throw new CommitValidationException(
"invalid author", invalidEmail("author", author, user, canonicalWebUrl));
"invalid author", invalidEmail("author", author, user, urlFormatter));
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_AUTHOR");
throw new CommitValidationException("internal auth error");
@ -580,13 +574,13 @@ public class CommitValidators {
public static class CommitterUploaderValidator implements CommitValidationListener {
private final IdentifiedUser user;
private final PermissionBackend.ForRef perm;
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
public CommitterUploaderValidator(
IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) {
IdentifiedUser user, PermissionBackend.ForRef perm, UrlFormatter urlFormatter) {
this.user = user;
this.perm = perm;
this.canonicalWebUrl = canonicalWebUrl;
this.urlFormatter = urlFormatter;
}
@Override
@ -601,7 +595,7 @@ public class CommitValidators {
return Collections.emptyList();
} catch (AuthException e) {
throw new CommitValidationException(
"invalid committer", invalidEmail("committer", committer, user, canonicalWebUrl));
"invalid committer", invalidEmail("committer", committer, user, urlFormatter));
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
throw new CommitValidationException("internal auth error");
@ -821,7 +815,7 @@ public class CommitValidators {
}
private static CommitValidationMessage invalidEmail(
String type, PersonIdent who, IdentifiedUser currentUser, String canonicalWebUrl) {
String type, PersonIdent who, IdentifiedUser currentUser, UrlFormatter urlFormatter) {
StringBuilder sb = new StringBuilder();
sb.append("email address ")
@ -839,11 +833,11 @@ public class CommitValidators {
}
}
if (canonicalWebUrl != null) {
sb.append("To register an email address, visit:\n");
sb.append(canonicalWebUrl).append("#").append(PageLinks.SETTINGS_CONTACT).append("\n");
if (urlFormatter.getSettingsUrl("").isPresent()) {
sb.append("To register an email address, visit:\n")
.append(urlFormatter.getSettingsUrl("EmailAddresses").get())
.append("\n\n");
}
sb.append("\n");
return new CommitValidationMessage(sb.toString(), true);
}

14
java/com/google/gerrit/server/mail/receive/MailProcessor.java
View File

@ -46,7 +46,7 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.mail.MailFilter;
import com.google.gerrit.server.mail.send.InboundEmailRejectionSender;
@ -97,7 +97,7 @@ public class MailProcessor {
private final CommentAdded commentAdded;
private final ApprovalsUtil approvalsUtil;
private final AccountCache accountCache;
private final Provider<String> canonicalUrl;
private final UrlFormatter urlFormatter;
@Inject
public MailProcessor(
@ -115,7 +115,7 @@ public class MailProcessor {
ApprovalsUtil approvalsUtil,
CommentAdded commentAdded,
AccountCache accountCache,
@CanonicalWebUrl Provider<String> canonicalUrl) {
UrlFormatter urlFormatter) {
this.emails = emails;
this.emailRejectionSender = emailRejectionSender;
this.retryHelper = retryHelper;
@ -130,7 +130,7 @@ public class MailProcessor {
this.commentAdded = commentAdded;
this.approvalsUtil = approvalsUtil;
this.accountCache = accountCache;
this.canonicalUrl = canonicalUrl;
this.urlFormatter = urlFormatter;
}
/**
@ -237,7 +237,11 @@ public class MailProcessor {
.sorted(CommentsUtil.COMMENT_ORDER)
.collect(toList());
Project.NameKey project = cd.project();
String changeUrl = canonicalUrl.get() + "c/" + cd.project().get() + "/+/" + cd.getId().get();
// If URL is not defined, we won't be able to parse line comments. We still attempt to get the
// other ones.
String changeUrl =
urlFormatter.getChangeViewUrl(cd.project(), cd.getId()).orElse("http://gerrit.invalid/");
List<MailComment> parsedComments;
if (useHtmlParser(message)) {

7
java/com/google/gerrit/server/mail/send/ChangeEmail.java
View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.send;
import com.google.common.base.Splitter;
import com.google.common.collect.ListMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
@ -217,11 +218,9 @@ public abstract class ChangeEmail extends NotificationEmail {
}
/** Get a link to the change; null if the server doesn't know its own address. */
@Nullable
public String getChangeUrl() {
if (getGerritUrl() != null) {
return getGerritUrl() + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
}
return null;
return args.urlFormatter.getChangeViewUrl(change.getProject(), change.getId()).orElse(null);
}
public String getChangeMessageThreadId() {

9
java/com/google/gerrit/server/mail/send/EmailArguments.java
View File

@ -14,7 +14,6 @@
package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
@ -27,10 +26,10 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritInstanceName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mail.EmailSettings;
import com.google.gerrit.server.notedb.ChangeNotes;
@ -67,7 +66,7 @@ public class EmailArguments {
final AnonymousUser anonymousUser;
final String anonymousCowardName;
final PersonIdent gerritPersonIdent;
final Provider<String> urlProvider;
final UrlFormatter urlFormatter;
final AllProjectsName allProjectsName;
final List<String> sshAddresses;
final SitePaths site;
@ -100,7 +99,7 @@ public class EmailArguments {
AnonymousUser anonymousUser,
@AnonymousCowardName String anonymousCowardName,
GerritPersonIdentProvider gerritPersonIdentProvider,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
UrlFormatter urlFormatter,
AllProjectsName allProjectsName,
ChangeQueryBuilder queryBuilder,
Provider<ReviewDb> db,
@ -129,7 +128,7 @@ public class EmailArguments {
this.anonymousUser = anonymousUser;
this.anonymousCowardName = anonymousCowardName;
this.gerritPersonIdent = gerritPersonIdentProvider.get();
this.urlProvider = urlProvider;
this.urlFormatter = urlFormatter;
this.allProjectsName = allProjectsName;
this.queryBuilder = queryBuilder;
this.db = db;

2
java/com/google/gerrit/server/mail/send/OutgoingEmail.java
View File

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

23
java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
View File

@ -14,8 +14,6 @@
package com.google.gerrit.server.project;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
@ -29,7 +27,7 @@ import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@ -40,7 +38,7 @@ import java.util.List;
@Singleton
public class ContributorAgreementsChecker {
private final String canonicalWebUrl;
private final UrlFormatter urlFormatter;
private final ProjectCache projectCache;
private final Metrics metrics;
@ -59,10 +57,8 @@ public class ContributorAgreementsChecker {
@Inject
ContributorAgreementsChecker(
@CanonicalWebUrl @Nullable String canonicalWebUrl,
ProjectCache projectCache,
Metrics metrics) {
this.canonicalWebUrl = canonicalWebUrl;
UrlFormatter urlFormatter, ProjectCache projectCache, Metrics metrics) {
this.urlFormatter = urlFormatter;
this.projectCache = projectCache;
this.metrics = metrics;
}
@ -113,15 +109,8 @@ public class ContributorAgreementsChecker {
.append(" (id=")
.append(iUser.getAccountId())
.append(")");
if (canonicalWebUrl != null) {
msg.append(":\n\n ");
msg.append(canonicalWebUrl);
msg.append("#");
msg.append(PageLinks.SETTINGS_AGREEMENTS);
msg.append("\n");
} else {
msg.append(".");
}
msg.append(urlFormatter.getSettingsUrl("Agreements").orElse(""));
throw new AuthException(msg.toString());
}
}

20
java/com/google/gerrit/server/restapi/project/GarbageCollect.java
View File

@ -16,6 +16,7 @@ package com.google.gerrit.server.restapi.project;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Preconditions;
import com.google.gerrit.common.data.GarbageCollectionResult;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
@ -24,7 +25,7 @@ import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GarbageCollection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
@ -33,13 +34,13 @@ import com.google.gerrit.server.ioutil.HexFormat;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.restapi.project.GarbageCollect.Input;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.util.Collections;
import java.util.Optional;
@RequiresCapability(GlobalCapability.RUN_GC)
@Singleton
@ -54,16 +55,16 @@ public class GarbageCollect
private final boolean canGC;
private final GarbageCollection.Factory garbageCollectionFactory;
private final WorkQueue workQueue;
private final Provider<String> canonicalUrl;
private final UrlFormatter urlFormatter;
@Inject
GarbageCollect(
GitRepositoryManager repoManager,
GarbageCollection.Factory garbageCollectionFactory,
WorkQueue workQueue,
@CanonicalWebUrl Provider<String> canonicalUrl) {
UrlFormatter urlFormatter) {
this.workQueue = workQueue;
this.canonicalUrl = canonicalUrl;
this.urlFormatter = urlFormatter;
this.canGC = repoManager instanceof LocalDiskRepositoryManager;
this.garbageCollectionFactory = garbageCollectionFactory;
}
@ -97,10 +98,11 @@ public class GarbageCollect
@SuppressWarnings("unchecked")
WorkQueue.Task<Void> task = (WorkQueue.Task<Void>) workQueue.getDefaultQueue().submit(job);
String location =
canonicalUrl.get() + "a/config/server/tasks/" + HexFormat.fromInt(task.getTaskId());
return Response.accepted(location);
Optional<String> url =
urlFormatter.getRestUrl("a/config/server/tasks/" + HexFormat.fromInt(task.getTaskId()));
// We're in a HTTP handler, so must be present.
Preconditions.checkState(url.isPresent());
return Response.accepted(url.get());
}
@SuppressWarnings("resource")

4
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@ -2585,7 +2585,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<String> expectedFooters =
Arrays.asList(
"Change-Id: " + r2.getChangeId(),
"Reviewed-on: " + canonicalWebUrl.get() + r2.getChange().getId(),
"Reviewed-on: " + canonicalWebUrl.get() + "c/" + r2.getChange().getId(),
"Reviewed-by: Administrator <admin@example.com>",
"Custom2: Administrator <admin@example.com>",
"Tested-by: Administrator <admin@example.com>");
@ -2627,7 +2627,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<String> expectedFooters =
Arrays.asList(
"Change-Id: " + change.getChangeId(),
"Reviewed-on: " + canonicalWebUrl.get() + change.getChange().getId(),
"Reviewed-on: " + canonicalWebUrl.get() + "c/" + change.getChange().getId(),
"Custom: refs/heads/master");
assertThat(footers).containsExactlyElementsIn(expectedFooters);
}

2
javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
View File

@ -232,7 +232,7 @@ public class GerritPublicKeyCheckerTest {
assertProblems(
checker.check(key.getPublicKey()),
Status.BAD,
"No identities found for user; check http://test/#/settings/web-identities");
"No identities found for user; check http://test/settings#Identities");
checker = checkerFactory.create().setStore(store).disableTrust();
assertProblems(

3
javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
View File

@ -42,9 +42,11 @@ import com.google.gerrit.server.config.AllUsersNameProvider;
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.DefaultBrowseUrls;
import com.google.gerrit.server.config.DisableReverseDnsLookup;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager;
@ -162,6 +164,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
migration.setFrom(NotesMigrationState.FINAL);
bind(MutableNotesMigration.class).toInstance(migration);
bind(NotesMigration.class).to(MutableNotesMigration.class);
bind(UrlFormatter.class).to(DefaultBrowseUrls.class);
// Tests don't support ReviewDb at all, but bindings are required via NoteDbModule.
bind(new TypeLiteral<SchemaFactory<ReviewDb>>() {})

2
plugins/hooks

@ -1 +1 @@
Subproject commit 7185e5ce46646e952071befd9cf8f4267560b51d
Subproject commit de469e8e2598779773652abb43a0356650e257b3

2
plugins/reviewnotes

@ -1 +1 @@
Subproject commit 54525ffaed5e8925d97657a622532a00a0006347
Subproject commit 543ae1f0a24dda61d4f36b173b9bddfd52ead3b9