Merge "Migrate from SoyTofu to SoySauce"
diff --git a/.bazelversion b/.bazelversion
index cffeade..1b58cc1 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-0.27.0rc5
\ No newline at end of file
+0.27.0
diff --git a/Documentation/dev-crafting-changes.txt b/Documentation/dev-crafting-changes.txt
index 3440e85..a22bea9 100644
--- a/Documentation/dev-crafting-changes.txt
+++ b/Documentation/dev-crafting-changes.txt
@@ -110,7 +110,7 @@
link:https://github.com/google/google-java-format[`google-java-format`]
tool (version 1.7), and to format Bazel BUILD, WORKSPACE and .bzl files the
link:https://github.com/bazelbuild/buildtools/tree/master/buildifier[`buildifier`]
-tool (version 0.22.0).
+tool (version 0.26.0).
These tools automatically apply format according to the style guides; this
streamlines code review by reducing the need for time-consuming, tedious,
and contentious discussions about trivial issues like whitespace.
diff --git a/WORKSPACE b/WORKSPACE
index f160f64..855309f 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -66,7 +66,7 @@
load("@bazel_skylib//lib:versions.bzl", "versions")
-versions.check(minimum_bazel_version = "0.25.0")
+versions.check(minimum_bazel_version = "0.26.1")
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_repositories")
@@ -83,8 +83,11 @@
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "6776d68ebb897625dead17ae510eac3d5f6342367327875210df44dbe2aeeb19",
- url = "https://github.com/bazelbuild/rules_go/releases/download/0.17.1/rules_go-0.17.1.tar.gz",
+ sha256 = "f04d2373bcaf8aa09bccb08a98a57e721306c8f6043a2a0ee610fd6853dcde3d",
+ urls = [
+ "https://storage.googleapis.com/bazel-mirror/github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
+ "https://github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
+ ],
)
load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
@@ -772,24 +775,24 @@
)
# When updating Bouncy Castle, also update it in bazlets.
-BC_VERS = "1.60"
+BC_VERS = "1.61"
maven_jar(
name = "bcprov",
artifact = "org.bouncycastle:bcprov-jdk15on:" + BC_VERS,
- sha1 = "bd47ad3bd14b8e82595c7adaa143501e60842a84",
+ sha1 = "00df4b474e71be02c1349c3292d98886f888d1f7",
)
maven_jar(
name = "bcpg",
artifact = "org.bouncycastle:bcpg-jdk15on:" + BC_VERS,
- sha1 = "13c7a199c484127daad298996e95818478431a2c",
+ sha1 = "422656435514ab8a28752b117d5d2646660a0ace",
)
maven_jar(
name = "bcpkix",
artifact = "org.bouncycastle:bcpkix-jdk15on:" + BC_VERS,
- sha1 = "d0c46320fbc07be3a24eb13a56cee4e3d38e0c75",
+ sha1 = "89bb3aa5b98b48e584eee2a7401b7682a46779b4",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java b/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java
index 286b045..e04a643 100644
--- a/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java
+++ b/java/com/google/gerrit/acceptance/ChangeIndexedCounter.java
@@ -37,17 +37,12 @@
countsByChange.clear();
}
- long getCount(ChangeInfo info) {
- return countsByChange.get(info._number);
- }
-
public void assertReindexOf(ChangeInfo info) {
assertReindexOf(info, 1);
}
- public void assertReindexOf(ChangeInfo info, int expectedCount) {
- assertThat(getCount(info)).isEqualTo(expectedCount);
- assertThat(countsByChange).hasSize(1);
+ public void assertReindexOf(ChangeInfo info, long expectedCount) {
+ assertThat(countsByChange.asMap()).containsExactly(info._number, expectedCount);
clear();
}
}
diff --git a/java/com/google/gerrit/extensions/validators/CommentForValidation.java b/java/com/google/gerrit/extensions/validators/CommentForValidation.java
new file mode 100644
index 0000000..51ae5ae
--- /dev/null
+++ b/java/com/google/gerrit/extensions/validators/CommentForValidation.java
@@ -0,0 +1,48 @@
+// Copyright (C) 2019 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.extensions.validators;
+
+import com.google.auto.value.AutoValue;
+
+/**
+ * Holds a comment's text and {@link CommentType} in order to pass it to a validation plugin.
+ *
+ * @see CommentValidator
+ */
+@AutoValue
+public abstract class CommentForValidation {
+
+ /** The type of comment. */
+ public enum CommentType {
+ /** A regular (inline) comment. */
+ INLINE_COMMENT,
+ /** A file comment. */
+ FILE_COMMENT,
+ /** A change message. */
+ CHANGE_MESSAGE
+ }
+
+ public static CommentForValidation create(CommentType type, String text) {
+ return new AutoValue_CommentForValidation(type, text);
+ }
+
+ public abstract CommentType getType();
+
+ public abstract String getText();
+
+ public CommentValidationFailure failValidation(String message) {
+ return CommentValidationFailure.create(this, message);
+ }
+}
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidationFailure.java b/java/com/google/gerrit/extensions/validators/CommentValidationFailure.java
new file mode 100644
index 0000000..1a832760
--- /dev/null
+++ b/java/com/google/gerrit/extensions/validators/CommentValidationFailure.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2019 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.extensions.validators;
+
+import com.google.auto.value.AutoValue;
+
+/** A comment or review message was rejected by a {@link CommentValidator}. */
+@AutoValue
+public abstract class CommentValidationFailure {
+ static CommentValidationFailure create(
+ CommentForValidation commentForValidation, String message) {
+ return new AutoValue_CommentValidationFailure(commentForValidation, message);
+ }
+
+ /** Returns the offending comment. */
+ public abstract CommentForValidation getComment();
+
+ /** A friendly message set by the {@link CommentValidator}. */
+ public abstract String getMessage();
+}
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidator.java b/java/com/google/gerrit/extensions/validators/CommentValidator.java
new file mode 100644
index 0000000..cfefdef
--- /dev/null
+++ b/java/com/google/gerrit/extensions/validators/CommentValidator.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 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.extensions.validators;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+/**
+ * Validates review comments and messages. Rejecting any comment/message will prevent all comments
+ * from being published.
+ */
+@ExtensionPoint
+public interface CommentValidator {
+
+ /**
+ * Validate the specified comments.
+ *
+ * @return An empty list if all comments are valid, or else a list of validation failures.
+ */
+ ImmutableList<CommentValidationFailure> validateComments(
+ ImmutableList<CommentForValidation> comments);
+}
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index 8ae5bcd..6a1c859 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -18,13 +18,18 @@
import static com.google.gerrit.reviewdb.client.PatchLineComment.Status.PUBLISHED;
import static java.util.stream.Collectors.toSet;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.update.ChangeContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -77,4 +82,20 @@
private static PatchSet.Id psId(ChangeNotes notes, Comment c) {
return PatchSet.id(notes.getChangeId(), c.key.patchSetId);
}
+
+ /**
+ * Helper to run the specified set of {@link CommentValidator}-s on the specified comments.
+ *
+ * @return See {@link CommentValidator#validateComments(ImmutableList)}.
+ */
+ public static ImmutableList<CommentValidationFailure> findInvalidComments(
+ PluginSetContext<CommentValidator> commentValidators,
+ ImmutableList<CommentForValidation> commentsForValidation) {
+ ImmutableList.Builder<CommentValidationFailure> commentValidationFailures =
+ new ImmutableList.Builder<>();
+ commentValidators.runEach(
+ listener ->
+ commentValidationFailures.addAll(listener.validateComments(commentsForValidation)));
+ return commentValidationFailures.build();
+ }
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index eaa0cd8..afe4e52 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -62,6 +62,7 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.systemstatus.MessageOfTheDay;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.extensions.webui.BranchWebLink;
import com.google.gerrit.extensions.webui.DiffWebLink;
import com.google.gerrit.extensions.webui.FileHistoryWebLink;
@@ -341,6 +342,7 @@
DynamicSet.bind(binder(), EventListener.class).to(EventsMetrics.class);
DynamicSet.setOf(binder(), UserScopedEventListener.class);
DynamicSet.setOf(binder(), CommitValidationListener.class);
+ DynamicSet.setOf(binder(), CommentValidator.class);
DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
DynamicSet.setOf(binder(), OnSubmitValidationListener.class);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index f69e364..77d9049 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -17,9 +17,9 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.flogger.LazyArgs.lazy;
-import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
import static com.google.gerrit.reviewdb.client.RefNames.isConfigRef;
@@ -81,19 +81,26 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.BranchNameKey;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CreateGroupPermissionSyncer;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.NotifyResolver;
@@ -301,6 +308,8 @@
private final ChangeNotes.Factory notesFactory;
private final ChangeReportFormatter changeFormatter;
private final CmdLineParser.Factory optionParserFactory;
+ private final CommentsUtil commentsUtil;
+ private final PluginSetContext<CommentValidator> commentValidators;
private final BranchCommitValidator.Factory commitValidatorFactory;
private final CreateGroupPermissionSyncer createGroupPermissionSyncer;
private final CreateRefControl createRefControl;
@@ -378,11 +387,13 @@
ChangeNotes.Factory notesFactory,
DynamicItem<ChangeReportFormatter> changeFormatterProvider,
CmdLineParser.Factory optionParserFactory,
+ CommentsUtil commentsUtil,
BranchCommitValidator.Factory commitValidatorFactory,
CreateGroupPermissionSyncer createGroupPermissionSyncer,
CreateRefControl createRefControl,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
PluginSetContext<ReceivePackInitializer> initializers,
+ PluginSetContext<CommentValidator> commentValidators,
MergedByPushOp.Factory mergedByPushOpFactory,
PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil,
@@ -415,6 +426,8 @@
this.batchUpdateFactory = batchUpdateFactory;
this.changeFormatter = changeFormatterProvider.get();
this.changeInserterFactory = changeInserterFactory;
+ this.commentsUtil = commentsUtil;
+ this.commentValidators = commentValidators;
this.commitValidatorFactory = commitValidatorFactory;
this.createRefControl = createRefControl;
this.createGroupPermissionSyncer = createGroupPermissionSyncer;
@@ -1301,7 +1314,6 @@
Optional<AuthException> err = checkRefPermission(cmd, RefPermission.DELETE);
if (!err.isPresent()) {
validRefOperation(cmd);
-
} else {
rejectProhibited(cmd, err.get());
}
@@ -1377,6 +1389,13 @@
final ReceiveCommand cmd;
final LabelTypes labelTypes;
private final boolean defaultPublishComments;
+ /**
+ * Result of running {@link CommentValidator}-s on drafts that are published with the commit
+ * (which happens iff {@code --publish-comments} is set). Remains {@code true} if none are
+ * installed.
+ */
+ private boolean commentsValid = true;
+
BranchNameKey dest;
PermissionBackend.ForRef perm;
Set<String> reviewer = Sets.newLinkedHashSet();
@@ -1579,7 +1598,15 @@
.collect(toImmutableSet());
}
+ void setCommentsValid(boolean commentsValid) {
+ this.commentsValid = commentsValid;
+ }
+
boolean shouldPublishComments() {
+ if (!commentsValid) {
+ // Validation messages of type WARNING have already been added, now withhold the comments.
+ return false;
+ }
if (publishComments) {
return true;
} else if (noPublishComments) {
@@ -1976,7 +2003,7 @@
messages.addAll(validationResult.messages());
if (validationResult.isValid()) {
logger.atFine().log("Replacing change %s", changeEnt.getId());
- requestReplace(cmd, true, changeEnt, newCommit);
+ requestReplaceAndValidateComments(cmd, true, changeEnt, newCommit);
}
} catch (IOException e) {
reject(cmd, "I/O exception validating commit");
@@ -1984,10 +2011,12 @@
}
/**
- * Add an update for an existing change. Returns true if it succeeded; rejects the command if it
- * failed.
+ * Update an existing change. If draft comments are to be published, these are validated and may
+ * be withheld.
+ *
+ * @return True if the command succeeded, false if it was rejected.
*/
- private boolean requestReplace(
+ private boolean requestReplaceAndValidateComments(
ReceiveCommand cmd, boolean checkMergedInto, Change change, RevCommit newCommit) {
if (change.isClosed()) {
reject(
@@ -2002,6 +2031,30 @@
reject(cmd, "duplicate request");
return false;
}
+
+ if (magicBranch != null && magicBranch.shouldPublishComments()) {
+ List<Comment> drafts =
+ commentsUtil.draftByChangeAuthor(notesFactory.createChecked(change), user.getAccountId());
+ ImmutableList<CommentForValidation> draftsForValidation =
+ drafts.stream()
+ .map(
+ comment ->
+ CommentForValidation.create(
+ comment.lineNbr > 0
+ ? CommentType.INLINE_COMMENT
+ : CommentType.FILE_COMMENT,
+ comment.message))
+ .collect(toImmutableList());
+ ImmutableList<CommentValidationFailure> commentValidationFailures =
+ PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ magicBranch.setCommentsValid(commentValidationFailures.isEmpty());
+ commentValidationFailures.forEach(
+ failure ->
+ addMessage(
+ "Comment validation failure: " + failure.getMessage(),
+ ValidationMessage.Type.WARNING));
+ }
+
replaceByChange.put(req.ontoChange, req);
return true;
}
@@ -2101,7 +2154,7 @@
}
}
- List<String> idList = c.getFooterLines(CHANGE_ID);
+ List<String> idList = c.getFooterLines(FooterConstants.CHANGE_ID);
if (!idList.isEmpty()) {
pending.put(c, lookupByChangeKey(c, Change.key(idList.get(idList.size() - 1).trim())));
} else {
@@ -2218,7 +2271,8 @@
continue;
}
}
- if (requestReplace(magicBranch.cmd, false, changes.get(0).change(), p.commit)) {
+ if (requestReplaceAndValidateComments(
+ magicBranch.cmd, false, changes.get(0).change(), p.commit)) {
continue;
}
return Collections.emptyList();
@@ -3197,7 +3251,7 @@
}
}
- for (String changeId : c.getFooterLines(CHANGE_ID)) {
+ for (String changeId : c.getFooterLines(FooterConstants.CHANGE_ID)) {
if (byKey == null) {
byKey = executeIndexQuery(() -> openChangesByKeyByBranch(branch));
}
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 3655369..034bcc9 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.Side;
@@ -26,6 +27,9 @@
import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.HtmlParser;
import com.google.gerrit.mail.MailComment;
import com.google.gerrit.mail.MailHeaderParser;
@@ -43,6 +47,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
@@ -54,6 +59,7 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.BatchUpdate;
@@ -83,6 +89,15 @@
public class MailProcessor {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final ImmutableMap<MailComment.CommentType, CommentForValidation.CommentType>
+ MAIL_COMMENT_TYPE_TO_VALIDATION_TYPE =
+ ImmutableMap.of(
+ MailComment.CommentType.CHANGE_MESSAGE,
+ CommentForValidation.CommentType.CHANGE_MESSAGE,
+ MailComment.CommentType.FILE_COMMENT, CommentForValidation.CommentType.FILE_COMMENT,
+ MailComment.CommentType.INLINE_COMMENT,
+ CommentForValidation.CommentType.INLINE_COMMENT);
+
private final Emails emails;
private final InboundEmailRejectionSender.Factory emailRejectionSender;
private final RetryHelper retryHelper;
@@ -98,6 +113,7 @@
private final ApprovalsUtil approvalsUtil;
private final AccountCache accountCache;
private final DynamicItem<UrlFormatter> urlFormatter;
+ private final PluginSetContext<CommentValidator> commentValidators;
@Inject
public MailProcessor(
@@ -115,7 +131,8 @@
ApprovalsUtil approvalsUtil,
CommentAdded commentAdded,
AccountCache accountCache,
- DynamicItem<UrlFormatter> urlFormatter) {
+ DynamicItem<UrlFormatter> urlFormatter,
+ PluginSetContext<CommentValidator> commentValidators) {
this.emails = emails;
this.emailRejectionSender = emailRejectionSender;
this.retryHelper = retryHelper;
@@ -131,6 +148,7 @@
this.approvalsUtil = approvalsUtil;
this.accountCache = accountCache;
this.urlFormatter = urlFormatter;
+ this.commentValidators = commentValidators;
}
/**
@@ -259,6 +277,21 @@
return;
}
+ ImmutableList<CommentForValidation> parsedCommentsForValidation =
+ parsedComments.stream()
+ .map(
+ comment ->
+ CommentForValidation.create(
+ MAIL_COMMENT_TYPE_TO_VALIDATION_TYPE.get(comment.getType()),
+ comment.getMessage()))
+ .collect(ImmutableList.toImmutableList());
+ ImmutableList<CommentValidationFailure> commentValidationFailures =
+ PublishCommentUtil.findInvalidComments(commentValidators, parsedCommentsForValidation);
+ if (!commentValidationFailures.isEmpty()) {
+ sendRejectionEmail(message, InboundEmailRejectionSender.Error.COMMENT_REJECTED);
+ return;
+ }
+
Op o = new Op(PatchSet.id(cd.getId(), metadata.patchSet), parsedComments, message.id());
BatchUpdate batchUpdate = buf.create(project, ctx.getUser(), TimeUtil.nowTs());
batchUpdate.addOp(cd.getId(), o);
diff --git a/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java b/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
index b5d384d..f0b3c66 100644
--- a/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
+++ b/java/com/google/gerrit/server/mail/send/InboundEmailRejectionSender.java
@@ -32,7 +32,8 @@
PARSING_ERROR,
INACTIVE_ACCOUNT,
UNKNOWN_ACCOUNT,
- INTERNAL_EXCEPTION;
+ INTERNAL_EXCEPTION,
+ COMMENT_REJECTED
}
public interface Factory {
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index ef38b05..b8ea310 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -16,6 +16,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
@@ -29,9 +30,11 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
+import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing;
@@ -61,6 +64,10 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.Account;
@@ -106,12 +113,14 @@
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.update.CommentsRejectedException;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -137,6 +146,7 @@
import java.util.OptionalInt;
import java.util.Set;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -173,6 +183,7 @@
private final WorkInProgressOp.Factory workInProgressOpFactory;
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
+ private final PluginSetContext<CommentValidator> commentValidators;
private final boolean strictLabels;
@Inject
@@ -195,7 +206,8 @@
@GerritServerConfig Config gerritConfig,
WorkInProgressOp.Factory workInProgressOpFactory,
ProjectCache projectCache,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ PluginSetContext<CommentValidator> commentValidators) {
super(retryHelper);
this.changeResourceFactory = changeResourceFactory;
this.changeDataFactory = changeDataFactory;
@@ -215,6 +227,7 @@
this.workInProgressOpFactory = workInProgressOpFactory;
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
+ this.commentValidators = commentValidators;
this.strictLabels = gerritConfig.getBoolean("change", "strictLabels", false);
}
@@ -858,7 +871,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws ResourceConflictException, UnprocessableEntityException, IOException,
- PatchListNotAvailableException {
+ PatchListNotAvailableException, CommentsRejectedException {
user = ctx.getIdentifiedUser();
notes = ctx.getNotes();
ps = psUtil.get(ctx.getNotes(), psId);
@@ -891,7 +904,8 @@
}
private boolean insertComments(ChangeContext ctx)
- throws UnprocessableEntityException, PatchListNotAvailableException {
+ throws UnprocessableEntityException, PatchListNotAvailableException,
+ CommentsRejectedException {
Map<String, List<CommentInput>> inputComments = in.comments;
if (inputComments == null) {
inputComments = Collections.emptyMap();
@@ -910,6 +924,7 @@
}
}
+ // This will be populated with Comment-s created from inputComments.
List<Comment> toPublish = new ArrayList<>();
Set<CommentSetEntry> existingComments =
@@ -954,11 +969,13 @@
switch (in.drafts) {
case PUBLISH:
case PUBLISH_ALL_REVISIONS:
+ validateComments(Streams.concat(drafts.values().stream(), toPublish.stream()));
publishCommentUtil.publish(ctx, psId, drafts.values(), in.tag);
comments.addAll(drafts.values());
break;
case KEEP:
default:
+ validateComments(toPublish.stream());
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
@@ -967,6 +984,24 @@
return !toPublish.isEmpty();
}
+ private void validateComments(Stream<Comment> comments) throws CommentsRejectedException {
+ ImmutableList<CommentForValidation> draftsForValidation =
+ comments
+ .map(
+ comment ->
+ CommentForValidation.create(
+ comment.lineNbr > 0
+ ? CommentForValidation.CommentType.INLINE_COMMENT
+ : CommentType.FILE_COMMENT,
+ comment.message))
+ .collect(toImmutableList());
+ ImmutableList<CommentValidationFailure> draftValidationFailures =
+ PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ if (!draftValidationFailures.isEmpty()) {
+ throw new CommentsRejectedException(draftValidationFailures);
+ }
+ }
+
private boolean insertRobotComments(ChangeContext ctx) throws PatchListNotAvailableException {
if (in.robotComments == null) {
return false;
@@ -1337,7 +1372,7 @@
return current;
}
- private boolean insertMessage(ChangeContext ctx) {
+ private boolean insertMessage(ChangeContext ctx) throws CommentsRejectedException {
String msg = Strings.nullToEmpty(in.message).trim();
StringBuilder buf = new StringBuilder();
@@ -1350,6 +1385,15 @@
buf.append(String.format("\n\n(%d comments)", comments.size()));
}
if (!msg.isEmpty()) {
+ ImmutableList<CommentValidationFailure> messageValidationFailure =
+ PublishCommentUtil.findInvalidComments(
+ commentValidators,
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.CHANGE_MESSAGE, msg)));
+ if (!messageValidationFailure.isEmpty()) {
+ throw new CommentsRejectedException(messageValidationFailure);
+ }
buf.append("\n\n").append(msg);
} else if (in.ready) {
buf.append("\n\n" + START_REVIEW_MESSAGE);
diff --git a/java/com/google/gerrit/server/schema/MysqlAccountPatchReviewStore.java b/java/com/google/gerrit/server/schema/MysqlAccountPatchReviewStore.java
index 35bf2cb..677b2de 100644
--- a/java/com/google/gerrit/server/schema/MysqlAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/schema/MysqlAccountPatchReviewStore.java
@@ -38,7 +38,7 @@
@Override
public StorageException convertError(String op, SQLException err) {
- switch (getSQLStateInt(err)) {
+ switch (err.getErrorCode()) {
case 1022: // ER_DUP_KEY
case 1062: // ER_DUP_ENTRY
case 1169: // ER_DUP_UNIQUE;
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index 2a958af..6dee241 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -34,6 +34,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -187,6 +188,10 @@
|| e instanceof NoSuchRefException
|| e instanceof NoSuchProjectException) {
throw new ResourceNotFoundException(e.getMessage(), e);
+ } else if (e instanceof CommentsRejectedException) {
+ // SC_BAD_REQUEST is not ideal because it's not a syntactic error, but there is no better
+ // status code and it's isolated in monitoring.
+ throw new BadRequestException(e.getMessage(), e);
}
Throwables.throwIfUnchecked(e);
@@ -290,7 +295,7 @@
private enum ChangeResult {
SKIPPED,
UPSERTED,
- DELETED;
+ DELETED
}
private final GitRepositoryManager repoManager;
@@ -567,9 +572,7 @@
handle.setResult(id, ChangeResult.SKIPPED);
continue;
}
- for (ChangeUpdate u : ctx.updates.values()) {
- handle.manager.add(u);
- }
+ ctx.updates.values().forEach(handle.manager::add);
if (ctx.deleted) {
logDebug("Change %s was deleted", id);
handle.manager.deleteChange(id);
diff --git a/java/com/google/gerrit/server/update/CommentsRejectedException.java b/java/com/google/gerrit/server/update/CommentsRejectedException.java
new file mode 100644
index 0000000..6b0c04d
--- /dev/null
+++ b/java/com/google/gerrit/server/update/CommentsRejectedException.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2019 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.update;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.validators.CommentValidationFailure;
+import java.util.Collection;
+import java.util.stream.Collectors;
+
+/** Thrown when comment validation rejected a comment, preventing it from being published. */
+public class CommentsRejectedException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ private final ImmutableList<CommentValidationFailure> commentValidationFailures;
+
+ public CommentsRejectedException(Collection<CommentValidationFailure> commentValidationFailures) {
+ this.commentValidationFailures = ImmutableList.copyOf(commentValidationFailures);
+ }
+
+ @Override
+ public String getMessage() {
+ return "One or more comments were rejected in validation: "
+ + commentValidationFailures.stream()
+ .map(CommentValidationFailure::getMessage)
+ .collect(Collectors.joining("; "));
+ }
+
+ /**
+ * Returns the validation failures that caused this exception. By contract this list is never
+ * empty.
+ */
+ public ImmutableList<CommentValidationFailure> getCommentValidationFailures() {
+ return commentValidationFailures;
+ }
+}
diff --git a/java/com/google/gerrit/testing/TestCommentHelper.java b/java/com/google/gerrit/testing/TestCommentHelper.java
new file mode 100644
index 0000000..b72cca7
--- /dev/null
+++ b/java/com/google/gerrit/testing/TestCommentHelper.java
@@ -0,0 +1,107 @@
+// Copyright (C) 2019 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.testing;
+
+import static java.util.stream.Collectors.toList;
+
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.client.Comment;
+import com.google.gerrit.extensions.client.Comment.Range;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.inject.Inject;
+import java.util.Collection;
+
+/** Test helper for dealing with comments/drafts. */
+public class TestCommentHelper {
+ private final GerritApi gApi;
+
+ @Inject
+ public TestCommentHelper(GerritApi gerritApi) {
+ gApi = gerritApi;
+ }
+
+ public DraftInput newDraft(String message) {
+ return populate(new DraftInput(), "file", message);
+ }
+
+ public DraftInput newDraft(String path, Side side, int line, String message) {
+ DraftInput d = new DraftInput();
+ return populate(d, path, side, line, message);
+ }
+
+ public void addDraft(String changeId, String revId, DraftInput in) throws Exception {
+ gApi.changes().id(changeId).revision(revId).createDraft(in).get();
+ }
+
+ public Collection<CommentInfo> getPublishedComments(String changeId) throws Exception {
+ return gApi.changes().id(changeId).comments().values().stream()
+ .flatMap(Collection::stream)
+ .collect(toList());
+ }
+
+ public static <C extends Comment> C populate(C c, String path, String message) {
+ return populate(c, path, createLineRange(), message);
+ }
+
+ private static <C extends Comment> C populate(C c, String path, Range range, String message) {
+ int line = range.startLine;
+ c.path = path;
+ c.side = Side.REVISION;
+ c.parent = null;
+ c.line = line != 0 ? line : null;
+ c.message = message;
+ c.unresolved = false;
+ if (line != 0) c.range = range;
+ return c;
+ }
+
+ private static <C extends Comment> C populate(
+ C c, String path, Side side, Range range, String message) {
+ int line = range.startLine;
+ c.path = path;
+ c.side = side;
+ c.parent = null;
+ c.line = line != 0 ? line : null;
+ c.message = message;
+ c.unresolved = false;
+ if (line != 0) c.range = range;
+ return c;
+ }
+
+ private static <C extends Comment> C populate(
+ C c, String path, Side side, int line, String message) {
+ return populate(c, path, side, createLineRange(line), message);
+ }
+
+ private static Range createLineRange() {
+ Range range = new Range();
+ range.startLine = 0;
+ range.startCharacter = 1;
+ range.endLine = 0;
+ range.endCharacter = 5;
+ return range;
+ }
+
+ private static Range createLineRange(int line) {
+ Range range = new Range();
+ range.startLine = line;
+ range.startCharacter = 1;
+ range.endLine = line;
+ range.endCharacter = 5;
+ return range;
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index f5954d9..228f233 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -3291,10 +3291,6 @@
countsByAccount.clear();
}
- long getCount(Account.Id accountId) {
- return countsByAccount.get(accountId.get());
- }
-
void assertReindexOf(TestAccount testAccount) {
assertReindexOf(testAccount, 1);
}
@@ -3303,19 +3299,18 @@
assertReindexOf(Account.id(accountInfo._accountId), 1);
}
- void assertReindexOf(TestAccount testAccount, int expectedCount) {
- assertThat(getCount(testAccount.id())).isEqualTo(expectedCount);
- assertThat(countsByAccount).hasSize(1);
+ void assertReindexOf(TestAccount testAccount, long expectedCount) {
+ assertThat(countsByAccount.asMap()).containsExactly(testAccount.id().get(), expectedCount);
clear();
}
- void assertReindexOf(Account.Id accountId, int expectedCount) {
- assertThat(getCount(accountId)).isEqualTo(expectedCount);
+ void assertReindexOf(Account.Id accountId, long expectedCount) {
+ assertThat(countsByAccount.asMap()).containsEntry(accountId.get(), expectedCount);
countsByAccount.remove(accountId.get());
}
void assertNoReindex() {
- assertThat(countsByAccount).isEmpty();
+ assertThat(countsByAccount.asMap()).isEmpty();
}
}
@@ -3339,23 +3334,17 @@
countsByProjectRefs.clear();
}
- long getCount(String projectRef) {
- return countsByProjectRefs.get(projectRef);
- }
-
void assertRefUpdateFor(String... projectRefs) {
- Map<String, Integer> expectedRefUpdateCounts = new HashMap<>();
+ Map<String, Long> expectedRefUpdateCounts = new HashMap<>();
for (String projectRef : projectRefs) {
- expectedRefUpdateCounts.put(projectRef, 1);
+ expectedRefUpdateCounts.put(projectRef, 1L);
}
assertRefUpdateFor(expectedRefUpdateCounts);
}
- void assertRefUpdateFor(Map<String, Integer> expectedProjectRefUpdateCounts) {
- for (Map.Entry<String, Integer> e : expectedProjectRefUpdateCounts.entrySet()) {
- assertThat(getCount(e.getKey())).isEqualTo(e.getValue());
- }
- assertThat(countsByProjectRefs).hasSize(expectedProjectRefUpdateCounts.size());
+ void assertRefUpdateFor(Map<String, Long> expectedProjectRefUpdateCounts) {
+ assertThat(countsByProjectRefs.asMap())
+ .containsExactlyEntriesIn(expectedProjectRefUpdateCounts);
clear();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
new file mode 100644
index 0000000..ebed15c
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -0,0 +1,288 @@
+// Copyright (C) 2019 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.acceptance.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.server.restapi.change.PostReview;
+import com.google.gerrit.server.update.CommentsRejectedException;
+import com.google.gerrit.testing.TestCommentHelper;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import java.sql.Timestamp;
+import org.easymock.Capture;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests for comment validation in {@link PostReview}. */
+public class PostReviewIT extends AbstractDaemonTest {
+ @Inject private CommentValidator mockCommentValidator;
+ @Inject private TestCommentHelper testCommentHelper;
+
+ private static final String COMMENT_TEXT = "The comment text";
+
+ private Capture<ImmutableList<CommentForValidation>> capture = new Capture<>();
+
+ @Override
+ public Module createModule() {
+ return new FactoryModule() {
+ @Override
+ public void configure() {
+ CommentValidator mockCommentValidator = EasyMock.createMock(CommentValidator.class);
+ bind(CommentValidator.class)
+ .annotatedWith(Exports.named(mockCommentValidator.getClass()))
+ .toInstance(mockCommentValidator);
+ bind(CommentValidator.class).toInstance(mockCommentValidator);
+ }
+ };
+ }
+
+ @Before
+ public void resetMock() {
+ EasyMock.reset(mockCommentValidator);
+ }
+
+ @After
+ public void verifyMock() {
+ EasyMock.verify(mockCommentValidator);
+ }
+
+ @Test
+ public void validateCommentsInInput_commentOK() throws Exception {
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+
+ PushOneCommit.Result r = createChange();
+
+ ReviewInput input = new ReviewInput();
+ CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
+ comment.updated = new Timestamp(0);
+ input.comments = ImmutableMap.of(comment.path, ImmutableList.of(comment));
+
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).hasSize(1);
+ }
+
+ @Test
+ public void validateCommentsInInput_commentRejected() throws Exception {
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+
+ PushOneCommit.Result r = createChange();
+
+ ReviewInput input = new ReviewInput();
+ CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
+ comment.updated = new Timestamp(0);
+ input.comments = ImmutableMap.of(comment.path, ImmutableList.of(comment));
+
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+ BadRequestException badRequestException =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().review(input));
+ assertThat(badRequestException.getCause()).isInstanceOf(CommentsRejectedException.class);
+ assertThat(
+ Iterables.getOnlyElement(
+ ((CommentsRejectedException) badRequestException.getCause())
+ .getCommentValidationFailures())
+ .getComment()
+ .getText())
+ .isEqualTo(COMMENT_TEXT);
+ assertThat(badRequestException.getCause()).hasMessageThat().contains("Oh no!");
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+ }
+
+ @Test
+ public void validateDrafts_draftOK() throws Exception {
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+
+ PushOneCommit.Result r = createChange();
+
+ DraftInput draft =
+ testCommentHelper.newDraft(
+ r.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().getName()).createDraft(draft).get();
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+
+ ReviewInput input = new ReviewInput();
+ input.drafts = DraftHandling.PUBLISH;
+
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).hasSize(1);
+ }
+
+ @Test
+ public void validateDrafts_draftRejected() throws Exception {
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentType.INLINE_COMMENT, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result r = createChange();
+
+ DraftInput draft =
+ testCommentHelper.newDraft(
+ r.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
+ testCommentHelper.addDraft(r.getChangeId(), r.getCommit().getName(), draft);
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+
+ ReviewInput input = new ReviewInput();
+ input.drafts = DraftHandling.PUBLISH;
+ BadRequestException badRequestException =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().review(input));
+ assertThat(badRequestException.getCause()).isInstanceOf(CommentsRejectedException.class);
+ assertThat(
+ Iterables.getOnlyElement(
+ ((CommentsRejectedException) badRequestException.getCause())
+ .getCommentValidationFailures())
+ .getComment()
+ .getText())
+ .isEqualTo(draft.message);
+ assertThat(badRequestException.getCause()).hasMessageThat().contains("Oh no!");
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+ }
+
+ @Test
+ public void validateDrafts_inlineVsFileComments_allOK() throws Exception {
+ PushOneCommit.Result r = createChange();
+ DraftInput draftInline =
+ testCommentHelper.newDraft(
+ r.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
+ testCommentHelper.addDraft(r.getChangeId(), r.getCommit().getName(), draftInline);
+ DraftInput draftFile = testCommentHelper.newDraft(COMMENT_TEXT);
+ testCommentHelper.addDraft(r.getChangeId(), r.getCommit().getName(), draftFile);
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
+
+ EasyMock.expect(mockCommentValidator.validateComments(EasyMock.capture(capture)))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+
+ ReviewInput input = new ReviewInput();
+ input.drafts = DraftHandling.PUBLISH;
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).hasSize(2);
+
+ assertThat(capture.getValues()).hasSize(1);
+ assertThat(capture.getValue())
+ .containsExactly(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.INLINE_COMMENT, draftInline.message),
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, draftFile.message));
+ }
+
+ @Test
+ public void validateCommentsInChangeMessage_messageOK() throws Exception {
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result r = createChange();
+
+ ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
+ int numMessages = gApi.changes().id(r.getChangeId()).get().messages.size();
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ assertThat(gApi.changes().id(r.getChangeId()).get().messages).hasSize(numMessages + 1);
+ ChangeMessageInfo message =
+ Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages);
+ assertThat(message.message).contains(COMMENT_TEXT);
+ }
+
+ @Test
+ public void validateCommentsInChangeMessage_messageRejected() throws Exception {
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result r = createChange();
+
+ ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
+ assertThat(gApi.changes().id(r.getChangeId()).get().messages)
+ .hasSize(1); // From the initial commit.
+ BadRequestException badRequestException =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().review(input));
+ assertThat(badRequestException.getCause()).isInstanceOf(CommentsRejectedException.class);
+ assertThat(
+ Iterables.getOnlyElement(
+ ((CommentsRejectedException) badRequestException.getCause())
+ .getCommentValidationFailures())
+ .getComment()
+ .getText())
+ .isEqualTo(COMMENT_TEXT);
+ assertThat(badRequestException.getCause()).hasMessageThat().contains("Oh no!");
+ assertThat(gApi.changes().id(r.getChangeId()).get().messages)
+ .hasSize(1); // Unchanged from before.
+ ChangeMessageInfo message =
+ Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages);
+ assertThat(message.message).doesNotContain(COMMENT_TEXT);
+ }
+
+ private static CommentInput newComment(String path) {
+ return TestCommentHelper.populate(new CommentInput(), path, PostReviewIT.COMMENT_TEXT);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 9eb1270..d908ee5f 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -16,7 +16,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.GitUtil.deleteRef;
import static com.google.gerrit.acceptance.GitUtil.fetch;
@@ -31,6 +30,7 @@
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -1564,24 +1564,18 @@
countsByGroup.clear();
}
- long getCount(AccountGroup.UUID groupUuid) {
- return countsByGroup.get(groupUuid.get());
- }
-
void assertReindexOf(AccountGroup.UUID groupUuid) {
assertReindexOf(ImmutableList.of(groupUuid));
}
void assertReindexOf(List<AccountGroup.UUID> groupUuids) {
- for (AccountGroup.UUID groupUuid : groupUuids) {
- assertWithMessage(groupUuid.get()).that(getCount(groupUuid)).isEqualTo(1);
- }
- assertThat(countsByGroup).hasSize(groupUuids.size());
+ Map<String, Long> expected = groupUuids.stream().collect(toMap(u -> u.get(), u -> 1L));
+ assertThat(countsByGroup.asMap()).containsExactlyEntriesIn(expected);
clear();
}
void assertNoReindex() {
- assertThat(countsByGroup).isEmpty();
+ assertThat(countsByGroup.asMap()).isEmpty();
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index dce0ab2..91c8d22 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -723,14 +723,13 @@
assertReindexOf(projectName, 1);
}
- void assertReindexOf(String projectName, int expectedCount) {
- assertThat(getCount(projectName)).isEqualTo(expectedCount);
- assertThat(countsByProject).hasSize(1);
+ void assertReindexOf(String projectName, long expectedCount) {
+ assertThat(countsByProject.asMap()).containsExactly(projectName, expectedCount);
clear();
}
void assertNoReindex() {
- assertThat(countsByProject).isEmpty();
+ assertThat(countsByProject.asMap()).isEmpty();
}
void assertReindexExactly(ImmutableMap<String, Long> expected) {
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/BUILD b/javatests/com/google/gerrit/acceptance/server/git/receive/BUILD
new file mode 100644
index 0000000..760e7f4
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/BUILD
@@ -0,0 +1,8 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "receive",
+ labels = ["server"],
+ deps = [],
+)
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
new file mode 100644
index 0000000..cb5add3
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
@@ -0,0 +1,143 @@
+// Copyright (C) 2019 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.acceptance.server.git.receive;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentForValidation.CommentType;
+import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.testing.TestCommentHelper;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import org.easymock.Capture;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Tests for comment validation when publishing drafts via the {@code --publish-comments} option.
+ */
+public class ReceiveCommitsCommentValidationIT extends AbstractDaemonTest {
+ @Inject private CommentValidator mockCommentValidator;
+ @Inject private TestCommentHelper testCommentHelper;
+
+ private static final String COMMENT_TEXT = "The comment text";
+
+ private Capture<ImmutableList<CommentForValidation>> capture = new Capture<>();
+
+ @Override
+ public Module createModule() {
+ return new FactoryModule() {
+ @Override
+ public void configure() {
+ CommentValidator mockCommentValidator = EasyMock.createMock(CommentValidator.class);
+ bind(CommentValidator.class)
+ .annotatedWith(Exports.named(mockCommentValidator.getClass()))
+ .toInstance(mockCommentValidator);
+ bind(CommentValidator.class).toInstance(mockCommentValidator);
+ }
+ };
+ }
+
+ @Before
+ public void resetMock() {
+ EasyMock.reset(mockCommentValidator);
+ }
+
+ @After
+ public void verifyMock() {
+ EasyMock.verify(mockCommentValidator);
+ }
+
+ @Test
+ public void validateComments_commentOK() throws Exception {
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
+ DraftInput comment = testCommentHelper.newDraft(COMMENT_TEXT);
+ testCommentHelper.addDraft(changeId, revId, comment);
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ Result amendResult = amendChange(changeId, "refs/for/master%publish-comments", admin, testRepo);
+ amendResult.assertOkStatus();
+ amendResult.assertNotMessage("Comment validation failure:");
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).hasSize(1);
+ }
+
+ @Test
+ public void validateComments_commentRejected() throws Exception {
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
+ DraftInput comment = testCommentHelper.newDraft(COMMENT_TEXT);
+ testCommentHelper.addDraft(changeId, revId, comment);
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ Result amendResult = amendChange(changeId, "refs/for/master%publish-comments", admin, testRepo);
+ amendResult.assertOkStatus();
+ amendResult.assertMessage("Comment validation failure:");
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ }
+
+ @Test
+ public void validateComments_inlineVsFileComments_allOK() throws Exception {
+ EasyMock.expect(mockCommentValidator.validateComments(EasyMock.capture(capture)))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
+ DraftInput draftFile = testCommentHelper.newDraft(COMMENT_TEXT);
+ testCommentHelper.addDraft(changeId, revId, draftFile);
+ DraftInput draftInline =
+ testCommentHelper.newDraft(
+ result.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
+ testCommentHelper.addDraft(changeId, revId, draftInline);
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).isEmpty();
+ amendChange(changeId, "refs/for/master%publish-comments", admin, testRepo);
+ assertThat(testCommentHelper.getPublishedComments(result.getChangeId())).hasSize(2);
+ assertThat(capture.getValues()).hasSize(1);
+ assertThat(capture.getValue())
+ .containsExactly(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.INLINE_COMMENT, draftInline.message),
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, draftFile.message));
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index f917fd8..6e14635 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -16,25 +16,61 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailProcessingUtil;
import com.google.gerrit.server.mail.receive.MailProcessor;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
+import com.google.inject.Module;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.List;
+import org.easymock.EasyMock;
+import org.junit.Before;
import org.junit.Test;
public class MailProcessorIT extends AbstractMailIT {
@Inject private MailProcessor mailProcessor;
@Inject private AccountOperations accountOperations;
+ @Inject private CommentValidator mockCommentValidator;
+ @Inject private TestCommentHelper testCommentHelper;
+
+ private static final String COMMENT_TEXT = "The comment text";
+
+ @Override
+ public Module createModule() {
+ return new FactoryModule() {
+ @Override
+ public void configure() {
+ CommentValidator mockCommentValidator = EasyMock.createMock(CommentValidator.class);
+ bind(CommentValidator.class)
+ .annotatedWith(Exports.named(mockCommentValidator.getClass()))
+ .toInstance(mockCommentValidator);
+ bind(CommentValidator.class).toInstance(mockCommentValidator);
+ }
+ };
+ }
+
+ @Before
+ public void setUp() {
+ // Let the mock comment validator accept all comments during test setup.
+ EasyMock.reset(mockCommentValidator);
+ EasyMock.expect(mockCommentValidator.validateComments(EasyMock.anyObject()))
+ .andReturn(ImmutableList.of());
+ EasyMock.replay(mockCommentValidator);
+ }
@Test
public void parseAndPersistChangeMessage() throws Exception {
@@ -223,6 +259,108 @@
assertThat(message.headers()).containsKey("Subject");
}
+ @Test
+ public void validateChangeMessage_rejected() throws Exception {
+ String changeId = createChangeWithReview();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ List<CommentInfo> comments = gApi.changes().id(changeId).current().commentsAsList();
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
+
+ EasyMock.reset(mockCommentValidator);
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+
+ MailMessage.Builder b = messageBuilderWithDefaultFields();
+ String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", COMMENT_TEXT, null, null, null);
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
+
+ Collection<CommentInfo> commentsBefore = testCommentHelper.getPublishedComments(changeId);
+ mailProcessor.process(b.build());
+ assertThat(testCommentHelper.getPublishedComments(changeId)).isEqualTo(commentsBefore);
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body()).contains("rejected one or more comments");
+ EasyMock.verify(mockCommentValidator);
+ }
+
+ @Test
+ public void validateInlineComment_rejected() throws Exception {
+ String changeId = createChangeWithReview();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ List<CommentInfo> comments = gApi.changes().id(changeId).current().commentsAsList();
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
+
+ EasyMock.reset(mockCommentValidator);
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+
+ MailMessage.Builder b = messageBuilderWithDefaultFields();
+ String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, COMMENT_TEXT, null, null);
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
+
+ Collection<CommentInfo> commentsBefore = testCommentHelper.getPublishedComments(changeId);
+ mailProcessor.process(b.build());
+ assertThat(testCommentHelper.getPublishedComments(changeId)).isEqualTo(commentsBefore);
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body()).contains("rejected one or more comments");
+ EasyMock.verify(mockCommentValidator);
+ }
+
+ @Test
+ public void validateFileComment_rejected() throws Exception {
+ String changeId = createChangeWithReview();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ List<CommentInfo> comments = gApi.changes().id(changeId).current().commentsAsList();
+ String ts =
+ MailProcessingUtil.rfcDateformatter.format(
+ ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
+
+ EasyMock.reset(mockCommentValidator);
+ CommentForValidation commentForValidation =
+ CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
+ EasyMock.expect(
+ mockCommentValidator.validateComments(
+ ImmutableList.of(
+ CommentForValidation.create(
+ CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
+ .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ EasyMock.replay(mockCommentValidator);
+
+ MailMessage.Builder b = messageBuilderWithDefaultFields();
+ String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, null, COMMENT_TEXT, null);
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
+
+ Collection<CommentInfo> commentsBefore = testCommentHelper.getPublishedComments(changeId);
+ mailProcessor.process(b.build());
+ assertThat(testCommentHelper.getPublishedComments(changeId)).isEqualTo(commentsBefore);
+
+ assertNotifyTo(user);
+ Message message = sender.nextMessage();
+ assertThat(message.body()).contains("rejected one or more comments");
+ EasyMock.verify(mockCommentValidator);
+ }
+
private String getChangeUrl(ChangeInfo changeInfo) {
return canonicalWebUrl.get() + "c/" + changeInfo.project + "/+/" + changeInfo._number;
}
diff --git a/package.json b/package.json
index 8a18318f..6b9a38d 100644
--- a/package.json
+++ b/package.json
@@ -8,6 +8,7 @@
"eslint-config-google": "^0.13.0",
"eslint-plugin-html": "^5.0.5",
"fried-twinkie": "^0.2.2",
+ "polylint": "^2.10.4",
"typescript": "^2.x.x",
"web-component-tester": "^6.5.0"
},
@@ -15,7 +16,8 @@
"start": "polygerrit-ui/run-server.sh",
"test": "WCT_HEADLESS_MODE=1 WCT_ARGS='--verbose -l chrome' ./polygerrit-ui/app/run_test.sh",
"eslint": "./node_modules/eslint/bin/eslint.js --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app || exit 0",
- "test-template": "./polygerrit-ui/app/run_template_test.sh"
+ "test-template": "./polygerrit-ui/app/run_template_test.sh",
+ "polylint": "bazel test polygerrit-ui/app:polylint_test"
},
"repository": {
"type": "git",
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index 9ceda8c..a73f243 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -32,7 +32,6 @@
```sh
npm install
-sudo npm install -g polylint
```
It may complain about a missing `typescript@2.3.4` peer dependency, which is
@@ -175,6 +174,13 @@
```sh
bazel test //polygerrit-ui/app:polylint_test
```
+
+or
+
+```sh
+npm run polylint
+```
+
## Template Type Safety
Polymer elements are not type checked against the element definition, making it
trivial to break the display when refactoring or moving code. We now run
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
index 398ed9c..42b3788 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.html
@@ -100,12 +100,17 @@
<iron-icon id="icon" icon="gr-icons:create"></iron-icon>
</gr-button>
</div>
- <input
- id="editRefInput"
+ <iron-input
bind-value="{{section.id}}"
- is="iron-input"
type="text"
on-input="_handleValueChange">
+ <input
+ id="editRefInput"
+ bind-value="{{section.id}}"
+ is="iron-input"
+ type="text"
+ on-input="_handleValueChange">
+ </iron-input>
<gr-button
link
id="deleteBtn"
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.html b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.html
index c448f5b..b2a3105 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.html
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.html
@@ -18,7 +18,6 @@
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-list-view-behavior/gr-list-view-behavior.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-table-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
index 252dcfd..518905c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
@@ -69,23 +69,33 @@
<section class$="[[_computeBranchClass(baseChange)]]">
<span class="title">Provide base commit sha1 for change</span>
<span class="value">
- <input
- is="iron-input"
- id="baseCommitInput"
+ <iron-input
maxlength="40"
placeholder="(optional)"
bind-value="{{baseCommit}}">
+ <input
+ is="iron-input"
+ id="baseCommitInput"
+ maxlength="40"
+ placeholder="(optional)"
+ bind-value="{{baseCommit}}">
+ </iron-input>
</span>
</section>
<section>
<span class="title">Enter topic for new change</span>
<span class="value">
- <input
- is="iron-input"
- id="tagNameInput"
+ <iron-input
maxlength="1024"
placeholder="(optional)"
bind-value="{{topic}}">
+ <input
+ is="iron-input"
+ id="tagNameInput"
+ maxlength="1024"
+ placeholder="(optional)"
+ bind-value="{{topic}}">
+ </iron-input>
</span>
</section>
<section id="description">
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.html
index 73a3f4e..ed631de 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.html
@@ -39,10 +39,13 @@
<div id="form">
<section>
<span class="title">Group name</span>
- <input
- is="iron-input"
- id="groupNameInput"
+ <iron-input
bind-value="{{_name}}">
+ <input
+ is="iron-input"
+ id="groupNameInput"
+ bind-value="{{_name}}">
+ </iron-input>
</section>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
index a1fbd37..8986a71 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
@@ -44,27 +44,39 @@
<div id="form">
<section>
<span class="title">[[detailType]] name</span>
- <input
- is="iron-input"
- id="itemNameInput"
+ <iron-input
placeholder="[[detailType]] Name"
bind-value="{{_itemName}}">
+ <input
+ is="iron-input"
+ id="itemNameInput"
+ placeholder="[[detailType]] Name"
+ bind-value="{{_itemName}}">
+ </iron-input>
</section>
<section>
<span class="title">Initial Revision</span>
- <input
- is="iron-input"
- id="itemRevisionInput"
+ <iron-input
placeholder="Revision (Branch or SHA-1)"
bind-value="{{_itemRevision}}">
+ <input
+ is="iron-input"
+ id="itemRevisionInput"
+ placeholder="Revision (Branch or SHA-1)"
+ bind-value="{{_itemRevision}}">
+ </iron-input>
</section>
<section class$="[[_computeHideItemClass(itemDetail)]]">
<span class="title">Annotation</span>
- <input
- is="iron-input"
- id="itemAnnotationInput"
+ <iron-input
placeholder="Annotation (Optional)"
bind-value="{{_itemAnnotation}}">
+ <input
+ is="iron-input"
+ id="itemAnnotationInput"
+ placeholder="Annotation (Optional)"
+ bind-value="{{_itemAnnotation}}">
+ </iron-input>
</section>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
index 0feeced..5c48de1 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
@@ -54,10 +54,13 @@
<div id="form">
<section>
<span class="title">Repository name</span>
- <input is="iron-input"
- id="repoNameInput"
- autocomplete="on"
- bind-value="{{_repoConfig.name}}">
+ <iron-input autocomplete="on"
+ bind-value="{{_repoConfig.name}}">
+ <input is="iron-input"
+ id="repoNameInput"
+ autocomplete="on"
+ bind-value="{{_repoConfig.name}}">
+ </iron-input>
</section>
<section>
<span class="title">Rights inherit from</span>
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
index 787f2b4..52cbfb3 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
@@ -19,7 +19,6 @@
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="/bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/gr-subpage-styles.html">
<link rel="import" href="../../../styles/gr-table-styles.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
index 9f49e4b..a3bbddc 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
@@ -17,7 +17,6 @@
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="/bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/gr-subpage-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-config-array-editor/gr-plugin-config-array-editor.html b/polygerrit-ui/app/elements/admin/gr-plugin-config-array-editor/gr-plugin-config-array-editor.html
index 2625d65..9ef408b 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-config-array-editor/gr-plugin-config-array-editor.html
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-config-array-editor/gr-plugin-config-array-editor.html
@@ -83,11 +83,15 @@
<div class="row placeholder">None configured.</div>
</template>
<div class$="row [[_computeShowInputRow(disabled)]]">
- <input
- is="iron-input"
- id="input"
+ <iron-input
on-keydown="_handleInputKeydown"
- bind-value="{{_newValue}}"/>
+ bind-value="{{_newValue}}">
+ <input
+ is="iron-input"
+ id="input"
+ on-keydown="_handleInputKeydown"
+ bind-value="{{_newValue}}">
+ </iron-input>
<gr-button
id="addButton"
disabled$="[[!_newValue.length]]"
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
index ea4d165..63c2a97 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
@@ -17,7 +17,6 @@
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="/bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/gr-subpage-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
index 44b94e1..1c02b3c 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
@@ -124,10 +124,14 @@
class="editBtn">
edit
</gr-button>
- <input
- is=iron-input
+ <iron-input
bind-value="{{_revisedRef}}"
class="editItem">
+ <input
+ is=iron-input
+ bind-value="{{_revisedRef}}"
+ class="editItem">
+ </iron-input>
<gr-button
link
on-tap="_handleCancelRevision"
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
index 1a728ab..52db4c2 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
@@ -17,7 +17,6 @@
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-list-view-behavior/gr-list-view-behavior.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-table-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.html b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.html
index 69c86d9..872d282 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.html
@@ -87,12 +87,18 @@
</gr-select>
</template>
<template is="dom-if" if="[[_isString(option.info.type)]]">
- <input
- is="iron-input"
+ <iron-input
value="[[option.info.value]]"
on-input="_handleStringChange"
data-option-key$="[[option._key]]"
- disabled$="[[_computeDisabled(option.info.editable)]]"></input>
+ disabled$="[[_computeDisabled(option.info.editable)]]">
+ <input
+ is="iron-input"
+ value="[[option.info.value]]"
+ on-input="_handleStringChange"
+ data-option-key$="[[option._key]]"
+ disabled$="[[_computeDisabled(option.info.editable)]]">
+ </iron-input>
</template>
<template is="dom-if" if="[[option.info.inherited_value]]">
<span class="inherited">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
index f6b6d29..29cc2bd 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
@@ -270,12 +270,17 @@
<section>
<span class="title">Maximum Git object size limit</span>
<span class="value">
- <input
- id="maxGitObjSizeInput"
+ <iron-input
bind-value="{{_repoConfig.max_object_size_limit.configured_value}}"
- is="iron-input"
type="text"
disabled$="[[_readOnly]]">
+ <input
+ id="maxGitObjSizeInput"
+ bind-value="{{_repoConfig.max_object_size_limit.configured_value}}"
+ is="iron-input"
+ type="text"
+ disabled$="[[_readOnly]]">
+ </iron-input>
<template is="dom-if" if="[[_repoConfig.max_object_size_limit.value]]">
effective: [[_repoConfig.max_object_size_limit.value]] bytes
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index bcb0df4..e69d48f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -19,7 +19,6 @@
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../admin/gr-create-change-dialog/gr-create-change-dialog.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
index 508e881..767b62d 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
@@ -15,8 +15,9 @@
limitations under the License.
-->
-<link rel="import" href="/bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
@@ -77,12 +78,17 @@
<label for="baseInput">
Provide base commit sha1 for cherry-pick
</label>
- <input
- is="iron-input"
- id="baseCommitInput"
+ <iron-input
maxlength="40"
placeholder="(optional)"
bind-value="{{baseCommit}}">
+ <input
+ is="iron-input"
+ id="baseCommitInput"
+ maxlength="40"
+ placeholder="(optional)"
+ bind-value="{{baseCommit}}">
+ </iron-input>
<label for="messageInput">
Cherry Pick Commit Message
</label>
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.html b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.html
index a749431..d5d8128 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.html
@@ -16,6 +16,7 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
@@ -77,11 +78,15 @@
link
on-tap="_handleCloseTap">Close</gr-button>
</span>
- <input
- id="filterInput"
- is="iron-input"
+ <iron-input
placeholder="Filter"
on-bind-value-changed="_onFilterChanged">
+ <input
+ id="filterInput"
+ is="iron-input"
+ placeholder="Filter"
+ on-bind-value-changed="_onFilterChanged">
+ </iron-input>
</header>
<div class$="[[_computeLoadingClass(_loaded)]]">Loading...</div>
<template
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.html b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.html
index 72c5b44..46f973e 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.html
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.html
@@ -16,7 +16,6 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../shared/gr-account-chip/gr-account-chip.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index d9cf979..c992a51 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -21,6 +21,7 @@
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="/bower_components/iron-dropdown/iron-dropdown.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
@@ -303,7 +304,9 @@
<gr-endpoint-decorator name="annotation-toggler">
<span hidden id="annotation-span">
<label for="annotation-checkbox" id="annotation-label"></label>
- <input is="iron-input" type="checkbox" id="annotation-checkbox" disabled>
+ <iron-input type="checkbox" disabled>
+ <input is="iron-input" type="checkbox" id="annotation-checkbox" disabled>
+ </iron-input>
</span>
</gr-endpoint-decorator>
</div>
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html
index 616172b..5072b9d 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html
@@ -18,7 +18,6 @@
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-list-view-behavior/gr-list-view-behavior.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-table-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-list-view/gr-list-view.html">
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
index ea5024a..2d66bc2 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
@@ -132,11 +132,16 @@
placeholder="Enter an existing full file path."
query="[[_query]]"
text="{{_path}}"></gr-autocomplete>
- <input
- class="newPathInput"
- is="iron-input"
+ <iron-input
+ class="newPathIronInput"
bind-value="{{_newPath}}"
- placeholder="Enter the new path."/>
+ placeholder="Enter the new path.">
+ <input
+ class="newPathInput"
+ is="iron-input"
+ bind-value="{{_newPath}}"
+ placeholder="Enter the new path.">
+ </iron-input>
</div>
</gr-dialog>
<gr-dialog
@@ -148,10 +153,14 @@
on-cancel="_handleDialogCancel">
<div class="header" slot="header">Restore this file?</div>
<div class="main" slot="main">
- <input
- is="iron-input"
+ <iron-input
disabled
- bind-value="{{_path}}"/>
+ bind-value="{{_path}}">
+ <input
+ is="iron-input"
+ disabled
+ bind-value="{{_path}}">
+ </iron-input>
</div>
</gr-dialog>
</gr-overlay>
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
index 75a371f8..df029ed 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
@@ -191,6 +191,9 @@
let navStub;
let renameStub;
let renameAutocomplete;
+ const inputSelector = Polymer.Element ?
+ '.newPathIronInput' :
+ '.newPathInput';
setup(() => {
navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
@@ -210,7 +213,7 @@
assert.isTrue(queryStub.called);
assert.isTrue(element.$.renameDialog.disabled);
- element.$.renameDialog.querySelector('.newPathInput').bindValue =
+ element.$.renameDialog.querySelector(inputSelector).bindValue =
'src/test.newPath';
assert.isFalse(element.$.renameDialog.disabled);
@@ -238,7 +241,7 @@
assert.isTrue(queryStub.called);
assert.isTrue(element.$.renameDialog.disabled);
- element.$.renameDialog.querySelector('.newPathInput').bindValue =
+ element.$.renameDialog.querySelector(inputSelector).bindValue =
'src/test.newPath';
assert.isFalse(element.$.renameDialog.disabled);
@@ -260,7 +263,7 @@
assert.isTrue(element.$.renameDialog.disabled);
element.$.renameDialog.querySelector('gr-autocomplete').text =
'src/test.cpp';
- element.$.renameDialog.querySelector('.newPathInput').bindValue =
+ element.$.renameDialog.querySelector(inputSelector).bindValue =
'src/test.newPath';
assert.isFalse(element.$.renameDialog.disabled);
MockInteractions.tap(element.$.renameDialog.$$('gr-button'));
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
index 7d469a3..8de8db8 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
@@ -79,12 +79,17 @@
<span
hidden$="[[!usernameMutable]]"
class="value">
- <input
- is="iron-input"
- id="usernameInput"
+ <iron-input
disabled="[[_saving]]"
on-keydown="_handleKeydown"
bind-value="{{_username}}">
+ <input
+ is="iron-input"
+ id="usernameInput"
+ disabled="[[_saving]]"
+ on-keydown="_handleKeydown"
+ bind-value="{{_username}}">
+ </iron-input>
</span>
</section>
<section id="nameSection">
@@ -95,24 +100,34 @@
<span
hidden$="[[!nameMutable]]"
class="value">
- <input
- is="iron-input"
- id="nameInput"
+ <iron-input
disabled="[[_saving]]"
on-keydown="_handleKeydown"
bind-value="{{_account.name}}">
+ <input
+ is="iron-input"
+ id="nameInput"
+ disabled="[[_saving]]"
+ on-keydown="_handleKeydown"
+ bind-value="{{_account.name}}">
+ </iron-input>
</span>
</section>
<section>
<span class="title">Status (e.g. "Vacation")</span>
<span class="value">
- <input
- is="iron-input"
- id="statusInput"
+ <iron-input
disabled="[[_saving]]"
on-keydown="_handleKeydown"
bind-value="{{_account.status}}">
- </span>
+ <input
+ is="iron-input"
+ id="statusInput"
+ disabled="[[_saving]]"
+ on-keydown="_handleKeydown"
+ bind-value="{{_account.status}}">
+ </iron-input>
+ </span>
</section>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
index 43bb2db..8b32be9 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
@@ -16,7 +16,6 @@
-->
<link rel="import" href="../../../behaviors/gr-change-table-behavior/gr-change-table-behavior.html">
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
index bb5c069..2d5fd26 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
@@ -95,7 +95,13 @@
</div>
<div class$="agreementsTextBox [[_computeHideAgreementClass(_agreementName, _serverConfig.auth.contributor_agreements)]]">
<h3 class="smallHeading">Complete the agreement:</h3>
- <input id="input-agreements" is="iron-input" bind-value="{{_agreementsText}}" placeholder="Enter 'I agree' here" />
+ <iron-input bind-value="{{_agreementsText}}"
+ placeholder="Enter 'I agree' here">
+ <input id="input-agreements"
+ is="iron-input"
+ bind-value="{{_agreementsText}}"
+ placeholder="Enter 'I agree' here">
+ </iron-input>
<gr-button on-tap="_handleSaveAgreements" disabled="[[_disableAgreementsText(_agreementsText)]]">
Submit
</gr-button>
diff --git a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
index 33fc879..53a30c3 100644
--- a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
+++ b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
@@ -16,6 +16,7 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
@@ -29,40 +30,64 @@
<section>
<span class="title">Tab width</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.tab_size}}"
on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{editPrefs.tab_size}}"
+ on-keypress="_handleEditPrefsChanged"
+ on-change="_handleEditPrefsChanged">
+ </iron-input>
</span>
</section>
<section>
<span class="title">Columns</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.line_length}}"
on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{editPrefs.line_length}}"
+ on-keypress="_handleEditPrefsChanged"
+ on-change="_handleEditPrefsChanged">
+ </iron-input>
</span>
</section>
<section>
<span class="title">Indent unit</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.indent_unit}}"
on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{editPrefs.indent_unit}}"
+ on-keypress="_handleEditPrefsChanged"
+ on-change="_handleEditPrefsChanged">
+ </iron-input>
</span>
</section>
<section>
diff --git a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.html b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.html
index 98b9b6d..f508288 100644
--- a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.html
@@ -63,14 +63,22 @@
<tr>
<td class="emailColumn">[[item.email]]</td>
<td class="preferredControl" on-tap="_handlePreferredControlTap">
- <input
- is="iron-input"
+ <iron-input
class="preferredRadio"
type="radio"
on-change="_handlePreferredChange"
name="preferred"
value="[[item.email]]"
checked$="[[item.preferred]]">
+ <input
+ is="iron-input"
+ class="preferredRadio"
+ type="radio"
+ on-change="_handlePreferredChange"
+ name="preferred"
+ value="[[item.email]]"
+ checked$="[[item.preferred]]">
+ </iron-input>
</td>
<td>
<gr-button
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js
index cda6da7..99f4504 100644
--- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js
+++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.js
@@ -27,6 +27,10 @@
_passwordUrl: String,
},
+ attached() {
+ this.loadData();
+ },
+
loadData() {
const promises = [];
diff --git a/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.html b/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.html
index 56f4cfa..703c34c 100644
--- a/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.html
@@ -85,19 +85,30 @@
<tfoot>
<tr>
<th>
- <input
- is="iron-input"
+ <iron-input
placeholder="New Title"
on-keydown="_handleInputKeydown"
bind-value="{{_newName}}">
+ <input
+ is="iron-input"
+ placeholder="New Title"
+ on-keydown="_handleInputKeydown"
+ bind-value="{{_newName}}">
+ </iron-input>
</th>
<th>
- <input
+ <iron-input
class="newUrlInput"
- is="iron-input"
placeholder="New URL"
on-keydown="_handleInputKeydown"
bind-value="{{_newUrl}}">
+ <input
+ class="newUrlInput"
+ is="iron-input"
+ placeholder="New URL"
+ on-keydown="_handleInputKeydown"
+ bind-value="{{_newUrl}}">
+ </iron-input>
</th>
<th></th>
<th></th>
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
index 66b06f8..cdc0825 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
@@ -16,6 +16,7 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
@@ -81,19 +82,27 @@
<hr>
<section>
<div class="title">Full Name</div>
- <input
- is="iron-input"
- id="name"
+ <iron-input
bind-value="{{_account.name}}"
disabled="[[_saving]]">
+ <input
+ is="iron-input"
+ id="name"
+ bind-value="{{_account.name}}"
+ disabled="[[_saving]]">
+ </iron-input>
</section>
<section class$="[[_computeUsernameClass(_usernameMutable)]]">
<div class="title">Username</div>
- <input
- is="iron-input"
- id="username"
+ <iron-input
bind-value="{{_account.username}}"
disabled="[[_saving]]">
+ <input
+ is="iron-input"
+ id="username"
+ bind-value="{{_account.username}}"
+ disabled="[[_saving]]">
+ </iron-input>
</section>
<section>
<div class="title">Preferred Email</div>
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
index 68518a0..d193041 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -16,6 +16,7 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../behaviors/docs-url-behavior/docs-url-behavior.html">
<link rel="import" href="/bower_components/paper-toggle-button/paper-toggle-button.html">
@@ -49,7 +50,7 @@
:host {
color: var(--primary-text-color);
}
- #newEmailInput {
+ .newEmailInput {
width: 20em;
}
#email {
@@ -365,14 +366,22 @@
<section>
<span class="title">New email address</span>
<span class="value">
- <input
- id="newEmailInput"
+ <iron-input
+ class="newEmailInput"
bind-value="{{_newEmail}}"
- is="iron-input"
type="text"
disabled="[[_addingEmail]]"
on-keydown="_handleNewEmailKeydown"
placeholder="email@example.com">
+ <input
+ class="newEmailInput"
+ bind-value="{{_newEmail}}"
+ is="iron-input"
+ type="text"
+ disabled="[[_addingEmail]]"
+ on-keydown="_handleNewEmailKeydown"
+ placeholder="email@example.com">
+ </iron-input>
</span>
</section>
<section
@@ -387,12 +396,14 @@
disabled="[[!_computeAddEmailButtonEnabled(_newEmail, _addingEmail)]]"
on-tap="_handleAddEmailButton">Send verification</gr-button>
</fieldset>
- <div hidden$="[[!_showHttpAuth(_serverConfig)]]">
- <h2 id="HTTPCredentials">HTTP Credentials</h2>
- <fieldset>
- <gr-http-password id="httpPass"></gr-http-password>
- </fieldset>
- </div>
+ <template is="dom-if" if="[[_showHttpAuth(_serverConfig)]]">
+ <div>
+ <h2 id="HTTPCredentials">HTTP Credentials</h2>
+ <fieldset>
+ <gr-http-password id="httpPass"></gr-http-password>
+ </fieldset>
+ </div>
+ </template>
<div hidden$="[[!_serverConfig.sshd]]">
<h2
id="SSHKeys"
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
index d776b54..041dc64 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
@@ -166,7 +166,6 @@
this.$.accountInfo.loadData(),
this.$.watchedProjectsEditor.loadData(),
this.$.groupList.loadData(),
- this.$.httpPass.loadData(),
this.$.identities.loadData(),
this.$.editPrefs.loadData(),
this.$.diffPrefs.loadData(),
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
index e314608..35d43c7 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
@@ -15,6 +15,7 @@
limitations under the License.
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
@@ -103,11 +104,15 @@
placeholder="Repo"></gr-autocomplete>
</th>
<th colspan$="[[_getTypeCount()]]">
- <input
- id="newFilter"
+ <iron-input
class="newFilterInput"
- is="iron-input"
placeholder="branch:name, or other search expression">
+ <input
+ id="newFilter"
+ class="newFilterInput"
+ is="iron-input"
+ placeholder="branch:name, or other search expression">
+ </iron-input>
</th>
<th>
<gr-button link on-tap="_handleAddProject">Add</gr-button>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
index f6b4a27..8fea7e2 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
@@ -47,20 +47,29 @@
}
</style>
<div class="text">
- <input id="input" is="iron-input"
+ <iron-input
+ class$="copyText [[_computeInputClass(hideInput)]]"
+ type="text"
+ bind-value="[[text]]"
+ on-tap="_handleInputTap"
+ readonly>
+ <input
+ id="input"
+ is="iron-input"
class$="copyText [[_computeInputClass(hideInput)]]"
type="text"
bind-value="[[text]]"
on-tap="_handleInputTap"
readonly>
- <gr-button id="button"
- link
- has-tooltip="[[hasTooltip]]"
- class="copyToClipboard"
- title="[[buttonTitle]]"
- on-tap="_copyToClipboard">
- <iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon>
- </gr-button>
+ </iron-input>
+ <gr-button id="button"
+ link
+ has-tooltip="[[hasTooltip]]"
+ class="copyToClipboard"
+ title="[[buttonTitle]]"
+ on-tap="_copyToClipboard">
+ <iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon>
+ </gr-button>
</div>
</template>
<script src="gr-copy-clipboard.js"></script>
diff --git a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.html b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.html
index e21e5a4..9d85d44 100644
--- a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.html
+++ b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.html
@@ -60,43 +60,67 @@
<section>
<span class="title">Diff width</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
- id="columnsInput"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{diffPrefs.line_length}}"
on-keypress="_handleDiffPrefsChanged"
on-change="_handleDiffPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ id="columnsInput"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{diffPrefs.line_length}}"
+ on-keypress="_handleDiffPrefsChanged"
+ on-change="_handleDiffPrefsChanged">
+ </iron-input>
</span>
</section>
<section>
<span class="title">Tab width</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
- id="tabSizeInput"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{diffPrefs.tab_size}}"
on-keypress="_handleDiffPrefsChanged"
on-change="_handleDiffPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ id="tabSizeInput"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{diffPrefs.tab_size}}"
+ on-keypress="_handleDiffPrefsChanged"
+ on-change="_handleDiffPrefsChanged">
+ </iron-input>
</span>
</section>
<section hidden$="[[!diffPrefs.font_size]]">
<span class="title">Font size</span>
<span class="value">
- <input
- is="iron-input"
+ <iron-input
type="number"
- id="fontSizeInput"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{diffPrefs.font_size}}"
on-keypress="_handleDiffPrefsChanged"
on-change="_handleDiffPrefsChanged">
+ <input
+ is="iron-input"
+ type="number"
+ id="fontSizeInput"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{diffPrefs.font_size}}"
+ on-keypress="_handleDiffPrefsChanged"
+ on-change="_handleDiffPrefsChanged">
+ </iron-input>
</span>
</section>
<section>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html
index 78f9650..987b551 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html
@@ -30,7 +30,9 @@
<template>
<span hidden id="annotation-span">
<label for="annotation-checkbox" id="annotation-label"></label>
- <input is="iron-input" type="checkbox" id="annotation-checkbox" disabled>
+ <iron-input type="checkbox" disabled>
+ <input is="iron-input" type="checkbox" id="annotation-checkbox" disabled>
+ </iron-input>
</span>
</template>
</test-fixture>
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
index 1ee168d..765345c 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
@@ -15,6 +15,7 @@
limitations under the License.
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
@@ -68,10 +69,15 @@
<div id="topContainer">
<div class="filterContainer">
<label>Filter:</label>
- <input is="iron-input"
+ <iron-input
type="text"
- id="filter"
bind-value="{{filter}}">
+ <input
+ is="iron-input"
+ type="text"
+ id="filter"
+ bind-value="{{filter}}">
+ </iron-input>
</div>
<div id="createNewContainer"
class$="[[_computeCreateClass(createNew)]]">
diff --git a/polygerrit-ui/app/polylint_test.sh b/polygerrit-ui/app/polylint_test.sh
index ee69ce2..f6880a1 100755
--- a/polygerrit-ui/app/polylint_test.sh
+++ b/polygerrit-ui/app/polylint_test.sh
@@ -8,13 +8,13 @@
exit 1
fi
-polylint_bin=$(which polylint)
-if [[ -z "$polylint_bin" ]]; then
- echo "You must install polylint and its dependencies from NPM."
- echo "> npm install -g polylint"
+npx_bin=$(which npx)
+if [[ -z "$npx_bin" ]]; then
+ echo "NPX must be on the path."
+ echo "> npm i -g npx"
exit 1
fi
unzip -o polygerrit-ui/polygerrit_components.bower_components.zip -d polygerrit-ui/app
-${polylint_bin} --root polygerrit-ui/app --input elements/gr-app.html --b 'bower_components'
\ No newline at end of file
+npx polylint --root polygerrit-ui/app --input elements/gr-app.html --b 'bower_components' --verbose
diff --git a/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy b/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
index e997776..e88c424 100644
--- a/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
+++ b/resources/com/google/gerrit/server/mail/InboundEmailRejection.soy
@@ -62,3 +62,8 @@
This might be caused by an ongoing maintenance or a data corruption.
{call .InboundEmailRejectionFooter /}
{/template}
+
+{template .InboundEmailRejection_COMMENT_REJECTED kind="text"}
+ Gerrit Code Review rejected one or more comments because they did not pass validation.
+ {call .InboundEmailRejectionFooter /}
+{/template}
diff --git a/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy b/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
index f879270..e17508d 100644
--- a/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
+++ b/resources/com/google/gerrit/server/mail/InboundEmailRejectionHtml.soy
@@ -78,3 +78,10 @@
<p>
{call .InboundEmailRejectionFooterHtml /}
{/template}
+
+{template .InboundEmailRejectionHtml_COMMENT_REJECTED}
+ <p>
+ Gerrit Code Review rejected one or more comments because they did not pass validation.
+ </p>
+ {call .InboundEmailRejectionFooterHtml /}
+{/template}
diff --git a/resources/com/google/gerrit/server/mime/mime-types.properties b/resources/com/google/gerrit/server/mime/mime-types.properties
index 8159ac1..6fbd341 100644
--- a/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -124,7 +124,7 @@
mbox = application/mbox
md = text/x-markdown
mirc = text/mirc
-mjs = text/x-mjs
+mjs = text/javascript
mkd = text/x-markdown
ml = text/x-ocaml
mli = text/x-ocaml