Merge branch 'stable-3.0' into stable-3.1
* stable-3.0:
Open relative commentlinks in same tab
VersionedMetaData: Emit debug log after update has been committed
VersionedMetaData: Trim message sent to log
ChangeIdHandler: Make assumption on number of query results explicit
ChangeIdHandler: Remove unnecessary 'final' modifiers
Update git submodules
AbstractQueryChangesTest: Add method to create change as private
AbstractQueryChangesTest: Fix comment in byPrivate
AbstractQueryChangesTest: Use overloaded newChange method where possible
AbstractQueryChangesTest: Extend testing of visibleto predicate
ChangeQueryBuilder: Rename status_open to statusOpen
ChangeQueryBuilder: Rename is_visible to isVisible
ChangeNotificationsIT: Fix eclipse warning(s) about static use
Extract duplicated code of {Ssh|Http}LogJsonLayout
Add option to log SSH events in JSON format
Remove duplicated constants storing key names of Ssh logs
Add option to log HTTP logging events in JSON format
CacheMetrics: Make F_NAME a final constant in class scope
Fix more cases where Collection.isEmpty() should be used
Use Set instead of EnumSet in method signatures
Replace negation of anyMatch() with noneMatch()
AllExternalIds: Replace lambdas with method references
PostReview: Replace filter and findFirst with anyMatch
XContentBuilder: Use UTF_8 from Java's StandardCharsets rather than Guava
Update git submodules
Update git submodules
Update git submodules
QueryDocumentationExecutor: Make WEIGHTS final and use ImmutableMap
BooleanProjectConfigTransformations: Make MAPPER final
TestSubmitRuleInfo: Make INSTANCE final
TrackingFootersProvider: Make constants final
ChangeCleanupConfig: Make constants final
SetParent: Simplify lambda expression
ProjectsConsistencyChecker: Simplify lambda expression
Remove redundant null checks used with "instanceof"
Remove unnecessary array creation for varargs parameters
Remove redundant parentheses in lambda statements
PreferencesBox: Remove unnecessary boxing to Boolean
GitwebServlet: Rename bufferSize to BUFFER_SIZE and make static
GetPatch: Make FILE_NOT_FOUND static
DeleteZombieCommentsRefs: Make constants static
AbstractNotificationTest: Make constants static and rename
TabFile: Make TRIM final
Remove redundant calls to toString()
OpenIdServiceImpl: Fix incorrect null comparison of Optional
Consistently use isEmpty to check for empty collection
PageLinks: Reuse ADMIN_PROJECTS constant
PageLinks: Fix typo in method name
FlushCaches: Use isEmpty to check for empty list
JGitMetricModule: Replace anonymous Supplier instances with method references
Adapt to deprecation of WindowCacheStats methods in JGit
Documentation: Fix collapsed words and broken link
Use Flogger instead of System.out.println in DeleteZombieCommentsRefs
Fix a dependency injection runtime error in DeleteZombieDrafts pgm
Update git submodules
Bazel: Move runtime dependencies only from deps to runtime_deps
Update rename project documentation in project owner guide
Fix issues with caching edited commit message
Add "edit" button to diff view
Remove egit plugin from project owner guide Documentation
Error Prone: Enable and fix OperatorPrecedence
Bazel: Remove superfluous dependencies flagged by unused_deps
GerritBaseTests: Activate test logging
Revert "Enable optional DEBUG level logs for query tests"
Allow to control the Gerrit log level for running tests from system var
Revert "Acceptance: set log threshold level for tests"
Reduce log level for tests to INFO
Bazel: Update time attribute of file entries in plugin artifact
Add Jetty connection metrics
Add additional JGit WindowCache metrics
Update JGit to 5.1.13.202002110435-r
BucketedCallback: fix prune() to remove unset sub-metrics from registry
Modify draft ref updates commits to point to an empty parent
Change-Id: I0039bf12f018706b400f905491a515e2e957b3ec
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 38c4737..020141a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1632,6 +1632,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
@@ -3498,7 +3513,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 5a03082..8c2b0f9 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -281,7 +281,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:
@@ -364,21 +364,25 @@
Note that Bazel currently does not show
link:https://github.com/bazelbuild/bazel/issues/3476[the skipped tests].
-[[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
@@ -580,7 +584,6 @@
wrapper script. For an example, see the use of `crisper` in `tools/bzl/js.bzl`.
-
[[RBE]]
== Google Remote Build Support
@@ -615,8 +618,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 1f98291..f7aed5e 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -586,13 +586,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] 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] plugin:
+
@@ -759,7 +752,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].
@@ -768,11 +770,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] 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 d10e574..a360220 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/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 e719f83..646d8f0 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -49,8 +49,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 678bc31..a234e00 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.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.account.AccountOperationsImpl;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
@@ -50,6 +48,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;
@@ -75,11 +74,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;
@@ -117,8 +111,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(
@@ -154,8 +147,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) {
@@ -177,14 +169,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
@@ -224,8 +208,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");
@@ -262,46 +244,6 @@
}
}
- private static final ImmutableMap<String, Level> LOG_LEVELS =
- ImmutableMap.<String, Level>builder()
- .put("com.google.gerrit", Level.DEBUG)
-
- // 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 boolean forceLocalDisk() {
String value = Strings.nullToEmpty(System.getenv("GERRIT_FORCE_LOCAL_DISK"));
if (value.isEmpty()) {
@@ -417,7 +359,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(
@@ -518,25 +460,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/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 f9cd562..90b0930 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -128,7 +128,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 864839a..da5ae92 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/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 25eb7a8..8b1ae3c 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 bcb49de1..9b9a8a4 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 {
/**
@@ -78,7 +79,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;
@@ -116,7 +117,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;
}
@@ -149,7 +150,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 f5ece53..83a2179 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1358,7 +1358,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 16d66b6..e576d73 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -577,7 +577,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/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..6ceb242 100644
--- a/java/com/google/gerrit/pgm/http/jetty/BUILD
+++ b/java/com/google/gerrit/pgm/http/jetty/BUILD
@@ -10,9 +10,11 @@
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/util/http",
+ "//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..33916ac
--- /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.server.logging.JsonLayout;
+import com.google.gerrit.server.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 5c89282..0bbb51d 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -43,6 +43,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;
@@ -116,9 +117,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() {
@@ -152,12 +155,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(
@@ -171,7 +211,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 4d41ed7..0a38ee8 100644
--- a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
+++ b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
@@ -25,12 +25,12 @@
@Singleton
public class ChangeCleanupConfig {
- 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 c284f7f4..dd39198 100644
--- a/java/com/google/gerrit/server/git/GroupCollector.java
+++ b/java/com/google/gerrit/server/git/GroupCollector.java
@@ -141,7 +141,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/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/meta/TabFile.java b/java/com/google/gerrit/server/git/meta/TabFile.java
index 4c0378a..64ae6dd 100644
--- a/java/com/google/gerrit/server/git/meta/TabFile.java
+++ b/java/com/google/gerrit/server/git/meta/TabFile.java
@@ -33,7 +33,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 c6c9b39..ead76cc 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/logging/JsonLayout.java b/java/com/google/gerrit/server/logging/JsonLayout.java
new file mode 100644
index 0000000..3eb4515
--- /dev/null
+++ b/java/com/google/gerrit/server/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.server.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().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/server/logging/JsonLogEntry.java b/java/com/google/gerrit/server/logging/JsonLogEntry.java
new file mode 100644
index 0000000..bc16c70
--- /dev/null
+++ b/java/com/google/gerrit/server/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.server.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/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index 934a0a0..2530d7e 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.asUser(args.anonymousUser);
} else {
qb = args.queryBuilder.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 ce88f07..ee3ccd6 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -184,6 +184,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 +232,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 b55ce49..05fdee9 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -287,6 +287,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/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/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index a525e29..e0c5927 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -501,12 +501,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 f6aba34..a3b4126 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -239,7 +239,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(
@@ -266,13 +266,12 @@
List<ChangeData> queryResult =
retryHelper.execute(
ActionType.INDEX_QUERY,
- () -> {
- // Execute the query.
- return changeQueryProvider
- .get()
- .setRequestedFields(ChangeField.CHANGE, ChangeField.PATCH_SET)
- .query(and(basePredicate, or(predicates)));
- },
+ () ->
+ // Execute the query.
+ changeQueryProvider
+ .get()
+ .setRequestedFields(ChangeField.CHANGE, ChangeField.PATCH_SET)
+ .query(and(basePredicate, or(predicates))),
StorageException.class::isInstance);
// Result for this query that we want to return to the client.
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 d2fc77d..cdf1243 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -484,7 +484,7 @@
return ChangeStatusPredicate.parse(statusName);
}
- public Predicate<ChangeData> status_open() {
+ public Predicate<ChangeData> statusOpen() {
return ChangeStatusPredicate.open();
}
@@ -533,7 +533,7 @@
}
if ("visible".equalsIgnoreCase(value)) {
- return is_visible();
+ return isVisible();
}
if ("reviewed".equalsIgnoreCase(value)) {
@@ -942,7 +942,7 @@
public Predicate<ChangeData> visibleto(String who)
throws QueryParseException, IOException, ConfigInvalidException {
if (isSelf(who)) {
- return is_visible();
+ return isVisible();
}
try {
return Predicate.or(
@@ -977,7 +977,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 82b445f..0b51bf8 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
@@ -70,7 +70,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/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java
index a9b3aa0..39c5a3b 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDiff.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java
@@ -253,7 +253,7 @@
throw new IllegalStateException("unknown change type: " + ps.getChangeType());
}
- if (ps.getPatchHeader().size() > 0) {
+ if (!ps.getPatchHeader().isEmpty()) {
result.diffHeader = ps.getPatchHeader();
}
result.content = content.lines;
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/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 899ed00..d583b8e 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;
@@ -235,7 +235,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/server/update/RetryingRestCollectionModifyView.java b/java/com/google/gerrit/server/update/RetryingRestCollectionModifyView.java
index 96c2ed3..bce1209 100644
--- a/java/com/google/gerrit/server/update/RetryingRestCollectionModifyView.java
+++ b/java/com/google/gerrit/server/update/RetryingRestCollectionModifyView.java
@@ -45,7 +45,7 @@
.onAutoTrace(traceId::set)
.build();
return retryHelper
- .execute((updateFactory) -> applyImpl(updateFactory, parentResource, input), retryOptions)
+ .execute(updateFactory -> applyImpl(updateFactory, parentResource, input), retryOptions)
.traceId(traceId.get());
} catch (Exception e) {
Throwables.throwIfInstanceOf(e, RestApiException.class);
diff --git a/java/com/google/gerrit/server/update/RetryingRestModifyView.java b/java/com/google/gerrit/server/update/RetryingRestModifyView.java
index 275dc55..56c3eec 100644
--- a/java/com/google/gerrit/server/update/RetryingRestModifyView.java
+++ b/java/com/google/gerrit/server/update/RetryingRestModifyView.java
@@ -40,7 +40,7 @@
.onAutoTrace(traceId::set)
.build();
return retryHelper
- .execute((updateFactory) -> applyImpl(updateFactory, resource, input), retryOptions)
+ .execute(updateFactory -> applyImpl(updateFactory, resource, input), retryOptions)
.traceId(traceId.get());
} catch (Exception e) {
Throwables.throwIfInstanceOf(e, RestApiException.class);
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..8495ed1
--- /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.server.logging.JsonLayout;
+import com.google.gerrit.server.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 5e2fbea..c610d07 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -44,6 +44,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..2510602
--- /dev/null
+++ b/java/com/google/gerrit/testing/TestLoggingActivator.java
@@ -0,0 +1,94 @@
+// 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)
+ .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/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index b1194b1..7ac803e 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -29,6 +29,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
@@ -80,6 +81,7 @@
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;
@@ -385,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) {
@@ -1110,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/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/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index 49371f4..1445707 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -6,7 +6,6 @@
srcs = glob(["**/*.java"]),
deps = [
"//java/com/google/gerrit/pgm",
- "//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/pgm/init",
"//java/com/google/gerrit/pgm/init/api",
"//java/com/google/gerrit/server",
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 aa305b2..32104b7 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -130,8 +130,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;
@@ -143,9 +141,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;
@@ -208,20 +204,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();
@@ -443,7 +425,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);
@@ -941,11 +923,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());
@@ -1046,11 +1028,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);
@@ -1091,7 +1073,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);
@@ -1788,25 +1770,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);
}
@@ -3085,12 +3069,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)
@@ -3104,21 +3088,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(
@@ -3127,7 +3115,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());
@@ -3145,7 +3134,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 c122a9e..d0162d3 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -32,9 +32,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/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 40a02a3..7f97fec 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -137,11 +137,13 @@
align-items: center;
display: flex;
}
- .diffModeSelector {
+ .diffModeSelector,
+ .editButton {
align-items: center;
display: flex;
}
- .diffModeSelector span {
+ .diffModeSelector span,
+ .editButton span {
margin-right: var(--spacing-xs);
}
.diffModeSelector.hide,
@@ -153,6 +155,9 @@
text-transform: none;
}
}
+ .editButtona a {
+ text-decoration: none;
+ }
@media screen and (max-width: 50em) {
header {
padding: var(--spacing-s) var(--spacing-l);
@@ -284,6 +289,14 @@
on-click="_toggleBlame">[[_computeBlameToggleLabel(_isBlameLoaded, _isBlameLoading)]]</gr-button>
<span class="separator"></span>
</span>
+ <template is="dom-if" if="[[_computeIsLoggedIn(_loggedIn)]]">
+ <span class="editButton">
+ <a href$="[[_computeEditURL(_change, _patchRange, _path)]]">
+ <iron-icon icon="gr-icons:edit"></iron-icon>
+ </a>
+ <span class="separator"></span>
+ </span>
+ </template>
<div class$="diffModeSelector [[_computeModeSelectHideClass(_isImageDiff)]]">
<span>Diff view:</span>
<gr-diff-mode-selector
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index c64dd5c..164513d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -566,6 +566,14 @@
return this._getDiffUrl(this._change, this._patchRange, newPath.path);
},
+ _computeEditURL(change, patchRange, path) {
+ if ([change, patchRange, path].some(arg => arg === undefined)) {
+ return '';
+ }
+ return Gerrit.Nav.getEditUrlForDiff(
+ change, path, patchRange.patchNum);
+ },
+
/**
* Gives an object representing the target of navigating either left or
* right through the change. The resulting object will have one of the
@@ -1171,5 +1179,9 @@
_handleReloadingDiffPreference() {
this._getDiffPreferences();
},
+
+ _computeIsLoggedIn(loggedIn) {
+ return loggedIn ? true : false;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
index 75c0201..ee41103 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.js
@@ -85,13 +85,34 @@
if (newContent.length) {
this.$.storage.setEditableContentItem(this.storageKey, newContent);
} else {
+ // This does not really happen, because we don't clear newContent
+ // after saving (see below). So this only occurs when the user clears
+ // all the content in the editable textarea. But <gr-storage> cleans
+ // up itself after one day, so we are not so concerned about leaving
+ // some garbage behind.
this.$.storage.eraseEditableContentItem(this.storageKey);
}
}, STORAGE_DEBOUNCE_INTERVAL_MS);
},
_editingChanged(editing) {
- if (!editing) { return; }
+ // This method is for initializing _newContent when you start editing.
+ // Restoring content from local storage is not perfect and has
+ // some issues:
+ //
+ // 1. When you start editing in multiple tabs, then we are vulnerable to
+ // race conditions between the tabs.
+ // 2. The stored content is keyed by revision, so when you upload a new
+ // patchset and click "reload" and then click "cancel" on the content-
+ // editable, then you won't be able to recover the content anymore.
+ //
+ // Because of these issues we believe that it is better to only recover
+ // content from local storage when you enter editing mode for the first
+ // time. Otherwise it is better to just keep the last editing state from
+ // the same session.
+ if (!editing || this._newContent) {
+ return;
+ }
let content;
if (this.storageKey) {
@@ -126,12 +147,15 @@
return true;
}
- return disabled || (content === newContent);
+ return disabled || !newContent || content === newContent;
},
_handleSave(e) {
e.preventDefault();
this.fire('editable-content-save', {content: this._newContent});
+ // It would be nice, if we would set this._newContent = undefined here,
+ // but we can only do that when we are sure that the save operation has
+ // succeeded.
},
_handleCancel(e) {
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.html b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.html
index 24db69f..ee5adb0 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.html
@@ -65,11 +65,11 @@
MockInteractions.tap(element.$$('gr-button:not([primary])'));
});
- test('enabling editing updates edit field contents', () => {
+ test('enabling editing keeps old content', () => {
element.content = 'current content';
- element._newContent = 'stale content';
+ element._newContent = 'old content';
element.editing = true;
- assert.equal(element._newContent, 'current content');
+ assert.equal(element._newContent, 'old content');
});
test('disabling editing does not update edit field contents', () => {
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 6eb9f46..48a615a 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
@@ -69,10 +69,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 e60dd42..99f4f7e 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
@@ -144,7 +144,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);
@@ -162,7 +162,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);
@@ -203,7 +203,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/tools/bzl/plugin.bzl b/tools/bzl/plugin.bzl
index ed64d1b..3c6d597 100644
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -49,16 +49,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"],
