opendev/gerrit
Code Issues Proposed changes

Merge "Remove support for Velocity templates"

Browse Source
This commit is contained in:
Dave Borowitz
2017-10-18 00:52:06 +00:00
committed by Gerrit Code Review
parent 50a732db6a cb6597828b
commit 18baedb1eb
13 changed files with 18 additions and 376 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/config-mail.txt
View File

@@ -7,8 +7,9 @@ These defaults are also provided as examples so that administrators may copy
them and easily modify them to tweak their contents.
*Compatibility Note:* previously, Velocity Template Language (VTL) was used as
the template language for Gerrit emails. VTL has now been deprecated in favor of
Soy, but Velocity templates that modify text emails remain supported for now.
the template language for Gerrit emails. Support for VTL has now been removed
in favor of Soy, and Velocity templates that modify text emails are no longer
supported.
== Template Locations and Extensions:

6
WORKSPACE
View File

@@ -204,12 +204,6 @@ maven_jar(
sha1 = GUAVA_BIN_SHA1,
)
maven_jar(
name = "velocity",
artifact = "org.apache.velocity:velocity:1.7",
sha1 = "2ceb567b8f3f21118ecdec129fe1271dbc09aa7a",
)
maven_jar(
name = "jsch",
artifact = "com.jcraft:jsch:0.1.54",

1
gerrit-plugin-api/BUILD
View File

@@ -48,7 +48,6 @@ EXPORTS = [
"//lib:protobuf",
"//lib:servlet-api-3_1-without-neverlink",
"//lib:soy",
"//lib:velocity",
]
java_binary(

2
gerrit-server/BUILD
View File

@@ -76,7 +76,6 @@ java_library(
"//lib:servlet-api-3_1",
"//lib:soy",
"//lib:tukaani-xz",
"//lib:velocity",
"//lib/auto:auto-value",
"//lib/bouncycastle:bcpkix-neverlink",
"//lib/bouncycastle:bcprov-neverlink",
@@ -121,7 +120,6 @@ java_library(
"//lib:blame-cache",
"//lib:guava",
"//lib:soy",
"//lib:velocity",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
],

3
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -152,7 +152,6 @@ import com.google.gerrit.server.mail.send.MergedSender;
import com.google.gerrit.server.mail.send.RegisterNewEmailSender;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.mail.send.SetAssigneeSender;
import com.google.gerrit.server.mail.send.VelocityRuntimeProvider;
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry;
import com.google.gerrit.server.notedb.NoteDbModule;
@@ -189,7 +188,6 @@ import com.google.inject.TypeLiteral;
import com.google.inject.internal.UniqueAnnotations;
import com.google.template.soy.tofu.SoyTofu;
import java.util.List;
import org.apache.velocity.runtime.RuntimeInstance;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.transport.PostReceiveHook;
import org.eclipse.jgit.transport.PostUploadHook;
@@ -284,7 +282,6 @@ public class GerritGlobalModule extends FactoryModule {
bind(ApprovalsUtil.class);
bind(RuntimeInstance.class).toProvider(VelocityRuntimeProvider.class);
bind(SoyTofu.class).annotatedWith(MailTemplates.class).toProvider(MailSoyTofuProvider.class);
bind(FromAddressGenerator.class).toProvider(FromAddressGeneratorProvider.class).in(SINGLETON);
bind(Boolean.class)

29
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
View File

@@ -23,7 +23,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
@@ -116,11 +115,6 @@ public abstract class ChangeEmail extends NotificationEmail {
patchSetInfo = psi;
}
@Deprecated
public void setChangeMessage(ChangeMessage cm) {
setChangeMessage(cm.getMessage(), cm.getWrittenOn());
}
public void setChangeMessage(String cm, Timestamp t) {
changeMessage = cm;
timestamp = t;
@@ -222,7 +216,7 @@ public abstract class ChangeEmail extends NotificationEmail {
}
}
private void setChangeSubjectHeader() throws EmailException {
private void setChangeSubjectHeader() {
setHeader("Subject", textTemplate("ChangeSubject"));
}
@@ -237,8 +231,14 @@ public abstract class ChangeEmail extends NotificationEmail {
return null;
}
public String getChangeMessageThreadId() throws EmailException {
return velocify("<gerrit.${change.createdOn.time}.$change.key.get()@$email.gerritHost>");
public String getChangeMessageThreadId() {
return "<gerrit."
+ change.getCreatedOn().getTime()
+ "."
+ change.getKey().get()
+ "@"
+ this.getGerritHost()
+ ">";
}
/** Format the sender's "cover letter", {@link #getCoverLetter()}. */
@@ -445,17 +445,6 @@ public abstract class ChangeEmail extends NotificationEmail {
return authors;
}
@Override
protected void setupVelocityContext() {
super.setupVelocityContext();
velocityContext.put("change", change);
velocityContext.put("changeId", change.getKey());
velocityContext.put("coverLetter", getCoverLetter());
velocityContext.put("fromName", getNameFor(fromId));
velocityContext.put("patchSet", patchSet);
velocityContext.put("patchSetInfo", patchSetInfo);
}
@Override
protected void setupSoyContext() {
super.setupSoyContext();

115
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
View File

@@ -190,38 +190,6 @@ public class CommentSender extends ReplyToChangeSender {
}
}
/** No longer used outside Velocity. Remove this method when VTL support is removed. */
@Deprecated
public boolean hasInlineComments() {
return !inlineComments.isEmpty();
}
/** No longer used outside Velocity. Remove this method when VTL support is removed. */
@Deprecated
public String getInlineComments() {
return getInlineComments(1);
}
/** No longer used outside Velocity. Remove this method when VTL support is removed. */
@Deprecated
public String getInlineComments(int lines) {
try (Repository repo = getRepository()) {
StringBuilder cmts = new StringBuilder();
for (FileCommentGroup group : getGroupedInlineComments(repo)) {
String link = group.getLink();
if (link != null) {
cmts.append(link).append('\n');
}
cmts.append(group.getTitle()).append(":\n\n");
for (Comment c : group.comments) {
appendComment(cmts, lines, group.fileData, c);
}
cmts.append("\n\n");
}
return cmts.toString();
}
}
/**
* @return a list of FileCommentGroup objects representing the inline comments grouped by the
* file.
@@ -275,39 +243,6 @@ public class CommentSender extends ReplyToChangeSender {
return groups;
}
/** No longer used except for Velocity. Remove this method when VTL support is removed. */
@Deprecated
private void appendComment(
StringBuilder out, int contextLines, PatchFile currentFileData, Comment comment) {
if (comment instanceof RobotComment) {
RobotComment robotComment = (RobotComment) comment;
out.append("Robot Comment from ")
.append(robotComment.robotId)
.append(" (run ID ")
.append(robotComment.robotRunId)
.append("):\n");
}
if (comment.range != null) {
appendRangedComment(out, currentFileData, comment);
} else {
appendLineComment(out, contextLines, currentFileData, comment);
}
}
/** No longer used except for Velocity. Remove this method when VTL support is removed. */
@Deprecated
private void appendRangedComment(StringBuilder out, PatchFile fileData, Comment comment) {
String prefix = getCommentLinePrefix(comment);
String emptyPrefix = Strings.padStart(": ", prefix.length(), ' ');
boolean firstLine = true;
for (String line : getLinesByRange(comment.range, fileData, comment.side)) {
out.append(firstLine ? prefix : emptyPrefix).append(line).append('\n');
firstLine = false;
}
appendQuotedParent(out, comment);
out.append(comment.message.trim()).append('\n');
}
private String getCommentLinePrefix(Comment comment) {
int lineNbr = comment.range == null ? comment.lineNbr : comment.range.startLine;
StringBuilder sb = new StringBuilder();
@@ -339,56 +274,6 @@ public class CommentSender extends ReplyToChangeSender {
return lines;
}
/** No longer used except for Velocity. Remove this method when VTL support is removed. */
@Deprecated
private void appendLineComment(
StringBuilder out, int contextLines, PatchFile currentFileData, Comment comment) {
short side = comment.side;
int lineNbr = comment.lineNbr;
// Initialize maxLines to the known line number.
int maxLines = lineNbr;
try {
maxLines = currentFileData.getLineCount(side);
} catch (IOException err) {
// The file could not be read, leave the max as is.
log.warn(String.format("Failed to read file %s on side %d", comment.key.filename, side), err);
} catch (NoSuchEntityException err) {
// The file could not be read, leave the max as is.
log.warn(String.format("Side %d of file %s didn't exist", side, comment.key.filename), err);
}
int startLine = Math.max(1, lineNbr - contextLines + 1);
int stopLine = Math.min(maxLines, lineNbr + contextLines);
for (int line = startLine; line <= lineNbr; ++line) {
appendFileLine(out, currentFileData, side, line);
}
appendQuotedParent(out, comment);
out.append(comment.message.trim()).append('\n');
for (int line = lineNbr + 1; line < stopLine; ++line) {
appendFileLine(out, currentFileData, side, line);
}
}
/** No longer used except for Velocity. Remove this method when VTL support is removed. */
@Deprecated
private void appendFileLine(StringBuilder cmts, PatchFile fileData, short side, int line) {
String lineStr = getLine(fileData, side, line);
cmts.append("Line ").append(line).append(": ").append(lineStr).append("\n");
}
/** No longer used except for Velocity. Remove this method when VTL support is removed. */
@Deprecated
private void appendQuotedParent(StringBuilder out, Comment child) {
Optional<Comment> parent = getParent(child);
if (parent.isPresent()) {
out.append("> ").append(getShortenedCommentMessage(parent.get())).append('\n');
}
}
/**
* Get the parent comment of a given comment.
*

4
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/EmailArguments.java
View File

@@ -46,7 +46,6 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.template.soy.tofu.SoyTofu;
import java.util.List;
import org.apache.velocity.runtime.RuntimeInstance;
import org.eclipse.jgit.lib.PersonIdent;
public class EmailArguments {
@@ -75,7 +74,6 @@ public class EmailArguments {
final ChangeQueryBuilder queryBuilder;
final Provider<ReviewDb> db;
final ChangeData.Factory changeDataFactory;
final RuntimeInstance velocityRuntime;
final SoyTofu soyTofu;
final EmailSettings settings;
final DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners;
@@ -106,7 +104,6 @@ public class EmailArguments {
ChangeQueryBuilder queryBuilder,
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RuntimeInstance velocityRuntime,
@MailTemplates SoyTofu soyTofu,
EmailSettings settings,
@SshAdvertisedAddresses List<String> sshAddresses,
@@ -136,7 +133,6 @@ public class EmailArguments {
this.queryBuilder = queryBuilder;
this.db = db;
this.changeDataFactory = changeDataFactory;
this.velocityRuntime = velocityRuntime;
this.soyTofu = soyTofu;
this.settings = settings;
this.sshAddresses = sshAddresses;

19
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java
View File

@@ -45,18 +45,16 @@ public abstract class NotificationEmail extends OutgoingEmail {
setListIdHeader();
}
private void setListIdHeader() throws EmailException {
private void setListIdHeader() {
// Set a reasonable list id so that filters can be used to sort messages
setVHeader("List-Id", "<$email.listId.replace('@', '.')>");
setHeader(
"List-Id",
"<gerrit-" + branch.getParentKey().get().replace('/', '-') + "." + getGerritHost() + ">");
if (getSettingsUrl() != null) {
setVHeader("List-Unsubscribe", "<$email.settingsUrl>");
setHeader("List-Unsubscribe", "<" + getSettingsUrl() + ">");
}
}
public String getListId() throws EmailException {
return velocify("gerrit-$projectName.replace('/', '-')@$email.gerritHost");
}
/** Include users and groups that want notification of events. */
protected void includeWatchers(NotifyType type) {
includeWatchers(type, true);
@@ -102,13 +100,6 @@ public abstract class NotificationEmail extends OutgoingEmail {
return host;
}
@Override
protected void setupVelocityContext() {
super.setupVelocityContext();
velocityContext.put("projectName", branch.getParentKey().get());
velocityContext.put("branch", branch);
}
@Override
protected void setupSoyContext() {
super.setupSoyContext();

76
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.mail.send;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
@@ -36,12 +35,8 @@ import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
import com.google.template.soy.data.SanitizedContent;
import java.io.StringReader;
import java.io.StringWriter;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
@@ -53,12 +48,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
import org.apache.commons.lang.StringUtils;
import org.apache.velocity.Template;
import org.apache.velocity.VelocityContext;
import org.apache.velocity.context.InternalContextAdapterImpl;
import org.apache.velocity.runtime.RuntimeInstance;
import org.apache.velocity.runtime.parser.node.SimpleNode;
import org.eclipse.jgit.util.SystemReader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -78,7 +67,6 @@ public abstract class OutgoingEmail {
private StringBuilder textBody;
private StringBuilder htmlBody;
private ListMultimap<RecipientType, Account.Id> accountsToNotify = ImmutableListMultimap.of();
protected VelocityContext velocityContext;
protected Map<String, Object> soyContext;
protected Map<String, Object> soyContextEmailData;
protected List<String> footers;
@@ -237,7 +225,6 @@ public abstract class OutgoingEmail {
* @throws EmailException if an error occurred.
*/
protected void init() throws EmailException {
setupVelocityContext();
setupSoyContext();
smtpFromAddress = args.fromAddressGenerator.from(fromId);
@@ -309,11 +296,6 @@ public abstract class OutgoingEmail {
return args.urlProvider.get();
}
/** Set a header in the outgoing message using a template. */
protected void setVHeader(String name, String value) throws EmailException {
setHeader(name, velocify(value));
}
/** Set a header in the outgoing message. */
protected void setHeader(String name, String value) {
headers.put(name, new EmailHeader.String(value));
@@ -537,14 +519,6 @@ public abstract class OutgoingEmail {
return new Address(a.getFullName(), e);
}
protected void setupVelocityContext() {
velocityContext = new VelocityContext();
velocityContext.put("email", this);
velocityContext.put("messageClass", messageClass);
velocityContext.put("StringUtils", StringUtils.class);
}
protected void setupSoyContext() {
soyContext = new HashMap<>();
footers = new ArrayList<>();
@@ -559,41 +533,6 @@ public abstract class OutgoingEmail {
soyContext.put("email", soyContextEmailData);
}
protected String velocify(String template) throws EmailException {
try {
RuntimeInstance runtime = args.velocityRuntime;
String templateName = "OutgoingEmail";
SimpleNode tree = runtime.parse(new StringReader(template), templateName);
InternalContextAdapterImpl ica = new InternalContextAdapterImpl(velocityContext);
ica.pushCurrentTemplateName(templateName);
try {
tree.init(ica, runtime);
StringWriter w = new StringWriter();
tree.render(ica, w);
return w.toString();
} finally {
ica.popCurrentTemplateName();
}
} catch (Exception e) {
throw new EmailException("Cannot format velocity template: " + template, e);
}
}
protected String velocifyFile(String name) throws EmailException {
try {
RuntimeInstance runtime = args.velocityRuntime;
if (runtime.getLoaderNameForResource(name) == null) {
name = "com/google/gerrit/server/mail/" + name;
}
Template template = runtime.getTemplate(name, UTF_8.name());
StringWriter w = new StringWriter();
template.merge(velocityContext, w);
return w.toString();
} catch (Exception e) {
throw new EmailException("Cannot format velocity template " + name, e);
}
}
private String soyTemplate(String name, SanitizedContent.ContentKind kind) {
return args.soyTofu
.newRenderer("com.google.gerrit.server.mail.template." + name)
@@ -602,7 +541,7 @@ public abstract class OutgoingEmail {
.render();
}
protected String soyTextTemplate(String name) {
protected String textTemplate(String name) {
return soyTemplate(name, SanitizedContent.ContentKind.TEXT);
}
@@ -610,19 +549,6 @@ public abstract class OutgoingEmail {
return soyTemplate(name, SanitizedContent.ContentKind.HTML);
}
/**
* Evaluate the named template according to the following priority: 1) Velocity file override,
* OR... 2) Soy file override, OR... 3) Soy resource.
*/
protected String textTemplate(String name) throws EmailException {
String velocityName = name + ".vm";
Path filePath = args.site.mail_dir.resolve(velocityName);
if (Files.isRegularFile(filePath)) {
return velocifyFile(velocityName);
}
return soyTextTemplate(name);
}
public String joinStrings(Iterable<Object> in, String joiner) {
return joinStrings(in.iterator(), joiner);
}

118
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/VelocityRuntimeProvider.java
View File

@@ -1,118 +0,0 @@
// Copyright (C) 2011 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 com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import java.nio.file.Files;
import java.util.Properties;
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.RuntimeInstance;
import org.apache.velocity.runtime.RuntimeServices;
import org.apache.velocity.runtime.log.LogChute;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Configures Velocity template engine for sending email. */
@Singleton
public class VelocityRuntimeProvider implements Provider<RuntimeInstance> {
private final SitePaths site;
@Inject
VelocityRuntimeProvider(SitePaths site) {
this.site = site;
}
@Override
public RuntimeInstance get() {
String rl = "resource.loader";
String pkg = "org.apache.velocity.runtime.resource.loader";
Properties p = new Properties();
p.setProperty(RuntimeConstants.VM_PERM_INLINE_LOCAL, "true");
p.setProperty(RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS, Slf4jLogChute.class.getName());
p.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, "true");
p.setProperty("runtime.log.logsystem.log4j.category", "velocity");
if (Files.isDirectory(site.mail_dir)) {
p.setProperty(rl, "file, class");
p.setProperty("file." + rl + ".class", pkg + ".FileResourceLoader");
p.setProperty("file." + rl + ".path", site.mail_dir.toAbsolutePath().toString());
p.setProperty("class." + rl + ".class", pkg + ".ClasspathResourceLoader");
} else {
p.setProperty(rl, "class");
p.setProperty("class." + rl + ".class", pkg + ".ClasspathResourceLoader");
}
RuntimeInstance ri = new RuntimeInstance();
try {
ri.init(p);
} catch (Exception err) {
throw new ProvisionException("Cannot configure Velocity templates", err);
}
return ri;
}
/** Connects Velocity to sfl4j. */
public static class Slf4jLogChute implements LogChute {
// Logger should be named 'velocity' for consistency with log4j config
private static final Logger log = LoggerFactory.getLogger("velocity");
@Override
public void init(RuntimeServices rs) {}
@Override
public boolean isLevelEnabled(int level) {
switch (level) {
default:
case DEBUG_ID:
return log.isDebugEnabled();
case INFO_ID:
return log.isInfoEnabled();
case WARN_ID:
return log.isWarnEnabled();
case ERROR_ID:
return log.isErrorEnabled();
}
}
@Override
public void log(int level, String message) {
log(level, message, null);
}
@Override
public void log(int level, String msg, Throwable err) {
switch (level) {
default:
case DEBUG_ID:
log.debug(msg, err);
break;
case INFO_ID:
log.info(msg, err);
break;
case WARN_ID:
log.warn(msg, err);
break;
case ERROR_ID:
log.error(msg, err);
break;
}
}
}
}

4
gerrit-war/src/main/resources/log4j.properties
View File

@@ -46,9 +46,5 @@ log4j.logger.com.mchange.v2.c3p0=WARN
log4j.logger.com.mchange.v2.resourcepool=WARN
log4j.logger.com.mchange.v2.sql=WARN
# Silence non-critical messages from Velocity
#
log4j.logger.velocity=WARN
# Silence non-critical messages from apache.http
log4j.logger.org.apache.http=WARN

12
lib/BUILD
View File

@@ -82,18 +82,6 @@ java_library(
exports = ["@guava//jar"],
)
java_library(
name = "velocity",
data = ["//lib:LICENSE-Apache2.0"],
visibility = ["//visibility:public"],
exports = ["@velocity//jar"],
runtime_deps = [
"//lib/commons:collections",
"//lib/commons:lang",
"//lib/commons:oro",
],
)
java_library(
name = "jsch",
data = ["//lib:LICENSE-jsch"],