Merge "Upgrade Truth to 1.0.1"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 880626e..6968e83 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4784,6 +4784,15 @@
used for suggesting accounts when adding members to a group.
+
By default 0.
+[[suggest.relevantChanges]]suggest.relevantChanges::
++
+When suggesting reviewers, we go over recent changes of the user, and
+give priority to users that are present as reviewers in any of those
+changes. The number of changes we go over is `sugggest.relevantChanges`.
++
+By default 50. This nubmer is a tradeoff between speed and accuracy.
+A high number would be accurate but slow, and a low number would be
+fast but inaccurate.
[[tracing]]
=== Section tracing
diff --git a/WORKSPACE b/WORKSPACE
index 0f2ed8a..6e9fb38 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -266,31 +266,6 @@
sha1 = "1dcf1de382a0bf95a3d8b0849546c88bac1292c9",
)
-CAFFEINE_VERS = "2.8.0"
-
-maven_jar(
- name = "caffeine",
- artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
- sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
-)
-
-# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
-# naming collision between caffeine guava adapater and guava library itself.
-# Remove this renaming procedure, once this upstream issue is fixed:
-# https://github.com/ben-manes/caffeine/issues/364.
-http_file(
- name = "caffeine-guava-renamed",
- downloaded_file_path = "caffeine-guava-" + CAFFEINE_VERS + ".jar",
- sha256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1",
- urls = [
- "https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/guava/" +
- CAFFEINE_VERS +
- "/guava-" +
- CAFFEINE_VERS +
- ".jar",
- ],
-)
-
maven_jar(
name = "jsch",
artifact = "com.jcraft:jsch:0.1.54",
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index a933dac..ca105f6 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -193,7 +193,13 @@
@RunWith(ConfigSuite.class)
public abstract class AbstractDaemonTest {
+
+ /**
+ * Test methods without special annotations will use a common server for efficiency reasons. The
+ * server is torn down after the test class is done.
+ */
private static GerritServer commonServer;
+
private static Description firstTest;
@ClassRule public static TemporaryFolder temporaryFolder = new TemporaryFolder();
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index e4a993c..135a80e 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -7,6 +7,75 @@
"testsuite/ThrowingFunction.java",
]
+DEPLOY_ENV = [
+ "//java/com/google/gerrit/exceptions",
+ "//java/com/google/gerrit/gpg",
+ "//java/com/google/gerrit/git",
+ "//java/com/google/gerrit/httpd/auth/openid",
+ "//java/com/google/gerrit/index:query_exception",
+ "//java/com/google/gerrit/launcher",
+ "//java/com/google/gerrit/lifecycle",
+ "//java/com/google/gerrit/pgm",
+ "//java/com/google/gerrit/pgm/http/jetty",
+ "//java/com/google/gerrit/pgm/util",
+ "//java/com/google/gerrit/common:annotations",
+ "//java/com/google/gerrit/common:server",
+ "//java/com/google/gerrit/entities",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/httpd",
+ "//java/com/google/gerrit/index",
+ "//java/com/google/gerrit/index/project",
+ "//java/com/google/gerrit/json",
+ "//java/com/google/gerrit/lucene",
+ "//java/com/google/gerrit/mail",
+ "//java/com/google/gerrit/metrics",
+ "//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/audit",
+ "//java/com/google/gerrit/server/git/receive",
+ "//java/com/google/gerrit/server/logging",
+ "//java/com/google/gerrit/server/restapi",
+ "//java/com/google/gerrit/server/schema",
+ "//java/com/google/gerrit/server/util/git",
+ "//java/com/google/gerrit/server/util/time",
+ "//java/com/google/gerrit/sshd",
+ "//lib/auto:auto-value",
+ "//lib/auto:auto-value-annotations",
+ "//lib:args4j",
+ "//lib:gson",
+ "//lib:guava-retrying",
+ "//lib:jgit",
+ "//lib:jsch",
+ "//lib/commons:compress",
+ "//lib/commons:lang",
+ "//lib/flogger:api",
+ "//lib/guice",
+ "//lib/guice:guice-assistedinject",
+ "//lib/guice:guice-servlet",
+ "//lib/jetty:servlet",
+ "//lib/mail",
+ "//lib/mina:sshd",
+ "//lib/log:impl-log4j",
+ "//lib/log:log4j",
+ "//lib:guava",
+ "//lib/bouncycastle:bcpg",
+ "//lib/bouncycastle:bcprov",
+ "//prolog:gerrit-prolog-common",
+]
+
+TEST_DEPS = [
+ "//java/com/google/gerrit/truth",
+ "//java/com/google/gerrit/acceptance/config",
+ "//java/com/google/gerrit/acceptance/testsuite/project",
+ "//java/com/google/gerrit/server/fixes/testing",
+ "//java/com/google/gerrit/server/group/testing",
+ "//java/com/google/gerrit/server/project/testing:project-test-util",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//java/com/google/gerrit/extensions/common/testing:common-test-util",
+ "//java/com/google/gerrit/extensions/restapi/testing:restapi-test-util",
+ "//java/com/google/gerrit/gpg/testing:gpg-test-util",
+ "//java/com/google/gerrit/git/testing",
+]
+
java_library(
name = "lib",
testonly = True,
@@ -15,61 +84,25 @@
visibility = ["//visibility:public"],
exports = [
":framework-lib",
- "//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
- "//java/com/google/gerrit/entities",
- "//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/extensions/common/testing:common-test-util",
- "//java/com/google/gerrit/extensions/restapi/testing:restapi-test-util",
- "//java/com/google/gerrit/git",
- "//java/com/google/gerrit/git/testing",
- "//java/com/google/gerrit/gpg/testing:gpg-test-util",
- "//java/com/google/gerrit/httpd",
- "//java/com/google/gerrit/index",
- "//java/com/google/gerrit/json",
- "//java/com/google/gerrit/launcher",
- "//java/com/google/gerrit/lucene",
- "//java/com/google/gerrit/mail",
- "//java/com/google/gerrit/metrics",
- "//java/com/google/gerrit/pgm",
- "//java/com/google/gerrit/pgm/init",
- "//java/com/google/gerrit/pgm/util",
- "//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/audit",
- "//java/com/google/gerrit/server/git/receive",
- "//java/com/google/gerrit/server/project/testing:project-test-util",
- "//java/com/google/gerrit/server/restapi",
- "//java/com/google/gerrit/sshd",
- "//java/com/google/gerrit/testing:gerrit-test-util",
- "//java/com/google/gerrit/truth",
- "//lib:args4j",
- "//lib:gson",
- "//lib:guava-retrying",
- "//lib:h2",
- "//lib:jgit",
- "//lib:jimfs",
- "//lib:jsch",
- "//lib:servlet-api-without-neverlink",
- "//lib/bouncycastle:bcpg",
- "//lib/bouncycastle:bcprov",
- "//lib/commons:compress",
- "//lib/flogger:api",
- "//lib/guice",
- "//lib/guice:guice-assistedinject",
- "//lib/guice:guice-servlet",
- "//lib/mina:sshd",
- "//prolog:gerrit-prolog-common",
- ],
+ ] + DEPLOY_ENV + TEST_DEPS,
)
java_binary(
name = "framework",
testonly = True,
+ deploy_env = [":framework-deploy-env"],
main_class = "Dummy",
visibility = ["//visibility:public"],
runtime_deps = [":framework-lib"],
)
+java_binary(
+ name = "framework-deploy-env",
+ testonly = True,
+ main_class = "Dummy",
+ runtime_deps = DEPLOY_ENV,
+)
+
java_library2(
name = "framework-lib",
testonly = True,
@@ -79,73 +112,19 @@
),
exported_deps = [
":function",
- "//java/com/google/gerrit/acceptance/config",
- "//java/com/google/gerrit/acceptance/testsuite/project",
- "//java/com/google/gerrit/exceptions",
- "//java/com/google/gerrit/gpg",
- "//java/com/google/gerrit/httpd/auth/openid",
- "//java/com/google/gerrit/index:query_exception",
- "//java/com/google/gerrit/launcher",
- "//java/com/google/gerrit/lifecycle",
- "//java/com/google/gerrit/pgm:daemon",
- "//java/com/google/gerrit/pgm/http/jetty",
- "//java/com/google/gerrit/pgm/util",
- "//java/com/google/gerrit/server/fixes/testing",
- "//java/com/google/gerrit/server/group/testing",
- "//java/com/google/gerrit/server/project/testing:project-test-util",
- "//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:guava",
"//lib:jgit-junit",
"//lib:jimfs",
- "//lib/auto:auto-value",
- "//lib/auto:auto-value-annotations",
+ "//lib:servlet-api",
"//lib/httpcomponents:fluent-hc",
"//lib/httpcomponents:httpclient",
"//lib/httpcomponents:httpcore",
- "//lib/jetty:servlet",
- "//lib/log:impl-log4j",
- "//lib/log:log4j",
"//lib/mockito",
"//lib/truth",
"//lib/truth:truth-java8-extension",
- "//prolog:gerrit-prolog-common",
- ],
- visibility = ["//visibility:public"],
- deps = [
- "//java/com/google/gerrit/common:annotations",
- "//java/com/google/gerrit/common:server",
- "//java/com/google/gerrit/entities",
- "//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/httpd",
- "//java/com/google/gerrit/index",
- "//java/com/google/gerrit/index/project",
- "//java/com/google/gerrit/json",
- "//java/com/google/gerrit/lucene",
- "//java/com/google/gerrit/mail",
- "//java/com/google/gerrit/metrics",
- "//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/audit",
- "//java/com/google/gerrit/server/git/receive",
- "//java/com/google/gerrit/server/logging",
- "//java/com/google/gerrit/server/restapi",
- "//java/com/google/gerrit/server/schema",
- "//java/com/google/gerrit/server/util/git",
- "//java/com/google/gerrit/server/util/time",
- "//java/com/google/gerrit/sshd",
- "//lib:args4j",
- "//lib:gson",
- "//lib:guava-retrying",
- "//lib:jgit",
- "//lib:jsch",
- "//lib:servlet-api",
- "//lib/commons:lang",
"//lib/greenmail",
- "//lib/guice",
- "//lib/guice:guice-assistedinject",
- "//lib/guice:guice-servlet",
- "//lib/mail",
- "//lib/mina:sshd",
- ],
+ ] + TEST_DEPS,
+ visibility = ["//visibility:public"],
+ deps = DEPLOY_ENV,
)
java_library(
diff --git a/java/com/google/gerrit/extensions/BUILD b/java/com/google/gerrit/extensions/BUILD
index 3683449..da5dc8b 100644
--- a/java/com/google/gerrit/extensions/BUILD
+++ b/java/com/google/gerrit/extensions/BUILD
@@ -26,7 +26,6 @@
],
)
-#TODO(davido): There is no provided_deps argument to java_library rule
java_library(
name = "api",
srcs = glob(["**/*.java"]),
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidationContext.java b/java/com/google/gerrit/extensions/validators/CommentValidationContext.java
new file mode 100644
index 0000000..1cb00e3
--- /dev/null
+++ b/java/com/google/gerrit/extensions/validators/CommentValidationContext.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 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 validators context in order to pass it to a validation plugin.
+ *
+ * <p>This is used to provided additional context around that comment that can be used by the
+ * validator to determine what validations should be run. For example, a comment validator may only
+ * want to validate a comment if it's on a change in the project foo.
+ *
+ * @see CommentValidator
+ */
+@AutoValue
+public abstract class CommentValidationContext {
+
+ /** Returns the change id the comment is being added to. */
+ public abstract int getChangeId();
+
+ /** Returns the project the comment is being added to. */
+ public abstract String getProject();
+
+ public static Builder builder() {
+ return new AutoValue_CommentValidationContext.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder changeId(int value);
+
+ public abstract Builder project(String value);
+
+ public abstract CommentValidationContext build();
+ }
+}
diff --git a/java/com/google/gerrit/extensions/validators/CommentValidator.java b/java/com/google/gerrit/extensions/validators/CommentValidator.java
index cfefdef..ba73e46 100644
--- a/java/com/google/gerrit/extensions/validators/CommentValidator.java
+++ b/java/com/google/gerrit/extensions/validators/CommentValidator.java
@@ -30,5 +30,5 @@
* @return An empty list if all comments are valid, or else a list of validation failures.
*/
ImmutableList<CommentValidationFailure> validateComments(
- ImmutableList<CommentForValidation> comments);
+ CommentValidationContext ctx, ImmutableList<CommentForValidation> comments);
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index d9bb3b9..722955c 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -678,17 +678,24 @@
cause = Optional.of(e);
statusCode = SC_INTERNAL_SERVER_ERROR;
+ Optional<ExceptionHook.Status> status = getStatus(e);
+ statusCode = status.map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
+
if (res.isCommitted()) {
- logger.atSevere().withCause(e).log(
- "Error in %s %s, response already committed", req.getMethod(), uriForLogging(req));
responseBytes = 0;
+ if (statusCode == SC_INTERNAL_SERVER_ERROR) {
+ logger.atSevere().withCause(e).log(
+ "Error in %s %s, response already committed", req.getMethod(), uriForLogging(req));
+ } else {
+ logger.atWarning().log(
+ "Response for %s %s already committed, wanted to set status %d",
+ req.getMethod(), uriForLogging(req), statusCode);
+ }
} else {
res.reset();
traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- Optional<ExceptionHook.Status> status = getStatus(e);
if (status.isPresent()) {
- statusCode = status.get().statusCode();
responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
}
responseBytes = replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
@@ -1182,8 +1189,14 @@
}
return OutputFormat.JSON.newGson().fromJson(json, type);
} finally {
- // Reader.close won't consume the rest of the input. Explicitly consume the request body.
- br.skip(Long.MAX_VALUE);
+ try {
+ // Reader.close won't consume the rest of the input. Explicitly consume the request
+ // body.
+ br.skip(Long.MAX_VALUE);
+ } catch (Exception e) {
+ // ignore, e.g. trying to consume the rest of the input may fail if the request was
+ // cancelled
+ }
}
}
}
@@ -1786,13 +1799,15 @@
return replyError(req, res, status.statusCode(), msg.toString(), err);
}
- private static long replyInternalServerError(
+ private long replyInternalServerError(
HttpServletRequest req,
HttpServletResponse res,
Throwable err,
ImmutableList<String> userMessages)
throws IOException {
- logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uriForLogging(req));
+ logger.atSevere().withCause(err).log(
+ "Error in %s %s: %s",
+ req.getMethod(), uriForLogging(req), globals.retryHelper.formatCause(err));
StringBuilder msg = new StringBuilder("Internal server error");
if (!userMessages.isEmpty()) {
diff --git a/java/com/google/gerrit/pgm/BUILD b/java/com/google/gerrit/pgm/BUILD
index 08916a3..03b7c3d 100644
--- a/java/com/google/gerrit/pgm/BUILD
+++ b/java/com/google/gerrit/pgm/BUILD
@@ -1,17 +1,7 @@
load("@rules_java//java:defs.bzl", "java_library")
-# TODO(davido): This indirection doesn't avoid unwanted depdencies
-# in acceptance-framework and should be removed. Instead, provided_deps
-# should be used, once https://github.com/bazelbuild/bazel/issues/1402
-# is fixed.
-alias(
- name = "pgm",
- actual = ":daemon",
- visibility = ["//visibility:public"],
-)
-
java_library(
- name = "daemon",
+ name = "pgm",
srcs = glob(["**/*.java"]),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/pgm"],
diff --git a/java/com/google/gerrit/server/ExceptionHookImpl.java b/java/com/google/gerrit/server/ExceptionHookImpl.java
index 8393451..5a3f077 100644
--- a/java/com/google/gerrit/server/ExceptionHookImpl.java
+++ b/java/com/google/gerrit/server/ExceptionHookImpl.java
@@ -20,6 +20,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.git.LockFailureException;
import java.util.Optional;
+import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.RefUpdate;
/**
@@ -42,6 +43,9 @@
if (isLockFailure(throwable)) {
return Optional.of(RefUpdate.Result.LOCK_FAILURE.name());
}
+ if (isMissingObjectException(throwable)) {
+ return Optional.of("missing_object");
+ }
return Optional.empty();
}
@@ -65,6 +69,10 @@
return isMatching(throwable, t -> t instanceof LockFailureException);
}
+ private static boolean isMissingObjectException(Throwable throwable) {
+ return isMatching(throwable, t -> t instanceof MissingObjectException);
+ }
+
/**
* Check whether the given exception or any of its causes matches the given predicate.
*
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index 09042ab..3d34d6b 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -118,16 +119,18 @@
/**
* Helper to run the specified set of {@link CommentValidator}-s on the specified comments.
*
- * @return See {@link CommentValidator#validateComments(ImmutableList)}.
+ * @return See {@link CommentValidator#validateComments(CommentValidationContext,ImmutableList)}.
*/
public static ImmutableList<CommentValidationFailure> findInvalidComments(
+ CommentValidationContext ctx,
PluginSetContext<CommentValidator> commentValidators,
ImmutableList<CommentForValidation> commentsForValidation) {
ImmutableList.Builder<CommentValidationFailure> commentValidationFailures =
new ImmutableList.Builder<>();
commentValidators.runEach(
validator ->
- commentValidationFailures.addAll(validator.validateComments(commentsForValidation)));
+ commentValidationFailures.addAll(
+ validator.validateComments(ctx, commentsForValidation)));
return commentValidationFailures.build();
}
}
diff --git a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
index 8c9ebeb..1bd8b45 100644
--- a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
+++ b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
@@ -95,7 +95,6 @@
return Iterables.getOnlyElement(result);
}
- @SuppressWarnings("deprecation")
@Override
public Map<String, Ref> getRefs(String prefix) throws IOException {
List<Ref> refs = getDelegate().getRefDatabase().getRefsByPrefix(prefix);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 3d531b2..d8aa054 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -93,6 +93,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.ApprovalsUtil;
@@ -2014,8 +2015,13 @@
: CommentType.FILE_COMMENT,
comment.message))
.collect(toImmutableList());
+ CommentValidationContext ctx =
+ CommentValidationContext.builder()
+ .changeId(change.getChangeId())
+ .project(change.getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> commentValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ PublishCommentUtil.findInvalidComments(ctx, commentValidators, draftsForValidation);
magicBranch.setCommentsValid(commentValidationFailures.isEmpty());
commentValidationFailures.forEach(
failure ->
@@ -3337,7 +3343,8 @@
}
logger.atFine().log(
- "Auto-closing %d changes with existing patch sets and %d with new patch sets",
+ "Auto-closing %d changes with existing patch sets and %d with new patch"
+ + " sets",
existingPatchSets, newPatchSets);
bu.execute();
} catch (IOException | StorageException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
index 8237e69..3a8bcac 100644
--- a/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
+++ b/java/com/google/gerrit/server/git/validators/CommentLimitsValidator.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.validators.CommentForValidation;
+import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -33,7 +34,7 @@
@Override
public ImmutableList<CommentValidationFailure> validateComments(
- ImmutableList<CommentForValidation> comments) {
+ CommentValidationContext ctx, ImmutableList<CommentForValidation> comments) {
return comments.stream()
.filter(c -> c.getText().length() > maxCommentLength)
.map(
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 71d8c15..e79696a 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -34,6 +34,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.HtmlParser;
@@ -287,8 +288,14 @@
MAIL_COMMENT_TYPE_TO_VALIDATION_TYPE.get(comment.getType()),
comment.getMessage()))
.collect(ImmutableList.toImmutableList());
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(cd.change().getChangeId())
+ .project(cd.change().getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> commentValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, parsedCommentsForValidation);
+ PublishCommentUtil.findInvalidComments(
+ commentValidationCtx, commentValidators, parsedCommentsForValidation);
if (!commentValidationFailures.isEmpty()) {
sendRejectionEmail(message, InboundEmailRejectionSender.Error.COMMENT_REJECTED);
return;
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 03c2fc4..324069d 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -77,6 +77,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.json.OutputFormat;
@@ -999,16 +1000,22 @@
}
}
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(ctx.getChange().getChangeId())
+ .project(ctx.getChange().getProject().get())
+ .build();
switch (in.drafts) {
case PUBLISH:
case PUBLISH_ALL_REVISIONS:
- validateComments(Streams.concat(drafts.values().stream(), toPublish.stream()));
+ validateComments(
+ commentValidationCtx, Streams.concat(drafts.values().stream(), toPublish.stream()));
publishCommentUtil.publish(ctx, ctx.getUpdate(psId), drafts.values(), in.tag);
comments.addAll(drafts.values());
break;
case KEEP:
default:
- validateComments(toPublish.stream());
+ validateComments(commentValidationCtx, toPublish.stream());
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
@@ -1017,7 +1024,8 @@
return !toPublish.isEmpty();
}
- private void validateComments(Stream<Comment> comments) throws CommentsRejectedException {
+ private void validateComments(CommentValidationContext ctx, Stream<Comment> comments)
+ throws CommentsRejectedException {
ImmutableList<CommentForValidation> draftsForValidation =
comments
.map(
@@ -1029,7 +1037,7 @@
comment.message))
.collect(toImmutableList());
ImmutableList<CommentValidationFailure> draftValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ PublishCommentUtil.findInvalidComments(ctx, commentValidators, draftsForValidation);
if (!draftValidationFailures.isEmpty()) {
throw new CommentsRejectedException(draftValidationFailures);
}
@@ -1415,8 +1423,14 @@
buf.append(String.format("\n\n(%d comments)", comments.size()));
}
if (!msg.isEmpty()) {
+ CommentValidationContext commentValidationCtx =
+ CommentValidationContext.builder()
+ .changeId(ctx.getChange().getChangeId())
+ .project(ctx.getChange().getProject().get())
+ .build();
ImmutableList<CommentValidationFailure> messageValidationFailure =
PublishCommentUtil.findInvalidComments(
+ commentValidationCtx,
commentValidators,
ImmutableList.of(
CommentForValidation.create(
diff --git a/java/com/google/gerrit/server/restapi/change/PutDescription.java b/java/com/google/gerrit/server/restapi/change/PutDescription.java
index d84ab3e..f442a42 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDescription.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDescription.java
@@ -87,17 +87,21 @@
if (oldDescription.equals(newDescription)) {
return false;
}
- String summary;
- if (oldDescription.isEmpty()) {
- summary = "Description set to \"" + newDescription + "\"";
- } else if (newDescription.isEmpty()) {
- summary = "Description \"" + oldDescription + "\" removed";
- } else {
- summary = "Description changed to \"" + newDescription + "\"";
- }
-
update.setPsDescription(newDescription);
+ String summary;
+ if (oldDescription.isEmpty()) {
+ summary =
+ String.format("Description of patch set %d set to \"%s\"", psId.get(), newDescription);
+ } else if (newDescription.isEmpty()) {
+ summary =
+ String.format(
+ "Description \"%s\" removed from patch set %d", oldDescription, psId.get());
+ } else {
+ summary =
+ String.format(
+ "Description of patch set %d changed to \"%s\"", psId.get(), newDescription);
+ }
ChangeMessage cmsg =
ChangeMessagesUtil.newMessage(
psId, ctx.getUser(), ctx.getWhen(), summary, ChangeMessagesUtil.TAG_SET_DESCRIPTION);
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
index f07d815..39df82d 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -22,7 +22,6 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.ApprovalsUtil;
@@ -202,23 +201,25 @@
private Map<Account.Id, MutableDouble> baseRanking(
double baseWeight, String query, List<Account.Id> candidateList)
throws IOException, ConfigInvalidException {
- // Get the user's last 25 changes, check approvals
+ int numberOfRelevantChanges = config.getInt("suggest", "relevantChanges", 50);
+ // Get the user's last 25 changes, check reviewers
try {
List<ChangeData> result =
queryProvider
.get()
- .setLimit(25)
- .setRequestedFields(ChangeField.APPROVAL)
+ .setLimit(numberOfRelevantChanges)
+ .setRequestedFields(ChangeField.REVIEWER)
.query(changeQueryBuilder.owner("self"));
Map<Account.Id, MutableDouble> suggestions = new LinkedHashMap<>();
// Put those candidates at the bottom of the list
candidateList.stream().forEach(id -> suggestions.put(id, new MutableDouble(0)));
for (ChangeData cd : result) {
- for (PatchSetApproval approval : cd.currentApprovals()) {
- Account.Id id = approval.accountId();
- if (Strings.isNullOrEmpty(query) || accountMatchesQuery(id, query)) {
- suggestions.computeIfAbsent(id, (ignored) -> new MutableDouble(0)).add(baseWeight);
+ for (Account.Id reviewer : cd.reviewers().all()) {
+ if (Strings.isNullOrEmpty(query) || accountMatchesQuery(reviewer, query)) {
+ suggestions
+ .computeIfAbsent(reviewer, (ignored) -> new MutableDouble(0))
+ .add(baseWeight);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index 7156c8d..524a05e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -37,6 +38,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.server.update.CommentsRejectedException;
@@ -80,14 +82,17 @@
@Test
public void validateCommentsInInput_commentOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
-
ReviewInput input = new ReviewInput();
CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
comment.updated = new Timestamp(0);
@@ -101,14 +106,17 @@
@Test
public void validateCommentsInInput_commentRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
-
ReviewInput input = new ReviewInput();
CommentInput comment = newComment(r.getChange().currentFilePaths().get(0));
comment.updated = new Timestamp(0);
@@ -151,14 +159,17 @@
@Test
public void validateDrafts_draftOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
-
DraftInput draft =
testCommentHelper.newDraft(
r.getChange().currentFilePaths().get(0), Side.REVISION, 1, COMMENT_TEXT);
@@ -174,14 +185,18 @@
@Test
public void validateDrafts_draftRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.INLINE_COMMENT, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
DraftInput draft =
testCommentHelper.newDraft(
@@ -218,7 +233,8 @@
testCommentHelper.addDraft(r.getChangeId(), r.getCommit().getName(), draftFile);
assertThat(testCommentHelper.getPublishedComments(r.getChangeId())).isEmpty();
- when(mockCommentValidator.validateComments(capture.capture())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), capture.capture()))
+ .thenReturn(ImmutableList.of());
ReviewInput input = new ReviewInput();
input.drafts = DraftHandling.PUBLISH;
@@ -236,11 +252,15 @@
@Test
public void validateCommentsInChangeMessage_messageOK() throws Exception {
+ PushOneCommit.Result r = createChange();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- PushOneCommit.Result r = createChange();
ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
int numMessages = gApi.changes().id(r.getChangeId()).get().messages.size();
@@ -253,13 +273,17 @@
@Test
public void validateCommentsInChangeMessage_messageRejected() throws Exception {
+ PushOneCommit.Result r = createChange();
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(r.getChange().getId().get())
+ .project(r.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- PushOneCommit.Result r = createChange();
ReviewInput input = new ReviewInput().message(COMMENT_TEXT);
assertThat(gApi.changes().id(r.getChangeId()).get().messages)
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index c4dec31..e6b2190 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1320,10 +1320,24 @@
public void description() throws Exception {
PushOneCommit.Result r = createChange();
assertDescription(r, "");
- gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("test");
- assertDescription(r, "test");
+
+ // set description
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("foo");
+ assertDescription(r, "foo");
+ assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message)
+ .isEqualTo("Description of patch set 1 set to \"foo\"");
+
+ // update description
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("bar");
+ assertDescription(r, "bar");
+ assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message)
+ .isEqualTo("Description of patch set 1 changed to \"bar\"");
+
+ // remove description
gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("");
assertDescription(r, "");
+ assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message)
+ .isEqualTo("Description \"bar\" removed from patch set 1");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 568c63b..1bd2d99 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -39,7 +39,6 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
@@ -397,19 +396,13 @@
requestScopeOperations.setApiUser(user1.id());
String changeId1 = createChangeFromApi();
- requestScopeOperations.setApiUser(reviewer1.id());
- reviewChange(changeId1);
+ reviewChange(changeId1, reviewer1);
- requestScopeOperations.setApiUser(user1.id());
String changeId2 = createChangeFromApi();
- requestScopeOperations.setApiUser(reviewer1.id());
- reviewChange(changeId2);
+ reviewChange(changeId2, reviewer1);
+ reviewChange(changeId2, reviewer2);
- requestScopeOperations.setApiUser(reviewer2.id());
- reviewChange(changeId2);
-
- requestScopeOperations.setApiUser(user1.id());
String changeId3 = createChangeFromApi();
List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId3, null, 4);
assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList()))
@@ -440,13 +433,11 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- requestScopeOperations.setApiUser(foo1.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo1);
assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue();
TestAccount foo2 = accountCreator.create(name + "-2");
- requestScopeOperations.setApiUser(foo2.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo2);
assertThat(gApi.accounts().id(foo2.username()).getActive()).isTrue();
assertReviewers(
@@ -466,12 +457,10 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- requestScopeOperations.setApiUser(foo1.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo1);
TestAccount foo2 = accountCreator.create(name + "-2");
- requestScopeOperations.setApiUser(foo2.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo2);
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
@@ -488,12 +477,10 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- requestScopeOperations.setApiUser(foo1.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo1);
TestAccount foo2 = accountCreator.create(name + "-2");
- requestScopeOperations.setApiUser(foo2.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo2);
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
@@ -514,12 +501,10 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- requestScopeOperations.setApiUser(foo1.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo1);
TestAccount foo2 = accountCreator.create(name + "-2");
- requestScopeOperations.setApiUser(foo2.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, foo2);
assertReviewers(suggestCcs(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
@@ -575,8 +560,7 @@
String changeIdReviewed = createChangeFromApi();
TestAccount reviewer = accountCreator.create("newReviewer");
- requestScopeOperations.setApiUser(reviewer.id());
- reviewChange(changeIdReviewed);
+ reviewChange(changeIdReviewed, reviewer);
List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, "new", 4);
assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList()))
@@ -624,10 +608,8 @@
return user(name, fullName, name);
}
- private void reviewChange(String changeId) throws RestApiException {
- ReviewInput ri = new ReviewInput();
- ri.label("Code-Review", 1);
- gApi.changes().id(changeId).current().review(ri);
+ private void reviewChange(String changeId, TestAccount reviewer) throws RestApiException {
+ gApi.changes().id(changeId).addReviewer(reviewer.id().toString());
}
private String createChangeFromApi() throws RestApiException {
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
index d8b65b7..ccfe783 100644
--- a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
@@ -32,6 +32,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
@@ -53,6 +54,7 @@
private static final String COMMENT_TEXT = "The comment text";
@Captor private ArgumentCaptor<ImmutableList<CommentForValidation>> capture;
+ @Captor private ArgumentCaptor<CommentValidationContext> captureCtx;
@Override
public Module createModule() {
@@ -76,14 +78,18 @@
@Test
public void validateComments_commentOK() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(result.getChange().getId().get())
+ .project(result.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of());
- 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();
@@ -97,14 +103,18 @@
public void validateComments_commentRejected() throws Exception {
CommentForValidation commentForValidation =
CommentForValidation.create(CommentType.FILE_COMMENT, COMMENT_TEXT);
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ String revId = result.getCommit().getName();
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder()
+ .changeId(result.getChange().getId().get())
+ .project(result.getChange().project().get())
+ .build(),
ImmutableList.of(
CommentForValidation.create(
CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- 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();
@@ -116,7 +126,8 @@
@Test
public void validateComments_inlineVsFileComments_allOK() throws Exception {
- when(mockCommentValidator.validateComments(capture.capture())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(captureCtx.capture(), capture.capture()))
+ .thenReturn(ImmutableList.of());
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
String revId = result.getCommit().getName();
@@ -132,6 +143,9 @@
assertThat(capture.getAllValues()).hasSize(1);
+ assertThat(captureCtx.getValue().getProject()).isEqualTo(result.getChange().project().get());
+ assertThat(captureCtx.getValue().getChangeId()).isEqualTo(result.getChange().getId().get());
+
assertThat(capture.getAllValues().get(0))
.containsExactly(
CommentForValidation.create(
@@ -143,7 +157,7 @@
@Test
@GerritConfig(name = "change.maxCommentLength", value = "" + MAX_COMMENT_LENGTH)
public void validateComments_enforceLimits_commentTooLarge() throws Exception {
- when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), any())).thenReturn(ImmutableList.of());
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
int commentLength = MAX_COMMENT_LENGTH + 1;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 5531709..2409f52 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -29,6 +29,7 @@
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.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.mail.MailMessage;
import com.google.gerrit.mail.MailProcessingUtil;
@@ -70,7 +71,7 @@
@BeforeClass
public static void setUpMock() {
// Let the mock comment validator accept all comments during test setup.
- when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ when(mockCommentValidator.validateComments(any(), any())).thenReturn(ImmutableList.of());
}
@Before
@@ -274,7 +275,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.CHANGE_MESSAGE);
+ setupFailValidation(
+ CommentForValidation.CommentType.CHANGE_MESSAGE, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", COMMENT_TEXT, null, null, null);
@@ -298,7 +300,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.INLINE_COMMENT);
+ setupFailValidation(
+ CommentForValidation.CommentType.INLINE_COMMENT, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, COMMENT_TEXT, null, null);
@@ -322,7 +325,8 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- setupFailValidation(CommentForValidation.CommentType.FILE_COMMENT);
+ setupFailValidation(
+ CommentForValidation.CommentType.FILE_COMMENT, changeInfo.project, changeInfo._number);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, null, COMMENT_TEXT, null);
@@ -341,10 +345,12 @@
return canonicalWebUrl.get() + "c/" + changeInfo.project + "/+/" + changeInfo._number;
}
- private void setupFailValidation(CommentForValidation.CommentType type) {
+ private void setupFailValidation(
+ CommentForValidation.CommentType type, String failProject, int failChange) {
CommentForValidation commentForValidation = CommentForValidation.create(type, COMMENT_TEXT);
when(mockCommentValidator.validateComments(
+ CommentValidationContext.builder().changeId(failChange).project(failProject).build(),
ImmutableList.of(CommentForValidation.create(type, COMMENT_TEXT))))
.thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
}
diff --git a/javatests/com/google/gerrit/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index eb0bf25..0cae937 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -5,9 +5,7 @@
name = "pgm_tests",
srcs = glob(["**/*.java"]),
deps = [
- "//java/com/google/gerrit/pgm",
"//java/com/google/gerrit/pgm/http",
- "//java/com/google/gerrit/pgm/init",
"//java/com/google/gerrit/pgm/init/api",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/securestore/testing",
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index dcf40f0..336fcbf 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -20,19 +20,12 @@
httpcore-nio
j2objc
jackson-core
-javassist
jna
jruby
mina-core
nekohtml
objenesis
openid-consumer
-powermock-api-easymock
-powermock-api-support
-powermock-core
-powermock-module-junit4
-powermock-module-junit4-common
-powermock-reflect
sshd
sshd-common
sshd-mina
diff --git a/lib/truth/BUILD b/lib/truth/BUILD
index bb30945..dc1d802 100644
--- a/lib/truth/BUILD
+++ b/lib/truth/BUILD
@@ -2,6 +2,7 @@
java_library(
name = "truth",
+ testonly = True,
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:public"],
exports = ["@truth//jar"],
@@ -14,6 +15,7 @@
java_library(
name = "truth-java8-extension",
+ testonly = True,
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:public"],
exports = ["@truth-java8-extension//jar"],
@@ -25,6 +27,7 @@
java_library(
name = "truth-liteproto-extension",
+ testonly = True,
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:private"],
exports = ["@truth-liteproto-extension//jar"],
@@ -37,6 +40,7 @@
java_library(
name = "diffutils",
+ testonly = True,
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:private"],
exports = ["@diffutils//jar"],
@@ -44,6 +48,7 @@
java_library(
name = "truth-proto-extension",
+ testonly = True,
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:public"],
exports = [
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 f4bd6a6..283bd74 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
@@ -205,6 +205,9 @@
<gr-confirm-revert-dialog id="confirmRevertDialog"
class="confirmDialog"
on-confirm="_handleRevertDialogConfirm"
+ commit-message="[[commitMessage]]"
+ change="[[change]]"
+ changes="[[_revertChanges]]"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-revert-dialog>
<gr-confirm-revert-submission-dialog id="confirmRevertSubmissionDialog"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 2c4ca82..a4ed899 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -192,6 +192,11 @@
const AWAIT_CHANGE_ATTEMPTS = 5;
const AWAIT_CHANGE_TIMEOUT_MS = 1000;
+ const REVERT_TYPES = {
+ REVERT_SINGLE_CHANGE: 1,
+ REVERT_SUBMISSION: 2,
+ };
+
/**
* @appliesMixin Gerrit.FireMixin
* @appliesMixin Gerrit.PatchSetMixin
@@ -421,6 +426,7 @@
type: Boolean,
value: true,
},
+ _revertChanges: Array,
};
}
@@ -915,16 +921,13 @@
return null;
}
- _modifyRevertMsg() {
- return this.$.jsAPI.modifyRevertMsg(this.change,
- this.$.confirmRevertDialog.message, this.commitMessage);
- }
-
showRevertDialog() {
- this.$.confirmRevertDialog.populateRevertMessage(
- this.commitMessage, this.change.current_revision);
- this.$.confirmRevertDialog.message = this._modifyRevertMsg();
- this._showActionDialog(this.$.confirmRevertDialog);
+ const query = 'submissionid:' + this.change.submission_id;
+ this.$.restAPI.getChanges('', query)
+ .then(changes => {
+ this._revertChanges = changes;
+ this._showActionDialog(this.$.confirmRevertDialog);
+ });
}
showRevertSubmissionDialog() {
@@ -932,7 +935,7 @@
this.$.restAPI.getChanges('', query)
.then(changes => {
this.$.confirmRevertSubmissionDialog.
- populateRevertSubmissionMessage(
+ _populateRevertSubmissionMessage(
this.commitMessage, this.change, changes);
this._showActionDialog(this.$.confirmRevertSubmissionDialog);
});
@@ -1143,20 +1146,24 @@
);
}
- _handleRevertDialogConfirm() {
+ _handleRevertDialogConfirm(e) {
+ const revertType = e.detail.revertType;
+ const message = e.detail.message;
const el = this.$.confirmRevertDialog;
this.$.overlay.close();
el.hidden = true;
- this._fireAction('/revert', this.actions.revert, false,
- {message: el.message});
- }
-
- _handleRevertSubmissionDialogConfirm() {
- const el = this.$.confirmRevertSubmissionDialog;
- this.$.overlay.close();
- el.hidden = true;
- this._fireAction('/revert_submission', this.actions.revert_submission,
- false, {message: el.message});
+ switch (revertType) {
+ case REVERT_TYPES.REVERT_SINGLE_CHANGE:
+ this._fireAction('/revert', this.actions.revert, false,
+ {message});
+ break;
+ case REVERT_TYPES.REVERT_SUBMISSION:
+ this._fireAction('/revert_submission', this.actions.revert_submission,
+ false, {message});
+ break;
+ default:
+ console.error('invalid revert type');
+ }
}
_handleAbandonDialogConfirm() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 1c894ee..9c1a527 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -37,6 +37,7 @@
</test-fixture>
<script>
+ // TODO(dhruvsri): remove use of _populateRevertMessage as it's private
suite('gr-change-actions tests', () => {
let element;
let sandbox;
@@ -795,12 +796,12 @@
});
suite('revert change', () => {
- let alertStub;
let fireActionStub;
setup(() => {
fireActionStub = sandbox.stub(element, '_fireAction');
- alertStub = sandbox.stub(window, 'alert');
+ element.commitMessage = 'random commit message';
+ element.change.current_revision = 'abcdef';
element.actions = {
revert: {
method: 'POST',
@@ -813,50 +814,149 @@
});
test('revert change with plugin hook', done => {
+ const newRevertMsg = 'Modified revert msg';
+ sandbox.stub(element.$.confirmRevertDialog, '_modifyRevertMsg',
+ () => newRevertMsg);
element.change = {
current_revision: 'abc1234',
};
- const newRevertMsg = 'Modified revert msg';
- sandbox.stub(element, '_modifyRevertMsg',
- () => newRevertMsg);
- sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
- () => 'original msg');
+ sandbox.stub(element.$.confirmRevertDialog,
+ '_populateRevertSubmissionMessage', () => 'original msg');
flush(() => {
- const revertButton =
- element.$$('gr-button[data-action-key="revert"]');
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
MockInteractions.tap(revertButton);
-
- assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
- done();
+ flush(() => {
+ assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
+ done();
+ });
});
});
- test('works', () => {
- element.change = {
- current_revision: 'abc1234',
- };
- sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
- () => 'original msg');
- const revertButton = element.$$('gr-button[data-action-key="revert"]');
- MockInteractions.tap(revertButton);
+ suite('revert change submitted together', () => {
+ setup(() => {
+ element.change = {
+ submission_id: '199',
+ current_revision: '2000',
+ };
+ sandbox.stub(element.$.restAPI, 'getChanges')
+ .returns(Promise.resolve([
+ {change_id: '12345678901234', topic: 'T', subject: 'random'},
+ {change_id: '23456', topic: 'T', subject: 'a'.repeat(100)},
+ ]));
+ });
- element.$.confirmRevertDialog.message = 'foo message';
- element._handleRevertDialogConfirm();
- assert.notOk(alertStub.called);
+ test('confirm revert dialog shows both options', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const revertSingleChangeLabel = confirmRevertDialog
+ .shadowRoot.querySelector('.revertSingleChange');
+ const revertSubmissionLabel = confirmRevertDialog.
+ shadowRoot.querySelector('.revertSubmission');
+ assert(revertSingleChangeLabel.innerText.trim() ===
+ 'Revert single change');
+ assert(revertSubmissionLabel.innerText.trim() ===
+ 'Revert entire submission (2 Changes)');
+ let expectedMsg = 'Revert submission 199' + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+ 'Reverted Changes:' + '\n' +
+ '1234567890:random' + '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ assert.equal(confirmRevertDialog.message, expectedMsg);
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ expectedMsg = 'Revert "random commit message"\n\nThis reverts '
+ + 'commit 2000.\n\nReason'
+ + ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, expectedMsg);
+ done();
+ });
+ });
+ });
- const action = {
- __key: 'revert',
- __type: 'change',
- __primary: false,
- enabled: true,
- label: 'Revert',
- method: 'POST',
- title: 'Revert the change',
- };
- assert.deepEqual(fireActionStub.lastCall.args, [
- '/revert', action, false, {
- message: 'foo message',
- }]);
+ test('message modification is retained on switching', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ const revertSubmissionMsg = 'Revert submission 199' + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+ 'Reverted Changes:' + '\n' +
+ '1234567890:random' + '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ const singleChangeMsg =
+ 'Revert "random commit message"\n\nThis reverts '
+ + 'commit 2000.\n\nReason'
+ + ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, revertSubmissionMsg);
+ const newRevertMsg = revertSubmissionMsg + 'random';
+ const newSingleChangeMsg = singleChangeMsg + 'random';
+ confirmRevertDialog.message = newRevertMsg;
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, singleChangeMsg);
+ confirmRevertDialog.message = newSingleChangeMsg;
+ MockInteractions.tap(radioInputs[1]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, newRevertMsg);
+ MockInteractions.tap(radioInputs[0]);
+ flush(() => {
+ assert.equal(confirmRevertDialog.message, newSingleChangeMsg);
+ done();
+ });
+ });
+ });
+ });
+ });
+ });
+
+ suite('revert single change', () => {
+ setup(() => {
+ element.change = {
+ submission_id: '199',
+ current_revision: '2000',
+ };
+ sandbox.stub(element.$.restAPI, 'getChanges')
+ .returns(Promise.resolve([
+ {change_id: '12345678901234', topic: 'T', subject: 'random'},
+ ]));
+ });
+
+ test('confirm revert dialog shows one radio button', done => {
+ const revertButton = element.shadowRoot
+ .querySelector('gr-button[data-action-key="revert"]');
+ MockInteractions.tap(revertButton);
+ flush(() => {
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const radioInputs = confirmRevertDialog.shadowRoot
+ .querySelectorAll('input[name="revertOptions"]');
+ assert.equal(radioInputs.length, 1);
+ const msg = 'Revert "random commit message"\n\n'
+ + 'This reverts commit 2000.\n\nReason '
+ + 'for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog.message, msg);
+ const confirmButton = element.$.confirmRevertDialog.shadowRoot
+ .querySelector('gr-dialog')
+ .shadowRoot.querySelector('#confirm');
+ MockInteractions.tap(confirmButton);
+ flush(() => {
+ assert.equal(fireActionStub.getCall(0).args[0], '/revert');
+ assert.equal(fireActionStub.getCall(0).args[1].__key, 'revert');
+ assert.equal(fireActionStub.getCall(0).args[3].message, msg);
+ done();
+ });
+ });
+ });
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
index fc3a8c1..7bffe8a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
@@ -21,6 +21,7 @@
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<dom-module id="gr-confirm-revert-dialog">
<template>
@@ -37,6 +38,13 @@
display: block;
width: 100%;
}
+ .revertSubmissionLayout {
+ display: flex;
+ }
+ .label {
+ margin-left: var(--spacing-m);
+ margin-bottom: var(--spacing-m);
+ }
iron-autogrow-textarea {
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
@@ -50,7 +58,45 @@
on-cancel="_handleCancelTap">
<div class="header" slot="header">Revert Merged Change</div>
<div class="main" slot="main">
+ <div class="revertSubmissionLayout">
+ <input
+ name="revertOptions"
+ type="radio"
+ id="revertSingleChange"
+ on-change="_handleRevertSingleChangeClicked"
+ checked="[[_computeIfSingleRevert(_revertType)]]">
+ <label for="revertSingleChange" class="label revertSingleChange">
+ Revert single change
+ </label>
+ </div>
+ <template is="dom-if" if="[[_showRevertSubmission]]">
+ <div on-click="_handleRevertSubmissionClicked" class="revertSubmissionLayout">
+ <input
+ name="revertOptions"
+ type="radio"
+ id="revertSubmission"
+ checked="[[_computeIfRevertSubmission(_revertType)]]">
+ <label for="revertSubmission" class="label revertSubmission">
+ Revert entire submission ([[changes.length]] Changes)
+ </label>
+ </template>
<gr-endpoint-decorator name="confirm-revert-change">
+ <!-- Duplicating the text-area as a plugin in the case of a single
+ revert will override the entire textarea which should not happen
+ for multiple revert -->
+ <template is="dom-if" if="[[_computeIfSingleRevert(_revertType)]]">
+ <label for="messageInput">
+ Revert Commit Message
+ </label>
+ <iron-autogrow-textarea
+ id="messageInput"
+ class="message"
+ autocomplete="on"
+ max-rows="15"
+ bind-value="{{message}}"></iron-autogrow-textarea>
+ </template>
+ </gr-endpoint-decorator>
+ <template is="dom-if" if="[[_computeIfRevertSubmission(_revertType)]]">
<label for="messageInput">
Revert Commit Message
</label>
@@ -60,9 +106,10 @@
autocomplete="on"
max-rows="15"
bind-value="{{message}}"></iron-autogrow-textarea>
- </gr-endpoint-decorator>
+ </template>
</div>
</gr-dialog>
+ <gr-js-api-interface id="jsAPI"></gr-js-api-interface>
</template>
<script src="gr-confirm-revert-dialog.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
index bf727ec..438440e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
@@ -19,6 +19,13 @@
const ERR_COMMIT_NOT_FOUND =
'Unable to find the commit hash of this change.';
+ const CHANGE_SUBJECT_LIMIT = 50;
+
+ // TODO(dhruvsri): clean up repeated definitions after moving to js modules
+ const REVERT_TYPES = {
+ REVERT_SINGLE_CHANGE: 1,
+ REVERT_SUBMISSION: 2,
+ };
/**
* @appliesMixin Gerrit.FireMixin
@@ -45,12 +52,55 @@
static get properties() {
return {
message: String,
+ _revertType: {
+ type: Number,
+ value: REVERT_TYPES.REVERT_SINGLE_CHANGE,
+ },
+ _showRevertSubmission: {
+ type: Boolean,
+ value: false,
+ },
+ changes: {
+ type: Array,
+ value() { return []; },
+ },
+ change: Object,
+ commitMessage: String,
};
}
- populateRevertMessage(message, commitHash) {
+ static get observers() {
+ return [
+ 'onInputUpdate(change, commitMessage, changes)',
+ ];
+ }
+
+ _computeIfSingleRevert(revertType) {
+ return revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _computeIfRevertSubmission(revertType) {
+ return revertType === REVERT_TYPES.REVERT_SUBMISSION;
+ }
+
+ _modifyRevertMsg(change, commitMessage, message) {
+ return this.$.jsAPI.modifyRevertMsg(change,
+ message, commitMessage);
+ }
+
+ onInputUpdate(change, commitMessage, changes) {
+ if (!change || !changes) return;
+ this._populateRevertSingleChangeMessage(
+ change, commitMessage, change.current_revision);
+ if (changes.length > 1) {
+ this._populateRevertSubmissionMessage(
+ change, changes);
+ }
+ }
+
+ _populateRevertSingleChangeMessage(change, commitMessage, commitHash) {
// Figure out what the revert title should be.
- const originalTitle = message.split('\n')[0];
+ const originalTitle = (commitMessage || '').split('\n')[0];
const revertTitle = `Revert "${originalTitle}"`;
if (!commitHash) {
this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
@@ -58,20 +108,77 @@
}
const revertCommitText = `This reverts commit ${commitHash}.`;
- this.message = `${revertTitle}\n\n${revertCommitText}\n\n` +
+ this.revertSingleChangeMessage =
+ `${revertTitle}\n\n${revertCommitText}\n\n` +
`Reason for revert: <INSERT REASONING HERE>\n`;
+ // This is to give plugins a chance to update message
+ this.revertSingleChangeMessage =
+ this._modifyRevertMsg(change, commitMessage,
+ this.revertSingleChangeMessage);
+ this.message = this.revertSingleChangeMessage;
+ }
+
+ _getTrimmedChangeSubject(subject) {
+ if (!subject) return '';
+ if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
+ return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
+ }
+
+ _modifyRevertSubmissionMsg(change) {
+ return this.$.jsAPI.modifyRevertSubmissionMsg(change,
+ this.revertSubmissionMessage, this.commitMessage);
+ }
+
+ _populateRevertSubmissionMessage(change, changes) {
+ // Follow the same convention of the revert
+ const commitHash = change.current_revision;
+ if (!commitHash) {
+ this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
+ return;
+ }
+ if (!changes || changes.length <= 1) return;
+ const submissionId = change.submission_id;
+ const revertTitle = 'Revert submission ' + submissionId;
+ this.changes = changes;
+ this.revertSubmissionMessage = revertTitle + '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n';
+ this.revertSubmissionMessage += 'Reverted Changes:\n';
+ changes.forEach(change => {
+ this.revertSubmissionMessage += change.change_id.substring(0, 10) + ':'
+ + this._getTrimmedChangeSubject(change.subject) + '\n';
+ });
+ this.revertSubmissionMessage = this._modifyRevertSubmissionMsg(change);
+ this.message = this.revertSubmissionMessage;
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ this._showRevertSubmission = true;
+ }
+
+ _handleRevertSingleChangeClicked() {
+ if (this._revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE) return;
+ this.revertSubmissionMessage = this.message;
+ this.message = this.revertSingleChangeMessage;
+ this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _handleRevertSubmissionClicked() {
+ if (this._revertType === REVERT_TYPES.REVERT_SUBMISSION) return;
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ this.revertSingleChangeMessage = this.message;
+ this.message = this.revertSubmissionMessage;
}
_handleConfirmTap(e) {
e.preventDefault();
e.stopPropagation();
- this.fire('confirm', null, {bubbles: false});
+ this.fire('confirm', {revertType: this._revertType,
+ message: this.message}, {bubbles: false});
}
_handleCancelTap(e) {
e.preventDefault();
e.stopPropagation();
- this.fire('cancel', null, {bubbles: false});
+ this.fire('cancel', {revertType: this._revertType},
+ {bubbles: false});
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
index dbdfba2..1d28d32 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
@@ -50,13 +50,14 @@
assert.isNotOk(element.message);
const alertStub = sandbox.stub();
element.addEventListener('show-alert', alertStub);
- element.populateRevertMessage('not a commitHash in sight', undefined);
+ element._populateRevertSingleChangeMessage({},
+ 'not a commitHash in sight', undefined);
assert.isTrue(alertStub.calledOnce);
});
test('single line', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "one line commit"\n\n' +
@@ -67,7 +68,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "many lines"\n\n' +
@@ -78,7 +79,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'much lines\nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "much lines"\n\n' +
@@ -89,7 +90,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
- element.populateRevertMessage(
+ element._populateRevertSingleChangeMessage({},
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert "Revert "one line commit""\n\n' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
index d59f5cd..ae8dfa5 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
@@ -50,7 +50,7 @@
};
}
- getTrimmedChangeSubject(subject) {
+ _getTrimmedChangeSubject(subject) {
if (!subject) return '';
if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
@@ -61,7 +61,7 @@
this.message, this.commitMessage);
}
- populateRevertSubmissionMessage(message, change, changes) {
+ _populateRevertSubmissionMessage(message, change, changes) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
@@ -77,7 +77,7 @@
changes = changes || [];
changes.forEach(change => {
this.message += change.change_id.substring(0, 10) + ': ' +
- this.getTrimmedChangeSubject(change.subject) + '\n';
+ this._getTrimmedChangeSubject(change.subject) + '\n';
});
this.message = this._modifyRevertSubmissionMsg(change);
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
index cc4bd54..af99c7e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
@@ -51,7 +51,7 @@
assert.isNotOk(element.message);
const alertStub = sandbox.stub();
element.addEventListener('show-alert', alertStub);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'not a commitHash in sight'
);
assert.isTrue(alertStub.calledOnce);
@@ -59,7 +59,7 @@
test('single line', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -69,7 +69,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -79,7 +79,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'test \nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
@@ -89,7 +89,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
- element.populateRevertSubmissionMessage(
+ element._populateRevertSubmissionMessage(
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123');
const expected = 'Revert submission\n\n' +
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
index 8821bcb..5dee2fe 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
@@ -104,6 +104,12 @@
fn.call(this);
} else if (iters++ < AWAIT_MAX_ITERS) {
step.call(this);
+ } else {
+ // TODO(crbug.com/gerrit/10774): Once this is confirmed as the root
+ // cause of the bug, fix it by either making sure to resolve the fn
+ // function or find a better way to listen on the overlay being
+ // shown.
+ console.warn('gr-overlay _awaitOpen failed to resolve');
}
}, AWAIT_STEP);
};
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 5cc1511..3af4a28 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -126,50 +126,6 @@
sha1 = "dc13ae4faca6df981fc7aeb5a522d9db446d5d50",
)
- POWERM_VERS = "1.6.1"
-
- maven_jar(
- name = "powermock-module-junit4",
- artifact = "org.powermock:powermock-module-junit4:" + POWERM_VERS,
- sha1 = "ea8530b2848542624f110a393513af397b37b9cf",
- )
-
- maven_jar(
- name = "powermock-module-junit4-common",
- artifact = "org.powermock:powermock-module-junit4-common:" + POWERM_VERS,
- sha1 = "7222ced54dabc310895d02e45c5428ca05193cda",
- )
-
- maven_jar(
- name = "powermock-reflect",
- artifact = "org.powermock:powermock-reflect:" + POWERM_VERS,
- sha1 = "97d25eda8275c11161bcddda6ef8beabd534c878",
- )
-
- maven_jar(
- name = "powermock-api-easymock",
- artifact = "org.powermock:powermock-api-easymock:" + POWERM_VERS,
- sha1 = "aa740ecf89a2f64d410b3d93ef8cd6833009ef00",
- )
-
- maven_jar(
- name = "powermock-api-support",
- artifact = "org.powermock:powermock-api-support:" + POWERM_VERS,
- sha1 = "592ee6d929c324109d3469501222e0c76ccf0869",
- )
-
- maven_jar(
- name = "powermock-core",
- artifact = "org.powermock:powermock-core:" + POWERM_VERS,
- sha1 = "5afc1efce8d44ed76b30af939657bd598e45d962",
- )
-
- maven_jar(
- name = "javassist",
- artifact = "org.javassist:javassist:3.22.0-GA",
- sha1 = "3e83394258ae2089be7219b971ec21a8288528ad",
- )
-
TESTCONTAINERS_VERSION = "1.12.4"
maven_jar(
