Add Gerrit's instance name and reference it in notification emails

Gerrit users active on several Gerrit servers may find it hard to
determine the gerrit instance related to an email.
This commit fixes it by adding a Gerrit instance name to the email
titles, right before the project's short name.
For instance, for a Gerrit instance called "Pear" and the project
"website/forum", the notification email titles will contain "Pear/forum".

Also add configuration to disable this behavior.
Change-Id: I6c842f33ce605125ec64ca7d09643f59aa96a02d
This commit is contained in:
Maxime Guerreiro
2018-03-21 18:59:27 +01:00
parent 44cb0fd77a
commit 8d129d41f5
13 changed files with 208 additions and 3 deletions

View File

@@ -2144,6 +2144,15 @@ Path prefix for PolyGerrit's static resources if using a CDN.
Path for PolyGerrit's favicon after link:#gerrit.canonicalWebUrl[default URL],
including icon name and extension (.ico should be used).
[[gerrit.instanceName]]gerrit.instanceName::
+
Short identifier for this Gerrit instance.
A good name should be short but precise enough so that users can identify the instance among others.
+
Defaults to the full hostname of the Gerrit server.
[[gerrit.ui]]gerrit.ui::
+
Default UI when the user does not request a different preference via argument
@@ -4127,6 +4136,17 @@ in the link:http://data.iana.org/TLD/[IANA list].
+
Defaults to an empty list, meaning no additional TLDs are allowed.
[[sendemail.addInstanceNameInSubject]]sendemail.addInstanceNameInSubject::
+
When set to true, Gerrit will add its short name to the email subject, allowing recipients to quickly identify
what Gerrit instance the email came from.
+
The short name can be customized via the gerrit.instanceName option.
+
Defaults to true.
[[site]]
=== Section site

View File

@@ -225,6 +225,10 @@ $shortProjectName::
+
The project name with the path abbreviated.
$instanceAndProjectName::
+
The Gerrit instance name, followed by the short project name
$sshHost::
+
SSH hostname for the Gerrit instance.

View File

@@ -52,6 +52,7 @@ import com.google.gerrit.server.config.AuthConfigModule;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.DownloadConfig;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerConfigModule;
@@ -366,6 +367,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
modules.add(createIndexModule());
modules.add(new WorkQueue.Module());
modules.add(new GerritInstanceNameModule());
modules.add(
new CanonicalWebUrlModule() {
@Override

View File

@@ -62,6 +62,7 @@ import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DownloadConfig;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.EventBroker;
@@ -433,6 +434,7 @@ public class Daemon extends SiteProgram {
modules.add(new RestCacheAdminModule());
modules.add(new GpgModule(config));
modules.add(new StartupChecks.Module());
modules.add(new GerritInstanceNameModule());
if (MoreObjects.firstNonNull(httpd, true)) {
modules.add(
new CanonicalWebUrlModule() {

View File

@@ -0,0 +1,30 @@
// 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 static java.lang.annotation.RetentionPolicy.RUNTIME;
import com.google.inject.BindingAnnotation;
import java.lang.annotation.Retention;
/**
* Marker on a {@link String} holding the instance name for this server.
*
* <p>Note that the String may be null, if the administrator has not configured the value. Clients
* must handle such cases explicitly.
*/
@Retention(RUNTIME)
@BindingAnnotation
public @interface GerritInstanceName {}

View File

@@ -0,0 +1,27 @@
// 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.inject.AbstractModule;
/** Supports binding the {@link GerritInstanceName} annotation. */
public class GerritInstanceNameModule extends AbstractModule {
@Override
protected void configure() {
bind(String.class)
.annotatedWith(GerritInstanceName.class)
.toProvider(GerritInstanceNameProvider.class);
}
}

View File

@@ -0,0 +1,48 @@
// 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.gerrit.common.Nullable;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
/** Provides {@link GerritInstanceName} from {@code gerrit.name}. */
@Singleton
public class GerritInstanceNameProvider implements Provider<String> {
private final String instanceName;
@Inject
public GerritInstanceNameProvider(
@GerritServerConfig Config config,
@CanonicalWebUrl @Nullable Provider<String> canonicalUrlProvider) {
this.instanceName = getInstanceName(config, canonicalUrlProvider);
}
private String getInstanceName(Config config, @Nullable Provider<String> canonicalUrlProvider) {
String instanceName = config.getString("gerrit", null, "instanceName");
if (instanceName != null || canonicalUrlProvider == null) {
return instanceName;
}
return canonicalUrlProvider.get();
}
@Override
public String get() {
return instanceName;
}
}

View File

@@ -27,6 +27,8 @@ 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.git.GitRepositoryManager;
import com.google.gerrit.server.mail.EmailSettings;
@@ -44,6 +46,7 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.template.soy.tofu.SoyTofu;
import java.util.List;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
public class EmailArguments {
@@ -75,6 +78,8 @@ public class EmailArguments {
final DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners;
final Provider<InternalAccountQuery> accountQueryProvider;
final OutgoingEmailValidator validator;
final boolean addInstanceNameInSubject;
final Provider<String> instanceNameProvider;
@Inject
EmailArguments(
@@ -104,7 +109,9 @@ public class EmailArguments {
SitePaths site,
DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners,
Provider<InternalAccountQuery> accountQueryProvider,
OutgoingEmailValidator validator) {
OutgoingEmailValidator validator,
@GerritInstanceName Provider<String> instanceNameProvider,
@GerritServerConfig Config cfg) {
this.server = server;
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
@@ -132,5 +139,8 @@ public class EmailArguments {
this.outgoingEmailValidationListeners = outgoingEmailValidationListeners;
this.accountQueryProvider = accountQueryProvider;
this.validator = validator;
this.instanceNameProvider = instanceNameProvider;
this.addInstanceNameInSubject = cfg.getBoolean("sendemail", "addInstanceNameInSubject", false);
}
}

View File

@@ -109,6 +109,11 @@ public abstract class NotificationEmail extends OutgoingEmail {
soyContext.put("projectName", projectName);
// shortProjectName is the project name with the path abbreviated.
soyContext.put("shortProjectName", projectName.replaceAll("/.*/", "..."));
// instanceAndProjectName is the instance's name followed by the abbreviated project path
soyContext.put(
"instanceAndProjectName",
getInstanceAndProjectName(args.instanceNameProvider.get(), projectName));
soyContext.put("addInstanceNameInSubject", args.addInstanceNameInSubject);
soyContextEmailData.put("sshHost", getSshHost());
@@ -119,4 +124,14 @@ public abstract class NotificationEmail extends OutgoingEmail {
footers.add(MailHeader.PROJECT.withDelimiter() + branch.getParentKey().get());
footers.add("Gerrit-Branch: " + branch.getShortName());
}
protected static String getInstanceAndProjectName(String instanceName, String projectName) {
if (instanceName == null || instanceName.isEmpty()) {
return projectName.replaceAll("/.*/", "...");
}
// Extract the project name (everything after the last slash) and prepends it with gerrit's
// instance name
return instanceName + "/" + projectName.substring(projectName.lastIndexOf("/") + 1);
}
}

View File

@@ -60,7 +60,7 @@ public abstract class OutgoingEmail {
private static final Logger log = LoggerFactory.getLogger(OutgoingEmail.class);
protected String messageClass;
private final HashSet<Account.Id> rcptTo = new HashSet<>();
private final Set<Account.Id> rcptTo = new HashSet<>();
private final Map<String, EmailHeader> headers;
private final Set<Address> smtpRcptTo = new HashSet<>();
private Address smtpFromAddress;
@@ -541,11 +541,16 @@ public abstract class OutgoingEmail {
soyContextEmailData = new HashMap<>();
soyContextEmailData.put("settingsUrl", getSettingsUrl());
soyContextEmailData.put("instanceName", getInstanceName());
soyContextEmailData.put("gerritHost", getGerritHost());
soyContextEmailData.put("gerritUrl", getGerritUrl());
soyContext.put("email", soyContextEmailData);
}
private String getInstanceName() {
return args.instanceNameProvider.get();
}
private String soyTemplate(String name, SanitizedContent.ContentKind kind) {
return args.soyTofu
.newRenderer("com.google.gerrit.server.mail.template." + name)

View File

@@ -42,6 +42,7 @@ import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
@@ -203,6 +204,7 @@ public class InMemoryModule extends FactoryModule {
bind(Key.get(schemaFactory, ReviewDbFactory.class)).to(InMemoryDatabase.class);
install(NoSshKeyCache.module());
install(new GerritInstanceNameModule());
install(
new CanonicalWebUrlModule() {
@Override

View File

@@ -0,0 +1,34 @@
// 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.mail.send;
import static com.google.common.truth.Truth.assertThat;
import org.junit.Test;
public class NotificationEmailTest {
@Test
public void getInstanceAndProjectName_returnsTheRightValue() {
String instanceAndProjectName = NotificationEmail.getInstanceAndProjectName("test", "/my/api");
assertThat(instanceAndProjectName).isEqualTo("test/api");
}
@Test
public void getInstanceAndProjectName_handlesNull() {
String instanceAndProjectName = NotificationEmail.getInstanceAndProjectName(null, "/my/api");
assertThat(instanceAndProjectName).isEqualTo("...api");
}
}

View File

@@ -22,7 +22,13 @@
* @param branch
* @param change
* @param shortProjectName
* @param instanceAndProjectName
* @param addInstanceNameInSubject boolean
*/
{template .ChangeSubject kind="text"}
Change in {$shortProjectName}[{$branch.shortName}]: {$change.shortSubject}
{if $addInstanceNameInSubject}
Change in {$shortProjectName}[{$branch.shortName}]: {$change.shortSubject}
{else}
Change in {$instanceAndProjectName}[{$branch.shortName}]: {$change.shortSubject}
{/if}
{/template}