Merge "Fix `_selectedRevision` for edit patchset"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 95d78de..0e77e20 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1696,6 +1696,21 @@
+
Common unit suffixes of 'k', 'm', or 'g' are supported.
+[[core.packedGitUseStrongRefs]]core.packedGitUseStrongRefs::
++
+Set to `true` in order to use strong references to reference packfile
+pages cached in the WindowCache. Otherwise SoftReferences are used.
+If this option is set to `false`, the Java garbage collector will
+flush the WindowCache to free memory if the used heap comes close to
+the maximum heap size. This has the advantage that it can quickly
+reclaim memory which was used by the WindowCache but comes at the
+price that the previously cached pack file content needs to be again
+copied from the file system cache to the Gerrit process.
+Setting this option to `true` prevents flushing the WindowCache
+which provides more predictable performance.
++
+Default is `false`.
+
[[core.deltaBaseCaseLimit]]core.deltaBaseCacheLimit::
+
Maximum number of bytes to reserve for caching base objects
@@ -3571,7 +3586,8 @@
[[log.jsonLogging]]log.jsonLogging::
+
-If set to true, enables error logging in JSON format (file name: "logs/error_log.json").
+If set to true, enables error, ssh and http logging in JSON format (file name:
+"logs/{error|sshd|httpd}_log.json").
+
Defaults to false.
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 103ae0a..af5bfd3 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -293,7 +293,7 @@
Debugging tests:
----
- bazel test --test_output=streamed --test_filter=com.gerrit.TestClass.testMethod testTarget
+ bazel test --test_output=streamed --test_filter=com.gerrit.TestClass.testMethod testTarget
----
Debug test example:
@@ -376,21 +376,25 @@
Note that Bazel currently does not show
link:https://github.com/bazelbuild/bazel/issues/3476[the skipped tests,role=external,window=_blank].
-[[debug]]
-=== Index Query Tests
+[[logging]]
+=== Controlling logging level
-The `DEBUG` log level can optionally be enabled for the index query tests. That log level applies to
-both Elasticsearch and Lucene tests.
+Per default, logging level is set to `INFO` level for all tests. The `DEBUG`
+log level can be enabled for the tests.
-In Eclipse, set `-Ddebug=true` as a VM argument under the Run Configuration's `Arguments` tab.
-
-With `bazel`, here is an example for the Lucene `account` test:
+In IDE, set `-Dgerrit.logLevel=debug` as a VM argument. With `bazel`, pass
+`GERRIT_LOG_LEVEL=debug` environment variable:
----
-bazel test --jvmopt='-Ddebug=true' \
-javatests/com/google/gerrit/server/query/account:lucene_query_test
+ bazel test --test_filter=com.gerrit.server.notedb.ChangeNotesTest \
+ --test_env=GERRIT_LOG_LEVEL=debug \
+ javatests/com/google/gerrit/server:server_tests
----
+The log results can be found in:
+`bazel-testlogs/javatests/com/google/gerrit/server/server_tests/test.log`.
+
+
== Dependencies
Dependency JARs are normally downloaded as needed, but you can
@@ -596,7 +600,6 @@
link:https://classic.yarnpkg.com/en/docs/cli/upgrade/[yarn upgrade doc,role=external,window=_blank].
-
[[RBE]]
== Google Remote Build Support
@@ -631,8 +634,6 @@
```
-
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt
index 7f1e82b..7f932da 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -587,13 +587,6 @@
Gerrit administrators may configure which of the commands are shown on
the change screen.
-- link:https://gerrit-review.googlesource.com/admin/repos/plugins/egit[
- egit,role=external,window=_blank] plugin:
-+
-The `egit` plugin provides the change ref as a download command, which is
-needed for downloading a change from within
-link:https://www.eclipse.org/egit/[EGit].
-
- link:https://gerrit-review.googlesource.com/admin/repos/plugins/project-download-commands[
project-download-commands,role=external,window=_blank] plugin:
+
@@ -760,7 +753,16 @@
Gerrit core does not support the renaming of projects.
-As workaround you can perform the following steps:
+If the link:https://gerrit-review.googlesource.com/admin/repos/plugins/rename-project[rename-project]
+plugin is installed, projects can be renamed using the
+link:https://gerrit.googlesource.com/plugins/rename-project/+/refs/heads/master/src/main/resources/Documentation/cmd-rename.md[rename-project]
+ssh command.
+
+Find details about prerequisites in the
+link:https://gerrit.googlesource.com/plugins/rename-project/+/refs/heads/master/src/main/resources/Documentation/about.md[plugin documentation].
+
+If you don't want to use the rename-project plugin you can perform the following steps as
+a workaround:
. link:#project-creation[Create a new project] with the new name.
. link:#import-history[Import the history of the old project].
@@ -769,11 +771,6 @@
Please note that a drawback of this workaround is that the whole review
history (changes, review comments) is lost.
-Alternatively, you can use the
-link:https://gerrit.googlesource.com/plugins/importer/[importer,role=external,window=_blank] plugin
-to copy the project _including the review history_, and then
-link:#project-deletion[delete the old project].
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 41c241f..075de2b 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -67,6 +67,15 @@
==== Jetty
+* `http/server/jetty/connections/connections`: The current number of open connections
+* `http/server/jetty/connections/connections_total`: The total number of connections opened
+* `http/server/jetty/connections/connections_duration_max`: The max duration of a connection in ms
+* `http/server/jetty/connections/connections_duration_mean`: The mean duration of a connection in ms
+* `http/server/jetty/connections/connections_duration_stdev`: The standard deviation of the duration of a connection in ms
+* `http/server/jetty/connections/received_messages`: The total number of messages received
+* `http/server/jetty/connections/sent_messages`: The total number of messages sent
+* `http/server/jetty/connections/received_bytes`: Total number of bytes received by tracked connections
+* `http/server/jetty/connections/sent_bytes`: Total number of bytes sent by tracked connections"
* `http/server/jetty/threadpool/active_threads`: Active threads
* `http/server/jetty/threadpool/idle_threads`: Idle threads
* `http/server/jetty/threadpool/reserved_threads`: Reserved threads
@@ -137,6 +146,18 @@
* `jgit/block_cache/cache_used`: Bytes of memory retained in JGit block cache.
* `jgit/block_cache/open_files`: File handles held open by JGit block cache.
+* `avg_load_time` Average time to load a cache entry for JGit block cache.
+* `eviction_count` : Cache evictions for JGit block cache.
+* `eviction_ratio` : Cache eviction ratio for JGit block cache.
+* `hit_count` : Cache hits for JGit block cache.
+* `hit_ratio` : Cache hit ratio for JGit block cache.
+* `load_failure_count` : Failed cache loads for JGit block cache.
+* `load_failure_ratio` : Failed cache load ratio for JGit block cache.
+* `load_success_count` : Successful cache loads for JGit block cache.
+* `miss_count` : Cache misses for JGit block cache.
+* `miss_ratio` : Cache miss ratio for JGit block cache.
+* `cache_used_per_repository` : Bytes of memory retained per repository for the top N repositories
+having most data in the cache. The number N of reported repositories is limited to 1000.
=== Git
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index ca105f6..cfee970 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1282,7 +1282,7 @@
}
protected void assertNotifyCc(TestAccount expected) {
- assertNotifyCc(expected.getEmailAddress());
+ assertNotifyCc(expected.getNameEmail());
}
protected void assertNotifyCc(String expectedEmail, String expectedFullname) {
@@ -1302,7 +1302,7 @@
protected void assertNotifyBcc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(expected.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(expected.getNameEmail());
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(m.headers().get("Cc").isEmpty()).isTrue();
}
diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index 088de23..a372089 100644
--- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -319,6 +319,9 @@
}
protected class StagedUsers {
+ public static final String REVIEWER_BY_EMAIL = "reviewerByEmail@example.com";
+ public static final String CC_BY_EMAIL = "ccByEmail@example.com";
+
public final TestAccount owner;
public final TestAccount author;
public final TestAccount uploader;
@@ -327,8 +330,6 @@
public final TestAccount starrer;
public final TestAccount assignee;
public final TestAccount watchingProjectOwner;
- public final String reviewerByEmail = "reviewerByEmail@example.com";
- public final String ccerByEmail = "ccByEmail@example.com";
private final Map<NotifyType, TestAccount> watchers = new HashMap<>();
private final Map<String, TestAccount> accountsByEmail = new HashMap<>();
@@ -429,9 +430,9 @@
ReviewInput in =
ReviewInput.noScore()
.reviewer(reviewer.email())
- .reviewer(reviewerByEmail)
+ .reviewer(REVIEWER_BY_EMAIL)
.reviewer(ccer.email(), ReviewerState.CC, false)
- .reviewer(ccerByEmail, ReviewerState.CC, false);
+ .reviewer(CC_BY_EMAIL, ReviewerState.CC, false);
ReviewResult result = gApi.changes().id(r.getChangeId()).current().review(in);
supportReviewersByEmail = true;
if (result.reviewers.values().stream().anyMatch(v -> v.error != null)) {
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index 89d1f3d..541e479 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -48,8 +48,6 @@
"//lib/guice:guice-servlet",
"//lib/mail",
"//lib/mina:sshd",
- "//lib/log:impl-log4j",
- "//lib/log:log4j",
"//lib:guava",
"//lib/bouncycastle:bcpg",
"//lib/bouncycastle:bcprov",
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 95afad7..b25dcc3 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -18,13 +18,11 @@
import static com.google.common.base.Preconditions.checkState;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.util.Objects.requireNonNull;
-import static org.apache.log4j.Logger.getLogger;
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.config.ConfigAnnotationParser;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.config.GerritConfigs;
@@ -55,6 +53,7 @@
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.SshMode;
+import com.google.gerrit.testing.TestLoggingActivator;
import com.google.inject.AbstractModule;
import com.google.inject.BindingAnnotation;
import com.google.inject.Injector;
@@ -80,11 +79,6 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
-import org.apache.log4j.ConsoleAppender;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
-import org.apache.log4j.PatternLayout;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.RepositoryCache;
import org.eclipse.jgit.util.FS;
@@ -122,8 +116,7 @@
null, // @GerritConfig is only valid on methods.
null, // @GerritConfigs is only valid on methods.
null, // @GlobalPluginConfig is only valid on methods.
- null, // @GlobalPluginConfigs is only valid on methods.
- getLogLevelThresholdAnnotation(testDesc));
+ null); // @GlobalPluginConfigs is only valid on methods.
}
public static Description forTestMethod(
@@ -159,8 +152,7 @@
testDesc.getAnnotation(GerritConfig.class),
testDesc.getAnnotation(GerritConfigs.class),
testDesc.getAnnotation(GlobalPluginConfig.class),
- testDesc.getAnnotation(GlobalPluginConfigs.class),
- getLogLevelThresholdAnnotation(testDesc));
+ testDesc.getAnnotation(GlobalPluginConfigs.class));
}
private static boolean has(Class<? extends Annotation> annotation, Class<?> clazz) {
@@ -182,14 +174,6 @@
return null;
}
- private static Level getLogLevelThresholdAnnotation(org.junit.runner.Description testDesc) {
- LogThreshold logLevelThreshold = testDesc.getTestClass().getAnnotation(LogThreshold.class);
- if (logLevelThreshold == null) {
- return Level.DEBUG;
- }
- return Level.toLevel(logLevelThreshold.level());
- }
-
abstract org.junit.runner.Description testDescription();
@Nullable
@@ -229,8 +213,6 @@
@Nullable
abstract GlobalPluginConfigs pluginConfigs();
- abstract Level logLevelThreshold();
-
private void checkValidAnnotations() {
if (useClockStep() != null && useSystemTime()) {
throw new IllegalStateException("Use either @UseClockStep or @UseSystemTime, not both");
@@ -267,54 +249,6 @@
}
}
- private static final ImmutableMap<String, Level> LOG_LEVELS =
- ImmutableMap.<String, Level>builder()
- .put("com.google.gerrit", getGerritLogLevel())
-
- // Silence non-critical messages from MINA SSHD.
- .put("org.apache.mina", Level.WARN)
- .put("org.apache.sshd.common", Level.WARN)
- .put("org.apache.sshd.server", Level.WARN)
- .put("org.apache.sshd.common.keyprovider.FileKeyPairProvider", Level.INFO)
- .put("com.google.gerrit.sshd.GerritServerSession", Level.WARN)
-
- // Silence non-critical messages from mime-util.
- .put("eu.medsea.mimeutil", Level.WARN)
-
- // Silence non-critical messages from openid4java.
- .put("org.apache.xml", Level.WARN)
- .put("org.openid4java", Level.WARN)
- .put("org.openid4java.consumer.ConsumerManager", Level.FATAL)
- .put("org.openid4java.discovery.Discovery", Level.ERROR)
- .put("org.openid4java.server.RealmVerifier", Level.ERROR)
- .put("org.openid4java.message.AuthSuccess", Level.ERROR)
-
- // Silence non-critical messages from c3p0 (if used).
- .put("com.mchange.v2.c3p0", Level.WARN)
- .put("com.mchange.v2.resourcepool", Level.WARN)
- .put("com.mchange.v2.sql", Level.WARN)
-
- // Silence non-critical messages from apache.http.
- .put("org.apache.http", Level.WARN)
-
- // Silence non-critical messages from Jetty.
- .put("org.eclipse.jetty", Level.WARN)
-
- // Silence non-critical messages from JGit.
- .put("org.eclipse.jgit.transport.PacketLineIn", Level.WARN)
- .put("org.eclipse.jgit.transport.PacketLineOut", Level.WARN)
- .put("org.eclipse.jgit.internal.storage.file.FileSnapshot", Level.WARN)
- .put("org.eclipse.jgit.util.FS", Level.WARN)
- .build();
-
- private static Level getGerritLogLevel() {
- String value = Strings.nullToEmpty(System.getenv("GERRIT_LOG_LEVEL"));
- if (value.isEmpty()) {
- value = Strings.nullToEmpty(System.getProperty("gerrit.logLevel"));
- }
- return Level.toLevel(value, Level.INFO);
- }
-
private static boolean forceLocalDisk() {
String value = Strings.nullToEmpty(System.getenv("GERRIT_FORCE_LOCAL_DISK"));
if (value.isEmpty()) {
@@ -430,7 +364,7 @@
throws Exception {
checkArgument(site != null, "site is required (even for in-memory server");
desc.checkValidAnnotations();
- configureLogging(desc.logLevelThreshold());
+ TestLoggingActivator.configureLogging();
CyclicBarrier serverStarted = new CyclicBarrier(2);
Daemon daemon =
new Daemon(
@@ -531,25 +465,6 @@
return new GerritServer(desc, site, createTestInjector(daemon), daemon, daemonService);
}
- private static void configureLogging(Level threshold) {
- LogManager.resetConfiguration();
-
- PatternLayout layout = new PatternLayout();
- layout.setConversionPattern("%-5p %c %x: %m%n");
-
- ConsoleAppender dst = new ConsoleAppender();
- dst.setLayout(layout);
- dst.setTarget("System.err");
- dst.setThreshold(threshold);
- dst.activateOptions();
-
- Logger root = LogManager.getRootLogger();
- root.removeAllAppenders();
- root.addAppender(dst);
-
- LOG_LEVELS.entrySet().stream().forEach(e -> getLogger(e.getKey()).setLevel(e.getValue()));
- }
-
private static void mergeTestConfig(Config cfg) {
String forceEphemeralPort = String.format("%s:0", getLocalHost().getHostName());
String url = "http://" + forceEphemeralPort + "/";
diff --git a/java/com/google/gerrit/acceptance/LogThreshold.java b/java/com/google/gerrit/acceptance/LogThreshold.java
deleted file mode 100644
index 36831f3..0000000
--- a/java/com/google/gerrit/acceptance/LogThreshold.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// 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;
-
-import static java.lang.annotation.ElementType.METHOD;
-import static java.lang.annotation.ElementType.TYPE;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import java.lang.annotation.Inherited;
-import java.lang.annotation.Retention;
-import java.lang.annotation.Target;
-
-@Target({TYPE, METHOD})
-@Retention(RUNTIME)
-@Inherited
-public @interface LogThreshold {
- String level() default "DEBUG";
-}
diff --git a/java/com/google/gerrit/acceptance/TestAccount.java b/java/com/google/gerrit/acceptance/TestAccount.java
index 07bb739..8b39c9e 100644
--- a/java/com/google/gerrit/acceptance/TestAccount.java
+++ b/java/com/google/gerrit/acceptance/TestAccount.java
@@ -79,7 +79,7 @@
.toString();
}
- public Address getEmailAddress() {
+ public Address getNameEmail() {
// Address is weird enough that it's safer and clearer to create a new instance in a
// non-abstract method rather than, say, having an abstract emailAddress() as part of this
// AutoValue class. Specifically:
diff --git a/java/com/google/gerrit/common/PageLinks.java b/java/com/google/gerrit/common/PageLinks.java
index 9f06364..38de5b1 100644
--- a/java/com/google/gerrit/common/PageLinks.java
+++ b/java/com/google/gerrit/common/PageLinks.java
@@ -90,16 +90,16 @@
return ADMIN_PROJECTS + p.get();
}
- public static String toProjectAcceess(Project.NameKey p) {
- return "/admin/projects/" + p.get() + ",access";
+ public static String toProjectAccess(Project.NameKey p) {
+ return ADMIN_PROJECTS + p.get() + ",access";
}
public static String toProjectBranches(Project.NameKey p) {
- return "/admin/projects/" + p.get() + ",branches";
+ return ADMIN_PROJECTS + p.get() + ",branches";
}
public static String toProjectTags(Project.NameKey p) {
- return "/admin/projects/" + p.get() + ",tags";
+ return ADMIN_PROJECTS + p.get() + ",tags";
}
public static String toAccountQuery(String fullname, Status status) {
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 964cf67..3a68414 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -133,7 +133,7 @@
maxNegative = Short.MIN_VALUE;
maxPositive = Short.MAX_VALUE;
- if (values.size() > 0) {
+ if (!values.isEmpty()) {
if (values.get(0).getValue() < 0) {
maxNegative = values.get(0).getValue();
}
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index 5f412ce..730af90 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -392,7 +392,7 @@
@Override
public ResultSet<V> read() {
- return readImpl((doc) -> AbstractElasticIndex.this.fromDocument(doc, opts.fields()));
+ return readImpl(doc -> AbstractElasticIndex.this.fromDocument(doc, opts.fields()));
}
@Override
diff --git a/java/com/google/gerrit/elasticsearch/builders/XContentBuilder.java b/java/com/google/gerrit/elasticsearch/builders/XContentBuilder.java
index 061a373..9c44583 100644
--- a/java/com/google/gerrit/elasticsearch/builders/XContentBuilder.java
+++ b/java/com/google/gerrit/elasticsearch/builders/XContentBuilder.java
@@ -14,6 +14,7 @@
package com.google.gerrit.elasticsearch.builders;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.time.format.DateTimeFormatter.ISO_INSTANT;
import com.fasterxml.jackson.core.JsonEncoding;
@@ -21,7 +22,6 @@
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.json.JsonReadFeature;
import com.fasterxml.jackson.core.json.JsonWriteFeature;
-import com.google.common.base.Charsets;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.IOException;
@@ -139,7 +139,7 @@
public String string() {
close();
byte[] bytesArray = bos.toByteArray();
- return new String(bytesArray, Charsets.UTF_8);
+ return new String(bytesArray, UTF_8);
}
private void writeValue(Object value) throws IOException {
diff --git a/java/com/google/gerrit/entities/AttentionStatus.java b/java/com/google/gerrit/entities/AttentionStatus.java
new file mode 100644
index 0000000..c488ccd
--- /dev/null
+++ b/java/com/google/gerrit/entities/AttentionStatus.java
@@ -0,0 +1,72 @@
+// 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.entities;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import java.time.Instant;
+
+/**
+ * A single update to the attention set. To reconstruct the attention set these instances are parsed
+ * in reverse chronological order. Since each update contains all required information and
+ * invalidates all previous state (hence the name -Status rather than -Update), only the most recent
+ * record is relevant for each user.
+ *
+ * <p>See <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">here</a> for
+ * details.
+ */
+@AutoValue
+public abstract class AttentionStatus {
+
+ /** Users can be added to or removed from the attention set. */
+ public enum Operation {
+ ADD,
+ REMOVE
+ }
+
+ /**
+ * The time at which this status was set. This is null for instances to be written because the
+ * timestamp in the commit message will be used.
+ */
+ @Nullable
+ public abstract Instant timestamp();
+
+ /** The user included in or excluded from the attention set. */
+ public abstract Account.Id account();
+
+ /** Indicates whether the user is added to or removed from the attention set. */
+ public abstract Operation operation();
+
+ /** A short human readable reason that explains this status (e.g. "manual"). */
+ public abstract String reason();
+
+ /**
+ * Create an instance from data read from NoteDB. This includes the timestamp taken from the
+ * commit.
+ */
+ public static AttentionStatus createFromRead(
+ Instant timestamp, Account.Id account, Operation operation, String reason) {
+ return new AutoValue_AttentionStatus(timestamp, account, operation, reason);
+ }
+
+ /**
+ * Create an instance to be written to NoteDB. This has no timestamp because the timestamp of the
+ * commit will be used.
+ */
+ public static AttentionStatus createForWrite(
+ Account.Id account, Operation operation, String reason) {
+ return new AutoValue_AttentionStatus(null, account, operation, reason);
+ }
+}
diff --git a/java/com/google/gerrit/extensions/api/accounts/Accounts.java b/java/com/google/gerrit/extensions/api/accounts/Accounts.java
index db7f506..15fca9a 100644
--- a/java/com/google/gerrit/extensions/api/accounts/Accounts.java
+++ b/java/com/google/gerrit/extensions/api/accounts/Accounts.java
@@ -21,6 +21,7 @@
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
+import java.util.Set;
public interface Accounts {
/**
@@ -138,7 +139,7 @@
private int limit;
private int start;
private boolean suggest;
- private EnumSet<ListAccountsOption> options = EnumSet.noneOf(ListAccountsOption.class);
+ private Set<ListAccountsOption> options = EnumSet.noneOf(ListAccountsOption.class);
/** Execute query and return a list of accounts. */
public abstract List<AccountInfo> get() throws RestApiException;
@@ -185,7 +186,7 @@
}
/** Set options on the request, replacing existing options. */
- public QueryRequest withOptions(EnumSet<ListAccountsOption> options) {
+ public QueryRequest withOptions(Set<ListAccountsOption> options) {
this.options = options;
return this;
}
@@ -206,7 +207,7 @@
return suggest;
}
- public EnumSet<ListAccountsOption> getOptions() {
+ public Set<ListAccountsOption> getOptions() {
return options;
}
}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
index 6bd4b73..a26068a 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
@@ -22,6 +22,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import java.util.EnumSet;
import java.util.Optional;
+import java.util.Set;
/**
* An API for the change edit of a change. A change edit is similar to a patch set and will become
@@ -51,7 +52,7 @@
return base;
}
- public EnumSet<ChangeEditDetailOption> options() {
+ public Set<ChangeEditDetailOption> options() {
return options;
}
}
diff --git a/java/com/google/gerrit/extensions/api/changes/Changes.java b/java/com/google/gerrit/extensions/api/changes/Changes.java
index 8609207..d8741f5 100644
--- a/java/com/google/gerrit/extensions/api/changes/Changes.java
+++ b/java/com/google/gerrit/extensions/api/changes/Changes.java
@@ -24,6 +24,7 @@
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
+import java.util.Set;
public interface Changes {
/**
@@ -80,7 +81,7 @@
private int limit;
private int start;
private boolean isNoLimit;
- private EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
+ private Set<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
private ListMultimap<String, String> pluginOptions = ArrayListMultimap.create();
public abstract List<ChangeInfo> get() throws RestApiException;
@@ -118,7 +119,7 @@
}
/** Set options on the request, replacing existing options. */
- public QueryRequest withOptions(EnumSet<ListChangesOption> options) {
+ public QueryRequest withOptions(Set<ListChangesOption> options) {
this.options = options;
return this;
}
@@ -151,7 +152,7 @@
return start;
}
- public EnumSet<ListChangesOption> getOptions() {
+ public Set<ListChangesOption> getOptions() {
return options;
}
diff --git a/java/com/google/gerrit/extensions/api/groups/Groups.java b/java/com/google/gerrit/extensions/api/groups/Groups.java
index 86c2d77..81b5f47 100644
--- a/java/com/google/gerrit/extensions/api/groups/Groups.java
+++ b/java/com/google/gerrit/extensions/api/groups/Groups.java
@@ -24,6 +24,7 @@
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
public interface Groups {
/**
@@ -166,7 +167,7 @@
return this;
}
- public EnumSet<ListGroupsOption> getOptions() {
+ public Set<ListGroupsOption> getOptions() {
return options;
}
@@ -224,7 +225,7 @@
private String query;
private int limit;
private int start;
- private EnumSet<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
+ private Set<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
/** Execute query and returns the matched groups as list. */
public abstract List<GroupInfo> get() throws RestApiException;
@@ -266,7 +267,7 @@
}
/** Set options on the request, replacing existing options. */
- public QueryRequest withOptions(EnumSet<ListGroupsOption> options) {
+ public QueryRequest withOptions(Set<ListGroupsOption> options) {
this.options = options;
return this;
}
@@ -283,7 +284,7 @@
return start;
}
- public EnumSet<ListGroupsOption> getOptions() {
+ public Set<ListGroupsOption> getOptions() {
return options;
}
}
diff --git a/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java b/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java
index bd7ebcb..4170797 100644
--- a/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java
+++ b/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java
@@ -32,7 +32,7 @@
public static class None {
private None() {}
- public static None INSTANCE = new None();
+ public static final None INSTANCE = new None();
}
@Override
diff --git a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
index b09dad0..dcfb614 100644
--- a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
+++ b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -192,7 +192,7 @@
// We might already have this account on file. Look for it.
//
try {
- return accountManager.lookup(aReq.getIdentity()) == null;
+ return !accountManager.lookup(aReq.getIdentity()).isPresent();
} catch (AccountException e) {
logger.atWarning().withCause(e).log("Cannot determine if user account exists");
return true;
@@ -333,7 +333,7 @@
areq.setEmailAddress(fetchRsp.getAttributeValue("Email"));
}
- if (openIdDomains != null && openIdDomains.size() > 0) {
+ if (openIdDomains != null && !openIdDomains.isEmpty()) {
// Administrator limited email domains, which can be used for OpenID.
// Login process will only work if the passed email matches one
// of these domains.
diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
index 2507163..4fabb18 100644
--- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
+++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
@@ -96,9 +96,9 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String PROJECT_LIST_ACTION = "project_list";
+ private static final int BUFFER_SIZE = 8192;
private final Set<String> deniedActions;
- private final int bufferSize = 8192;
private final Path gitwebCgi;
private final URI gitwebUrl;
private final LocalDiskRepositoryManager repoManager;
@@ -504,11 +504,11 @@
proc.getOutputStream().close();
}
- try (InputStream in = new BufferedInputStream(proc.getInputStream(), bufferSize)) {
+ try (InputStream in = new BufferedInputStream(proc.getInputStream(), BUFFER_SIZE)) {
readCgiHeaders(rsp, in);
try (OutputStream out = rsp.getOutputStream()) {
- final byte[] buf = new byte[bufferSize];
+ final byte[] buf = new byte[BUFFER_SIZE];
int n;
while ((n = in.read(buf)) > 0) {
out.write(buf, 0, n);
@@ -643,7 +643,7 @@
() -> {
try {
try {
- final byte[] buf = new byte[bufferSize];
+ final byte[] buf = new byte[BUFFER_SIZE];
int remaining = contentLength;
while (0 < remaining) {
final int max = Math.max(buf.length, remaining);
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 4af7872..b32da89 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1616,7 +1616,7 @@
for (String p : Splitter.on('/').split(path)) {
out.add(IdString.fromUrl(p));
}
- if (out.size() > 0 && out.get(out.size() - 1).isEmpty()) {
+ if (!out.isEmpty() && out.get(out.size() - 1).isEmpty()) {
out.remove(out.size() - 1);
}
return out;
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 375967b..495c7e5 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -606,7 +606,7 @@
private void decodeReviewedBy(ListMultimap<String, IndexableField> doc, ChangeData cd) {
Collection<IndexableField> reviewedBy = doc.get(REVIEWEDBY_FIELD);
- if (reviewedBy.size() > 0) {
+ if (!reviewedBy.isEmpty()) {
Set<Account.Id> accounts = Sets.newHashSetWithExpectedSize(reviewedBy.size());
for (IndexableField r : reviewedBy) {
int id = r.numericValue().intValue();
diff --git a/java/com/google/gerrit/mail/Address.java b/java/com/google/gerrit/mail/Address.java
index 24ab353..455cc90 100644
--- a/java/com/google/gerrit/mail/Address.java
+++ b/java/com/google/gerrit/mail/Address.java
@@ -16,6 +16,7 @@
import com.google.gerrit.common.Nullable;
+/** Represents an address (name + email) in an email message. */
public class Address {
public static Address parse(String in) {
final int lt = in.indexOf('<');
diff --git a/java/com/google/gerrit/mail/RawMailParser.java b/java/com/google/gerrit/mail/RawMailParser.java
index b7e2030..9c89d19 100644
--- a/java/com/google/gerrit/mail/RawMailParser.java
+++ b/java/com/google/gerrit/mail/RawMailParser.java
@@ -69,7 +69,7 @@
}
// Add From, To and Cc
- if (mimeMessage.getFrom() != null && mimeMessage.getFrom().size() > 0) {
+ if (mimeMessage.getFrom() != null && !mimeMessage.getFrom().isEmpty()) {
Mailbox from = mimeMessage.getFrom().get(0);
messageBuilder.from(new Address(from.getName(), from.getAddress()));
}
diff --git a/java/com/google/gerrit/metrics/dropwizard/BUILD b/java/com/google/gerrit/metrics/dropwizard/BUILD
index 3079809..4b3859f 100644
--- a/java/com/google/gerrit/metrics/dropwizard/BUILD
+++ b/java/com/google/gerrit/metrics/dropwizard/BUILD
@@ -8,7 +8,6 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/metrics",
- "//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/server",
"//lib:args4j",
"//lib:guava",
diff --git a/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java b/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
index e33c242..68f8021 100644
--- a/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
+++ b/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
@@ -22,6 +22,7 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Field;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
/** Abstract callback metric broken down into buckets. */
@@ -66,7 +67,13 @@
}
void doPrune() {
- cells.entrySet().removeIf(objectValueGaugeEntry -> !objectValueGaugeEntry.getValue().set);
+ Set<Map.Entry<Object, BucketedCallback<V>.ValueGauge>> entries = cells.entrySet();
+ for (Map.Entry<Object, ValueGauge> e : entries) {
+ if (!e.getValue().set) {
+ entries.remove(e);
+ registry.remove(submetric(e.getKey()));
+ }
+ }
}
void doEndSet() {
diff --git a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
index 3819786..62b2497 100644
--- a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
+++ b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
@@ -14,12 +14,18 @@
package com.google.gerrit.metrics.proc;
+import com.google.gerrit.metrics.CallbackMetric1;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.server.logging.Metadata;
+import java.util.Map;
import org.eclipse.jgit.storage.file.WindowCacheStats;
public class JGitMetricModule extends MetricModule {
+ private static final long MAX_REPO_COUNT = 1000;
+
@Override
protected void configure(MetricMaker metrics) {
metrics.newCallbackMetric(
@@ -28,12 +34,99 @@
new Description("Bytes of memory retained in JGit block cache.")
.setGauge()
.setUnit(Units.BYTES),
- () -> WindowCacheStats.getStats().getOpenByteCount());
+ WindowCacheStats.getStats()::getOpenByteCount);
metrics.newCallbackMetric(
"jgit/block_cache/open_files",
Long.class,
new Description("File handles held open by JGit block cache.").setGauge().setUnit("fds"),
- () -> WindowCacheStats.getStats().getOpenFileCount());
+ WindowCacheStats.getStats()::getOpenFileCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/avg_load_time",
+ Double.class,
+ new Description("Average time to load a cache entry for JGit block cache.")
+ .setGauge()
+ .setUnit(Units.NANOSECONDS),
+ WindowCacheStats.getStats()::getAverageLoadTime);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/eviction_count",
+ Long.class,
+ new Description("Cache evictions for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getEvictionCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/eviction_ratio",
+ Double.class,
+ new Description("Cache eviction ratio for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getEvictionRatio);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/hit_count",
+ Long.class,
+ new Description("Cache hits for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getHitCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/hit_ratio",
+ Double.class,
+ new Description("Cache hit ratio for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getHitRatio);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/load_failure_count",
+ Long.class,
+ new Description("Failed cache loads for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getLoadFailureCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/load_failure_ratio",
+ Double.class,
+ new Description("Failed cache load ratio for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getLoadFailureRatio);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/load_success_count",
+ Long.class,
+ new Description("Successfull cache loads for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getLoadSuccessCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/miss_count",
+ Long.class,
+ new Description("Cache misses for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getMissCount);
+
+ metrics.newCallbackMetric(
+ "jgit/block_cache/miss_ratio",
+ Double.class,
+ new Description("Cache miss ratio for JGit block cache.").setGauge(),
+ WindowCacheStats.getStats()::getMissRatio);
+
+ CallbackMetric1<String, Long> repoEnt =
+ metrics.newCallbackMetric(
+ "jgit/block_cache/cache_used_per_repository",
+ Long.class,
+ new Description(
+ "Bytes of memory retained per repository for the top repositories "
+ + "having most data in the cache.")
+ .setGauge()
+ .setUnit("byte"),
+ Field.ofString("repository_name", Metadata.Builder::projectName).build());
+ metrics.newTrigger(
+ repoEnt,
+ () -> {
+ Map<String, Long> cacheMap = WindowCacheStats.getStats().getOpenByteCountPerRepository();
+ if (cacheMap.isEmpty()) {
+ repoEnt.forceCreate("");
+ } else {
+ cacheMap.entrySet().stream()
+ .sorted(Map.Entry.<String, Long>comparingByValue().reversed())
+ .limit(MAX_REPO_COUNT)
+ .forEach(e -> repoEnt.set(e.getKey(), e.getValue()));
+ repoEnt.prune();
+ }
+ });
}
}
diff --git a/java/com/google/gerrit/pgm/BUILD b/java/com/google/gerrit/pgm/BUILD
index aa4df82..8b8f13c 100644
--- a/java/com/google/gerrit/pgm/BUILD
+++ b/java/com/google/gerrit/pgm/BUILD
@@ -22,6 +22,7 @@
"//java/com/google/gerrit/launcher",
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/lucene",
+ "//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/metrics/dropwizard",
"//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/pgm/init",
diff --git a/java/com/google/gerrit/pgm/DeleteZombieDrafts.java b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
index 90a60c1..c08e999 100644
--- a/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
+++ b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
@@ -14,6 +14,8 @@
package com.google.gerrit.pgm;
+import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.server.config.GerritServerConfigModule;
@@ -68,6 +70,7 @@
bind(String.class)
.annotatedWith(SecureStoreClassName.class)
.toProvider(Providers.of(getConfiguredSecureStoreClass()));
+ bind(MetricMaker.class).to(DisabledMetricMaker.class);
install(new FactoryModuleBuilder().build(DeleteZombieCommentsRefs.Factory.class));
}
});
diff --git a/java/com/google/gerrit/pgm/http/jetty/BUILD b/java/com/google/gerrit/pgm/http/jetty/BUILD
index 32247fb..cd188f5 100644
--- a/java/com/google/gerrit/pgm/http/jetty/BUILD
+++ b/java/com/google/gerrit/pgm/http/jetty/BUILD
@@ -13,6 +13,8 @@
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/util/http",
+ "//java/com/google/gerrit/util/logging",
+ "//lib:gson",
"//lib:guava",
"//lib:jgit",
"//lib:servlet-api",
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
index 809f7bc..4e4c93b 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
@@ -17,6 +17,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.httpd.GetUserFilter;
import com.google.gerrit.httpd.restapi.LogRedactUtil;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.util.SystemLog;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
@@ -28,11 +29,13 @@
import org.eclipse.jetty.server.RequestLog;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
+import org.eclipse.jgit.lib.Config;
/** Writes the {@code httpd_log} file with per-request data. */
class HttpLog extends AbstractLifeCycle implements RequestLog {
private static final Logger log = Logger.getLogger(HttpLog.class);
private static final String LOG_NAME = "httpd_log";
+ private static final String JSON_SUFFIX = ".json";
interface HttpLogFactory {
HttpLog get();
@@ -52,8 +55,20 @@
private final AsyncAppender async;
@Inject
- HttpLog(SystemLog systemLog) {
- async = systemLog.createAsyncAppender(LOG_NAME, new HttpLogLayout());
+ HttpLog(SystemLog systemLog, @GerritServerConfig Config config) {
+ boolean json = config.getBoolean("log", "jsonLogging", false);
+ boolean text = config.getBoolean("log", "textLogging", true) || !json;
+
+ async = new AsyncAppender();
+
+ if (text) {
+ async.addAppender(systemLog.createAsyncAppender(LOG_NAME, new HttpLogLayout()));
+ }
+
+ if (json) {
+ async.addAppender(
+ systemLog.createAsyncAppender(LOG_NAME + JSON_SUFFIX, new HttpLogJsonLayout()));
+ }
}
@Override
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
new file mode 100644
index 0000000..73d9ee4
--- /dev/null
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
@@ -0,0 +1,72 @@
+// 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.pgm.http.jetty;
+
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_CONTENT_LENGTH;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_HOST;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_METHOD;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_PROTOCOL;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_REFERER;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_RESOURCE;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_STATUS;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER;
+import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER_AGENT;
+
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
+import java.time.format.DateTimeFormatter;
+import org.apache.log4j.spi.LoggingEvent;
+
+public class HttpLogJsonLayout extends JsonLayout {
+
+ @Override
+ public DateTimeFormatter createDateTimeFormatter() {
+ return DateTimeFormatter.ofPattern("dd/MMM/yyyy:HH:mm:ss,SSS Z");
+ }
+
+ @Override
+ public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
+ return new HttpJsonLogEntry(event);
+ }
+
+ @SuppressWarnings("unused")
+ private class HttpJsonLogEntry extends JsonLogEntry {
+ public String host;
+ public String thread;
+ public String user;
+ public String timestamp;
+ public String method;
+ public String resource;
+ public String protocol;
+ public String status;
+ public String contentLength;
+ public String referer;
+ public String userAgent;
+
+ public HttpJsonLogEntry(LoggingEvent event) {
+ this.host = getMdcString(event, P_HOST);
+ this.thread = event.getThreadName();
+ this.user = getMdcString(event, P_USER);
+ this.timestamp = formatDate(event.getTimeStamp());
+ this.method = getMdcString(event, P_METHOD);
+ this.resource = getMdcString(event, P_RESOURCE);
+ this.protocol = getMdcString(event, P_PROTOCOL);
+ this.status = getMdcString(event, P_STATUS);
+ this.contentLength = getMdcString(event, P_CONTENT_LENGTH);
+ this.referer = getMdcString(event, P_REFERER);
+ this.userAgent = getMdcString(event, P_USER_AGENT);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java b/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java
index b6a2d38..80edd45 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java
@@ -17,6 +17,7 @@
import com.google.gerrit.metrics.CallbackMetric;
import com.google.gerrit.metrics.CallbackMetric0;
import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.MetricMaker;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -45,27 +46,83 @@
metrics.newCallbackMetric(
"http/server/jetty/threadpool/idle_threads",
Integer.class,
- new Description("Idle threads").setGauge().setUnit("threads"));
+ new Description("Idle threads").setGauge());
CallbackMetric0<Integer> busyThreads =
metrics.newCallbackMetric(
"http/server/jetty/threadpool/active_threads",
Integer.class,
- new Description("Active threads").setGauge().setUnit("threads"));
+ new Description("Active threads").setGauge());
CallbackMetric0<Integer> reservedThreads =
metrics.newCallbackMetric(
"http/server/jetty/threadpool/reserved_threads",
Integer.class,
- new Description("Reserved threads").setGauge().setUnit("threads"));
+ new Description("Reserved threads").setGauge());
CallbackMetric0<Integer> queueSize =
metrics.newCallbackMetric(
"http/server/jetty/threadpool/queue_size",
Integer.class,
- new Description("Queued requests waiting for a thread").setGauge().setUnit("requests"));
+ new Description("Queued requests waiting for a thread").setGauge());
CallbackMetric0<Boolean> lowOnThreads =
metrics.newCallbackMetric(
"http/server/jetty/threadpool/is_low_on_threads",
Boolean.class,
new Description("Whether thread pool is low on threads").setGauge());
+ CallbackMetric0<Long> connections =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/connections",
+ Long.class,
+ new Description("The current number of open connections").setGauge());
+ CallbackMetric0<Long> connectionsTotal =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/connections_total",
+ Long.class,
+ new Description("The total number of connections opened").setGauge());
+ CallbackMetric0<Long> connectionDurationMax =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/connections_duration_max",
+ Long.class,
+ new Description("The max duration of a connection")
+ .setGauge()
+ .setUnit(Units.MILLISECONDS));
+ CallbackMetric0<Double> connectionDurationMean =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/connections_duration_mean",
+ Double.class,
+ new Description("The mean duration of a connection")
+ .setGauge()
+ .setUnit(Units.MILLISECONDS));
+ CallbackMetric0<Double> connectionDurationStDev =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/connections_duration_stdev",
+ Double.class,
+ new Description("The standard deviation of the duration of a connection")
+ .setGauge()
+ .setUnit(Units.MILLISECONDS));
+ CallbackMetric0<Long> receivedMessages =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/received_messages",
+ Long.class,
+ new Description("The total number of messages received").setGauge());
+ CallbackMetric0<Long> sentMessages =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/sent_messages",
+ Long.class,
+ new Description("The total number of messages sent").setGauge());
+ CallbackMetric0<Long> receivedBytes =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/received_bytes",
+ Long.class,
+ new Description("Total number of bytes received by tracked connections")
+ .setGauge()
+ .setUnit(Units.BYTES));
+ CallbackMetric0<Long> sentBytes =
+ metrics.newCallbackMetric(
+ "http/server/jetty/connections/sent_bytes",
+ Long.class,
+ new Description("Total number of bytes sent by tracked connections")
+ .setGauge()
+ .setUnit(Units.BYTES));
+
JettyServer.Metrics jettyMetrics = jetty.getMetrics();
metrics.newTrigger(
ImmutableSet.<CallbackMetric<?>>of(
@@ -76,7 +133,16 @@
maxPoolSize,
poolSize,
queueSize,
- lowOnThreads),
+ lowOnThreads,
+ connections,
+ connectionsTotal,
+ connectionDurationMax,
+ connectionDurationMean,
+ connectionDurationStDev,
+ receivedMessages,
+ sentMessages,
+ receivedBytes,
+ sentBytes),
() -> {
minPoolSize.set(jettyMetrics.getMinThreads());
maxPoolSize.set(jettyMetrics.getMaxThreads());
@@ -86,6 +152,15 @@
reservedThreads.set(jettyMetrics.getReservedThreads());
queueSize.set(jettyMetrics.getQueueSize());
lowOnThreads.set(jettyMetrics.isLowOnThreads());
+ connections.set(jettyMetrics.getConnections());
+ connectionsTotal.set(jettyMetrics.getConnectionsTotal());
+ connectionDurationMax.set(jettyMetrics.getConnectionDurationMax());
+ connectionDurationMean.set(jettyMetrics.getConnectionDurationMean());
+ connectionDurationStDev.set(jettyMetrics.getConnectionDurationStdDev());
+ receivedMessages.set(jettyMetrics.getReceivedMessages());
+ sentMessages.set(jettyMetrics.getSentMessages());
+ receivedBytes.set(jettyMetrics.getReceivedBytes());
+ sentBytes.set(jettyMetrics.getSentBytes());
});
}
}
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
index 5851fd0..b59dfc9 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -45,6 +45,7 @@
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.io.ConnectionStatistics;
import org.eclipse.jetty.jmx.MBeanContainer;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.ForwardedRequestCustomizer;
@@ -118,9 +119,11 @@
static class Metrics {
private final QueuedThreadPool threadPool;
+ private ConnectionStatistics connStats;
- Metrics(QueuedThreadPool threadPool) {
+ Metrics(QueuedThreadPool threadPool, ConnectionStatistics connStats) {
this.threadPool = threadPool;
+ this.connStats = connStats;
}
public int getIdleThreads() {
@@ -154,12 +157,49 @@
public boolean isLowOnThreads() {
return threadPool.isLowOnThreads();
}
+
+ public long getConnections() {
+ return connStats.getConnections();
+ }
+
+ public long getConnectionsTotal() {
+ return connStats.getConnectionsTotal();
+ }
+
+ public long getConnectionDurationMax() {
+ return connStats.getConnectionDurationMax();
+ }
+
+ public double getConnectionDurationMean() {
+ return connStats.getConnectionDurationMean();
+ }
+
+ public double getConnectionDurationStdDev() {
+ return connStats.getConnectionDurationStdDev();
+ }
+
+ public long getReceivedMessages() {
+ return connStats.getReceivedMessages();
+ }
+
+ public long getSentMessages() {
+ return connStats.getSentMessages();
+ }
+
+ public long getReceivedBytes() {
+ return connStats.getReceivedBytes();
+ }
+
+ public long getSentBytes() {
+ return connStats.getSentBytes();
+ }
}
private final SitePaths site;
private final Server httpd;
private final Metrics metrics;
private boolean reverseProxy;
+ private ConnectionStatistics connStats;
@Inject
JettyServer(
@@ -173,7 +213,11 @@
QueuedThreadPool pool = threadPool(cfg, threadSettingsConfig);
httpd = new Server(pool);
httpd.setConnectors(listen(httpd, cfg));
- metrics = new Metrics(pool);
+ connStats = new ConnectionStatistics();
+ for (Connector connector : httpd.getConnectors()) {
+ connector.addBean(connStats);
+ }
+ metrics = new Metrics(pool, connStats);
Handler app = makeContext(env, cfg);
if (cfg.getBoolean("httpd", "requestLog", !reverseProxy)) {
diff --git a/java/com/google/gerrit/pgm/init/api/Section.java b/java/com/google/gerrit/pgm/init/api/Section.java
index cbf32a1..b5d35f4 100644
--- a/java/com/google/gerrit/pgm/init/api/Section.java
+++ b/java/com/google/gerrit/pgm/init/api/Section.java
@@ -69,7 +69,7 @@
all.addAll(Arrays.asList(flags.cfg.getStringList(section, subsection, name)));
if (value != null) {
- if (all.size() == 0 || all.size() == 1) {
+ if (all.isEmpty() || all.size() == 1) {
flags.cfg.setString(section, subsection, name, value);
} else {
all.set(0, value);
@@ -78,7 +78,7 @@
} else if (all.size() == 1) {
flags.cfg.unset(section, subsection, name);
- } else if (all.size() != 0) {
+ } else if (!all.isEmpty()) {
all.remove(0);
flags.cfg.setStringList(section, subsection, name, all);
}
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 6a16a53..4fe9daa 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -462,8 +462,8 @@
}
if (filteredExtIdsByScheme.size() > 1
- || !filteredExtIdsByScheme.stream()
- .anyMatch(e -> e.key().equals(who.getExternalIdKey()))) {
+ || filteredExtIdsByScheme.stream()
+ .noneMatch(e -> e.key().equals(who.getExternalIdKey()))) {
u.deleteExternalIds(filteredExtIdsByScheme);
}
});
diff --git a/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java b/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
index 19582da..db350c6 100644
--- a/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java
@@ -37,8 +37,8 @@
for (AccountState accountState : accounts.all()) {
Account account = accountState.account();
if (account.preferredEmail() != null) {
- if (!accountState.externalIds().stream()
- .anyMatch(e -> account.preferredEmail().equals(e.email()))) {
+ if (accountState.externalIds().stream()
+ .noneMatch(e -> account.preferredEmail().equals(e.email()))) {
addError(
String.format(
"Account '%s' has no external ID for its preferred email '%s'",
diff --git a/java/com/google/gerrit/server/args4j/ChangeIdHandler.java b/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
index ddcd4db..448c654 100644
--- a/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
+++ b/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.util.cli.Localizable.localizable;
import com.google.common.base.Splitter;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -35,34 +36,42 @@
import org.kohsuke.args4j.spi.Setter;
public class ChangeIdHandler extends OptionHandler<Change.Id> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final Provider<InternalChangeQuery> queryProvider;
@Inject
public ChangeIdHandler(
// TODO(dborowitz): Not sure whether this is injectable here.
Provider<InternalChangeQuery> queryProvider,
- @Assisted final CmdLineParser parser,
- @Assisted final OptionDef option,
- @Assisted final Setter<Change.Id> setter) {
+ @Assisted CmdLineParser parser,
+ @Assisted OptionDef option,
+ @Assisted Setter<Change.Id> setter) {
super(parser, option, setter);
this.queryProvider = queryProvider;
}
@Override
public final int parseArguments(Parameters params) throws CmdLineException {
- final String token = params.getParameter(0);
- final List<String> tokens = Splitter.on(',').splitToList(token);
+ String token = params.getParameter(0);
+ List<String> tokens = Splitter.on(',').splitToList(token);
if (tokens.size() != 3) {
throw new CmdLineException(
owner, localizable("change should be specified as <project>,<branch>,<change-id>"));
}
try {
- final Change.Key key = Change.Key.parse(tokens.get(2));
- final Project.NameKey project = Project.nameKey(tokens.get(0));
- final BranchNameKey branch = BranchNameKey.create(project, tokens.get(1));
- for (ChangeData cd : queryProvider.get().byBranchKey(branch, key)) {
- setter.addValue(cd.getId());
+ Change.Key key = Change.Key.parse(tokens.get(2));
+ Project.NameKey project = Project.nameKey(tokens.get(0));
+ BranchNameKey branch = BranchNameKey.create(project, tokens.get(1));
+ List<ChangeData> changes = queryProvider.get().byBranchKey(branch, key);
+ if (!changes.isEmpty()) {
+ if (changes.size() > 1) {
+ String msg = "\"%s\": resolves to multiple changes";
+ logger.atSevere().log(msg, token);
+ throw new CmdLineException(owner, localizable(msg), token);
+ }
+ setter.addValue(changes.get(0).getId());
return 1;
}
} catch (IllegalArgumentException e) {
diff --git a/java/com/google/gerrit/server/cache/CacheMetrics.java b/java/com/google/gerrit/server/cache/CacheMetrics.java
index 1ef5a3b..5d30952 100644
--- a/java/com/google/gerrit/server/cache/CacheMetrics.java
+++ b/java/com/google/gerrit/server/cache/CacheMetrics.java
@@ -32,10 +32,11 @@
@Singleton
public class CacheMetrics {
+ private static final Field<String> F_NAME =
+ Field.ofString("cache_name", Metadata.Builder::cacheName).build();
+
@Inject
public CacheMetrics(MetricMaker metrics, DynamicMap<Cache<?, ?>> cacheMap) {
- Field<String> F_NAME = Field.ofString("cache_name", Metadata.Builder::cacheName).build();
-
CallbackMetric1<String, Long> memEnt =
metrics.newCallbackMetric(
"caches/memory_cached",
diff --git a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
index bd51fc7..cb4bff8 100644
--- a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
+++ b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
@@ -29,12 +29,12 @@
public class ChangeCleanupConfig {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static String SECTION = "changeCleanup";
- private static String KEY_ABANDON_AFTER = "abandonAfter";
- private static String KEY_ABANDON_IF_MERGEABLE = "abandonIfMergeable";
- private static String KEY_ABANDON_MESSAGE = "abandonMessage";
- private static String KEY_CLEANUP_ACCOUNT_PATCH_REVIEW = "cleanupAccountPatchReview";
- private static String DEFAULT_ABANDON_MESSAGE =
+ private static final String SECTION = "changeCleanup";
+ private static final String KEY_ABANDON_AFTER = "abandonAfter";
+ private static final String KEY_ABANDON_IF_MERGEABLE = "abandonIfMergeable";
+ private static final String KEY_ABANDON_MESSAGE = "abandonMessage";
+ private static final String KEY_CLEANUP_ACCOUNT_PATCH_REVIEW = "cleanupAccountPatchReview";
+ private static final String DEFAULT_ABANDON_MESSAGE =
"Auto-Abandoned due to inactivity, see "
+ "${URL}\n"
+ "\n"
diff --git a/java/com/google/gerrit/server/config/TrackingFootersProvider.java b/java/com/google/gerrit/server/config/TrackingFootersProvider.java
index 2114b1a..1611da9 100644
--- a/java/com/google/gerrit/server/config/TrackingFootersProvider.java
+++ b/java/com/google/gerrit/server/config/TrackingFootersProvider.java
@@ -34,10 +34,10 @@
private static final int MAX_LENGTH = 10;
- private static String TRACKING_ID_TAG = "trackingid";
- private static String FOOTER_TAG = "footer";
- private static String SYSTEM_TAG = "system";
- private static String REGEX_TAG = "match";
+ private static final String TRACKING_ID_TAG = "trackingid";
+ private static final String FOOTER_TAG = "footer";
+ private static final String SYSTEM_TAG = "system";
+ private static final String REGEX_TAG = "match";
private final List<TrackingFooter> trackingFooters = new ArrayList<>();
@Inject
diff --git a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
index a15b429..59ae6f8 100644
--- a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
+++ b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
@@ -22,7 +22,6 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
-import java.util.Map;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
@@ -42,7 +41,7 @@
public class QueryDocumentationExecutor {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static Map<String, Float> WEIGHTS =
+ private static final ImmutableMap<String, Float> WEIGHTS =
ImmutableMap.of(
Constants.TITLE_FIELD, 2.0f,
Constants.DOC_FIELD, 1.0f);
diff --git a/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
index b040f38..18b6a5e 100644
--- a/java/com/google/gerrit/server/events/StreamEventsApiListener.java
+++ b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@@ -230,7 +230,7 @@
}
String[] hashtagArray(Collection<String> hashtags) {
- if (hashtags != null && hashtags.size() > 0) {
+ if (hashtags != null && !hashtags.isEmpty()) {
return Sets.newHashSet(hashtags).toArray(new String[hashtags.size()]);
}
return null;
diff --git a/java/com/google/gerrit/server/git/GroupCollector.java b/java/com/google/gerrit/server/git/GroupCollector.java
index 1f0dcd4..9e0f2ee 100644
--- a/java/com/google/gerrit/server/git/GroupCollector.java
+++ b/java/com/google/gerrit/server/git/GroupCollector.java
@@ -149,7 +149,7 @@
checkState(!done, "visit() called after getGroups()");
Set<RevCommit> interestingParents = getInterestingParents(c);
- if (interestingParents.size() == 0) {
+ if (interestingParents.isEmpty()) {
// All parents are uninteresting: treat this commit as the root of a new
// group of related changes.
groups.put(c, c.name());
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 39fd103..89a2036 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.naturalOrder;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Strings;
@@ -44,8 +43,6 @@
import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -62,7 +59,6 @@
import com.google.gerrit.server.submit.IntegrationException;
import com.google.gerrit.server.submit.MergeIdenticalTreeException;
import com.google.gerrit.server.submit.MergeSorter;
-import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
@@ -128,44 +124,6 @@
*/
private static final int NAME_ABBREV_LEN = 6;
- static class PluggableCommitMessageGenerator {
- private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
-
- @Inject
- PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
- this.changeMessageModifiers = changeMessageModifiers;
- }
-
- public String generate(
- RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
- requireNonNull(original.getRawBuffer());
- if (mergeTip != null) {
- requireNonNull(mergeTip.getRawBuffer());
- }
-
- int count = 0;
- String current = originalMessage;
- for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
- ChangeMessageModifier changeMessageModifier = ext.get();
- String className = changeMessageModifier.getClass().getName();
- current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
- checkState(
- current != null,
- "%s.onSubmit from plugin %s returned null instead of new commit message",
- className,
- ext.getPluginName());
- count++;
- logger.atFine().log(
- "Invoked %s from plugin %s, message length now %d",
- className, ext.getPluginName(), current.length());
- }
- logger.atFine().log(
- "Invoked %d ChangeMessageModifiers on message with original length %d",
- count, originalMessage.length());
- return current;
- }
- }
-
private static final String R_HEADS_MASTER = Constants.R_HEADS + Constants.MASTER;
public static boolean useRecursiveMerge(Config cfg) {
@@ -432,7 +390,33 @@
RevCommit originalCommit,
String mergeStrategy,
boolean allowConflicts,
- PersonIdent committerIndent,
+ PersonIdent committerIdent,
+ String commitMsg,
+ CodeReviewRevWalk rw)
+ throws IOException, MergeIdenticalTreeException, MergeConflictException,
+ InvalidMergeStrategyException {
+ return createMergeCommit(
+ inserter,
+ repoConfig,
+ mergeTip,
+ originalCommit,
+ mergeStrategy,
+ allowConflicts,
+ committerIdent,
+ committerIdent,
+ commitMsg,
+ rw);
+ }
+
+ public static CodeReviewCommit createMergeCommit(
+ ObjectInserter inserter,
+ Config repoConfig,
+ RevCommit mergeTip,
+ RevCommit originalCommit,
+ String mergeStrategy,
+ boolean allowConflicts,
+ PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMsg,
CodeReviewRevWalk rw)
throws IOException, MergeIdenticalTreeException, MergeConflictException,
@@ -496,8 +480,8 @@
CommitBuilder mergeCommit = new CommitBuilder();
mergeCommit.setTreeId(tree);
mergeCommit.setParentIds(mergeTip, originalCommit);
- mergeCommit.setAuthor(committerIndent);
- mergeCommit.setCommitter(committerIndent);
+ mergeCommit.setAuthor(authorIdent);
+ mergeCommit.setCommitter(committerIdent);
mergeCommit.setMessage(commitMsg);
CodeReviewCommit commit = rw.parseCommit(inserter.insert(mergeCommit));
commit.setFilesWithGitConflicts(filesWithGitConflicts);
diff --git a/java/com/google/gerrit/server/git/NotifyConfig.java b/java/com/google/gerrit/server/git/NotifyConfig.java
index 2ca2744a..429f15a 100644
--- a/java/com/google/gerrit/server/git/NotifyConfig.java
+++ b/java/com/google/gerrit/server/git/NotifyConfig.java
@@ -50,11 +50,11 @@
return types.contains(type) || types.contains(NotifyType.ALL);
}
- public EnumSet<NotifyType> getNotify() {
+ public Set<NotifyType> getNotify() {
return types;
}
- public void setTypes(EnumSet<NotifyType> newTypes) {
+ public void setTypes(Set<NotifyType> newTypes) {
types = EnumSet.copyOf(newTypes);
}
diff --git a/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
new file mode 100644
index 0000000..804a218
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
@@ -0,0 +1,71 @@
+// 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.server.git;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
+import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/** Helper to call plugins that want to change the commit message before a change is merged. */
+public class PluggableCommitMessageGenerator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+
+ @Inject
+ PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
+ this.changeMessageModifiers = changeMessageModifiers;
+ }
+
+ /**
+ * Returns the commit message as modified by plugins. The returned message can be equal to {@code
+ * originalMessage} in case no plugins are registered or the registered plugins decided not to
+ * modify the message.
+ */
+ public String generate(
+ RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
+ requireNonNull(original.getRawBuffer());
+ if (mergeTip != null) {
+ requireNonNull(mergeTip.getRawBuffer());
+ }
+
+ int count = 0;
+ String current = originalMessage;
+ for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
+ ChangeMessageModifier changeMessageModifier = ext.get();
+ String className = changeMessageModifier.getClass().getName();
+ current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
+ checkState(
+ current != null,
+ "%s.onSubmit from plugin %s returned null instead of new commit message",
+ className,
+ ext.getPluginName());
+ count++;
+ logger.atFine().log(
+ "Invoked %s from plugin %s, message length now %d",
+ className, ext.getPluginName(), current.length());
+ }
+ logger.atFine().log(
+ "Invoked %d ChangeMessageModifiers on message with original length %d",
+ count, originalMessage.length());
+ return current;
+ }
+}
diff --git a/java/com/google/gerrit/server/git/meta/TabFile.java b/java/com/google/gerrit/server/git/meta/TabFile.java
index f5d7037..c9a8e77 100644
--- a/java/com/google/gerrit/server/git/meta/TabFile.java
+++ b/java/com/google/gerrit/server/git/meta/TabFile.java
@@ -34,7 +34,7 @@
String parse(String str);
}
- public static Parser TRIM = String::trim;
+ public static final Parser TRIM = String::trim;
protected static class Row {
public String left;
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index 38c8d7d..8ab2779 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -412,7 +412,7 @@
// read the subject line and use it as reflog message
ru.setRefLogMessage("commit: " + reader.readLine(), true);
}
- logger.atFine().log("Saving commit: " + message);
+ logger.atFine().log("Saving commit '%s' on project '%s'", message.trim(), projectName);
inserter.flush();
RefUpdate.Result result = ru.update();
switch (result) {
@@ -420,6 +420,9 @@
case FAST_FORWARD:
revision = rw.parseCommit(ru.getNewObjectId());
update.fireGitRefUpdatedEvent(ru);
+ logger.atFine().log(
+ "Saved commit '%s' as revision '%s' on project '%s'",
+ message.trim(), revision.name(), projectName);
return revision;
case LOCK_FAILURE:
throw new LockFailureException(errorMsg(ru, db.getDirectory()), ru);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 4b6a4cd..3013b02 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2267,7 +2267,7 @@
return Collections.emptyList();
}
- if (changes.size() == 0) {
+ if (changes.isEmpty()) {
if (!isValidChangeId(p.changeKey.get())) {
reject(magicBranch.cmd, "invalid Change-Id");
return Collections.emptyList();
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
index 7e1353c..63c5297 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
@@ -78,7 +78,7 @@
* @return the maximal set of statuses that any changes matching the input predicates may have,
* based on examining boolean and {@link ChangeStatusPredicate}s.
*/
- public static EnumSet<Change.Status> getPossibleStatus(Predicate<ChangeData> in) {
+ public static Set<Change.Status> getPossibleStatus(Predicate<ChangeData> in) {
EnumSet<Change.Status> s = extractStatus(in);
return s != null ? s : EnumSet.allOf(Change.Status.class);
}
diff --git a/java/com/google/gerrit/server/logging/BUILD b/java/com/google/gerrit/server/logging/BUILD
index 2c2341f..7af34f7 100644
--- a/java/com/google/gerrit/server/logging/BUILD
+++ b/java/com/google/gerrit/server/logging/BUILD
@@ -10,11 +10,13 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server/util/time",
+ "//lib:gson",
"//lib:guava",
"//lib:jgit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/flogger:api",
"//lib/guice",
+ "//lib/log:log4j",
],
)
diff --git a/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index be36f55..d2fa69c 100644
--- a/java/com/google/gerrit/server/mail/send/ProjectWatch.java
+++ b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
@@ -244,7 +244,7 @@
qb = args.queryBuilder.get().asUser(args.anonymousUser.get());
} else {
qb = args.queryBuilder.get().asUser(user);
- p = qb.is_visible();
+ p = qb.isVisible();
}
if (filter != null) {
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index d301d34..7136d2b 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -110,7 +110,7 @@
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
checkUserType(u);
if (u instanceof IdentifiedUser) {
- return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
+ return noteUtil.newIdent(u.asIdentifiedUser().getAccount().id(), when, serverIdent);
} else if (u instanceof InternalUser) {
return serverIdent;
}
@@ -164,10 +164,6 @@
return accountId;
}
- protected PersonIdent newIdent(Account.Id authorId, Date when) {
- return noteUtil.newIdent(authorId, when, serverIdent);
- }
-
/** Whether no updates have been done. */
public abstract boolean isEmpty();
@@ -184,6 +180,14 @@
protected abstract String getRefName();
+ protected void setParentCommit(CommitBuilder cb, ObjectId parentCommitId) {
+ if (!parentCommitId.equals(ObjectId.zeroId())) {
+ cb.setParentId(parentCommitId);
+ } else {
+ cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
+ }
+ }
+
/**
* Whether to allow bypassing the check that an update does not exceed the max update count on an
* object.
@@ -224,11 +228,7 @@
}
cb.setAuthor(authorIdent);
cb.setCommitter(new PersonIdent(serverIdent, when));
- if (!curr.equals(z)) {
- cb.setParentId(curr);
- } else {
- cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
- }
+ setParentCommit(cb, curr);
if (cb.getTreeId() == null) {
if (curr.equals(z)) {
cb.setTreeId(emptyTree(ins)); // No parent, assume empty tree.
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 72a460c..4b538f3 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -289,6 +289,11 @@
}
@Override
+ protected void setParentCommit(CommitBuilder cb, ObjectId parentCommitId) {
+ cb.setParentIds(); // Draft updates should not keep history of parent commits
+ }
+
+ @Override
public boolean isEmpty() {
return delete.isEmpty() && put.isEmpty();
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index cebb67d..287f3e7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -15,10 +15,13 @@
package com.google.gerrit.server.notedb;
import com.google.auto.value.AutoValue;
-import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
+import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.config.GerritServerId;
+import com.google.gson.Gson;
import com.google.inject.Inject;
+import java.time.Instant;
import java.util.Date;
import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
@@ -27,42 +30,31 @@
import org.eclipse.jgit.util.RawParseUtils;
public class ChangeNoteUtil {
- public static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
- public static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
- public static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
- public static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
- public static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
- public static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
- public static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
- public static final FooterKey FOOTER_LABEL = new FooterKey("Label");
- public static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
- public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
- new FooterKey("Patch-set-description");
- public static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
- public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
- public static final FooterKey FOOTER_STATUS = new FooterKey("Status");
- public static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
- public static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
- public static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
- public static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
- public static final FooterKey FOOTER_TAG = new FooterKey("Tag");
- public static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
- public static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
- public static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
- static final String AUTHOR = "Author";
- static final String BASE_PATCH_SET = "Base-for-patch-set";
- static final String COMMENT_RANGE = "Comment-range";
- static final String FILE = "File";
- static final String LENGTH = "Bytes";
- static final String PARENT = "Parent";
- static final String PARENT_NUMBER = "Parent-number";
- static final String PATCH_SET = "Patch-set";
- static final String REAL_AUTHOR = "Real-author";
- static final String REVISION = "Revision";
- static final String UUID = "UUID";
- static final String UNRESOLVED = "Unresolved";
- static final String TAG = FOOTER_TAG.getName();
+ static final FooterKey FOOTER_ATTENTION = new FooterKey("Attention");
+ static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
+ static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
+ static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
+ static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
+ static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
+ static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
+ static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
+ static final FooterKey FOOTER_LABEL = new FooterKey("Label");
+ static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
+ static final FooterKey FOOTER_PATCH_SET_DESCRIPTION = new FooterKey("Patch-set-description");
+ static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
+ static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
+ static final FooterKey FOOTER_STATUS = new FooterKey("Status");
+ static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
+ static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
+ static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
+ static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
+ static final FooterKey FOOTER_TAG = new FooterKey("Tag");
+ static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
+ static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
+ static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
+
+ private static final Gson gson = OutputFormat.JSON_COMPACT.newGson();
private final ChangeNoteJson changeNoteJson;
private final String serverId;
@@ -77,21 +69,17 @@
return changeNoteJson;
}
- public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
+ public PersonIdent newIdent(Account.Id accountId, Date when, PersonIdent serverIdent) {
return new PersonIdent(
- "Gerrit User " + authorId.toString(),
- authorId.get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ getUsername(accountId), getEmailAddress(accountId), when, serverIdent.getTimeZone());
}
- @VisibleForTesting
- public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
- return new PersonIdent(
- "Gerrit User " + author.id(),
- author.id().get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ private static String getUsername(Account.Id accountId) {
+ return "Gerrit User " + accountId.toString();
+ }
+
+ private String getEmailAddress(Account.Id accountId) {
+ return accountId.get() + "@" + serverId;
}
public static Optional<CommitMessageRange> parseCommitMessageRange(RevCommit commit) {
@@ -151,6 +139,7 @@
@AutoValue
public abstract static class CommitMessageRange {
+
public abstract int subjectStart();
public abstract int subjectEnd();
@@ -165,6 +154,7 @@
@AutoValue.Builder
public abstract static class Builder {
+
abstract Builder subjectStart(int subjectStart);
abstract Builder subjectEnd(int subjectEnd);
@@ -176,4 +166,51 @@
abstract CommitMessageRange build();
}
}
+
+ /** Helper class for JSON serialization. Timestamp is taken from the commit. */
+ private static class AttentionStatusInNoteDb {
+
+ final String personIdent;
+ final AttentionStatus.Operation operation;
+ final String reason;
+
+ AttentionStatusInNoteDb(
+ String personIndent, AttentionStatus.Operation operation, String reason) {
+ this.personIdent = personIndent;
+ this.operation = operation;
+ this.reason = reason;
+ }
+ }
+
+ /** The returned {@link Optional} holds the parsed entity or is empty if parsing failed. */
+ static Optional<AttentionStatus> attentionStatusFromJson(
+ Instant timestamp, String attentionString) {
+ AttentionStatusInNoteDb inNoteDb =
+ gson.fromJson(attentionString, AttentionStatusInNoteDb.class);
+ PersonIdent personIdent = RawParseUtils.parsePersonIdent(inNoteDb.personIdent);
+ if (personIdent == null) {
+ return Optional.empty();
+ }
+ Optional<Account.Id> account = NoteDbUtil.parseIdent(personIdent);
+ return account.map(
+ id -> AttentionStatus.createFromRead(timestamp, id, inNoteDb.operation, inNoteDb.reason));
+ }
+
+ String attentionStatusToJson(AttentionStatus attentionStatus) {
+ PersonIdent personIdent =
+ new PersonIdent(
+ getUsername(attentionStatus.account()), getEmailAddress(attentionStatus.account()));
+ StringBuilder stringBuilder = new StringBuilder();
+ appendIdentString(stringBuilder, personIdent.getName(), personIdent.getEmailAddress());
+ return gson.toJson(
+ new AttentionStatusInNoteDb(
+ stringBuilder.toString(), attentionStatus.operation(), attentionStatus.reason()));
+ }
+
+ static void appendIdentString(StringBuilder stringBuilder, String name, String emailAddress) {
+ PersonIdent.appendSanitized(stringBuilder, name);
+ stringBuilder.append(" <");
+ PersonIdent.appendSanitized(stringBuilder, emailAddress);
+ stringBuilder.append('>');
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 8cf0046..2de2195 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -40,6 +40,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -211,6 +212,7 @@
return sr.all().stream().map(id -> scanOneChange(project, sr, id)).filter(Objects::nonNull);
}
+ @Nullable
private ChangeNotesResult scanOneChange(Project.NameKey project, ScanResult sr, Change.Id id) {
if (!sr.fromMetaRefs().contains(id)) {
// Stray patch set refs can happen due to normal error conditions, e.g. failed
@@ -219,10 +221,15 @@
}
// TODO(dborowitz): See discussion in BatchUpdate#newChangeContext.
- Change change = ChangeNotes.Factory.newChange(project, id);
-
- logger.atFine().log("adding change %s found in project %s", id, project);
- return toResult(change);
+ try {
+ Change change = ChangeNotes.Factory.newChange(project, id);
+ logger.atFine().log("adding change %s found in project %s", id, project);
+ return toResult(change);
+ } catch (InvalidServerIdException ise) {
+ logger.atWarning().withCause(ise).log(
+ "skipping change %d in project %s because of an invalid server id", id.get(), project);
+ return null;
+ }
}
@Nullable
@@ -369,6 +376,10 @@
return state.reviewerUpdates();
}
+ public ImmutableList<AttentionStatus> getAttentionUpdates() {
+ return state.attentionUpdates();
+ }
+
/**
* @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. The
* order of the set is the order in which they were assigned.
@@ -531,9 +542,9 @@
* be to bump the cache version, but that would invalidate all persistent cache entries, what we
* rather try to avoid.
*/
- checkState(
- Strings.isNullOrEmpty(stateServerId) || args.serverId.equals(stateServerId),
- String.format("invalid server id, expected %s: actual: %s", args.serverId, stateServerId));
+ if (!Strings.isNullOrEmpty(stateServerId) && !args.serverId.equals(stateServerId)) {
+ throw new InvalidServerIdException(args.serverId, stateServerId);
+ }
state.copyColumnsTo(change);
revisionNoteMap = v.revisionNoteMap();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 428df16..ed6039f 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -16,6 +16,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
@@ -43,6 +44,7 @@
import com.google.common.base.Enums;
import com.google.common.base.Splitter;
import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.ListMultimap;
@@ -57,6 +59,7 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@@ -75,6 +78,7 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -102,7 +106,6 @@
// Private final members initialized in the constructor.
private final ChangeNoteJson changeNoteJson;
-
private final NoteDbMetrics metrics;
private final Change.Id id;
private final ObjectId tip;
@@ -114,6 +117,9 @@
private final Table<Address, ReviewerStateInternal, Timestamp> reviewersByEmail;
private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates;
+ /** Holds only the most recent update per user. Older updates are discarded. */
+ private final Map<Account.Id, AttentionStatus> latestAttentionStatus;
+
private final List<AssigneeStatusUpdate> assigneeUpdates;
private final List<SubmitRecord> submitRecords;
private final ListMultimap<ObjectId, Comment> comments;
@@ -169,6 +175,7 @@
pendingReviewersByEmail = ReviewerByEmailSet.empty();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
+ latestAttentionStatus = new HashMap<>();
assigneeUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
@@ -239,6 +246,7 @@
pendingReviewersByEmail,
allPastReviewers,
buildReviewerUpdates(),
+ ImmutableList.copyOf(latestAttentionStatus.values()),
assigneeUpdates,
submitRecords,
buildAllMessages(),
@@ -359,6 +367,7 @@
}
parseHashtags(commit);
+ parseAttentionUpdates(commit);
parseAssigneeUpdates(ts, commit);
if (submissionId == null) {
@@ -569,6 +578,21 @@
}
}
+ private void parseAttentionUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
+ List<String> attentionStrings = commit.getFooterLineValues(FOOTER_ATTENTION);
+ for (String attentionString : attentionStrings) {
+
+ Optional<AttentionStatus> attentionStatus =
+ ChangeNoteUtil.attentionStatusFromJson(
+ Instant.ofEpochSecond(commit.getCommitTime()), attentionString);
+ if (!attentionStatus.isPresent()) {
+ throw invalidFooter(FOOTER_ATTENTION, attentionString);
+ }
+ // Processing is in reverse chronological order. Keep only the latest update.
+ latestAttentionStatus.putIfAbsent(attentionStatus.get().account(), attentionStatus.get());
+ }
+ }
+
private void parseAssigneeUpdates(Timestamp ts, ChangeNotesCommit commit)
throws ConfigInvalidException {
String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 064e43b..1931e7e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -35,6 +35,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -55,6 +56,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -66,6 +68,7 @@
import com.google.protobuf.ByteString;
import com.google.protobuf.MessageLite;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -83,11 +86,12 @@
*/
@AutoValue
public abstract class ChangeNotesState {
+
static ChangeNotesState empty(Change change) {
return Builder.empty(change.getId()).build();
}
- static Builder builder() {
+ private static Builder builder() {
return new AutoValue_ChangeNotesState.Builder();
}
@@ -115,6 +119,7 @@
ReviewerByEmailSet pendingReviewersByEmail,
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
+ List<AttentionStatus> attentionStatusUpdates,
List<AssigneeStatusUpdate> assigneeUpdates,
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
@@ -165,6 +170,7 @@
.pendingReviewersByEmail(pendingReviewersByEmail)
.allPastReviewers(allPastReviewers)
.reviewerUpdates(reviewerUpdates)
+ .attentionUpdates(attentionStatusUpdates)
.assigneeUpdates(assigneeUpdates)
.submitRecords(submitRecords)
.changeMessages(changeMessages)
@@ -180,6 +186,7 @@
*/
@AutoValue
abstract static class ChangeColumns {
+
static Builder builder() {
return new AutoValue_ChangeNotesState_ChangeColumns.Builder();
}
@@ -229,6 +236,7 @@
@AutoValue.Builder
abstract static class Builder {
+
abstract Builder changeKey(Change.Key changeKey);
abstract Builder createdOn(Timestamp createdOn);
@@ -297,6 +305,8 @@
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();
+ abstract ImmutableList<AttentionStatus> attentionUpdates();
+
abstract ImmutableList<AssigneeStatusUpdate> assigneeUpdates();
abstract ImmutableList<SubmitRecord> submitRecords();
@@ -361,6 +371,7 @@
@AutoValue.Builder
abstract static class Builder {
+
static Builder empty(Change.Id changeId) {
return new AutoValue_ChangeNotesState.Builder()
.changeId(changeId)
@@ -373,6 +384,7 @@
.pendingReviewersByEmail(ReviewerByEmailSet.empty())
.allPastReviewers(ImmutableList.of())
.reviewerUpdates(ImmutableList.of())
+ .attentionUpdates(ImmutableList.of())
.assigneeUpdates(ImmutableList.of())
.submitRecords(ImmutableList.of())
.changeMessages(ImmutableList.of())
@@ -406,6 +418,8 @@
abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
+ abstract Builder attentionUpdates(List<AttentionStatus> attentionUpdates);
+
abstract Builder assigneeUpdates(List<AssigneeStatusUpdate> assigneeUpdates);
abstract Builder submitRecords(List<SubmitRecord> submitRecords);
@@ -473,6 +487,7 @@
object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get()));
object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u)));
+ object.attentionUpdates().forEach(u -> b.addAttentionStatus(toAttentionStatusProto(u)));
object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u)));
object
.submitRecords()
@@ -556,6 +571,15 @@
.build();
}
+ private static AttentionStatusProto toAttentionStatusProto(AttentionStatus attentionStatus) {
+ return AttentionStatusProto.newBuilder()
+ .setDate(attentionStatus.timestamp().toEpochMilli())
+ .setAccount(attentionStatus.account().get())
+ .setOperation(attentionStatus.operation().name())
+ .setReason(attentionStatus.reason())
+ .build();
+ }
+
private static AssigneeStatusUpdateProto toAssigneeStatusUpdateProto(AssigneeStatusUpdate u) {
AssigneeStatusUpdateProto.Builder builder =
AssigneeStatusUpdateProto.newBuilder()
@@ -596,6 +620,7 @@
.allPastReviewers(
proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList()))
.reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList()))
+ .attentionUpdates(toAttentionUpdateList(proto.getAttentionStatusList()))
.assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList()))
.submitRecords(
proto.getSubmitRecordList().stream()
@@ -694,6 +719,20 @@
return b.build();
}
+ private static ImmutableList<AttentionStatus> toAttentionUpdateList(
+ List<AttentionStatusProto> protos) {
+ ImmutableList.Builder<AttentionStatus> b = ImmutableList.builder();
+ for (AttentionStatusProto proto : protos) {
+ b.add(
+ AttentionStatus.createFromRead(
+ Instant.ofEpochMilli(proto.getDate()),
+ Account.id(proto.getAccount()),
+ AttentionStatus.Operation.valueOf(proto.getOperation()),
+ proto.getReason()));
+ }
+ return b.build();
+ }
+
private static ImmutableList<AssigneeStatusUpdate> toAssigneeStatusUpdateList(
List<AssigneeStatusUpdateProto> protos) {
ImmutableList.Builder<AssigneeStatusUpdate> b = ImmutableList.builder();
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 2822f61..3ef631b 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -19,6 +19,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
@@ -52,6 +53,7 @@
import com.google.common.collect.TreeBasedTable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Project;
@@ -125,6 +127,7 @@
private String submissionId;
private String topic;
private String commit;
+ private List<AttentionStatus> attentionUpdates;
private Optional<Account.Id> assignee;
private Set<String> hashtags;
private String changeMessage;
@@ -368,6 +371,21 @@
this.hashtags = hashtags;
}
+ /**
+ * All updates must have a timestamp of null since we use the commit's timestamp. There also must
+ * not be multiple updates for a single user.
+ */
+ void setAttentionUpdates(List<AttentionStatus> attentionUpdates) {
+ checkArgument(
+ attentionUpdates.stream().noneMatch(x -> x.timestamp() != null),
+ "must not specify timestamp for write");
+ checkArgument(
+ attentionUpdates.stream().map(AttentionStatus::account).distinct().count()
+ == attentionUpdates.size(),
+ "must not specify multiple updates for single user");
+ this.attentionUpdates = attentionUpdates;
+ }
+
public void setAssignee(Account.Id assignee) {
checkArgument(assignee != null, "use removeAssignee");
this.assignee = Optional.of(assignee);
@@ -578,6 +596,12 @@
addFooter(msg, FOOTER_COMMIT, commit);
}
+ if (attentionUpdates != null) {
+ for (AttentionStatus attentionUpdate : attentionUpdates) {
+ addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionStatusToJson(attentionUpdate));
+ }
+ }
+
if (assignee != null) {
if (assignee.isPresent()) {
addFooter(msg, FOOTER_ASSIGNEE);
@@ -713,6 +737,7 @@
&& status == null
&& submissionId == null
&& submitRecords == null
+ && attentionUpdates == null
&& assignee == null
&& hashtags == null
&& topic == null
@@ -774,12 +799,8 @@
}
private StringBuilder addIdent(StringBuilder sb, Account.Id accountId) {
- PersonIdent ident = newIdent(accountId, when);
-
- PersonIdent.appendSanitized(sb, ident.getName());
- sb.append(" <");
- PersonIdent.appendSanitized(sb, ident.getEmailAddress());
- sb.append('>');
+ PersonIdent ident = noteUtil.newIdent(accountId, when, serverIdent);
+ ChangeNoteUtil.appendIdentString(sb, ident.getName(), ident.getEmailAddress());
return sb;
}
}
diff --git a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
index 9ec1d69..128e185 100644
--- a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
+++ b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.collect.Iterables;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.server.config.AllUsersName;
@@ -42,9 +43,12 @@
* and not get deleted. These refs point to an empty tree.
*/
public class DeleteZombieCommentsRefs {
- private final String EMPTY_TREE_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
- private final String DRAFT_REFS_PREFIX = "refs/draft-comments";
- private final int CHUNK_SIZE = 100; // log progress after deleting every CHUNK_SIZE refs
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private static final String EMPTY_TREE_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
+ private static final String DRAFT_REFS_PREFIX = "refs/draft-comments";
+ private static final int CHUNK_SIZE = 100; // log progress after deleting every CHUNK_SIZE refs
+
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final int cleanupPercentage;
@@ -70,18 +74,15 @@
List<Ref> draftRefs = allUsersRepo.getRefDatabase().getRefsByPrefix(DRAFT_REFS_PREFIX);
List<Ref> zombieRefs = filterZombieRefs(draftRefs);
- System.out.println(
- String.format(
- "Found a total of %d zombie draft refs in %s repo.",
- zombieRefs.size(), allUsers.get()));
+ logger.atInfo().log(
+ "Found a total of %d zombie draft refs in %s repo.", zombieRefs.size(), allUsers.get());
- System.out.println(String.format("Cleanup percentage = %d", cleanupPercentage));
+ logger.atInfo().log("Cleanup percentage = %d", cleanupPercentage);
zombieRefs =
zombieRefs.stream()
.filter(ref -> Change.Id.fromAllUsersRef(ref.getName()).get() % 100 < cleanupPercentage)
.collect(toImmutableList());
- System.out.println(
- String.format("Number of zombie refs to be cleaned = %d", zombieRefs.size()));
+ logger.atInfo().log("Number of zombie refs to be cleaned = %d", zombieRefs.size());
long zombieRefsCnt = zombieRefs.size();
long deletedRefsCnt = 0;
@@ -124,7 +125,7 @@
}
private void logProgress(long deletedRefsCount, long allRefsCount, long elapsed) {
- System.out.format(
+ logger.atInfo().log(
"Deleted %d/%d zombie draft refs (%d seconds)\n", deletedRefsCount, allRefsCount, elapsed);
}
}
diff --git a/java/com/google/gerrit/server/notedb/InvalidServerIdException.java b/java/com/google/gerrit/server/notedb/InvalidServerIdException.java
new file mode 100644
index 0000000..f79e07c
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/InvalidServerIdException.java
@@ -0,0 +1,25 @@
+// 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.server.notedb;
+
+public class InvalidServerIdException extends IllegalStateException {
+ private static final long serialVersionUID = 5302751510361680907L;
+
+ public InvalidServerIdException(String expectedServerId, String actualServerId) {
+ super(
+ String.format(
+ "invalid server id, expected %s: actual: %s", expectedServerId, actualServerId));
+ }
+}
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 8ee0fee..653c3b5f 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -484,12 +484,12 @@
public Set<LabelPermission.WithValue> testLabels(Collection<LabelType> types)
throws PermissionBackendException {
requireNonNull(types, "LabelType");
- return test(types.stream().flatMap((t) -> valuesOf(t).stream()).collect(toSet()));
+ return test(types.stream().flatMap(t -> valuesOf(t).stream()).collect(toSet()));
}
private static Set<LabelPermission.WithValue> valuesOf(LabelType label) {
return label.getValues().stream()
- .map((v) -> new LabelPermission.WithValue(label, v))
+ .map(v -> new LabelPermission.WithValue(label, v))
.collect(toSet());
}
}
diff --git a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
index 3a73d0c..ae9828a 100644
--- a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
+++ b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
@@ -27,7 +27,7 @@
/** Provides transformations to get and set BooleanProjectConfigs from the API. */
public class BooleanProjectConfigTransformations {
- private static ImmutableMap<BooleanProjectConfig, Mapper> MAPPER =
+ private static final ImmutableMap<BooleanProjectConfig, Mapper> MAPPER =
ImmutableMap.<BooleanProjectConfig, Mapper>builder()
.put(
BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS,
diff --git a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
index 110beaf..7ed2491 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -233,7 +233,7 @@
predicates.add(new CommitPredicate(commit.name()));
}
- if (predicates.size() > 0) {
+ if (!predicates.isEmpty()) {
// Execute the query with the remaining predicates that were collected.
autoCloseableChanges.addAll(
executeQueryAndAutoCloseChanges(
diff --git a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
index b233260..091edca 100644
--- a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
+++ b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
@@ -71,7 +71,7 @@
List<AccountState> accountStates = byExternalId(externalId);
if (accountStates.size() == 1) {
return accountStates.get(0);
- } else if (accountStates.size() > 0) {
+ } else if (!accountStates.isEmpty()) {
StringBuilder msg = new StringBuilder();
msg.append("Ambiguous external ID ").append(externalId).append(" for accounts: ");
Joiner.on(", ")
diff --git a/java/com/google/gerrit/server/query/change/AndChangeSource.java b/java/com/google/gerrit/server/query/change/AndChangeSource.java
index 4a3b936..749204f 100644
--- a/java/com/google/gerrit/server/query/change/AndChangeSource.java
+++ b/java/com/google/gerrit/server/query/change/AndChangeSource.java
@@ -35,9 +35,7 @@
@Override
public boolean hasChange() {
- return source != null
- && source instanceof ChangeDataSource
- && ((ChangeDataSource) source).hasChange();
+ return source instanceof ChangeDataSource && ((ChangeDataSource) source).hasChange();
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 327c21f..f8610db 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -512,7 +512,7 @@
return ChangeStatusPredicate.parse(statusName);
}
- public Predicate<ChangeData> status_open() {
+ public Predicate<ChangeData> statusOpen() {
return ChangeStatusPredicate.open();
}
@@ -561,7 +561,7 @@
}
if ("visible".equalsIgnoreCase(value)) {
- return is_visible();
+ return isVisible();
}
if ("reviewed".equalsIgnoreCase(value)) {
@@ -973,7 +973,7 @@
public Predicate<ChangeData> visibleto(String who)
throws QueryParseException, IOException, ConfigInvalidException {
if (isSelf(who)) {
- return is_visible();
+ return isVisible();
}
try {
return Predicate.or(
@@ -1008,7 +1008,7 @@
args.anonymousUserProvider);
}
- public Predicate<ChangeData> is_visible() throws QueryParseException {
+ public Predicate<ChangeData> isVisible() throws QueryParseException {
return visibleto(args.getUser());
}
diff --git a/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java b/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
index 218a89d..3a43fd3 100644
--- a/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
+++ b/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
@@ -77,13 +77,13 @@
} else if (f != null) {
r.add(f);
} else {
- r.add(builder.status_open());
+ r.add(builder.statusOpen());
}
}
if (r.isEmpty()) {
return ImmutableList.of(ChangeIndexPredicate.none());
} else if (checkIsVisible) {
- return ImmutableList.of(or(r), builder.is_visible());
+ return ImmutableList.of(or(r), builder.isVisible());
} else {
return ImmutableList.of(or(r));
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
index 442b6a4..80f0bb1 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
@@ -76,7 +76,7 @@
permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
}
- if (extIds == null || extIds.size() == 0) {
+ if (extIds == null || extIds.isEmpty()) {
throw new BadRequestException("external IDs are required");
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index a6a1eef..9e792d0 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -335,9 +335,10 @@
Timestamp now = TimeUtil.nowTs();
+ PersonIdent committer = me.newCommitterIdent(now, serverTimeZone);
PersonIdent author =
input.author == null
- ? me.newCommitterIdent(now, serverTimeZone)
+ ? committer
: new PersonIdent(input.author.name, input.author.email, now, serverTimeZone);
String commitMessage = getCommitMessage(input.subject, me);
@@ -345,7 +346,9 @@
CodeReviewCommit c;
if (input.merge != null) {
// create a merge commit
- c = newMergeCommit(git, oi, rw, projectState, mergeTip, input.merge, author, commitMessage);
+ c =
+ newMergeCommit(
+ git, oi, rw, projectState, mergeTip, input.merge, author, committer, commitMessage);
if (!c.getFilesWithGitConflicts().isEmpty()) {
logger.atFine().log(
"merge commit has conflicts in the following files: %s",
@@ -353,7 +356,7 @@
}
} else {
// create an empty commit
- c = newCommit(oi, rw, author, mergeTip, commitMessage);
+ c = newCommit(oi, rw, author, committer, mergeTip, commitMessage);
}
Change.Id changeId = Change.id(seq.nextChangeId());
@@ -495,6 +498,7 @@
ObjectInserter oi,
CodeReviewRevWalk rw,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
RevCommit mergeTip,
String commitMessage)
throws IOException {
@@ -507,7 +511,7 @@
commit.setParentId(mergeTip);
}
commit.setAuthor(authorIdent);
- commit.setCommitter(authorIdent);
+ commit.setCommitter(committerIdent);
commit.setMessage(commitMessage);
return rw.parseCommit(insert(oi, commit));
}
@@ -520,6 +524,7 @@
RevCommit mergeTip,
MergeInput merge,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMessage)
throws RestApiException, IOException {
logger.atFine().log(
@@ -556,6 +561,7 @@
mergeStrategy,
merge.allowConflicts,
authorIdent,
+ committerIdent,
commitMessage,
rw);
} catch (NoMergeBaseException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/GetPatch.java b/java/com/google/gerrit/server/restapi/change/GetPatch.java
index 66ccef3..187ebce 100644
--- a/java/com/google/gerrit/server/restapi/change/GetPatch.java
+++ b/java/com/google/gerrit/server/restapi/change/GetPatch.java
@@ -43,7 +43,7 @@
public class GetPatch implements RestReadView<RevisionResource> {
private final GitRepositoryManager repoManager;
- private final String FILE_NOT_FOUND = "File not found: %s.";
+ private static final String FILE_NOT_FOUND = "File not found: %s.";
@Option(name = "--zip")
private boolean zip;
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index b5035d8..e8395f0 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -308,8 +308,6 @@
cherryPickInput.message = revertInput.message;
ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
- // TODO (paiking): In the the future, the timestamp should be the same for all the revert
- // changes.
try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.setNotify(
notifyResolver.resolve(
diff --git a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
index caa256f..214a001 100644
--- a/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
+++ b/java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
@@ -46,6 +46,7 @@
import java.util.Comparator;
import java.util.EnumSet;
import java.util.List;
+import java.util.Set;
import org.kohsuke.args4j.Option;
public class SubmittedTogether implements RestReadView<ChangeResource> {
@@ -97,12 +98,12 @@
this.sorter = sorter;
}
- public SubmittedTogether addListChangesOption(EnumSet<ListChangesOption> o) {
+ public SubmittedTogether addListChangesOption(Set<ListChangesOption> o) {
jsonOpt.addAll(o);
return this;
}
- public SubmittedTogether addSubmittedTogetherOption(EnumSet<SubmittedTogetherOption> o) {
+ public SubmittedTogether addSubmittedTogetherOption(Set<SubmittedTogetherOption> o) {
options.addAll(o);
return this;
}
diff --git a/java/com/google/gerrit/server/restapi/group/ListGroups.java b/java/com/google/gerrit/server/restapi/group/ListGroups.java
index adc251c..a0e1e2e 100644
--- a/java/com/google/gerrit/server/restapi/group/ListGroups.java
+++ b/java/com/google/gerrit/server/restapi/group/ListGroups.java
@@ -86,7 +86,7 @@
private final Groups groups;
private final GroupResolver groupResolver;
- private EnumSet<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
+ private Set<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
private boolean visibleToAll;
private Account.Id user;
private boolean owned;
@@ -220,7 +220,7 @@
this.groupResolver = groupResolver;
}
- public void setOptions(EnumSet<ListGroupsOption> options) {
+ public void setOptions(Set<ListGroupsOption> options) {
this.options = options;
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index 345340b..6384282 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -350,7 +350,7 @@
queries.add(String.format("(state:%s)", state.name()));
}
- return Joiner.on(" AND ").join(queries).toString();
+ return Joiner.on(" AND ").join(queries);
}
private SortedMap<String, ProjectInfo> applyAsQuery(String query) throws BadRequestException {
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index 4cf0182..a610dd4 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -165,12 +165,7 @@
throw new ResourceConflictException("cannot set parent to self");
}
- if (Iterables.tryFind(
- parent.tree(),
- p -> {
- return p.getNameKey().equals(project);
- })
- .isPresent()) {
+ if (Iterables.tryFind(parent.tree(), p -> p.getNameKey().equals(project)).isPresent()) {
throw new ResourceConflictException(
"cycle exists between " + project.get() + " and " + parent.getName());
}
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index f567a3a..a5b88b4 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -22,6 +22,7 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/util/cli",
+ "//java/com/google/gerrit/util/logging",
"//lib:args4j",
"//lib:gson",
"//lib:guava",
diff --git a/java/com/google/gerrit/sshd/SshLog.java b/java/com/google/gerrit/sshd/SshLog.java
index e8df441..2b3052c 100644
--- a/java/com/google/gerrit/sshd/SshLog.java
+++ b/java/com/google/gerrit/sshd/SshLog.java
@@ -45,15 +45,18 @@
@Singleton
class SshLog implements LifecycleListener, GerritConfigListener {
private static final Logger log = Logger.getLogger(SshLog.class);
- private static final String LOG_NAME = "sshd_log";
- private static final String P_SESSION = "session";
- private static final String P_USER_NAME = "userName";
- private static final String P_ACCOUNT_ID = "accountId";
- private static final String P_WAIT = "queueWaitTime";
- private static final String P_EXEC = "executionTime";
- private static final String P_STATUS = "status";
- private static final String P_AGENT = "agent";
- private static final String P_MESSAGE = "message";
+
+ private static final String JSON_SUFFIX = ".json";
+
+ protected static final String LOG_NAME = "sshd_log";
+ protected static final String P_SESSION = "session";
+ protected static final String P_USER_NAME = "userName";
+ protected static final String P_ACCOUNT_ID = "accountId";
+ protected static final String P_WAIT = "queueWaitTime";
+ protected static final String P_EXEC = "executionTime";
+ protected static final String P_STATUS = "status";
+ protected static final String P_AGENT = "agent";
+ protected static final String P_MESSAGE = "message";
private final Provider<SshSession> session;
private final Provider<Context> context;
@@ -61,6 +64,9 @@
private final GroupAuditService auditService;
private final SystemLog systemLog;
+ private final boolean json;
+ private final boolean text;
+
private final Object lock = new Object();
@Inject
@@ -75,6 +81,9 @@
this.auditService = auditService;
this.systemLog = systemLog;
+ this.json = config.getBoolean("log", "jsonLogging", false);
+ this.text = config.getBoolean("log", "textLogging", true) || !json;
+
if (config.getBoolean("sshd", "requestLog", true)) {
enableLogging();
}
@@ -84,7 +93,16 @@
public boolean enableLogging() {
synchronized (lock) {
if (async == null) {
- async = systemLog.createAsyncAppender(LOG_NAME, new SshLogLayout());
+ async = new AsyncAppender();
+
+ if (text) {
+ async.addAppender(systemLog.createAsyncAppender(LOG_NAME, new SshLogLayout()));
+ }
+
+ if (json) {
+ async.addAppender(
+ systemLog.createAsyncAppender(LOG_NAME + JSON_SUFFIX, new SshLogJsonLayout()));
+ }
return true;
}
return false;
diff --git a/java/com/google/gerrit/sshd/SshLogJsonLayout.java b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
new file mode 100644
index 0000000..5ece5af
--- /dev/null
+++ b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
@@ -0,0 +1,96 @@
+// 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.sshd;
+
+import static com.google.gerrit.sshd.SshLog.P_ACCOUNT_ID;
+import static com.google.gerrit.sshd.SshLog.P_AGENT;
+import static com.google.gerrit.sshd.SshLog.P_EXEC;
+import static com.google.gerrit.sshd.SshLog.P_MESSAGE;
+import static com.google.gerrit.sshd.SshLog.P_SESSION;
+import static com.google.gerrit.sshd.SshLog.P_STATUS;
+import static com.google.gerrit.sshd.SshLog.P_USER_NAME;
+import static com.google.gerrit.sshd.SshLog.P_WAIT;
+
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
+import java.time.format.DateTimeFormatter;
+import org.apache.log4j.spi.LoggingEvent;
+
+public class SshLogJsonLayout extends JsonLayout {
+
+ @Override
+ public DateTimeFormatter createDateTimeFormatter() {
+ return DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss,SSS Z");
+ }
+
+ @Override
+ public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
+ return new SshJsonLogEntry(event);
+ }
+
+ @SuppressWarnings("unused")
+ private class SshJsonLogEntry extends JsonLogEntry {
+ public String timestamp;
+ public String session;
+ public String thread;
+ public String user;
+ public String accountId;
+ public String message;
+ public String waitTime;
+ public String execTime;
+ public String status;
+ public String agent;
+ public String timeNegotiating;
+ public String timeSearchReuse;
+ public String timeSearchSizes;
+ public String timeCounting;
+ public String timeCompressing;
+ public String timeWriting;
+ public String timeTotal;
+ public String bitmapIndexMisses;
+ public String deltasTotal;
+ public String objectsTotal;
+ public String bytesTotal;
+
+ public SshJsonLogEntry(LoggingEvent event) {
+ this.timestamp = formatDate(event.getTimeStamp());
+ this.session = getMdcString(event, P_SESSION);
+ this.thread = event.getThreadName();
+ this.user = getMdcString(event, P_USER_NAME);
+ this.accountId = getMdcString(event, P_ACCOUNT_ID);
+ this.message = (String) event.getMessage();
+ this.waitTime = getMdcString(event, P_WAIT);
+ this.execTime = getMdcString(event, P_EXEC);
+ this.status = getMdcString(event, P_STATUS);
+ this.agent = getMdcString(event, P_AGENT);
+
+ String metricString = getMdcString(event, P_MESSAGE);
+ if (metricString != null && !metricString.isEmpty()) {
+ String[] ssh_metrics = metricString.split(" ");
+ this.timeNegotiating = ssh_metrics[0];
+ this.timeSearchReuse = ssh_metrics[1];
+ this.timeSearchSizes = ssh_metrics[2];
+ this.timeCounting = ssh_metrics[3];
+ this.timeCompressing = ssh_metrics[4];
+ this.timeWriting = ssh_metrics[5];
+ this.timeTotal = ssh_metrics[6];
+ this.bitmapIndexMisses = ssh_metrics[7];
+ this.deltasTotal = ssh_metrics[8];
+ this.objectsTotal = ssh_metrics[9];
+ this.bytesTotal = ssh_metrics[10];
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/sshd/SshLogLayout.java b/java/com/google/gerrit/sshd/SshLogLayout.java
index f16dd73..c676be9 100644
--- a/java/com/google/gerrit/sshd/SshLogLayout.java
+++ b/java/com/google/gerrit/sshd/SshLogLayout.java
@@ -14,6 +14,15 @@
package com.google.gerrit.sshd;
+import static com.google.gerrit.sshd.SshLog.P_ACCOUNT_ID;
+import static com.google.gerrit.sshd.SshLog.P_AGENT;
+import static com.google.gerrit.sshd.SshLog.P_EXEC;
+import static com.google.gerrit.sshd.SshLog.P_MESSAGE;
+import static com.google.gerrit.sshd.SshLog.P_SESSION;
+import static com.google.gerrit.sshd.SshLog.P_STATUS;
+import static com.google.gerrit.sshd.SshLog.P_USER_NAME;
+import static com.google.gerrit.sshd.SshLog.P_WAIT;
+
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.TimeZone;
@@ -23,15 +32,6 @@
public final class SshLogLayout extends Layout {
- private static final String P_SESSION = "session";
- private static final String P_USER_NAME = "userName";
- private static final String P_ACCOUNT_ID = "accountId";
- private static final String P_WAIT = "queueWaitTime";
- private static final String P_EXEC = "executionTime";
- private static final String P_STATUS = "status";
- private static final String P_AGENT = "agent";
- private static final String P_MESSAGE = "message";
-
private final Calendar calendar;
private long lastTimeMillis;
private final char[] lastTimeString = new char[20];
diff --git a/java/com/google/gerrit/sshd/commands/FlushCaches.java b/java/com/google/gerrit/sshd/commands/FlushCaches.java
index 0c9bbb5..2afc009 100644
--- a/java/com/google/gerrit/sshd/commands/FlushCaches.java
+++ b/java/com/google/gerrit/sshd/commands/FlushCaches.java
@@ -57,14 +57,14 @@
protected void run() throws Failure {
try {
if (list) {
- if (all || caches.size() > 0) {
+ if (all || !caches.isEmpty()) {
throw die("cannot use --list with --all or --cache");
}
doList();
return;
}
- if (all && caches.size() > 0) {
+ if (all && !caches.isEmpty()) {
throw die("cannot combine --all and --cache");
} else if (!all && caches.size() == 1 && caches.contains("all")) {
caches.clear();
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index a76275b..16c15ad 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -45,6 +45,8 @@
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-servlet",
+ "//lib/log:impl-log4j",
+ "//lib/log:log4j",
"//lib/truth",
],
)
diff --git a/java/com/google/gerrit/testing/GerritTestName.java b/java/com/google/gerrit/testing/GerritTestName.java
index d003289..d287837 100644
--- a/java/com/google/gerrit/testing/GerritTestName.java
+++ b/java/com/google/gerrit/testing/GerritTestName.java
@@ -15,6 +15,7 @@
package com.google.gerrit.testing;
import com.google.common.base.CharMatcher;
+import org.junit.BeforeClass;
import org.junit.rules.TestName;
import org.junit.rules.TestRule;
import org.junit.runner.Description;
@@ -23,6 +24,11 @@
public class GerritTestName implements TestRule {
private final TestName delegate = new TestName();
+ @BeforeClass
+ public static void beforeClassTest() {
+ TestLoggingActivator.configureLogging();
+ }
+
public String getSanitizedMethodName() {
String name = delegate.getMethodName().toLowerCase();
name =
diff --git a/java/com/google/gerrit/testing/TestLoggingActivator.java b/java/com/google/gerrit/testing/TestLoggingActivator.java
new file mode 100644
index 0000000..2049bfd
--- /dev/null
+++ b/java/com/google/gerrit/testing/TestLoggingActivator.java
@@ -0,0 +1,102 @@
+// 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.testing;
+
+import static org.apache.log4j.Logger.getLogger;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
+import org.apache.log4j.ConsoleAppender;
+import org.apache.log4j.Level;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.log4j.PatternLayout;
+
+public class TestLoggingActivator {
+ private static final ImmutableMap<String, Level> LOG_LEVELS =
+ ImmutableMap.<String, Level>builder()
+ .put("com.google.gerrit", getGerritLogLevel())
+
+ // Silence non-critical messages from MINA SSHD.
+ .put("org.apache.mina", Level.WARN)
+ .put("org.apache.sshd.common", Level.WARN)
+ .put("org.apache.sshd.server", Level.WARN)
+ .put("org.apache.sshd.common.keyprovider.FileKeyPairProvider", Level.INFO)
+ .put("com.google.gerrit.sshd.GerritServerSession", Level.WARN)
+
+ // Silence non-critical messages from mime-util.
+ .put("eu.medsea.mimeutil", Level.WARN)
+
+ // Silence non-critical messages from openid4java.
+ .put("org.apache.xml", Level.WARN)
+ .put("org.openid4java", Level.WARN)
+ .put("org.openid4java.consumer.ConsumerManager", Level.FATAL)
+ .put("org.openid4java.discovery.Discovery", Level.ERROR)
+ .put("org.openid4java.server.RealmVerifier", Level.ERROR)
+ .put("org.openid4java.message.AuthSuccess", Level.ERROR)
+
+ // Silence non-critical messages from c3p0 (if used).
+ .put("com.mchange.v2.c3p0", Level.WARN)
+ .put("com.mchange.v2.resourcepool", Level.WARN)
+ .put("com.mchange.v2.sql", Level.WARN)
+
+ // Silence non-critical messages from apache.http.
+ .put("org.apache.http", Level.WARN)
+
+ // Silence non-critical messages from Jetty.
+ .put("org.eclipse.jetty", Level.WARN)
+
+ // Silence non-critical messages from JGit.
+ .put("org.eclipse.jgit.transport.PacketLineIn", Level.WARN)
+ .put("org.eclipse.jgit.transport.PacketLineOut", Level.WARN)
+ .put("org.eclipse.jgit.internal.storage.file.FileSnapshot", Level.WARN)
+ .put("org.eclipse.jgit.util.FS", Level.WARN)
+ .put("org.eclipse.jgit.util.SystemReader", Level.WARN)
+
+ // Silence non-critical messages from Elasticsearch.
+ .put("org.elasticsearch", Level.WARN)
+
+ // Silence non-critical messages from Docker for Elasticsearch query tests.
+ .put("org.testcontainers", Level.WARN)
+ .put("com.github.dockerjava.core", Level.WARN)
+ .build();
+
+ private static Level getGerritLogLevel() {
+ String value = Strings.nullToEmpty(System.getenv("GERRIT_LOG_LEVEL"));
+ if (value.isEmpty()) {
+ value = Strings.nullToEmpty(System.getProperty("gerrit.logLevel"));
+ }
+ return Level.toLevel(value, Level.INFO);
+ }
+
+ public static void configureLogging() {
+ LogManager.resetConfiguration();
+
+ PatternLayout layout = new PatternLayout();
+ layout.setConversionPattern("%-5p %c %x: %m%n");
+
+ ConsoleAppender dst = new ConsoleAppender();
+ dst.setLayout(layout);
+ dst.setTarget("System.err");
+ dst.setThreshold(Level.DEBUG);
+ dst.activateOptions();
+
+ Logger root = LogManager.getRootLogger();
+ root.removeAllAppenders();
+ root.addAppender(dst);
+
+ LOG_LEVELS.entrySet().stream().forEach(e -> getLogger(e.getKey()).setLevel(e.getValue()));
+ }
+}
diff --git a/java/com/google/gerrit/util/logging/BUILD b/java/com/google/gerrit/util/logging/BUILD
new file mode 100644
index 0000000..b8db49bd
--- /dev/null
+++ b/java/com/google/gerrit/util/logging/BUILD
@@ -0,0 +1,13 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "logging",
+ srcs = glob(
+ ["*.java"],
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//lib:gson",
+ "//lib/log:log4j",
+ ],
+)
diff --git a/java/com/google/gerrit/util/logging/JsonLayout.java b/java/com/google/gerrit/util/logging/JsonLayout.java
new file mode 100644
index 0000000..45fbd32
--- /dev/null
+++ b/java/com/google/gerrit/util/logging/JsonLayout.java
@@ -0,0 +1,72 @@
+// 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.util.logging;
+
+import com.google.gson.FieldNamingPolicy;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import org.apache.log4j.Layout;
+import org.apache.log4j.spi.LoggingEvent;
+
+public abstract class JsonLayout extends Layout {
+ private final DateTimeFormatter dateFormatter;
+ private final Gson gson;
+ private final ZoneOffset timeOffset;
+
+ public JsonLayout() {
+ dateFormatter = createDateTimeFormatter();
+ timeOffset = OffsetDateTime.now(ZoneId.systemDefault()).getOffset();
+
+ gson = newGson();
+ }
+
+ public abstract DateTimeFormatter createDateTimeFormatter();
+
+ public abstract JsonLogEntry toJsonLogEntry(LoggingEvent event);
+
+ @Override
+ public String format(LoggingEvent event) {
+ return gson.toJson(toJsonLogEntry(event)) + "\n";
+ }
+
+ private static Gson newGson() {
+ GsonBuilder gb =
+ new GsonBuilder()
+ .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
+ .disableHtmlEscaping();
+ return gb.create();
+ }
+
+ public String formatDate(long now) {
+ return ZonedDateTime.of(
+ LocalDateTime.ofInstant(Instant.ofEpochMilli(now), timeOffset), ZoneId.systemDefault())
+ .format(dateFormatter);
+ }
+
+ @Override
+ public void activateOptions() {}
+
+ @Override
+ public boolean ignoresThrowable() {
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/util/logging/JsonLogEntry.java b/java/com/google/gerrit/util/logging/JsonLogEntry.java
new file mode 100644
index 0000000..ceac427
--- /dev/null
+++ b/java/com/google/gerrit/util/logging/JsonLogEntry.java
@@ -0,0 +1,23 @@
+// 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.util.logging;
+
+import org.apache.log4j.spi.LoggingEvent;
+
+public abstract class JsonLogEntry {
+ public String getMdcString(LoggingEvent event, String key) {
+ return (String) event.getMDC(key);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 625d0fb..e2d2501 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -130,7 +130,6 @@
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.index.account.StalenessChecker;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.RefPattern;
@@ -213,7 +212,6 @@
@Inject private Sequences seq;
@Inject private StalenessChecker stalenessChecker;
@Inject private VersionedAuthorizedKeys.Accessor authorizedKeys;
- @Inject private PermissionBackend permissionBackend;
@Inject private ExtensionRegistry extensionRegistry;
@Inject private PluginSetContext<ExceptionHook> exceptionHooks;
@@ -859,7 +857,7 @@
gApi.changes().id(r.getChangeId()).abandon();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user2.getNameEmail());
accountIndexedCounter.assertNoReindex();
}
}
@@ -885,7 +883,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message message = messages.get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertMailReplyTo(message, admin.email());
accountIndexedCounter.assertNoReindex();
}
@@ -1842,7 +1840,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("new SSH keys have been added");
// Delete key
@@ -1854,7 +1852,7 @@
assertThat(sender.getMessages()).hasSize(1);
message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("SSH keys have been deleted");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 0eefe02..9399c3b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1518,7 +1518,7 @@
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.from().getName()).isEqualTo("Administrator (Code Review)");
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("I'd like you to do a code review");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
@@ -1587,7 +1587,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -1771,7 +1771,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -2162,7 +2162,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
@@ -2411,7 +2411,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
@@ -3065,7 +3065,7 @@
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.updated, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.updated, serverIdent.get());
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitPatchSetCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
@@ -3074,7 +3074,7 @@
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitChangeCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
@@ -4527,7 +4527,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index 2801b36..72ab2e6 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -41,7 +41,6 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -1011,7 +1010,6 @@
}
}
- @Nullable
protected RevCommit getRemoteHead(String project, String branch) throws Exception {
return projectOperations.project(Project.nameKey(project)).getHead(branch);
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 51dee72..4ee1f35 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -545,7 +545,7 @@
r.assertOkStatus();
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.ALL);
@@ -553,7 +553,7 @@
assertThat(sender.getMessages()).hasSize(1);
m = sender.getMessages().get(0);
assertThat(m.rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress(), user3.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail(), user3.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + user3.email());
@@ -1116,7 +1116,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
@Test
@@ -1141,7 +1141,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
/**
@@ -1838,7 +1838,7 @@
@Test
public void pushWithEmailInFooter() throws Exception {
- pushWithReviewerInFooter(user.getEmailAddress().toString(), user);
+ pushWithReviewerInFooter(user.getNameEmail().toString(), user);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 56d9ff2..1083377 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -1071,7 +1071,7 @@
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
tr.branch(RefNames.changeMetaRef(cd3.getId()))
.commit()
.author(author)
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
index 0f5def6..420ddda 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
@@ -61,7 +61,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index ac00e38..090146e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -209,7 +209,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt())
- .containsExactly(Address.parse(addInput.reviewer), user.getEmailAddress());
+ .containsExactly(Address.parse(addInput.reviewer), user.getNameEmail());
sender.clear();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index fbe6533..0a006fa 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -150,7 +150,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
}
@@ -188,7 +188,7 @@
Message m = messages.get(0);
List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
for (TestAccount u : firstUsers) {
- expectedAddresses.add(u.getEmailAddress());
+ expectedAddresses.add(u.getNameEmail());
}
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
@@ -216,9 +216,9 @@
m = messages.get(0);
expectedAddresses = new ArrayList<>(4);
for (int i = 0; i < 3; i++) {
- expectedAddresses.add(users.get(users.size() - i - 1).getEmailAddress());
+ expectedAddresses.add(users.get(users.size() - i - 1).getNameEmail());
}
- expectedAddresses.add(reviewer.getEmailAddress());
+ expectedAddresses.add(reviewer.getNameEmail());
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
}
@@ -398,13 +398,13 @@
assertThat(messages).hasSize(2);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
m = messages.get(1);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
}
@@ -635,7 +635,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).current().review(reviewInput);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -652,7 +652,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).addReviewer(addReviewer);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -829,7 +829,7 @@
}
private void assertThatUserIsOnlyReviewer(String changeId) throws Exception {
- AccountInfo userInfo = new AccountInfo(user.fullName(), user.getEmailAddress().getEmail());
+ AccountInfo userInfo = new AccountInfo(user.fullName(), user.getNameEmail().getEmail());
userInfo._accountId = user.id().get();
userInfo.username = user.username();
assertThat(gApi.changes().id(changeId).get().reviewers)
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 0af1295..d831b62 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -216,7 +216,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
// check that watcher is not notified if notify=NONE
@@ -299,9 +299,11 @@
ChangeInfo info = assertCreateSucceeds(input);
RevisionApi rApi = gApi.changes().id(info.id).current();
- GitPerson person = rApi.commit(false).author;
- assertThat(person).email().isEqualTo(input.author.email);
- assertThat(person).name().isEqualTo(input.author.name);
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(input.author.email);
+ assertThat(author).name().isEqualTo(input.author.name);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
}
@Test
@@ -331,7 +333,19 @@
changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
ChangeInput input = newChangeInput(ChangeStatus.NEW);
input.baseCommit = setup.get("master").getCommit().getId().name();
- assertCreateSucceeds(input);
+ ChangeInfo result = assertCreateSucceeds(input);
+ assertThat(gApi.changes().id(result.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(input.baseCommit);
+ }
+
+ @Test
+ public void createChangeWithParentChange() throws Exception {
+ Result change = createChange();
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseChange = change.getChangeId();
+ ChangeInfo result = assertCreateSucceeds(input);
+ assertThat(gApi.changes().id(result.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(change.getCommit().getId().name());
}
@Test
@@ -415,7 +429,7 @@
assertThat(commit.getShortMessage()).isEqualTo("Create change");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commit.getCommitterIdent())
@@ -437,6 +451,22 @@
}
@Test
+ public void createMergeChangeAuthor() throws Exception {
+ changeInTwoBranches("branchA", "a.txt", "branchB", "b.txt");
+ ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
+ in.author = new AccountInput();
+ in.author.name = "Gerritless Jane";
+ in.author.email = "gerritlessjane@invalid";
+ ChangeInfo change = assertCreateSucceeds(in);
+
+ RevisionApi rApi = gApi.changes().id(change.id).current();
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(in.author.email);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
+ }
+
+ @Test
public void createMergeChange_Conflicts() throws Exception {
changeInTwoBranches("branchA", "shared.txt", "branchB", "shared.txt");
ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
index 37fa2ce..25e5647 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
@@ -73,7 +73,7 @@
List<FakeEmailSender.Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
FakeEmailSender.Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index ba41d7e..b60cce5 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -81,10 +81,10 @@
public class CommentsIT extends AbstractDaemonTest {
@Inject private ChangeNoteUtil noteUtil;
@Inject private FakeEmailSender email;
+ @Inject private ProjectOperations projectOperations;
@Inject private Provider<ChangesCollection> changes;
@Inject private Provider<PostReview> postReview;
@Inject private RequestScopeOperations requestScopeOperations;
- @Inject ProjectOperations projectOperations;
private final Integer[] lines = {0, 1};
@@ -387,6 +387,39 @@
assertThat(list.stream().map(infoToInput(file))).containsExactlyElementsIn(expectedComments);
}
+ /**
+ * This test makes sure that the commits in the refs/draft-comments ref in NoteDb have no parent
+ * commits. This is important so that each new draft update (add, modify, delete) does not keep
+ * track of previous history.
+ */
+ @Test
+ public void commitsInDraftCommentsRefHaveNoParent() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ String draftRefName = RefNames.refsDraftComments(r.getChange().getId(), user.id());
+
+ DraftInput comment1 = newDraft("file_1", Side.REVISION, 1, "comment 1");
+ CommentInfo commentInfo1 = addDraft(changeId, revId, comment1);
+ assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
+
+ DraftInput comment2 = newDraft("file_2", Side.REVISION, 2, "comment 2");
+ CommentInfo commentInfo2 = addDraft(changeId, revId, comment2);
+ assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
+
+ deleteDraft(changeId, revId, commentInfo1.id);
+ assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
+ assertThat(
+ getDraftComments(changeId, revId).values().stream()
+ .flatMap(List::stream)
+ .map(commentInfo -> commentInfo.message))
+ .containsExactly("comment 2");
+
+ deleteDraft(changeId, revId, commentInfo2.id);
+ assertThat(projectOperations.project(allUsers).hasHead(draftRefName)).isFalse();
+ assertThat(getDraftComments(changeId, revId).values().stream().flatMap(List::stream)).isEmpty();
+ }
+
@Test
public void putDraft() throws Exception {
for (Integer line : lines) {
@@ -1112,6 +1145,12 @@
}
}
+ private RevCommit getHeadOfDraftCommentsRef(String refName) throws Exception {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ return getHead(repo, refName);
+ }
+ }
+
private static String extractComments(String msg) {
// Extract lines between start "....." and end "-- ".
Pattern p = Pattern.compile(".*[.]{5}\n+(.*)\\n+-- \n.*", Pattern.DOTALL);
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index ee5f117..069387c 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -830,7 +830,8 @@
private void addNoteDbCommit(Change.Id id, String commitMessage) throws Exception {
PersonIdent committer = serverIdent.get();
- PersonIdent author = noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
+ PersonIdent author =
+ noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
serverSideTestRepo
.branch(RefNames.changeMetaRef(id))
.commit()
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
index 3b0e27b..9e0de92 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
@@ -38,8 +38,8 @@
MailMessage.Builder messageBuilderWithDefaultFields() {
MailMessage.Builder b = MailMessage.builder();
b.id("some id");
- b.from(user.getEmailAddress());
- b.addTo(user.getEmailAddress()); // Not evaluated
+ b.from(user.getNameEmail());
+ b.addTo(user.getNameEmail()); // Not evaluated
b.subject("");
b.dateReceived(Instant.now());
return b;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index ae92bea..2dc1e24 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -104,7 +104,7 @@
assertThat(sender)
.sent("abandon", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -119,7 +119,7 @@
.sent("abandon", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -135,7 +135,7 @@
.sent("abandon", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -151,7 +151,7 @@
.sent("abandon", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, other)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -165,7 +165,7 @@
assertThat(sender)
.sent("abandon", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -217,7 +217,7 @@
assertThat(sender)
.sent("abandon", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -238,7 +238,7 @@
assertThat(sender)
.sent("abandon", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ABANDONED_CHANGES)
.noOneElse();
@@ -284,7 +284,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -308,7 +308,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.owner, sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -333,7 +333,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.owner, sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -358,7 +358,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.owner, sc.reviewer, other)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -382,7 +382,7 @@
.sent("newchange", sc)
.to(email)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -438,7 +438,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
}
@@ -451,7 +451,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -475,7 +475,7 @@
.sent("newchange", sc)
.to(reviewer)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -533,7 +533,7 @@
.sent("newchange", sc)
.to("nonexistent@example.com")
.cc(sc.reviewer)
- .cc(sc.ccerByEmail, sc.reviewerByEmail)
+ .cc(StagedUsers.CC_BY_EMAIL, StagedUsers.REVIEWER_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -555,7 +555,7 @@
.sent("newchange", sc)
.cc("nonexistent@example.com")
.cc(sc.reviewer)
- .cc(sc.ccerByEmail, sc.reviewerByEmail)
+ .cc(StagedUsers.CC_BY_EMAIL, StagedUsers.REVIEWER_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -641,7 +641,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -656,7 +656,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -671,7 +671,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -686,7 +686,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -702,7 +702,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -718,7 +718,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, other)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -732,7 +732,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -776,7 +776,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -802,7 +802,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -836,7 +836,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -850,7 +850,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -873,7 +873,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -888,7 +888,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -903,7 +903,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer, other)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -912,7 +912,7 @@
.sent("newchange", sc)
.to(other)
.cc(sc.reviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1082,7 +1082,7 @@
.sent("deleteReviewer", sc)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1098,7 +1098,7 @@
.sent("deleteReviewer", sc)
.to(sc.owner, extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1114,7 +1114,7 @@
.sent("deleteReviewer", sc)
.to(sc.owner, extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1131,7 +1131,7 @@
.sent("deleteReviewer", sc)
.to(sc.owner, extraReviewer)
.cc(admin, extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1147,7 +1147,7 @@
.sent("deleteReviewer", sc)
.to(extraCcer)
.cc(sc.reviewer, sc.ccer, extraReviewer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1163,7 +1163,7 @@
.sent("deleteReviewer", sc)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1222,7 +1222,7 @@
.sent("deleteReviewer", sc)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1250,7 +1250,7 @@
@Test
public void deleteReviewerByEmailFromWipChange() throws Exception {
StagedChange sc = stageWipChangeWithExtraReviewer();
- gApi.changes().id(sc.changeId).reviewer(sc.reviewerByEmail).remove();
+ gApi.changes().id(sc.changeId).reviewer(StagedUsers.REVIEWER_BY_EMAIL).remove();
assertThat(sender).didNotSend();
}
@@ -1317,7 +1317,7 @@
assertThat(sender)
.sent("deleteVote", sc)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1335,7 +1335,7 @@
.sent("deleteVote", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1352,7 +1352,7 @@
.sent("deleteVote", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1370,7 +1370,7 @@
.sent("deleteVote", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, admin, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1386,7 +1386,7 @@
assertThat(sender)
.sent("deleteVote", sc)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1402,7 +1402,7 @@
.sent("deleteVote", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1445,7 +1445,7 @@
assertThat(sender)
.sent("deleteVote", sc)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1461,7 +1461,7 @@
assertThat(sender)
.sent("deleteVote", sc)
.cc(sc.reviewer, sc.ccer, extraReviewer, extraCcer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -1524,7 +1524,7 @@
.that(sender)
.sent("merged", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
.noOneElse();
@@ -1540,7 +1540,7 @@
.sent("merged", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
.noOneElse();
@@ -1555,7 +1555,7 @@
.sent("merged", sc)
.to(sc.owner)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
.noOneElse();
@@ -1570,7 +1570,7 @@
.sent("merged", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
.noOneElse();
@@ -1585,7 +1585,7 @@
.sent("merged", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1667,7 +1667,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1683,7 +1683,7 @@
.notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer, other)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1699,7 +1699,7 @@
.notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer, other)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1716,7 +1716,7 @@
.to(sc.reviewer)
.to(other)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1732,7 +1732,7 @@
.to(sc.reviewer)
.to(other)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1791,7 +1791,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1806,7 +1806,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1829,7 +1829,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer, newReviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1853,7 +1853,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer, newReviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1868,7 +1868,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1893,7 +1893,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1908,7 +1908,7 @@
.sent("newpatchset", sc)
.to(sc.owner, sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1923,7 +1923,7 @@
.sent("newpatchset", sc)
.to(sc.owner, sc.reviewer, other)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1938,7 +1938,7 @@
.sent("newpatchset", sc)
.to(sc.owner, sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -1952,7 +1952,7 @@
.sent("newpatchset", sc)
.to(sc.owner, sc.reviewer)
.cc(sc.ccer, other)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -2018,7 +2018,7 @@
.sent("newpatchset", sc)
.to(sc.reviewer)
.cc(sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -2061,7 +2061,7 @@
assertThat(sender)
.sent("restore", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2075,7 +2075,7 @@
assertThat(sender)
.sent("restore", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2089,7 +2089,7 @@
assertThat(sender)
.sent("restore", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2104,7 +2104,7 @@
.sent("restore", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2119,7 +2119,7 @@
.sent("restore", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2134,7 +2134,7 @@
.sent("restore", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2173,7 +2173,7 @@
assertThat(sender)
.sent("revert", sc)
.cc(sc.reviewer, sc.ccer, admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2198,7 +2198,7 @@
.sent("revert", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2223,7 +2223,7 @@
.sent("revert", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2248,7 +2248,7 @@
.sent("revert", sc)
.to(sc.owner)
.cc(other, sc.reviewer, sc.ccer, admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2285,7 +2285,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2298,7 +2300,9 @@
assertThat(sender)
.sent("setassignee", sc)
.cc(sc.owner)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2310,7 +2314,9 @@
assign(sc, admin, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2323,7 +2329,9 @@
assertThat(sender)
.sent("setassignee", sc)
.cc(admin)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2335,7 +2343,9 @@
assign(sc, sc.owner, sc.owner);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -2349,7 +2359,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2363,7 +2375,9 @@
assign(sc, sc.owner, sc.owner);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.noOneElse();
assertThat(sender).didNotSend();
}
@@ -2374,7 +2388,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2386,7 +2402,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .cc(
+ StagedUsers.REVIEWER_BY_EMAIL,
+ StagedUsers.CC_BY_EMAIL) // TODO(logan): This is probably not intended!
.to(sc.assignee)
.noOneElse();
assertThat(sender).didNotSend();
@@ -2416,7 +2434,7 @@
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
@@ -2432,7 +2450,7 @@
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
- .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .cc(StagedUsers.REVIEWER_BY_EMAIL, StagedUsers.CC_BY_EMAIL)
.bcc(sc.starrer)
.bcc(ALL_COMMENTS)
.noOneElse();
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 502194a..49f25e4 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -237,7 +237,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
@@ -259,7 +259,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index 29574c4..8b12b9d 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -241,7 +241,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -273,7 +273,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -295,7 +295,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -322,7 +322,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -357,7 +357,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -385,7 +385,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -407,7 +407,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -434,7 +434,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -547,7 +547,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
diff --git a/javatests/com/google/gerrit/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index b68aacc..5a3a824 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -5,7 +5,6 @@
name = "pgm_tests",
srcs = glob(["**/*.java"]),
deps = [
- "//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/pgm/init/api",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/securestore/testing",
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 013939a..8ffcc8b 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -518,7 +518,9 @@
private RevCommit writeCommit(String body) throws Exception {
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
- body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
+ body,
+ noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
+ false);
}
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
@@ -529,7 +531,7 @@
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
body,
- noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
initWorkInProgress);
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 16981dc..68c7883 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -28,6 +28,7 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@@ -44,6 +45,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -55,6 +57,7 @@
import com.google.protobuf.ByteString;
import java.lang.reflect.Type;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -64,6 +67,7 @@
import org.junit.Test;
public class ChangeNotesStateTest {
+
private static final Change.Id ID = Change.id(123);
private static final ObjectId SHA =
ObjectId.fromString("1234567812345678123456781234567812345678");
@@ -95,6 +99,13 @@
return ChangeNotesState.Builder.empty(ID).metaId(SHA).columns(cols);
}
+ private ChangeNotesStateProto.Builder newProtoBuilder() {
+ return ChangeNotesStateProto.newBuilder()
+ .setMetaId(SHA_BYTES)
+ .setChangeId(ID.get())
+ .setColumns(colsProto);
+ }
+
@Test
public void serializeChangeKey() throws Exception {
assertRoundTrip(
@@ -589,6 +600,39 @@
}
@Test
+ public void serializeAttentionUpdates() throws Exception {
+ assertRoundTrip(
+ newBuilder()
+ .attentionUpdates(
+ ImmutableList.of(
+ AttentionStatus.createFromRead(
+ Instant.EPOCH.plusSeconds(23),
+ Account.id(1000),
+ AttentionStatus.Operation.ADD,
+ "reason 1"),
+ AttentionStatus.createFromRead(
+ Instant.EPOCH.plusSeconds(42),
+ Account.id(2000),
+ AttentionStatus.Operation.REMOVE,
+ "reason 2")))
+ .build(),
+ newProtoBuilder()
+ .addAttentionStatus(
+ AttentionStatusProto.newBuilder()
+ .setDate(23_000) // epoch millis
+ .setAccount(1000)
+ .setOperation("ADD")
+ .setReason("reason 1"))
+ .addAttentionStatus(
+ AttentionStatusProto.newBuilder()
+ .setDate(42_000) // epoch millis
+ .setAccount(2000)
+ .setOperation("REMOVE")
+ .setReason("reason 2"))
+ .build());
+ }
+
+ @Test
public void serializeAssigneeUpdates() throws Exception {
assertRoundTrip(
newBuilder()
@@ -745,6 +789,9 @@
"reviewerUpdates",
new TypeLiteral<ImmutableList<ReviewerStatusUpdate>>() {}.getType())
.put(
+ "attentionUpdates",
+ new TypeLiteral<ImmutableList<AttentionStatus>>() {}.getType())
+ .put(
"assigneeUpdates",
new TypeLiteral<ImmutableList<AssigneeStatusUpdate>>() {}.getType())
.put("submitRecords", new TypeLiteral<ImmutableList<SubmitRecord>>() {}.getType())
@@ -824,10 +871,10 @@
.hasFields(
ImmutableMap.of(
"table",
- new TypeLiteral<
- ImmutableTable<
- ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
- "accounts", new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
+ new TypeLiteral<
+ ImmutableTable<ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
+ "accounts",
+ new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
}
@Test
@@ -836,9 +883,10 @@
.hasFields(
ImmutableMap.of(
"table",
- new TypeLiteral<
- ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
- "users", new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
+ new TypeLiteral<
+ ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
+ "users",
+ new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
}
@Test
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index db0dec8..4e068ba 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -38,6 +38,8 @@
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
+import com.google.gerrit.entities.AttentionStatus.Operation;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -58,6 +60,7 @@
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -688,6 +691,92 @@
}
@Test
+ public void defaultAttentionSetIsEmpty() throws Exception {
+ Change c = newChange();
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).isEmpty();
+ }
+
+ @Test
+ public void addAttentionStatus() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
+ }
+
+ @Test
+ public void filterLatestAttentionStatus() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+ update = newUpdate(c, changeOwner);
+ attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
+ }
+
+ @Test
+ public void addAttentionStatus_rejectTimestamp() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createFromRead(
+ Instant.now(), changeOwner.getAccountId(), Operation.ADD, "test");
+ IllegalArgumentException thrown =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> update.setAttentionUpdates(ImmutableList.of(attentionStatus)));
+ assertThat(thrown).hasMessageThat().contains("must not specify timestamp for write");
+ }
+
+ @Test
+ public void addAttentionStatus_rejectMultiplePerUser() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus0 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
+ AttentionStatus attentionStatus1 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
+ IllegalArgumentException thrown =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1)));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("must not specify multiple updates for single user");
+ }
+
+ @Test
+ public void addAttentionStatusForMultipleUsers() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus0 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ AttentionStatus attentionStatus1 =
+ AttentionStatus.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
+
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates())
+ .containsExactly(addTimestamp(attentionStatus0, c), addTimestamp(attentionStatus1, c));
+ }
+
+ @Test
public void assigneeCommit() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -3140,4 +3229,13 @@
update.commit();
return tr.parseBody(commit);
}
+
+ private AttentionStatus addTimestamp(AttentionStatus attentionStatus, Change c) {
+ Timestamp timestamp = newNotes(c).getChange().getLastUpdatedOn();
+ return AttentionStatus.createFromRead(
+ timestamp.toInstant(),
+ attentionStatus.account(),
+ attentionStatus.operation(),
+ attentionStatus.reason());
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 31d2048..e7f0812 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -91,14 +91,10 @@
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
@@ -150,20 +146,6 @@
protected abstract Injector createInjector();
- @BeforeClass
- public static void setLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.DEBUG);
- }
- }
-
- @AfterClass
- public static void resetLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.INFO);
- }
- }
-
@Before
public void setUpInjector() throws Exception {
lifecycle = new LifecycleManager();
diff --git a/javatests/com/google/gerrit/server/query/account/BUILD b/javatests/com/google/gerrit/server/query/account/BUILD
index da37f26..5c910a0 100644
--- a/javatests/com/google/gerrit/server/query/account/BUILD
+++ b/javatests/com/google/gerrit/server/query/account/BUILD
@@ -23,10 +23,8 @@
"//lib:guava",
"//lib:jgit",
"//lib/guice",
- "//lib/log:log4j",
"//lib/truth",
"//lib/truth:truth-java8-extension",
- "//resources:log4j-config",
],
)
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 71bdd69..fb09c9f 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -132,8 +132,6 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -145,9 +143,7 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.SystemReader;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
@@ -210,20 +206,6 @@
protected abstract Injector createInjector();
- @BeforeClass
- public static void setLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.DEBUG);
- }
- }
-
- @AfterClass
- public static void resetLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.INFO);
- }
- }
-
@Before
public void setUpInjector() throws Exception {
lifecycle = new LifecycleManager();
@@ -445,7 +427,7 @@
gApi.changes().id(change1.getChangeId()).setPrivate(true, null);
- // Change1 is not private, but should be still visible to its owner.
+ // Change1 is private, but should be still visible to its owner.
assertQuery("is:open", change1, change2);
assertQuery("is:private", change1);
@@ -943,11 +925,11 @@
public void byLabel() throws Exception {
accountManager.authenticate(AuthRequest.forUser("anotheruser"));
TestRepository<Repo> repo = createProject("repo");
- ChangeInserter ins = newChange(repo, null, null, null, null, false);
- ChangeInserter ins2 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins3 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins4 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins5 = newChange(repo, null, null, null, null, false);
+ ChangeInserter ins = newChange(repo);
+ ChangeInserter ins2 = newChange(repo);
+ ChangeInserter ins3 = newChange(repo);
+ ChangeInserter ins4 = newChange(repo);
+ ChangeInserter ins5 = newChange(repo);
Change reviewMinus2Change = insert(repo, ins);
gApi.changes().id(reviewMinus2Change.getId().get()).current().review(ReviewInput.reject());
@@ -1048,11 +1030,11 @@
.update();
ReviewInput reviewVerified = new ReviewInput().label("Verified", 1);
- ChangeInserter ins = newChange(repo, null, null, null, null, false);
- ChangeInserter ins2 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins3 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins4 = newChange(repo, null, null, null, null, false);
- ChangeInserter ins5 = newChange(repo, null, null, null, null, false);
+ ChangeInserter ins = newChange(repo);
+ ChangeInserter ins2 = newChange(repo);
+ ChangeInserter ins3 = newChange(repo);
+ ChangeInserter ins4 = newChange(repo);
+ ChangeInserter ins5 = newChange(repo);
// CR+1
Change reviewCRplus1 = insert(repo, ins);
@@ -1093,7 +1075,7 @@
@Test
public void byLabelNotOwner() throws Exception {
TestRepository<Repo> repo = createProject("repo");
- ChangeInserter ins = newChange(repo, null, null, null, null, false);
+ ChangeInserter ins = newChange(repo);
Account.Id user1 = createAccount("user1");
Change reviewPlus1Change = insert(repo, ins);
@@ -1790,25 +1772,27 @@
public void visible() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
- Change change2 = insert(repo, newChange(repo));
-
- gApi.changes().id(change2.getChangeId()).setPrivate(true, "private");
+ Change change2 = insert(repo, newChangePrivate(repo));
String q = "project:repo";
- assertQuery(q, change2, change1);
- // Second user cannot see first user's private change.
- Account.Id user2 =
- accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
+ // Bad request for query with non-existent user
+ assertThatQueryException(q + " visibleto:notexisting");
+
+ // Current user can see all changes
+ assertQuery(q, change2, change1);
+ assertQuery(q + " visibleto:self", change2, change1);
+
+ // Second user cannot see first user's private change
+ Account.Id user2 = createAccount("anotheruser");
assertQuery(q + " visibleto:" + user2.get(), change1);
+ assertQuery(q + " visibleto:anotheruser", change1);
String g1 = createGroup("group1", "Administrators");
gApi.groups().id(g1).addMembers("anotheruser");
assertQuery(q + " visibleto:" + g1, change1);
- requestContext.setContext(
- newRequestContext(
- accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId()));
+ requestContext.setContext(newRequestContext(user2));
assertQuery("is:visible", change1);
}
@@ -3117,12 +3101,12 @@
}
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
- return newChange(repo, null, null, null, null, false);
+ return newChange(repo, null, null, null, null, false, false);
}
protected ChangeInserter newChangeForCommit(TestRepository<Repo> repo, RevCommit commit)
throws Exception {
- return newChange(repo, commit, null, null, null, false);
+ return newChange(repo, commit, null, null, null, false, false);
}
protected ChangeInserter newChangeWithFiles(TestRepository<Repo> repo, String... paths)
@@ -3136,21 +3120,25 @@
protected ChangeInserter newChangeForBranch(TestRepository<Repo> repo, String branch)
throws Exception {
- return newChange(repo, null, branch, null, null, false);
+ return newChange(repo, null, branch, null, null, false, false);
}
protected ChangeInserter newChangeWithStatus(TestRepository<Repo> repo, Change.Status status)
throws Exception {
- return newChange(repo, null, null, status, null, false);
+ return newChange(repo, null, null, status, null, false, false);
}
protected ChangeInserter newChangeWithTopic(TestRepository<Repo> repo, String topic)
throws Exception {
- return newChange(repo, null, null, null, topic, false);
+ return newChange(repo, null, null, null, topic, false, false);
}
protected ChangeInserter newChangeWorkInProgress(TestRepository<Repo> repo) throws Exception {
- return newChange(repo, null, null, null, null, true);
+ return newChange(repo, null, null, null, null, true, false);
+ }
+
+ protected ChangeInserter newChangePrivate(TestRepository<Repo> repo) throws Exception {
+ return newChange(repo, null, null, null, null, false, true);
}
protected ChangeInserter newChange(
@@ -3159,7 +3147,8 @@
@Nullable String branch,
@Nullable Change.Status status,
@Nullable String topic,
- boolean workInProgress)
+ boolean workInProgress,
+ boolean isPrivate)
throws Exception {
if (commit == null) {
commit = repo.parseBody(repo.commit().message("message").create());
@@ -3177,7 +3166,8 @@
.setValidate(false)
.setStatus(status)
.setTopic(topic)
- .setWorkInProgress(workInProgress);
+ .setWorkInProgress(workInProgress)
+ .setPrivate(isPrivate);
return ins;
}
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 520e65a..e5b51e7 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -34,9 +34,7 @@
"//lib:jgit",
"//lib:jgit-junit",
"//lib/guice",
- "//lib/log:log4j",
"//lib/truth",
- "//resources:log4j-config",
],
)
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 9b01f8d..d80eac0 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -68,12 +68,8 @@
import java.util.List;
import java.util.Locale;
import java.util.Optional;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
@@ -119,20 +115,6 @@
protected abstract Injector createInjector();
- @BeforeClass
- public static void setLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.DEBUG);
- }
- }
-
- @AfterClass
- public static void resetLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.INFO);
- }
- }
-
@Before
public void setUpInjector() throws Exception {
lifecycle = new LifecycleManager();
diff --git a/javatests/com/google/gerrit/server/query/group/BUILD b/javatests/com/google/gerrit/server/query/group/BUILD
index 8f6fd6d..e14350f 100644
--- a/javatests/com/google/gerrit/server/query/group/BUILD
+++ b/javatests/com/google/gerrit/server/query/group/BUILD
@@ -20,10 +20,8 @@
"//lib:guava",
"//lib:jgit",
"//lib/guice",
- "//lib/log:log4j",
"//lib/truth",
"//lib/truth:truth-java8-extension",
- "//resources:log4j-config",
],
)
diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
index e142e0c..dfd7928 100644
--- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
@@ -68,12 +68,8 @@
import java.util.List;
import java.util.Locale;
import java.util.Set;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
@@ -115,20 +111,6 @@
protected abstract Injector createInjector();
- @BeforeClass
- public static void setLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.DEBUG);
- }
- }
-
- @AfterClass
- public static void resetLogLevel() {
- if (Boolean.getBoolean("debug")) {
- LogManager.getRootLogger().setLevel(Level.INFO);
- }
- }
-
@Before
public void setUpInjector() throws Exception {
lifecycle = new LifecycleManager();
diff --git a/javatests/com/google/gerrit/server/query/project/BUILD b/javatests/com/google/gerrit/server/query/project/BUILD
index 996be2f..984d824 100644
--- a/javatests/com/google/gerrit/server/query/project/BUILD
+++ b/javatests/com/google/gerrit/server/query/project/BUILD
@@ -21,9 +21,7 @@
"//lib:guava",
"//lib:jgit",
"//lib/guice",
- "//lib/log:log4j",
"//lib/truth",
- "//resources:log4j-config",
],
)
diff --git a/plugins/replication b/plugins/replication
index 0bf9f5a..8be6db9 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 0bf9f5ae26220bfdcff0b3332e810f00aa9a7789
+Subproject commit 8be6db961b2c6c3a32e0b3db2f04475ca2c4c520
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index bfe5ddc..bd21015 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -535,25 +535,24 @@
</div>
</section>
- <template is="dom-if" if="[[_showPrimaryTabs]]">
- <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
- <paper-tab>Files</paper-tab>
- <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
- as="tabHeader">
- <paper-tab>
- <gr-endpoint-decorator name$="[[tabHeader]]">
- <gr-endpoint-param name="change" value="[[_change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </paper-tab>
- </template>
- </paper-tabs>
- </template>
-
<section class="patchInfo">
- <div hidden$="[[!_showFileTabContent]]">
+ <template is="dom-if" if="[[_showPrimaryTabs]]">
+ <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
+ <paper-tab data-name$="[[_files_tab_name]]">Files</paper-tab>
+ <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
+ as="tabHeader">
+ <paper-tab data-name$="[[tabHeader]]">
+ <gr-endpoint-decorator name$="[[tabHeader]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </paper-tab>
+ </template>
+ </paper-tabs>
+ </template>
+ <div hidden$="[[!_findIfTabMatches(_currentTabName, _files_tab_name)]]">
<gr-file-list-header
id="fileListHeader"
account="[[_account]]"
@@ -604,13 +603,13 @@
on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list>
</div>
- <template is="dom-if" if="[[!_showFileTabContent]]">
- <gr-endpoint-decorator name$="[[_selectedFilesTabPluginEndpoint]]">
- <gr-endpoint-param name="change" value="[[_change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
+ <template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _selectedTabPluginHeader)]]">
+ <gr-endpoint-decorator name$="[[_selectedTabPluginEndpoint]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
</template>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 1622ebd..b55d964 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -68,6 +68,7 @@
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
+ const FILES_TAB_NAME = 'files';
/**
* @appliesMixin Gerrit.FireMixin
@@ -322,7 +323,12 @@
_dynamicTabContentEndpoints: {
type: Array,
},
- _selectedFilesTabPluginEndpoint: {
+ // The dynamic content of the plugin added tab
+ _selectedTabPluginEndpoint: {
+ type: String,
+ },
+ // The dynamic heading of the plugin added tab
+ _selectedTabPluginHeader: {
type: String,
},
_robotCommentsPatchSetDropdownItems: {
@@ -333,6 +339,14 @@
_currentRobotCommentsPatchSet: {
type: Number,
},
+ _files_tab_name: {
+ type: String,
+ value: FILES_TAB_NAME,
+ },
+ _currentTabName: {
+ type: String,
+ value: FILES_TAB_NAME,
+ },
};
}
@@ -503,30 +517,44 @@
return currentView === view;
}
- _handleFileTabChange(e) {
- const selectedIndex = this.shadowRoot
- .querySelector('#primaryTabs').selected;
- this._showFileTabContent = selectedIndex === 0;
- // Initial tab is the static files list.
- const newSelectedTab =
- this._dynamicTabContentEndpoints[selectedIndex - 1];
- if (newSelectedTab !== this._selectedFilesTabPluginEndpoint) {
- this._selectedFilesTabPluginEndpoint = newSelectedTab;
+ _findIfTabMatches(currentTab, tab) {
+ return currentTab === tab;
+ }
- const tabName = this._selectedFilesTabPluginEndpoint || 'files';
- const source = e && e.type ? e.type : '';
- this.$.reporting.reportInteraction('tab-changed', {tabName, source});
+ _handleFileTabChange(e) {
+ const selectedIndex = e.target.selected;
+ const tabs = e.target.querySelectorAll('paper-tab');
+ this._currentTabName = tabs[selectedIndex] &&
+ tabs[selectedIndex].dataset.name;
+ const source = e && e.type ? e.type : '';
+ const pluginIndex = this._dynamicTabHeaderEndpoints.indexOf(
+ this._currentTabName);
+ if (pluginIndex !== -1) {
+ this._selectedTabPluginEndpoint = this._dynamicTabContentEndpoints[
+ pluginIndex];
+ this._selectedTabPluginHeader = this._dynamicTabHeaderEndpoints[
+ pluginIndex];
+ } else {
+ this._selectedTabPluginEndpoint = '';
+ this._selectedTabPluginHeader = '';
}
+ this.$.reporting.reportInteraction('tab-changed',
+ {tabName: this._currentTabName, source});
}
_handleShowTab(e) {
- const idx = this._dynamicTabContentEndpoints.indexOf(e.detail.tab);
+ const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
+ const tabs = primaryTabs.querySelectorAll('paper-tab');
+ let idx = -1;
+ tabs.forEach((tab, index) => {
+ if (tab.dataset.name === e.detail.tab) idx = index;
+ });
if (idx === -1) {
- console.warn(e.detail.tab + ' tab not found');
+ console.error(e.detail.tab + ' tab not found');
return;
}
- this.shadowRoot.querySelector('#primaryTabs').selected = idx + 1;
- this.shadowRoot.querySelector('#primaryTabs').scrollIntoView();
+ primaryTabs.selected = idx;
+ primaryTabs.scrollIntoView();
this.$.reporting.reportInteraction('show-tab', {tabName: e.detail.tab});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 934eeaf..3fe8faf 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -280,6 +280,20 @@
element = fixture('basic');
sandbox.stub(element.$.actions, 'reload').returns(Promise.resolve());
Gerrit._loadPlugins([]);
+ Gerrit.install(
+ plugin => {
+ plugin.registerDynamicCustomComponent(
+ 'change-view-tab-header',
+ 'gr-checks-change-view-tab-header-view'
+ );
+ plugin.registerDynamicCustomComponent(
+ 'change-view-tab-content',
+ 'gr-checks-view'
+ );
+ },
+ '0.1',
+ 'http://some/plugins/url.html'
+ );
});
teardown(done => {
@@ -305,6 +319,44 @@
assert.isTrue(replaceStateStub.called);
});
+ suite('plugins adding to file tab', () => {
+ setup(done => {
+ // Resolving it here instead of during setup() as other tests depend
+ // on flush() not being called during setup.
+ flush(() => done());
+ });
+
+ test('plugin added tab shows up as a dynamic endpoint', () => {
+ assert(element._dynamicTabHeaderEndpoints.includes(
+ 'change-view-tab-header-url'));
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ assert.equal(paperTabs.querySelectorAll('paper-tab').length, 2);
+ assert.equal(paperTabs.querySelectorAll('paper-tab')[1].dataset.name,
+ 'change-view-tab-header-url');
+ });
+
+ test('handleShowTab switched tab correctly', done => {
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ assert.equal(paperTabs.selected, 0);
+ element._handleShowTab({detail:
+ {tab: 'change-view-tab-header-url'}});
+ flush(() => {
+ assert.equal(paperTabs.selected, 1);
+ done();
+ });
+ });
+
+ test('switching tab sets _selectedTabPluginEndpoint', done => {
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ MockInteractions.tap(paperTabs.querySelectorAll('paper-tab')[1]);
+ flush(() => {
+ assert.equal(element._selectedTabPluginEndpoint,
+ 'change-view-tab-content-url');
+ done();
+ });
+ });
+ });
+
suite('keyboard shortcuts', () => {
test('t to add topic', () => {
const editStub = sandbox.stub(element.$.metadata, 'editTopic');
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
index 06edb9b..c650bb5 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
@@ -76,10 +76,10 @@
</div>
<div slot="footer" class="fix-picker" hidden$="[[hasSingleFix(_fixSuggestions)]]">
<span>Suggested fix [[addOneTo(_selectedFixIdx)]] of [[_fixSuggestions.length]]</span>
- <gr-button on-click="_onPrevFixClick" disabled$="[[_noPrevFix(_selectedFixIdx)]]">
+ <gr-button id="prevFix" on-click="_onPrevFixClick" disabled$="[[_noPrevFix(_selectedFixIdx)]]">
<iron-icon icon="gr-icons:chevron-left"></iron-icon>
</gr-button>
- <gr-button on-click="_onNextFixClick" disabled$="[[_noNextFix(_selectedFixIdx)]]">
+ <gr-button id="nextFix" on-click="_onNextFixClick" disabled$="[[_noNextFix(_selectedFixIdx, _fixSuggestions)]]">
<iron-icon icon="gr-icons:chevron-right"></iron-icon>
</gr-button>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
index 45e8c07..94549ee 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
@@ -105,11 +105,7 @@
})
.catch(err => {
this._close();
- this.dispatchEvent(new CustomEvent('show-error', {
- bubbles: true,
- composed: true,
- detail: {message: `Error generating fix preview: ${err}`},
- }));
+ throw err;
});
},
@@ -144,8 +140,8 @@
_onNextFixClick(e) {
if (e) e.stopPropagation();
- if (this._selectedFixIdx < this._fixSuggestions.length &&
- this._fixSuggestions != null) {
+ if (this._fixSuggestions &&
+ this._selectedFixIdx < this._fixSuggestions.length) {
this._selectedFixIdx += 1;
return this._showSelectedFixSuggestion(
this._fixSuggestions[this._selectedFixIdx]);
@@ -156,9 +152,9 @@
return _selectedFixIdx === 0;
},
- _noNextFix(_selectedFixIdx) {
- if (this._fixSuggestions == null) return true;
- return _selectedFixIdx === this._fixSuggestions.length - 1;
+ _noNextFix(_selectedFixIdx, fixSuggestions) {
+ if (fixSuggestions == null) return true;
+ return _selectedFixIdx === fixSuggestions.length - 1;
},
_close() {
@@ -189,14 +185,7 @@
this._currentFix.fix_id).then(res => {
Gerrit.Nav.navigateToChange(this.change, 'edit', this._patchNum);
this._close();
- })
- .catch(err => {
- this.dispatchEvent(new CustomEvent('show-error', {
- bubbles: true,
- composed: true,
- detail: {message: `Error applying fix suggestion: ${err}`},
- }));
- });
+ });
},
getFixDescription(currentFix) {
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
index d4ab8f1..f3f748f 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
@@ -17,11 +17,12 @@
-->
<meta name='viewport' content='width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes'>
<title>gr-apply-fix-dialog</title>
-<script src='/test/common-test-setup.js'></script>
+<link rel='import' href='../../../test/common-test-setup.html'>
<script src='/bower_components/webcomponentsjs/custom-elements-es5-adapter.js'></script>
<script src='/bower_components/webcomponentsjs/webcomponents-lite.js'></script>
<script src='/bower_components/web-component-tester/browser.js'></script>
+<script src="../../../test/test-pre-setup.js"></script>
<link rel='import' href='../../../test/common-test-setup.html' />
<link rel='import' href='./gr-apply-fix-dialog.html'>
@@ -39,11 +40,16 @@
await readyToTest();
let element;
let sandbox;
- const ROBOT_COMMENT = {
+ const ROBOT_COMMENT_WITH_TWO_FIXES = {
robot_id: 'robot_1',
fix_suggestions: [{fix_id: 'fix_1'}, {fix_id: 'fix_2'}],
};
+ const ROBOT_COMMENT_WITH_ONE_FIX = {
+ robot_id: 'robot_1',
+ fix_suggestions: [{fix_id: 'fix_1'}],
+ };
+
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
@@ -100,7 +106,7 @@
}));
sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}})
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_TWO_FIXES}})
.then(() => {
assert.equal(element._currentFix.fix_id, 'fix_1');
assert.equal(element._currentPreviews.length, 2);
@@ -109,16 +115,42 @@
});
});
+ test('next button state updated when suggestions changed', done => {
+ sandbox.stub(element.$.restAPI, 'getRobotCommentFixPreview')
+ .returns(Promise.resolve({}));
+ sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
+
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_ONE_FIX}})
+ .then(() => assert.isTrue(element.$.nextFix.disabled))
+ .then(() =>
+ element.open({detail: {patchNum: 2,
+ comment: ROBOT_COMMENT_WITH_TWO_FIXES}}))
+ .then(() => {
+ assert.isFalse(element.$.nextFix.disabled);
+ done();
+ });
+ });
+
test('preview endpoint throws error should reset dialog', done => {
- element.addEventListener('show-error', () => {
+ sandbox.stub(window, 'fetch', (url => {
+ if (url.endsWith('/preview')) {
+ return Promise.reject(new Error('backend error'));
+ }
+ return Promise.resolve({
+ ok: true,
+ text() { return Promise.resolve(''); },
+ status: 200,
+ });
+ }));
+ const errorStub = sinon.stub();
+ document.addEventListener('network-error', errorStub);
+ element.open({detail: {patchNum: 2,
+ comment: ROBOT_COMMENT_WITH_TWO_FIXES}});
+ flush(() => {
+ assert.isTrue(errorStub.called);
assert.deepEqual(element._currentFix, {});
- assert.equal(element._currentPreviews.length, 0);
done();
});
- sandbox.stub(element.$.restAPI, 'getRobotCommentFixPreview',
- () => Promise.reject(new Error('backend error')));
-
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}});
});
test('apply fix button should call apply' +
@@ -179,7 +211,7 @@
}));
sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}})
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_TWO_FIXES}})
.then(() => {
element._onNextFixClick();
assert.equal(element._currentFix.fix_id, 'fix_2');
@@ -188,5 +220,28 @@
done();
});
});
+
+ test('server-error should throw for failed apply call', done => {
+ sandbox.stub(window, 'fetch', (url => {
+ if (url.endsWith('/apply')) {
+ return Promise.reject(new Error('backend error'));
+ }
+ return Promise.resolve({
+ ok: true,
+ text() { return Promise.resolve(''); },
+ status: 200,
+ });
+ }));
+ const errorStub = sinon.stub();
+ document.addEventListener('network-error', errorStub);
+ sandbox.stub(Gerrit.Nav, 'navigateToChange');
+ element._currentFix = {fix_id: '123'};
+ element._handleApplyFix();
+ flush(() => {
+ assert.isFalse(Gerrit.Nav.navigateToChange.called);
+ assert.isTrue(errorStub.called);
+ done();
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index 7d60f13..2d9369f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -51,7 +51,9 @@
coverage-ranges="[[_coverageRanges]]"
blame="[[_blame]]"
layers="[[_layers]]"
- diff="[[diff]]">
+ diff="[[diff]]"
+ show-newline-warning-left="[[_showNewlineWarningLeft(diff)]]"
+ show-newline-warning-right="[[_showNewlineWarningRight(diff)]]">
</gr-diff>
<gr-syntax-layer
id="syntaxLayer"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index da22f6b..a44e366 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -1037,6 +1037,73 @@
'diff-context-expanded', {numLines: event.detail.numLines}
);
}
+
+ /**
+ * Find the last chunk for the given side.
+ *
+ * @param {!Object} diff
+ * @param {boolean} leftSide true if checking the base of the diff,
+ * false if testing the revision.
+ * @return {Object|null} returns the chunk object or null if there was
+ * no chunk for that side.
+ */
+ _lastChunkForSide(diff, leftSide) {
+ if (!diff.content.length) { return null; }
+
+ let chunkIndex = diff.content.length;
+ let chunk;
+
+ // Walk backwards until we find a chunk for the given side.
+ do {
+ chunkIndex--;
+ chunk = diff.content[chunkIndex];
+ } while (
+ // We haven't reached the beginning.
+ chunkIndex >= 0 &&
+
+ // The chunk doesn't have both sides.
+ !chunk.ab &&
+
+ // The chunk doesn't have the given side.
+ ((leftSide && (!chunk.a || !chunk.a.length)) ||
+ (!leftSide && (!chunk.b || !chunk.b.length))));
+
+ // If we reached the beginning of the diff and failed to find a chunk
+ // with the given side, return null.
+ if (chunkIndex === -1) { return null; }
+
+ return chunk;
+ }
+
+ /**
+ * Check whether the specified side of the diff has a trailing newline.
+ *
+ * @param {!Object} diff
+ * @param {boolean} leftSide true if checking the base of the diff,
+ * false if testing the revision.
+ * @return {boolean|null} Return true if the side has a trailing newline.
+ * Return false if it doesn't. Return null if not applicable (for
+ * example, if the diff has no content on the specified side).
+ */
+ _hasTrailingNewlines(diff, leftSide) {
+ const chunk = this._lastChunkForSide(diff, leftSide);
+ if (!chunk) { return null; }
+ let lines;
+ if (chunk.ab) {
+ lines = chunk.ab;
+ } else {
+ lines = leftSide ? chunk.a : chunk.b;
+ }
+ return lines[lines.length - 1] === '';
+ }
+
+ _showNewlineWarningLeft(diff) {
+ return this._hasTrailingNewlines(diff, true) === false;
+ }
+
+ _showNewlineWarningRight(diff) {
+ return this._hasTrailingNewlines(diff, false) === false;
+ }
}
customElements.define(GrDiffHost.is, GrDiffHost);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index fd93cb4..5a42fc4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -1530,5 +1530,102 @@
});
});
});
+
+ suite('trailing newlines', () => {
+ setup(() => {
+ });
+
+ suite('_lastChunkForSide', () => {
+ test('deltas', () => {
+ const diff = {content: [
+ {a: ['foo', 'bar'], b: ['baz']},
+ {ab: ['foo', 'bar', 'baz']},
+ {b: ['foo']},
+ ]};
+ assert.equal(element._lastChunkForSide(diff, false), diff.content[2]);
+ assert.equal(element._lastChunkForSide(diff, true), diff.content[1]);
+
+ diff.content.push({a: ['foo'], b: ['bar']});
+ assert.equal(element._lastChunkForSide(diff, false), diff.content[3]);
+ assert.equal(element._lastChunkForSide(diff, true), diff.content[3]);
+ });
+
+ test('addition with a undefined', () => {
+ const diff = {content: [
+ {b: ['foo', 'bar', 'baz']},
+ ]};
+ assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
+ assert.isNull(element._lastChunkForSide(diff, true));
+ });
+
+ test('addition with a empty', () => {
+ const diff = {content: [
+ {a: [], b: ['foo', 'bar', 'baz']},
+ ]};
+ assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
+ assert.isNull(element._lastChunkForSide(diff, true));
+ });
+
+ test('deletion with b undefined', () => {
+ const diff = {content: [
+ {a: ['foo', 'bar', 'baz']},
+ ]};
+ assert.isNull(element._lastChunkForSide(diff, false));
+ assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
+ });
+
+ test('deletion with b empty', () => {
+ const diff = {content: [
+ {a: ['foo', 'bar', 'baz'], b: []},
+ ]};
+ assert.isNull(element._lastChunkForSide(diff, false));
+ assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
+ });
+
+ test('empty', () => {
+ const diff = {content: []};
+ assert.isNull(element._lastChunkForSide(diff, false));
+ assert.isNull(element._lastChunkForSide(diff, true));
+ });
+ });
+
+ suite('_hasTrailingNewlines', () => {
+ test('shared no trailing', () => {
+ const diff = undefined;
+ sandbox.stub(element, '_lastChunkForSide')
+ .returns({ab: ['foo', 'bar']});
+ assert.isFalse(element._hasTrailingNewlines(diff, false));
+ assert.isFalse(element._hasTrailingNewlines(diff, true));
+ });
+
+ test('delta trailing in right', () => {
+ const diff = undefined;
+ sandbox.stub(element, '_lastChunkForSide')
+ .returns({a: ['foo', 'bar'], b: ['baz', '']});
+ assert.isTrue(element._hasTrailingNewlines(diff, false));
+ assert.isFalse(element._hasTrailingNewlines(diff, true));
+ });
+
+ test('addition', () => {
+ const diff = undefined;
+ sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
+ if (leftSide) { return null; }
+ return {b: ['foo', '']};
+ });
+ assert.isTrue(element._hasTrailingNewlines(diff, false));
+ assert.isNull(element._hasTrailingNewlines(diff, true));
+ });
+
+ test('deletion', () => {
+ const diff = undefined;
+ sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
+ if (!leftSide) { return null; }
+ return {a: ['foo']};
+ });
+ assert.isNull(element._hasTrailingNewlines(diff, false));
+ assert.isFalse(element._hasTrailingNewlines(diff, true));
+ });
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index ef22188..8e96147 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -249,9 +249,19 @@
parentIndex: Number,
+ showNewlineWarningLeft: {
+ type: Boolean,
+ value: false,
+ },
+ showNewlineWarningRight: {
+ type: Boolean,
+ value: false,
+ },
+
_newlineWarning: {
type: String,
- computed: '_computeNewlineWarning(diff)',
+ computed: '_computeNewlineWarning(' +
+ 'showNewlineWarningLeft, showNewlineWarningRight)',
},
_diffLength: Number,
@@ -916,76 +926,16 @@
}
/**
- * Find the last chunk for the given side.
- *
- * @param {!Object} diff
- * @param {boolean} leftSide true if checking the base of the diff,
- * false if testing the revision.
- * @return {Object|null} returns the chunk object or null if there was
- * no chunk for that side.
- */
- _lastChunkForSide(diff, leftSide) {
- if (!diff.content.length) { return null; }
-
- let chunkIndex = diff.content.length;
- let chunk;
-
- // Walk backwards until we find a chunk for the given side.
- do {
- chunkIndex--;
- chunk = diff.content[chunkIndex];
- } while (
- // We haven't reached the beginning.
- chunkIndex >= 0 &&
-
- // The chunk doesn't have both sides.
- !chunk.ab &&
-
- // The chunk doesn't have the given side.
- ((leftSide && (!chunk.a || !chunk.a.length)) ||
- (!leftSide && (!chunk.b || !chunk.b.length))));
-
- // If we reached the beginning of the diff and failed to find a chunk
- // with the given side, return null.
- if (chunkIndex === -1) { return null; }
-
- return chunk;
- }
-
- /**
- * Check whether the specified side of the diff has a trailing newline.
- *
- * @param {!Object} diff
- * @param {boolean} leftSide true if checking the base of the diff,
- * false if testing the revision.
- * @return {boolean|null} Return true if the side has a trailing newline.
- * Return false if it doesn't. Return null if not applicable (for
- * example, if the diff has no content on the specified side).
- */
- _hasTrailingNewlines(diff, leftSide) {
- const chunk = this._lastChunkForSide(diff, leftSide);
- if (!chunk) { return null; }
- let lines;
- if (chunk.ab) {
- lines = chunk.ab;
- } else {
- lines = leftSide ? chunk.a : chunk.b;
- }
- return lines[lines.length - 1] === '';
- }
-
- /**
- * @param {!Object} diff
+ * @param {!boolean} warnLeft
+ * @param {!boolean} warnRight
* @return {string|null}
*/
- _computeNewlineWarning(diff) {
- const hasLeft = this._hasTrailingNewlines(diff, true);
- const hasRight = this._hasTrailingNewlines(diff, false);
+ _computeNewlineWarning(warnLeft, warnRight) {
const messages = [];
- if (hasLeft === false) {
+ if (warnLeft) {
messages.push(NO_NEWLINE_BASE);
}
- if (hasRight === false) {
+ if (warnRight) {
messages.push(NO_NEWLINE_REVISION);
}
if (!messages.length) { return null; }
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 9fcf1ca..b14cc203 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -834,137 +834,60 @@
});
});
- suite('trailing newlines', () => {
+ suite('trailing newline warnings', () => {
+ const NO_NEWLINE_BASE = 'No newline at end of base file.';
+ const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
+
+ const getWarning = element =>
+ element.shadowRoot.querySelector('.newlineWarning').textContent;
+
setup(() => {
element = fixture('basic');
+ element.showNewlineWarningLeft = false;
+ element.showNewlineWarningRight = false;
});
- suite('_lastChunkForSide', () => {
- test('deltas', () => {
- const diff = {content: [
- {a: ['foo', 'bar'], b: ['baz']},
- {ab: ['foo', 'bar', 'baz']},
- {b: ['foo']},
- ]};
- assert.equal(element._lastChunkForSide(diff, false), diff.content[2]);
- assert.equal(element._lastChunkForSide(diff, true), diff.content[1]);
-
- diff.content.push({a: ['foo'], b: ['bar']});
- assert.equal(element._lastChunkForSide(diff, false), diff.content[3]);
- assert.equal(element._lastChunkForSide(diff, true), diff.content[3]);
- });
-
- test('addition with a undefined', () => {
- const diff = {content: [
- {b: ['foo', 'bar', 'baz']},
- ]};
- assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
- assert.isNull(element._lastChunkForSide(diff, true));
- });
-
- test('addition with a empty', () => {
- const diff = {content: [
- {a: [], b: ['foo', 'bar', 'baz']},
- ]};
- assert.equal(element._lastChunkForSide(diff, false), diff.content[0]);
- assert.isNull(element._lastChunkForSide(diff, true));
- });
-
- test('deletion with b undefined', () => {
- const diff = {content: [
- {a: ['foo', 'bar', 'baz']},
- ]};
- assert.isNull(element._lastChunkForSide(diff, false));
- assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
- });
-
- test('deletion with b empty', () => {
- const diff = {content: [
- {a: ['foo', 'bar', 'baz'], b: []},
- ]};
- assert.isNull(element._lastChunkForSide(diff, false));
- assert.equal(element._lastChunkForSide(diff, true), diff.content[0]);
- });
-
- test('empty', () => {
- const diff = {content: []};
- assert.isNull(element._lastChunkForSide(diff, false));
- assert.isNull(element._lastChunkForSide(diff, true));
- });
- });
-
- suite('_hasTrailingNewlines', () => {
- test('shared no trailing', () => {
- const diff = undefined;
- sandbox.stub(element, '_lastChunkForSide')
- .returns({ab: ['foo', 'bar']});
- assert.isFalse(element._hasTrailingNewlines(diff, false));
- assert.isFalse(element._hasTrailingNewlines(diff, true));
- });
-
- test('delta trailing in right', () => {
- const diff = undefined;
- sandbox.stub(element, '_lastChunkForSide')
- .returns({a: ['foo', 'bar'], b: ['baz', '']});
- assert.isTrue(element._hasTrailingNewlines(diff, false));
- assert.isFalse(element._hasTrailingNewlines(diff, true));
- });
-
- test('addition', () => {
- const diff = undefined;
- sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
- if (leftSide) { return null; }
- return {b: ['foo', '']};
- });
- assert.isTrue(element._hasTrailingNewlines(diff, false));
- assert.isNull(element._hasTrailingNewlines(diff, true));
- });
-
- test('deletion', () => {
- const diff = undefined;
- sandbox.stub(element, '_lastChunkForSide', (diff, leftSide) => {
- if (!leftSide) { return null; }
- return {a: ['foo']};
- });
- assert.isNull(element._hasTrailingNewlines(diff, false));
- assert.isFalse(element._hasTrailingNewlines(diff, true));
- });
- });
-
- test('_computeNewlineWarning', () => {
- const NO_NEWLINE_BASE = 'No newline at end of base file.';
- const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
-
- let hasLeft;
- let hasRight;
- sandbox.stub(element, '_hasTrailingNewlines', (diff, left) => {
- if (left) { return hasLeft; }
- return hasRight;
- });
- const diff = undefined;
-
- // The revision has a trailing newline, but the base doesn't.
- hasLeft = true;
- hasRight = false;
- assert.equal(element._computeNewlineWarning(diff), NO_NEWLINE_REVISION);
-
- // Trailing newlines were undetermined in the revision.
- hasLeft = true;
- hasRight = null;
- assert.isNull(element._computeNewlineWarning(diff));
-
- // Missing trailing newlines in the base.
- hasLeft = false;
- hasRight = null;
- assert.equal(element._computeNewlineWarning(diff), NO_NEWLINE_BASE);
-
- // Missing trailing newlines in both.
- hasLeft = false;
- hasRight = false;
- assert.equal(element._computeNewlineWarning(diff),
+ test('shows combined warning if both sides set to warn', () => {
+ element.showNewlineWarningLeft = true;
+ element.showNewlineWarningRight = true;
+ assert.include(getWarning(element),
NO_NEWLINE_BASE + ' — ' + NO_NEWLINE_REVISION);
});
+ suite('showNewlineWarningLeft', () => {
+ test('show warning if true', () => {
+ element.showNewlineWarningLeft = true;
+ assert.include(getWarning(element), NO_NEWLINE_BASE);
+ });
+
+ test('hide warning if false', () => {
+ element.showNewlineWarningLeft = false;
+ assert.notInclude(getWarning(element), NO_NEWLINE_BASE);
+ });
+
+ test('hide warning if undefined', () => {
+ element.showNewlineWarningLeft = undefined;
+ assert.notInclude(getWarning(element), NO_NEWLINE_BASE);
+ });
+ });
+
+ suite('showNewlineWarningRight', () => {
+ test('show warning if true', () => {
+ element.showNewlineWarningRight = true;
+ assert.include(getWarning(element), NO_NEWLINE_REVISION);
+ });
+
+ test('hide warning if false', () => {
+ element.showNewlineWarningRight = false;
+ assert.notInclude(getWarning(element), NO_NEWLINE_REVISION);
+ });
+
+ test('hide warning if undefined', () => {
+ element.showNewlineWarningRight = undefined;
+ assert.notInclude(getWarning(element), NO_NEWLINE_REVISION);
+ });
+ });
+
test('_computeNewlineWarningClass', () => {
const hidden = 'newlineWarning hidden';
const shown = 'newlineWarning';
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
index 5a2f104..1c10642 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
@@ -60,7 +60,7 @@
*/
_import(url) {
return new Promise((resolve, reject) => {
- (this.importHref || Polymer.importHref)(url, resolve, reject);
+ Polymer.importHref(url, resolve, reject);
});
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
index 2e3bee1..d12a571 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
@@ -44,7 +44,7 @@
if (this._urlsImported.includes(url)) { return Promise.resolve(); }
this._urlsImported.push(url);
return new Promise((resolve, reject) => {
- (this.importHref || Polymer.importHref)(url, resolve, reject);
+ Polymer.importHref(url, resolve, reject);
});
}
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 ac5c2e9..c344bf64 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
@@ -59,11 +59,9 @@
padding: 2px;
}
}
-
</style>
<div class="text">
<iron-input
- hidden$="[[hideInput]]"
class="copyText"
type="text"
bind-value="[[text]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
index 677a952..55eb198 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
@@ -68,13 +68,23 @@
});
test('_handleInputClick', () => {
- const inputElement = element.$$('input');
+ // iron-input as parent should never be hidden as copy won't work
+ // on nested hidden elements
+ const ironInputElement = element.shadowRoot.querySelector('iron-input');
+ assert.notEqual(getComputedStyle(ironInputElement).display, 'none');
+
+ const inputElement = element.shadowRoot.querySelector('input');
MockInteractions.tap(inputElement);
assert.equal(inputElement.selectionStart, 0);
assert.equal(inputElement.selectionEnd, element.text.length - 1);
});
test('hideInput', () => {
+ // iron-input as parent should never be hidden as copy won't work
+ // on nested hidden elements
+ const ironInputElement = element.shadowRoot.querySelector('iron-input');
+ assert.notEqual(getComputedStyle(ironInputElement).display, 'none');
+
assert.notEqual(getComputedStyle(element.$.input).display, 'none');
element.hideInput = true;
flushAsynchronousOperations();
diff --git a/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html b/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
index 6e62b15..305702a 100644
--- a/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
@@ -19,7 +19,7 @@
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-api-interface</title>
-<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<script src="../../../test/test-pre-setup.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
diff --git a/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js b/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
index defc6bd..81126ec 100644
--- a/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
+++ b/polygerrit-ui/app/elements/shared/gr-lib-loader/gr-lib-loader.js
@@ -77,7 +77,7 @@
*/
getDarkTheme() {
return new Promise((resolve, reject) => {
- (this.importHref || Polymer.importHref)(
+ Polymer.importHref(
this._getLibRoot() + DARK_THEME_PATH, () => {
const module = document.createElement('style');
module.setAttribute('include', 'dark-theme');
@@ -85,7 +85,8 @@
cs.appendChild(module);
resolve(cs);
- });
+ },
+ reject);
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
index bbe525e..10f7ed6 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
@@ -76,10 +76,16 @@
this._handleParseResult.bind(this), this.removeZeroWidthSpace);
parser.parse(content);
- // Ensure that links originating from HTML commentlink configs open in a
- // new tab. @see Issue 5567
+ // Ensure that external links originating from HTML commentlink configs
+ // open in a new tab. @see Issue 5567
+ // Ensure links to the same host originating from commentlink configs
+ // open in the same tab. @see Issue 4616
output.querySelectorAll('a').forEach(anchor => {
- anchor.setAttribute('target', '_blank');
+ if (anchor.hostname === window.location.hostname) {
+ anchor.setAttribute('target', '_self');
+ } else {
+ anchor.setAttribute('target', '_blank');
+ }
anchor.setAttribute('rel', 'noopener');
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 2b2fcfc..ff94bc7 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -146,7 +146,7 @@
const linkEl = element.$.output.childNodes[1];
assert.equal(textNode.textContent, prefix);
const url = '/q/' + changeID;
- assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.target, '_self');
// Since url is a path, the host is added automatically.
assert.isTrue(linkEl.href.endsWith(url));
assert.equal(linkEl.textContent, changeID);
@@ -164,7 +164,7 @@
const linkEl = element.$.output.childNodes[1];
assert.equal(textNode.textContent, prefix);
const url = '/r/q/' + changeID;
- assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.target, '_self');
// Since url is a path, the host is added automatically.
assert.isTrue(linkEl.href.endsWith(url));
assert.equal(linkEl.textContent, changeID);
@@ -205,7 +205,7 @@
assert.equal(textNode.textContent, prefix);
- assert.equal(changeLinkEl.target, '_blank');
+ assert.equal(changeLinkEl.target, '_self');
assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
assert.equal(changeLinkEl.textContent, changeID);
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
index 2c326df..0c8ca06 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
@@ -359,15 +359,16 @@
fetchOptions: options,
anonymizedUrl: req.reportUrlAsIs ? url : req.anonymizedUrl,
};
- const xhr = this.fetch(fetchReq).then(response => {
- if (!response.ok) {
- if (req.errFn) {
- return req.errFn.call(undefined, response);
- }
- this.fire('server-error', {request: fetchReq, response});
- }
- return response;
- })
+ const xhr = this.fetch(fetchReq)
+ .then(response => {
+ if (!response.ok) {
+ if (req.errFn) {
+ return req.errFn.call(undefined, response);
+ }
+ this.fire('server-error', {request: fetchReq, response});
+ }
+ return response;
+ })
.catch(err => {
this.fire('network-error', {error: err});
if (req.errFn) {
diff --git a/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
index 5fad3f4..f64d9ef 100644
--- a/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-email-suggestions-provider.js"></script>
diff --git a/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
index 7d89c2a..c46b443 100644
--- a/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-group-suggestions-provider.js"></script>
diff --git a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
index af7b4bd..9696c2e 100644
--- a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-reviewer-suggestions-provider.js"></script>
diff --git a/proto/cache.proto b/proto/cache.proto
index 8f73388..b881e3d 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -75,7 +75,7 @@
// Instead, we just take the tedious yet simple approach of having a "has_foo"
// field for each nullable field "foo", indicating whether or not foo is null.
//
-// Next ID: 23
+// Next ID: 24
message ChangeNotesStateProto {
// Effectively required, even though the corresponding ChangeNotesState field
// is optional, since the field is only absent when NoteDb is disabled, in
@@ -133,7 +133,7 @@
// which case attempting to use the ChangeNotesCache is programmer error.
ChangeColumnsProto columns = 3;
- reserved 4; // past_assignee
+ reserved 4; // past_assignee
repeated string hashtag = 5;
@@ -167,6 +167,7 @@
// Next ID: 5
message ReviewerStatusUpdateProto {
+ // Epoch millis.
int64 date = 1;
int32 updated_by = 2;
int32 reviewer = 3;
@@ -194,14 +195,26 @@
bool has_server_id = 21;
message AssigneeStatusUpdateProto {
+ // Epoch millis.
int64 date = 1;
int32 updated_by = 2;
int32 current_assignee = 3;
bool has_current_assignee = 4;
}
repeated AssigneeStatusUpdateProto assignee_update = 22;
-}
+ // An update to the attention set of the change. See class AttentionStatus for
+ // context.
+ message AttentionStatusProto {
+ // Epoch millis.
+ int64 date = 1;
+ int32 account = 2;
+ // Maps to enum AttentionStatus.Operation
+ string operation = 3;
+ string reason = 4;
+ }
+ repeated AttentionStatusProto attention_status = 23;
+}
// Serialized form of com.google.gerrit.server.query.change.ConflictKey
message ConflictKeyProto {
diff --git a/tools/bzl/plugin.bzl b/tools/bzl/plugin.bzl
index 64a6e22..3cffb79 100644
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -51,16 +51,21 @@
# TODO(davido): Remove manual merge of manifest file when this feature
# request is implemented: https://github.com/bazelbuild/bazel/issues/2009
+ # TODO(davido): Remove manual touch command when this issue is resolved:
+ # https://github.com/bazelbuild/bazel/issues/10789
genrule2(
name = name + target_suffix,
stamp = 1,
srcs = ["%s__non_stamped_deploy.jar" % name],
cmd = " && ".join([
+ "TZ=UTC",
+ "export TZ",
"GEN_VERSION=$$(cat bazel-out/stable-status.txt | grep -w STABLE_BUILD_%s_LABEL | cut -d ' ' -f 2)" % dir_name.upper(),
"cd $$TMP",
"unzip -q $$ROOT/$<",
"echo \"Implementation-Version: $$GEN_VERSION\n$$(cat META-INF/MANIFEST.MF)\" > META-INF/MANIFEST.MF",
- "zip -qr $$ROOT/$@ .",
+ "find . -exec touch '{}' ';'",
+ "zip -Xqr $$ROOT/$@ .",
]),
outs = ["%s%s.jar" % (name, target_suffix)],
visibility = ["//visibility:public"],
