Merge changes I7eb02d9e,Ifb1fa558
* changes:
Switch FileInfoJson to use new diff cache by default
PatchScriptFactory: Remove usage of old diff cache
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 8f36cfb..0bbf4fb 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -12,35 +12,112 @@
* `build/label`: Version of Gerrit server software.
* `events`: Triggered events.
+** `type`:
+ The type of the event.
=== Actions
* `action/retry_attempt_count`: Number of retry attempts made
-by RetryHelper to execute an action (0 == single attempt, no retry)
+ by RetryHelper to execute an action (0 == single attempt, no retry)
+** `action_type`:
+ The type of the action that was retried.
+** `operation_name`:
+ The name of the operation that was retried.
+** `cause`:
+ The original cause that triggered the retry.
* `action/retry_timeout_count`: Number of action executions of RetryHelper
-that ultimately timed out
+ that ultimately timed out
+** `action_type`:
+ The type of the action that was retried.
+** `operation_name`:
+ The name of the operation that was retried.
+** `cause`:
+ The original cause that triggered the retry.
* `action/auto_retry_count`: Number of automatic retries with tracing
+** `action_type`:
+ The type of the action that was retried.
+** `operation_name`:
+ The name of the operation that was retried.
+** `cause`:
+ The cause for the retry.
* `action/failures_on_auto_retry_count`: Number of failures on auto retry
+** `action_type`:
+ The type of the action that was retried.
+** `operation_name`:
+ The name of the operation that was retried.
+** `cause`:
+ The cause for the retry.
[[cancellations]]
=== Cancellations
* `cancellation/advisory_deadline_count`: Exceeded advisory deadlines by request
+** `request_type`:
+ The type of the request to which the advisory deadline applied.
+** `request_uri`:
+ The redacted URI of the request to which the advisory deadline applied (only
+ set for request_type = REST).
+** `deadline_id`:
+ The ID of the advisory deadline.
* `cancellation/cancelled_requests_count`: Number of request cancellations by
request
+** `request_type`:
+ The type of the request that was cancelled.
+** `request_uri`:
+ The redacted URI of the request that was cancelled (only set for
+ request_type = REST).
+** `cancellation_reason`:
+ The reason why the request was cancelled.
* `cancellation/receive_timeout_count`: Number of requests that are cancelled
because link:config.html#receive.timeout[receive.timout] is exceeded
+** `cancellation_type`:
+ The cancellation type (graceful or forceful).
+
+[[performance]]
+=== Performance
+
+* `performance/operations`: Latency of performing operations
+** `operation_name`:
+ The operation that was performed.
+** `change_identifier`:
+ The ID of the change for which the operation was performed (format =
+ '<project>~<numeric-change-id>').
+** `trace_id`:
+ The ID of the trace if tracing was done.
+* `performance/operations_count`: Number of performed operations
+** `operation_name`:
+ The operation that was performed.
+** `trace_id`:
+ The ID of the trace if tracing was done.
+** `request`:
+ The request for which the operation was performed (format = '<request-type>
+ <redacted-request-uri>').
+* `performance/plugin_operations_count`: Number of performed operations by
+ plugin
+** `operation_name`:
+ The operation that was performed.
+** `plugin`:
+ The name of the plugin that performed the operation.
+** `trace_id`:
+ The ID of the trace if tracing was done.
+
=== Pushes
-* `receivecommits/changes`: histogram of number of changes processed
-in a single upload, split up by update type (change created/updated,
-change autoclosed).
-* `receivecommits/latency`: latency per change for processing a push,
-split up by update type (create+replace, and autoclose)
-* `receivecommits/push_latency`: total latency for processing a push,
-split up by update type (create+replace, autoclose, normal)
-* `receivecommits/timeout`: number of timeouts during push processing.
+* `receivecommits/changes`: histogram of number of changes processed in a single
+ upload
+** `type`:
+ type of push (create/replace, autoclose)
+* `receivecommits/latency_per_push`: processing delay for a processing single
+ push
+** `type`:
+ type of push (create/replace, autoclose, normal)
+* `receivecommits/latency_per_push_per_change`: Processing delay per push
+ divided by the number of changes in said push. (Only includes pushes which
+ contain changes.)
+** `type`:
+ type of push (create/replace, autoclose, normal)
+* `receivecommits/timeout`: rate of push timeouts
=== Process
@@ -58,27 +135,58 @@
* `proc/jvm/memory/object_pending_finalization_count`: Approximate number of
objects needing finalization.
* `proc/jvm/gc/count`: Number of GCs.
+** `gc_name`:
+ The name of the garbage collector.
* `proc/jvm/gc/time`: Approximate accumulated GC elapsed time.
-* `proc/jvm/memory/pool/committed/<pool name>`: Committed amount of memory for pool.
-* `proc/jvm/memory/pool/max/<pool name>`: Maximum amount of memory for pool.
-* `proc/jvm/memory/pool/used/<pool name>`: Used amount of memory for pool.
+** `gc_name`:
+ The name of the garbage collector.
+* `proc/jvm/memory/pool/committed`: Committed amount of memory for pool.
+** `pool_name`:
+ The name of the memory pool.
+* `proc/jvm/memory/pool/max`: Maximum amount of memory for pool.
+** `pool_name`:
+ The name of the memory pool.
+* `proc/jvm/memory/pool/used`: Used amount of memory for pool.
+** `pool_name`:
+ The name of the memory pool.
* `proc/jvm/thread/num_live`: Current live thread count.
* `proc/jvm/thread/num_daemon_live`: Current live daemon threads count.
-* `proc/jvm/thread/num_peak_live`: Peak live thread count since the Java virtual machine started or peak was reset.
-* `proc/jvm/thread/num_total_started`: Total number of threads created and also started since the Java virtual machine started.
-* `proc/jvm/thread/num_deadlocked_threads`: Number of threads that are deadlocked waiting for object monitors or ownable synchronizers.
- If deadlocks waiting for ownable synchronizers can be monitored depends on the capabilities of the used JVM.
+* `proc/jvm/thread/num_peak_live`: Peak live thread count since the Java virtual
+ machine started or peak was reset.
+* `proc/jvm/thread/num_total_started`: Total number of threads created and also
+ started since the Java virtual machine started.
+* `proc/jvm/thread/num_deadlocked_threads`: Number of threads that are
+ deadlocked waiting for object monitors or ownable synchronizers.
+ If deadlocks waiting for ownable synchronizers can be monitored depends on the
+ capabilities of the used JVM.
=== Caches
* `caches/memory_cached`: Memory entries.
+** `cache_name`:
+ The name of the cache.
* `caches/memory_hit_ratio`: Memory hit ratio.
+** `cache_name`:
+ The name of the cache.
* `caches/memory_eviction_count`: Memory eviction count.
+** `cache_name`:
+ The name of the cache.
* `caches/disk_cached`: Disk entries used by persistent cache.
+** `cache_name`:
+ The name of the cache.
* `caches/disk_hit_ratio`: Disk hit ratio for persistent cache.
-* `caches/refresh_count`: The number of refreshes per cache with an indicator if a reload was necessary.
-* `caches/diff/timeouts`: The number of git file diff computations that resulted in timeouts.
-* `caches/diff/legacy/timeouts`: The number of git file diff computations (using the legacy cache) that resulted in timeouts.
+** `cache_name`:
+ The name of the cache.
+* `caches/refresh_count`: The number of refreshes per cache with an indicator if
+ a reload was necessary.
+** `cache`:
+ The name of the cache.
+** `outdated`:
+ Whether the cache entry was outdated on reload.
+* `caches/diff/timeouts`: The number of git file diff computations that resulted
+ in timeouts.
+* `caches/diff/legacy/timeouts`: The number of git file diff computations (using
+ the legacy cache) that resulted in timeouts.
Cache disk metrics are expensive to compute on larger installations and are not
computed by default. They can be enabled via the
@@ -87,65 +195,110 @@
=== Change
-* `change/submit_rule_evaluation`: Latency for evaluating submit rules on a change.
-* `change/submit_type_evaluation`: Latency for evaluating the submit type on a change.
+* `change/submit_rule_evaluation`: Latency for evaluating submit rules on a
+ change.
+* `change/submit_type_evaluation`: Latency for evaluating the submit type on a
+ change.
+* `change/post_review/draft_handling`: Total number of draft handling option
+ (KEEP, PUBLISH, PUBLISH_ALL_REVISIONS) selected by users while posting a
+ review.
+** `type`:
+ The type of the draft handling option (KEEP, PUBLISH, PUBLISH_ALL_REVISIONS).
=== Comments
-* `ported_comments/as_patchset_level`: Total number of comments ported as patchset-level comments.
-* `ported_comments/as_file_level`: Total number of comments ported as file-level comments.
-* `ported_comments/as_range_comments`: Total number of comments having line/range values in the ported patchset.
+* `ported_comments/as_patchset_level`: Total number of comments ported as
+ patchset-level comments.
+* `ported_comments/as_file_level`: Total number of comments ported as file-level
+ comments.
+* `ported_comments/as_range_comments`: Total number of comments having
+ line/range values in the ported patchset.
=== HTTP
==== 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/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
* `http/server/jetty/threadpool/max_pool_size`: Maximum thread pool size
* `http/server/jetty/threadpool/min_pool_size`: Minimum thread pool size
* `http/server/jetty/threadpool/pool_size`: Current thread pool size
-* `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a thread
+* `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a
+ thread
+* `http/server/jetty/threadpool/is_low_on_threads`: Whether thread pool is low
+ on threads
==== LDAP
* `ldap/login_latency`: Latency of logins.
* `ldap/user_search_latency`: Latency for searching the user account.
-* `ldap/group_search_latency`: Latency for querying the group memberships of an account.
+* `ldap/group_search_latency`: Latency for querying the group memberships of an
+ account.
* `ldap/group_expansion_latency`: Latency for expanding nested groups.
==== REST API
* `http/server/error_count`: Rate of REST API error responses.
+** `status`:
+ HTTP status code
* `http/server/success_count`: Rate of REST API success responses.
+** `status`:
+ HTTP status code
* `http/server/rest_api/count`: Rate of REST API calls by view.
+** `view`:
+ view implementation class
* `http/server/rest_api/change_id_type`: Rate of REST API calls by change ID type.
+** `change_id_type`:
+ The type of the change identifier.
* `http/server/rest_api/error_count`: Rate of REST API calls by view.
+** `view`:
+ view implementation class
+** `error_code`:
+ HTTP status code
+** `cause`:
+ The cause of the error.
* `http/server/rest_api/server_latency`: REST API call latency by view.
+** `view`:
+ view implementation class
* `http/server/rest_api/response_bytes`: Size of REST API response on network
-(may be gzip compressed) by view.
+ (may be gzip compressed) by view.
+** `view`:
+ view implementation class
* `http/server/rest_api/change_json/to_change_info_latency`: Latency for
-toChangeInfo invocations in ChangeJson.
+ toChangeInfo invocations in ChangeJson.
* `http/server/rest_api/change_json/to_change_infos_latency`: Latency for
-toChangeInfos invocations in ChangeJson.
+ toChangeInfos invocations in ChangeJson.
* `http/server/rest_api/change_json/format_query_results_latency`: Latency for
-formatQueryResults invocations in ChangeJson.
-* `http/server/rest_api/ui_actions/latency`: Latency for RestView#getDescription calls.
+ formatQueryResults invocations in ChangeJson.
+* `http/server/rest_api/ui_actions/latency`: Latency for RestView#getDescription
+ calls.
+** `view`:
+ view implementation class
=== Query
* `query/query_latency`: Successful query latency, accumulated over the life
-of the process.
+ of the process.
+** `index`: index name
=== Core Queues
@@ -164,11 +317,15 @@
Each queue provides the following metrics:
* `queue/<queue_name>/pool_size`: Current number of threads in the pool
-* `queue/<queue_name>/max_pool_size`: Maximum allowed number of threads in the pool
-* `queue/<queue_name>/active_threads`: Number of threads that are actively executing tasks
+* `queue/<queue_name>/max_pool_size`: Maximum allowed number of threads in the
+ pool
+* `queue/<queue_name>/active_threads`: Number of threads that are actively
+ executing tasks
* `queue/<queue_name>/scheduled_tasks`: Number of scheduled tasks in the queue
-* `queue/<queue_name>/total_scheduled_tasks_count`: Total number of tasks that have been scheduled
-* `queue/<queue_name>/total_completed_tasks_count`: Total number of tasks that have completed execution
+* `queue/<queue_name>/total_scheduled_tasks_count`: Total number of tasks that
+ have been scheduled
+* `queue/<queue_name>/total_completed_tasks_count`: Total number of tasks that
+ have completed execution
=== SSH sessions
@@ -180,7 +337,7 @@
* `topic/cross_project_submit`: number of cross-project topic submissions.
* `topic/cross_project_submit_completed`: number of cross-project
-topic submissions that concluded successfully.
+ topic submissions that concluded successfully.
=== JGit
@@ -197,23 +354,34 @@
* `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.
+* `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.
+** `repository_name`: The name of the repository.
=== Git
* `git/upload-pack/request_count`: Total number of git-upload-pack requests.
+** `operation`:
+ The name of the operation (CLONE, FETCH).
* `git/upload-pack/phase_counting`: Time spent in the 'Counting...' phase.
+** `operation`:
+ The name of the operation (CLONE, FETCH).
* `git/upload-pack/phase_compressing`: Time spent in the 'Compressing...' phase.
+** `operation`:
+ The name of the operation (CLONE, FETCH).
* `git/upload-pack/phase_writing`: Time spent transferring bytes to client.
+** `operation`:
+ The name of the operation (CLONE, FETCH).
* `git/upload-pack/pack_bytes`: Distribution of sizes of packs sent to clients.
+** `operation`:
+ The name of the operation (CLONE, FETCH).
* `git/auto-merge/num_operations`: Number of auto merge operations and context.
+** `operation`:
+ The type of the operation (CACHE_LOAD, IN_MEMORY_WRITE, ON_DISK_WRITE).
* `git/auto-merge/latency`: Latency of auto merge operations and context.
-
-=== BatchUpdate
-
-* `batch_update/execute_change_ops`: BatchUpdate change update latency,
-excluding reindexing
+** `operation`:
+ The type of the operation (CACHE_LOAD, IN_MEMORY_WRITE, ON_DISK_WRITE).
=== NoteDb
@@ -223,43 +391,63 @@
* `notedb/parse_latency`: NoteDb parse latency for changes.
* `notedb/external_id_cache_load_count`: Total number of times the external ID
cache loader was called.
-* `notedb/external_id_partial_read_latency`: Latency for generating a new external ID
- cache state from a prior state.
+** `partial`:
+ Whether the reload was partial.
+* `notedb/external_id_partial_read_latency`: Latency for generating a new
+ external ID cache state from a prior state.
* `notedb/external_id_update_count`: Total number of external ID updates.
* `notedb/read_all_external_ids_latency`: Latency for reading all
-external ID's from NoteDb.
+ external ID's from NoteDb.
* `notedb/read_single_account_config_latency`: Latency for reading a single
-account config from NoteDb.
+ account config from NoteDb.
* `notedb/read_single_external_id_latency`: Latency for reading a single
-external ID from NoteDb.
+ external ID from NoteDb.
=== Permissions
-* `permissions/permission_collection/filter_latency`: Latency to filter access sections
-by user and ref.
+* `permissions/permission_collection/filter_latency`: Latency for access filter
+ computations in PermissionCollection
* `permissions/ref_filter/full_filter_count`: Rate of full ref filter operations
-* `permissions/ref_filter/skip_filter_count`: Rate of ref filter operations where
-we skip full evaluation because the user can read all refs
+* `permissions/ref_filter/skip_filter_count`: Rate of ref filter operations
+ where we skip full evaluation because the user can read all refs
=== Reviewer Suggestion
* `reviewer_suggestion/query_accounts`: Latency for querying accounts for
-reviewer suggestion.
+ reviewer suggestion.
* `reviewer_suggestion/recommend_accounts`: Latency for recommending accounts
-for reviewer suggestion.
+ for reviewer suggestion.
* `reviewer_suggestion/load_accounts`: Latency for loading accounts for
-reviewer suggestion.
+ reviewer suggestion.
* `reviewer_suggestion/query_groups`: Latency for querying groups for reviewer
-suggestion.
+ suggestion.
+* `reviewer_suggestion/filter_visibility`: Latency for removing users that can't
+ see the change
=== Repo Sequences
* `sequence/next_id_latency`: Latency of requesting IDs from repo sequences.
+** `sequence`:
+ The sequence from which IDs were retrieved.
+** `multiple`:
+ Whether more than one ID was retrieved.
=== Plugin
* `plugin/latency`: Latency for plugin invocation.
+** `plugin_name`"
+ The name of the plugin.
+** `class`:
+ The class of the plugin that was invoked.
+** `export_value`:
+ The export name under which the invoked class is registered.
* `plugin/error_count`: Number of plugin errors.
+** `plugin_name`"
+ The name of the plugin.
+** `class`:
+ The class of the plugin that was invoked.
+** `export_value`:
+ The export name under which the invoked class is registered.
=== Group
@@ -268,11 +456,19 @@
=== Replication Plugin
* `plugins/replication/replication_latency`: Time spent pushing to remote
-destination.
+ destination.
+** `destination`: The destination of the replication.
* `plugins/replication/replication_delay`: Time spent waiting before pushing to
-remote destination.
+ remote destination.
+** `destination`: The destination of the replication.
* `plugins/replication/replication_retries`: Number of retries when pushing to
-remote destination.
+ remote destination.
+** `destination`: The destination of the replication.
+* `plugins/replication/latency_slower_than_threshold`: latency for project to
+ destination, where latency was slower than threshold
+** `slow_threshold`: The threshold.
+** `project`: The name of the project.
+** `destination`: The destination of the replication.
=== License
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 2ebe6bd..444c9ee 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -427,6 +427,11 @@
+
True if the change has unresolved comments.
+has:attention::
++
+True if the change has attention by the current user.
+
+
[[is]]
is:assigned::
+
@@ -442,6 +447,10 @@
+
True if the change does not have an assignee.
+is:attention::
++
+True if the change has attention by the current user.
+
is:watched::
+
True if this change matches one of the current user's watch filters,
diff --git a/java/com/google/gerrit/entities/Address.java b/java/com/google/gerrit/entities/Address.java
index 2324330..5d63476 100644
--- a/java/com/google/gerrit/entities/Address.java
+++ b/java/com/google/gerrit/entities/Address.java
@@ -15,6 +15,7 @@
package com.google.gerrit.entities;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.gerrit.common.Nullable;
/** Represents an address (name + email) in an email message. */
@@ -66,8 +67,9 @@
public abstract String email();
+ @Memoized
@Override
- public final int hashCode() {
+ public int hashCode() {
return email().hashCode();
}
diff --git a/java/com/google/gerrit/entities/GroupReference.java b/java/com/google/gerrit/entities/GroupReference.java
index 208ba0f..125153e 100644
--- a/java/com/google/gerrit/entities/GroupReference.java
+++ b/java/com/google/gerrit/entities/GroupReference.java
@@ -17,6 +17,7 @@
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.gerrit.common.Nullable;
/** Describes a group within a projects {@link AccessSection}s. */
@@ -78,8 +79,9 @@
return "?";
}
+ @Memoized
@Override
- public final int hashCode() {
+ public int hashCode() {
return uuid(this).hashCode();
}
diff --git a/java/com/google/gerrit/entities/NotifyConfig.java b/java/com/google/gerrit/entities/NotifyConfig.java
index 17da81f..5c0a3db 100644
--- a/java/com/google/gerrit/entities/NotifyConfig.java
+++ b/java/com/google/gerrit/entities/NotifyConfig.java
@@ -15,6 +15,7 @@
package com.google.gerrit.entities;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
@@ -106,8 +107,9 @@
return getName().compareTo(o.getName());
}
+ @Memoized
@Override
- public final int hashCode() {
+ public int hashCode() {
return getName().hashCode();
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 3c39ea1..91659f8 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -768,13 +768,12 @@
}
} else {
res.reset();
- traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+ TraceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
if (status.isPresent()) {
- responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
+ responseBytes = reply(req, res, e, status.get(), getUserMessages(e));
} else {
- responseBytes =
- replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
+ responseBytes = replyInternalServerError(req, res, e, getUserMessages(e));
}
}
}
@@ -984,7 +983,7 @@
throws Exception {
RetryableAction<T> retryableAction = globals.retryHelper.action(actionType, caller, action);
AtomicReference<Optional<String>> traceId = new AtomicReference<>(Optional.empty());
- if (!traceContext.isTracing()) {
+ if (!TraceContext.isTracing()) {
// enable automatic retry with tracing in case of non-recoverable failure
retryableAction
.retryWithTrace(t -> !(t instanceof RestApiException))
@@ -1876,9 +1875,9 @@
.findFirst();
}
- private ImmutableList<String> getUserMessages(TraceContext traceContext, Throwable err) {
+ private ImmutableList<String> getUserMessages(Throwable err) {
return globals.exceptionHooks.stream()
- .flatMap(h -> h.getUserMessages(err, traceContext.getTraceId().orElse(null)).stream())
+ .flatMap(h -> h.getUserMessages(err, TraceContext.getTraceId().orElse(null)).stream())
.collect(toImmutableList());
}
diff --git a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
index e8611b3..d64bd19 100644
--- a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
+++ b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
@@ -184,7 +184,9 @@
+ "having most data in the cache.")
.setGauge()
.setUnit("byte"),
- Field.ofString("repository_name", Metadata.Builder::projectName).build());
+ Field.ofString("repository_name", Metadata.Builder::projectName)
+ .description("The name of the repository.")
+ .build());
metrics.newTrigger(
repoEnt,
() -> {
diff --git a/java/com/google/gerrit/pgm/init/InitAuth.java b/java/com/google/gerrit/pgm/init/InitAuth.java
index c15cff3..948ec49 100644
--- a/java/com/google/gerrit/pgm/init/InitAuth.java
+++ b/java/com/google/gerrit/pgm/init/InitAuth.java
@@ -26,6 +26,7 @@
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.Section;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.mail.SignedToken;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -42,11 +43,13 @@
private final Section ldap;
private final Section receive;
private final InitFlags flags;
+ private final SitePaths site;
@Inject
- InitAuth(InitFlags flags, ConsoleUI ui, Section.Factory sections) {
+ InitAuth(InitFlags flags, ConsoleUI ui, final SitePaths site, Section.Factory sections) {
this.flags = flags;
this.ui = ui;
+ this.site = site;
this.auth = sections.get("auth", null);
this.ldap = sections.get("ldap", null);
this.receive = sections.get(RECEIVE, null);
@@ -62,6 +65,10 @@
}
initSignedPush();
+
+ if (site.isNew) {
+ initUserNameCaseSensitivity();
+ }
}
private void initAuthType() {
@@ -156,4 +163,9 @@
boolean enable = ui.yesno(def, "Enable signed push support");
receive.set("enableSignedPush", Boolean.toString(enable));
}
+
+ private void initUserNameCaseSensitivity() {
+ boolean enableCaseInsensitivity = ui.yesno(true, "Use case insensitive usernames");
+ auth.set("userNameCaseInsensitive", Boolean.toString(enableCaseInsensitivity));
+ }
}
diff --git a/java/com/google/gerrit/server/CancellationMetrics.java b/java/com/google/gerrit/server/CancellationMetrics.java
index 2d0b878..f534ccb 100644
--- a/java/com/google/gerrit/server/CancellationMetrics.java
+++ b/java/com/google/gerrit/server/CancellationMetrics.java
@@ -14,11 +14,6 @@
package com.google.gerrit.server;
-import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.metrics.Counter1;
import com.google.gerrit.metrics.Counter3;
@@ -48,7 +43,7 @@
.build(),
Field.ofString("request_uri", Metadata.Builder::restViewName)
.description(
- "The URI of the request to which the advisory deadline applied"
+ "The redacted URI of the request to which the advisory deadline applied"
+ " (only set for request_type = REST).")
.build(),
Field.ofString("deadline_id", (metadataBuilder, resolveAllUsers) -> {})
@@ -64,7 +59,7 @@
.build(),
Field.ofString("request_uri", Metadata.Builder::restViewName)
.description(
- "The URI of the request that was cancelled"
+ "The redacted URI of the request that was cancelled"
+ " (only set for request_type = REST).")
.build(),
Field.ofEnum(
@@ -87,17 +82,13 @@
public void countAdvisoryDeadline(RequestInfo requestInfo, String deadlineId) {
advisoryDeadlineCount.increment(
- requestInfo.requestType(),
- requestInfo.requestUri().map(CancellationMetrics::redactRequestUri).orElse(""),
- deadlineId);
+ requestInfo.requestType(), requestInfo.redactedRequestUri().orElse(""), deadlineId);
}
public void countCancelledRequest(
RequestInfo requestInfo, RequestStateProvider.Reason cancellationReason) {
cancelledRequestsCount.increment(
- requestInfo.requestType(),
- requestInfo.requestUri().map(CancellationMetrics::redactRequestUri).orElse(""),
- cancellationReason);
+ requestInfo.requestType(), requestInfo.redactedRequestUri().orElse(""), cancellationReason);
}
public void countCancelledRequest(
@@ -105,7 +96,7 @@
String requestUri,
RequestStateProvider.Reason cancellationReason) {
cancelledRequestsCount.increment(
- requestType.name(), CancellationMetrics.redactRequestUri(requestUri), cancellationReason);
+ requestType.name(), RequestInfo.redactRequestUri(requestUri), cancellationReason);
}
@UsedAt(UsedAt.Project.GOOGLE)
@@ -123,58 +114,4 @@
public void countForcefulReceiveTimeout() {
receiveTimeoutCount.increment("forceful");
}
-
- /**
- * Redacts resource IDs from the given request URI.
- *
- * <p>resource IDs in the request URI are replaced with '*'.
- *
- * @param requestUri a REST URI that has path segments that alternate between view name and
- * resource IDs (e.g. "/<view>", "/<view>/<id>", "/<view>/<id>/<view>",
- * "/<view>/<id>/<view>/<id>", "/<view>/<id>/<view>/<id>/<view>" etc.), must be given without
- * the '/a' prefix
- * @return the redacted request URI
- */
- @VisibleForTesting
- static String redactRequestUri(String requestUri) {
- requireNonNull(requestUri, "requestUri");
- checkState(
- !requestUri.startsWith("/a/"), "request URI must not start with '/a/': %s", requestUri);
-
- StringBuilder redactedRequestUri = new StringBuilder();
-
- boolean hasLeadingSlash = false;
- boolean hasTrailingSlash = false;
- if (requestUri.startsWith("/")) {
- hasLeadingSlash = true;
- requestUri = requestUri.substring(1);
- }
- if (requestUri.endsWith("/")) {
- hasTrailingSlash = true;
- requestUri = requestUri.substring(0, requestUri.length() - 1);
- }
-
- boolean idPathSegment = false;
- for (String pathSegment : Splitter.on('/').split(requestUri)) {
- if (!idPathSegment) {
- redactedRequestUri.append("/" + pathSegment);
- idPathSegment = true;
- } else {
- redactedRequestUri.append("/");
- if (!pathSegment.isEmpty()) {
- redactedRequestUri.append("*");
- }
- idPathSegment = false;
- }
- }
-
- if (!hasLeadingSlash) {
- redactedRequestUri.deleteCharAt(0);
- }
- if (hasTrailingSlash) {
- redactedRequestUri.append('/');
- }
-
- return redactedRequestUri.toString();
- }
}
diff --git a/java/com/google/gerrit/server/DeadlineChecker.java b/java/com/google/gerrit/server/DeadlineChecker.java
index 5662e50..f41b1e3 100644
--- a/java/com/google/gerrit/server/DeadlineChecker.java
+++ b/java/com/google/gerrit/server/DeadlineChecker.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.cancellation.RequestStateProvider;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.util.HashSet;
@@ -134,7 +135,7 @@
this(
serverConfig,
cancellationsMetrics,
- System.nanoTime(),
+ TimeUtil.nowNanos(),
requestInfo,
clientProvidedTimeoutValue);
}
@@ -236,7 +237,7 @@
@Override
public void checkIfCancelled(OnCancelled onCancelled) {
- long now = System.nanoTime();
+ long now = TimeUtil.nowNanos();
Set<String> exceededAdvisoryDeadlines = new HashSet<>();
advisoryDeadlines
diff --git a/java/com/google/gerrit/server/PerformanceMetrics.java b/java/com/google/gerrit/server/PerformanceMetrics.java
new file mode 100644
index 0000000..85452ce
--- /dev/null
+++ b/java/com/google/gerrit/server/PerformanceMetrics.java
@@ -0,0 +1,137 @@
+// Copyright (C) 2021 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;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.metrics.Counter3;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer3;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.server.logging.PerformanceLogger;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.concurrent.TimeUnit;
+
+/** Performance logger that records the execution times as a metric. */
+@Singleton
+public class PerformanceMetrics implements PerformanceLogger {
+ private static final String OPERATION_LATENCY_METRIC_NAME = "performance/operations";
+ private static final String OPERATION_COUNT_METRIC_NAME = "performance/operations_count";
+ private static final String PLUGIN_OPERATION_COUNT_METRIC_NAME =
+ "performance/plugin_operations_count";
+
+ public final Timer3<String, String, String> operationsLatency;
+ public final Counter3<String, String, String> operationsCounter;
+ public final Counter3<String, String, String> pluginOperationsCounter;
+
+ @Inject
+ PerformanceMetrics(MetricMaker metricMaker) {
+ Field<String> operationNameField =
+ Field.ofString(
+ "operation_name",
+ (metadataBuilder, fieldValue) -> metadataBuilder.operationName(fieldValue))
+ .description("The operation that was performed.")
+ .build();
+ Field<String> changeIdentifierField =
+ Field.ofString("change_identifier", (metadataBuilder, fieldValue) -> {})
+ .description(
+ "The ID of the change for which the operation was performed"
+ + " (format = '<project>~<numeric-change-id>').")
+ .build();
+ Field<String> traceIdField =
+ Field.ofString("trace_id", (metadataBuilder, fieldValue) -> {})
+ .description("The ID of the trace if tracing was done.")
+ .build();
+ Field<String> requestField =
+ Field.ofString("request", (metadataBuilder, fieldValue) -> {})
+ .description(
+ "The request for which the operation was performed"
+ + " (format = '<request-type> <redacted-request-uri>').")
+ .build();
+ Field<String> pluginField =
+ Field.ofString(
+ "plugin", (metadataBuilder, fieldValue) -> metadataBuilder.pluginName(fieldValue))
+ .description("The name of the plugin that performed the operation.")
+ .build();
+
+ this.operationsLatency =
+ metricMaker.newTimer(
+ OPERATION_LATENCY_METRIC_NAME,
+ new Description("Latency of performing operations")
+ .setCumulative()
+ .setUnit(Description.Units.MILLISECONDS),
+ operationNameField,
+ changeIdentifierField,
+ traceIdField);
+ this.operationsCounter =
+ metricMaker.newCounter(
+ OPERATION_COUNT_METRIC_NAME,
+ new Description("Number of performed operations").setRate(),
+ operationNameField,
+ traceIdField,
+ requestField);
+ this.pluginOperationsCounter =
+ metricMaker.newCounter(
+ PLUGIN_OPERATION_COUNT_METRIC_NAME,
+ new Description("Number of performed operations by plugin").setRate(),
+ operationNameField,
+ pluginField,
+ traceIdField);
+ }
+
+ @Override
+ public void log(String operation, long durationMs) {
+ log(operation, durationMs, /* metadata= */ null);
+ }
+
+ @Override
+ public void log(String operation, long durationMs, @Nullable Metadata metadata) {
+ if (OPERATION_LATENCY_METRIC_NAME.equals(operation)) {
+ // Recording the timer metric below triggers writing a performance log entry. If we are called
+ // for this performance log entry we must abort to avoid an endless loop.
+ // In practice this should not happen since PerformanceLoggers are only called on close() of
+ // the PerformanceLogContext, and hence the performance log that gets written by the metric
+ // below gets ignored.
+ return;
+ }
+
+ String traceId = TraceContext.getTraceId().orElse("");
+
+ operationsLatency.record(
+ operation, formatChangeIdentifier(metadata), traceId, durationMs, TimeUnit.MILLISECONDS);
+
+ String requestTag = TraceContext.getTag(TraceRequestListener.TAG_REQUEST).orElse("");
+ operationsCounter.increment(operation, traceId, requestTag);
+
+ TraceContext.getPluginTag()
+ .ifPresent(pluginName -> pluginOperationsCounter.increment(operation, pluginName, traceId));
+ }
+
+ private String formatChangeIdentifier(@Nullable Metadata metadata) {
+ if (metadata == null
+ || (!metadata.projectName().isPresent() && !metadata.changeId().isPresent())) {
+ return "";
+ }
+
+ StringBuilder sb = new StringBuilder();
+ sb.append(metadata.projectName().orElse("n/a"));
+ sb.append('~');
+ sb.append(metadata.changeId().map(String::valueOf).orElse("n/a"));
+ return sb.toString();
+ }
+}
diff --git a/java/com/google/gerrit/server/RequestInfo.java b/java/com/google/gerrit/server/RequestInfo.java
index 053b3ac..791e228 100644
--- a/java/com/google/gerrit/server/RequestInfo.java
+++ b/java/com/google/gerrit/server/RequestInfo.java
@@ -14,7 +14,12 @@
package com.google.gerrit.server;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
+import com.google.common.base.Splitter;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.logging.TraceContext;
@@ -55,6 +60,16 @@
*/
public abstract Optional<String> requestUri();
+ /**
+ * Redacted request URI.
+ *
+ * <p>Request URI where resource IDs are replaced by '*'.
+ */
+ @Memoized
+ public Optional<String> redactedRequestUri() {
+ return requestUri().map(RequestInfo::redactRequestUri);
+ }
+
/** The user that has sent the request. */
public abstract CurrentUser callingUser();
@@ -68,6 +83,67 @@
*/
public abstract Optional<Project.NameKey> project();
+ @Memoized
+ public String formatForLogging() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(requestType());
+ redactedRequestUri().ifPresent(redactedRequestUri -> sb.append(' ').append(redactedRequestUri));
+ return sb.toString();
+ }
+
+ /**
+ * Redacts resource IDs from the given request URI.
+ *
+ * <p>resource IDs in the request URI are replaced with '*'.
+ *
+ * @param requestUri a REST URI that has path segments that alternate between view name and
+ * resource IDs (e.g. "/<view>", "/<view>/<id>", "/<view>/<id>/<view>",
+ * "/<view>/<id>/<view>/<id>", "/<view>/<id>/<view>/<id>/<view>" etc.), must be given without
+ * the '/a' prefix
+ * @return the redacted request URI
+ */
+ static String redactRequestUri(String requestUri) {
+ requireNonNull(requestUri, "requestUri");
+ checkState(
+ !requestUri.startsWith("/a/"), "request URI must not start with '/a/': %s", requestUri);
+
+ StringBuilder redactedRequestUri = new StringBuilder();
+
+ boolean hasLeadingSlash = false;
+ boolean hasTrailingSlash = false;
+ if (requestUri.startsWith("/")) {
+ hasLeadingSlash = true;
+ requestUri = requestUri.substring(1);
+ }
+ if (requestUri.endsWith("/")) {
+ hasTrailingSlash = true;
+ requestUri = requestUri.substring(0, requestUri.length() - 1);
+ }
+
+ boolean idPathSegment = false;
+ for (String pathSegment : Splitter.on('/').split(requestUri)) {
+ if (!idPathSegment) {
+ redactedRequestUri.append("/" + pathSegment);
+ idPathSegment = true;
+ } else {
+ redactedRequestUri.append("/");
+ if (!pathSegment.isEmpty()) {
+ redactedRequestUri.append("*");
+ }
+ idPathSegment = false;
+ }
+ }
+
+ if (!hasLeadingSlash) {
+ redactedRequestUri.deleteCharAt(0);
+ }
+ if (hasTrailingSlash) {
+ redactedRequestUri.append('/');
+ }
+
+ return redactedRequestUri.toString();
+ }
+
public static RequestInfo.Builder builder(
RequestType requestType, CurrentUser callingUser, TraceContext traceContext) {
return builder().requestType(requestType).callingUser(callingUser).traceContext(traceContext);
diff --git a/java/com/google/gerrit/server/TraceRequestListener.java b/java/com/google/gerrit/server/TraceRequestListener.java
index 7136e47..6cc0982 100644
--- a/java/com/google/gerrit/server/TraceRequestListener.java
+++ b/java/com/google/gerrit/server/TraceRequestListener.java
@@ -28,6 +28,9 @@
*/
@Singleton
public class TraceRequestListener implements RequestListener {
+ public static String TAG_REQUEST = "request";
+
+ private static String TAG_PROJECT = "project";
private static String SECTION_TRACING = "tracing";
private final ImmutableList<RequestConfig> traceConfigs;
@@ -39,7 +42,8 @@
@Override
public void onRequest(RequestInfo requestInfo) {
- requestInfo.project().ifPresent(p -> requestInfo.traceContext().addTag("project", p));
+ requestInfo.traceContext().addTag(TAG_REQUEST, requestInfo.formatForLogging());
+ requestInfo.project().ifPresent(p -> requestInfo.traceContext().addTag(TAG_PROJECT, p));
traceConfigs.stream()
.filter(traceConfig -> traceConfig.matches(requestInfo))
.forEach(
diff --git a/java/com/google/gerrit/server/account/ServiceUserClassifierImpl.java b/java/com/google/gerrit/server/account/ServiceUserClassifierImpl.java
index 27ac9f4..db030f9 100644
--- a/java/com/google/gerrit/server/account/ServiceUserClassifierImpl.java
+++ b/java/com/google/gerrit/server/account/ServiceUserClassifierImpl.java
@@ -76,7 +76,7 @@
.get(toTraverse.remove(0))
.orElseThrow(() -> new IllegalStateException("invalid subgroup"));
if (seen.contains(currentGroup.getGroupUUID())) {
- logger.atWarning().log(
+ logger.atFine().log(
"Skipping %s because it's a cyclic subgroup", currentGroup.getGroupUUID());
continue;
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index bbee1b2..30f4094 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -19,6 +19,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
@@ -190,6 +191,7 @@
* notes branch.
*/
@SuppressWarnings("deprecation") // Use Hashing.sha1 for compatibility.
+ @Memoized
public ObjectId sha1() {
String keyString = isCaseInsensitive() ? get().toLowerCase(Locale.US) : get();
return ObjectId.fromRaw(Hashing.sha1().hashString(keyString, UTF_8).asBytes());
@@ -225,7 +227,8 @@
}
@Override
- public final int hashCode() {
+ @Memoized
+ public int hashCode() {
return Objects.hash(sha1());
}
@@ -301,8 +304,9 @@
&& Objects.equals(password(), o.password());
}
+ @Memoized
@Override
- public final int hashCode() {
+ public int hashCode() {
return Objects.hash(key(), accountId(), isCaseInsensitive(), email(), password());
}
@@ -320,7 +324,8 @@
* </pre>
*/
@Override
- public final String toString() {
+ @Memoized
+ public String toString() {
Config c = new Config();
writeToConfig(c);
return c.toText();
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
index 5d81a25..72d703b 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -95,7 +95,9 @@
new Description("Total number of external ID cache reloads from Git.")
.setRate()
.setUnit("updates"),
- Field.ofBoolean("partial", Metadata.Builder::partial).build());
+ Field.ofBoolean("partial", Metadata.Builder::partial)
+ .description("Whether the reload was partial.")
+ .build());
this.reloadDifferential =
metricMaker.newTimer(
"notedb/external_id_partial_read_latency",
diff --git a/java/com/google/gerrit/server/cache/CacheMetrics.java b/java/com/google/gerrit/server/cache/CacheMetrics.java
index 12194e7..f1fd4a8 100644
--- a/java/com/google/gerrit/server/cache/CacheMetrics.java
+++ b/java/com/google/gerrit/server/cache/CacheMetrics.java
@@ -35,7 +35,9 @@
@Singleton
public class CacheMetrics {
private static final Field<String> F_NAME =
- Field.ofString("cache_name", Metadata.Builder::cacheName).build();
+ Field.ofString("cache_name", Metadata.Builder::cacheName)
+ .description("The name of the cache.")
+ .build();
@Inject
public CacheMetrics(
diff --git a/java/com/google/gerrit/server/change/ChangeFinder.java b/java/com/google/gerrit/server/change/ChangeFinder.java
index ba104d8..9f253de 100644
--- a/java/com/google/gerrit/server/change/ChangeFinder.java
+++ b/java/com/google/gerrit/server/change/ChangeFinder.java
@@ -96,6 +96,7 @@
.setRate()
.setUnit("requests"),
Field.ofEnum(ChangeIdType.class, "change_id_type", Metadata.Builder::changeIdType)
+ .description("The type of the change identifier.")
.build());
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index ef1c0ae..35b16b4 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -85,6 +85,7 @@
import com.google.gerrit.server.ExceptionHookImpl;
import com.google.gerrit.server.ExternalUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PerformanceMetrics;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.TraceRequestListener;
import com.google.gerrit.server.account.AccountCacheImpl;
@@ -431,6 +432,7 @@
DynamicSet.setOf(binder(), SubmitRule.class);
DynamicSet.setOf(binder(), QuotaEnforcer.class);
DynamicSet.setOf(binder(), PerformanceLogger.class);
+ DynamicSet.bind(binder(), PerformanceLogger.class).to(PerformanceMetrics.class);
DynamicSet.setOf(binder(), RequestListener.class);
DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
DynamicSet.setOf(binder(), ChangeETagComputation.class);
diff --git a/java/com/google/gerrit/server/events/EventsMetrics.java b/java/com/google/gerrit/server/events/EventsMetrics.java
index 3c87cca..6d48c37 100644
--- a/java/com/google/gerrit/server/events/EventsMetrics.java
+++ b/java/com/google/gerrit/server/events/EventsMetrics.java
@@ -32,7 +32,9 @@
metricMaker.newCounter(
"events",
new Description("Triggered events").setRate().setUnit("triggered events"),
- Field.ofString("type", Metadata.Builder::eventType).build());
+ Field.ofString("type", Metadata.Builder::eventType)
+ .description("The type of the event.")
+ .build());
}
@Override
diff --git a/java/com/google/gerrit/server/extensions/webui/UiActions.java b/java/com/google/gerrit/server/extensions/webui/UiActions.java
index a7f6b48..34c3c20 100644
--- a/java/com/google/gerrit/server/extensions/webui/UiActions.java
+++ b/java/com/google/gerrit/server/extensions/webui/UiActions.java
@@ -72,7 +72,9 @@
new com.google.gerrit.metrics.Description("Latency for RestView#getDescription calls")
.setCumulative()
.setUnit(Units.MILLISECONDS),
- Field.ofString("view", Metadata.Builder::restViewName).build());
+ Field.ofString("view", Metadata.Builder::restViewName)
+ .description("view implementation class")
+ .build());
}
public <R extends RestResource> Iterable<UiAction.Description> from(
diff --git a/java/com/google/gerrit/server/git/UploadPackMetricsHook.java b/java/com/google/gerrit/server/git/UploadPackMetricsHook.java
index 4afff2b..1619add 100644
--- a/java/com/google/gerrit/server/git/UploadPackMetricsHook.java
+++ b/java/com/google/gerrit/server/git/UploadPackMetricsHook.java
@@ -45,7 +45,9 @@
@Inject
UploadPackMetricsHook(MetricMaker metricMaker) {
Field<Operation> operationField =
- Field.ofEnum(Operation.class, "operation", Metadata.Builder::gitOperation).build();
+ Field.ofEnum(Operation.class, "operation", Metadata.Builder::gitOperation)
+ .description("The name of the operation (CLONE, FETCH).")
+ .build();
requestCount =
metricMaker.newCounter(
"git/upload-pack/request_count",
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 42fc916..f7f58fc 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -627,7 +627,7 @@
ReceiveCommitsResult processCommands(
Collection<ReceiveCommand> commands, MultiProgressMonitor progress) throws StorageException {
checkState(!used, "Tried to re-use a ReceiveCommits objects that is single-use only");
- long start = System.nanoTime();
+ long start = TimeUtil.nowNanos();
parsePushOptions();
String clientProvidedDeadlineValue =
Iterables.getLast(pushOptions.get("deadline"), /* defaultValue= */ null);
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 02ec5ea..bfe1ee1 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -342,6 +342,11 @@
integer(ChangeQueryBuilder.FIELD_ATTENTION_SET_USERS)
.buildRepeatable(ChangeField::getAttentionSetUserIds);
+ /** Number of changes that contain attention set. */
+ public static final FieldDef<ChangeData, Integer> ATTENTION_SET_USERS_COUNT =
+ intRange(ChangeQueryBuilder.FIELD_ATTENTION_SET_USERS_COUNT)
+ .build(cd -> additionsOnly(cd.attentionSet()).size());
+
/**
* The full attention set data including timestamp, reason and possible future fields.
*
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index a758377..30ab6e6a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -173,9 +173,14 @@
new Schema.Builder<ChangeData>().add(V67).add(ChangeField.SUBMIT_RULE_RESULT).build();
/** Added new field {@link ChangeField#CHERRY_PICK}. */
+ @Deprecated
static final Schema<ChangeData> V69 =
new Schema.Builder<ChangeData>().add(V68).add(ChangeField.CHERRY_PICK).build();
+ /** Added new field {@link ChangeField#ATTENTION_SET_USERS_COUNT}. */
+ static final Schema<ChangeData> V70 =
+ new Schema.Builder<ChangeData>().add(V69).add(ChangeField.ATTENTION_SET_USERS_COUNT).build();
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 681dfbc..487e0af 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -268,15 +268,23 @@
return this;
}
- public boolean isTracing() {
+ public static boolean isTracing() {
return LoggingContext.getInstance().isLoggingForced();
}
- public Optional<String> getTraceId() {
+ public static Optional<String> getTraceId() {
return LoggingContext.getInstance().getTagsAsMap().get(RequestId.Type.TRACE_ID.name()).stream()
.findFirst();
}
+ public static Optional<String> getPluginTag() {
+ return getTag(PLUGIN_TAG);
+ }
+
+ public static Optional<String> getTag(String tagName) {
+ return LoggingContext.getInstance().getTagsAsMap().get(tagName).stream().findFirst();
+ }
+
public TraceContext enableAclLogging() {
if (stopAclLoggingOnClose) {
return this;
@@ -286,11 +294,7 @@
return this;
}
- public boolean isAclLoggingEnabled() {
- return LoggingContext.getInstance().isAclLogging();
- }
-
- public ImmutableList<String> getAclLogRecords() {
+ public static ImmutableList<String> getAclLogRecords() {
return LoggingContext.getInstance().getAclLogRecords();
}
diff --git a/java/com/google/gerrit/server/notedb/Sequences.java b/java/com/google/gerrit/server/notedb/Sequences.java
index 7a8e28f..7ae98778 100644
--- a/java/com/google/gerrit/server/notedb/Sequences.java
+++ b/java/com/google/gerrit/server/notedb/Sequences.java
@@ -112,8 +112,11 @@
.setCumulative()
.setUnit(Units.MILLISECONDS),
Field.ofEnum(SequenceType.class, "sequence", Metadata.Builder::noteDbSequenceType)
+ .description("The sequence from which IDs were retrieved.")
.build(),
- Field.ofBoolean("multiple", Metadata.Builder::multiple).build());
+ Field.ofBoolean("multiple", Metadata.Builder::multiple)
+ .description("Whether more than one ID was retrieved.")
+ .build());
}
public int nextAccountId() {
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index 97910400..2529c04 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -100,18 +100,22 @@
MetricMaker metricMaker,
@GerritServerConfig Config cfg,
@GerritPersonIdent Provider<PersonIdent> gerritIdentProvider) {
+ Field<OperationType> operationTypeField =
+ Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName)
+ .description("The type of the operation (CACHE_LOAD, IN_MEMORY_WRITE, ON_DISK_WRITE).")
+ .build();
this.counter =
metricMaker.newCounter(
"git/auto-merge/num_operations",
new Description("AutoMerge computations").setRate().setUnit("auto merge computations"),
- Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName).build());
+ operationTypeField);
this.latency =
metricMaker.newTimer(
"git/auto-merge/latency",
new Description("AutoMerge computation latency")
.setCumulative()
.setUnit("milliseconds"),
- Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName).build());
+ operationTypeField);
this.save = cacheAutomerge(cfg);
this.gerritIdentProvider = gerritIdentProvider;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
diff --git a/java/com/google/gerrit/server/permissions/SectionSortCache.java b/java/com/google/gerrit/server/permissions/SectionSortCache.java
index d800782..e64f8b6 100644
--- a/java/com/google/gerrit/server/permissions/SectionSortCache.java
+++ b/java/com/google/gerrit/server/permissions/SectionSortCache.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
@@ -126,22 +127,22 @@
public abstract List<String> patterns();
- public abstract int cachedHashCode();
-
static EntryKey create(String refName, List<AccessSection> sections) {
- int hc = refName.hashCode();
List<String> patterns = new ArrayList<>(sections.size());
for (AccessSection s : sections) {
- String n = s.getName();
- patterns.add(n);
- hc = hc * 31 + n.hashCode();
+ patterns.add(s.getName());
}
- return new AutoValue_SectionSortCache_EntryKey(refName, ImmutableList.copyOf(patterns), hc);
+ return new AutoValue_SectionSortCache_EntryKey(refName, ImmutableList.copyOf(patterns));
}
+ @Memoized
@Override
- public final int hashCode() {
- return cachedHashCode();
+ public int hashCode() {
+ int hc = ref().hashCode();
+ for (String n : patterns()) {
+ hc = hc * 31 + n.hashCode();
+ }
+ return hc;
}
}
diff --git a/java/com/google/gerrit/server/plugincontext/PluginContext.java b/java/com/google/gerrit/server/plugincontext/PluginContext.java
index 3c30745..ef2e181 100644
--- a/java/com/google/gerrit/server/plugincontext/PluginContext.java
+++ b/java/com/google/gerrit/server/plugincontext/PluginContext.java
@@ -121,11 +121,17 @@
@Inject
PluginMetrics(MetricMaker metricMaker) {
Field<String> pluginNameField =
- Field.ofString("plugin_name", Metadata.Builder::pluginName).build();
+ Field.ofString("plugin_name", Metadata.Builder::pluginName)
+ .description("The name of the plugin.")
+ .build();
Field<String> classNameField =
- Field.ofString("class_name", Metadata.Builder::className).build();
+ Field.ofString("class_name", Metadata.Builder::className)
+ .description("The class of the plugin that was invoked.")
+ .build();
Field<String> exportValueField =
- Field.ofString("export_value", Metadata.Builder::exportValue).build();
+ Field.ofString("export_value", Metadata.Builder::exportValue)
+ .description("The export name under which the invoked class is registered.")
+ .build();
this.latency =
metricMaker.newTimer(
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 2874a34..de27afa 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -345,9 +345,16 @@
refreshCounter =
metricMaker.newCounter(
"caches/refresh_count",
- new Description("count").setRate(),
- Field.ofString("cache", Metadata.Builder::className).build(),
- Field.ofBoolean("outdated", Metadata.Builder::outdated).build());
+ new Description(
+ "The number of refreshes per cache with an indicator if a reload was"
+ + " necessary.")
+ .setRate(),
+ Field.ofString("cache", Metadata.Builder::className)
+ .description("The name of the cache.")
+ .build(),
+ Field.ofBoolean("outdated", Metadata.Builder::outdated)
+ .description("Whether the cache entry was outdated on reload.")
+ .build());
this.allProjectsName = allProjectsName;
this.allProjectsConfigProvider = allProjectsConfigProvider;
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 8525eb4..f1fe520 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -142,6 +142,7 @@
public static final String FIELD_ADDED = "added";
public static final String FIELD_AGE = "age";
public static final String FIELD_ATTENTION_SET_USERS = "attentionusers";
+ public static final String FIELD_ATTENTION_SET_USERS_COUNT = "attentionuserscount";
public static final String FIELD_ATTENTION_SET_FULL = "attentionfull";
public static final String FIELD_ASSIGNEE = "assignee";
public static final String FIELD_AUTHOR = "author";
@@ -614,6 +615,14 @@
return ChangePredicates.editBy(self());
}
+ if ("attention".equalsIgnoreCase(value)) {
+ if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
+ throw new QueryParseException(
+ "'has:attention' operator is not supported by change index version");
+ }
+ return new IsAttentionPredicate();
+ }
+
if ("unresolved".equalsIgnoreCase(value)) {
return new IsUnresolvedPredicate();
}
@@ -687,6 +696,14 @@
"'is:private' operator is not supported by change index version");
}
+ if ("attention".equalsIgnoreCase(value)) {
+ if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
+ throw new QueryParseException(
+ "'is:attention' operator is not supported by change index version");
+ }
+ return new IsAttentionPredicate();
+ }
+
if ("assigned".equalsIgnoreCase(value)) {
return Predicate.not(ChangePredicates.assignee(Account.id(ChangeField.NO_ASSIGNEE)));
}
diff --git a/java/com/google/gerrit/server/query/change/IsAttentionPredicate.java b/java/com/google/gerrit/server/query/change/IsAttentionPredicate.java
new file mode 100644
index 0000000..d20d64a
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/IsAttentionPredicate.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2021 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.query.change;
+
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.index.change.ChangeField;
+
+public class IsAttentionPredicate extends IntegerRangeChangePredicate {
+ public IsAttentionPredicate() throws QueryParseException {
+ this(">0");
+ }
+
+ public IsAttentionPredicate(String value) throws QueryParseException {
+ super(ChangeField.ATTENTION_SET_USERS_COUNT, value);
+ }
+
+ @Override
+ protected Integer getValueInt(ChangeData changeData) {
+ return ChangeField.ATTENTION_SET_USERS_COUNT.get(changeData);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 6d3e222..f70379b 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -33,6 +33,7 @@
"//lib/auto:auto-value-annotations",
"//lib/commons:compress",
"//lib/commons:lang",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 4dbb6ee..5002a82 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -80,6 +80,10 @@
import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommentsUtil;
@@ -151,6 +155,29 @@
public class PostReview implements RestModifyView<RevisionResource, ReviewInput> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ @Singleton
+ private static class Metrics {
+ final Counter1<String> draftHandling;
+
+ @Inject
+ Metrics(MetricMaker metricMaker) {
+ draftHandling =
+ metricMaker.newCounter(
+ "change/post_review/draft_handling",
+ new Description(
+ "Total number of draft handling option "
+ + "(KEEP, PUBLISH, PUBLISH_ALL_REVISIONS) "
+ + "selected by users while posting a review.")
+ .setRate()
+ .setUnit("count"),
+ Field.ofString("type", Metadata.Builder::eventType)
+ .description(
+ "The type of the draft handling option"
+ + " (KEEP, PUBLISH, PUBLISH_ALL_REVISIONS).")
+ .build());
+ }
+ }
+
private static final String ERROR_ADDING_REVIEWER = "error adding reviewer";
public static final String ERROR_WIP_READY_MUTUALLY_EXCLUSIVE =
"work_in_progress and ready are mutually exclusive";
@@ -170,6 +197,7 @@
private final EmailReviewComments.Factory email;
private final CommentAdded commentAdded;
private final ReviewerModifier reviewerModifier;
+ private final Metrics metrics;
private final ModifyReviewersEmail modifyReviewersEmail;
private final NotifyResolver notifyResolver;
private final WorkInProgressOp.Factory workInProgressOpFactory;
@@ -196,6 +224,7 @@
EmailReviewComments.Factory email,
CommentAdded commentAdded,
ReviewerModifier reviewerModifier,
+ Metrics metrics,
ModifyReviewersEmail modifyReviewersEmail,
NotifyResolver notifyResolver,
@GerritServerConfig Config gerritConfig,
@@ -218,6 +247,7 @@
this.email = email;
this.commentAdded = commentAdded;
this.reviewerModifier = reviewerModifier;
+ this.metrics = metrics;
this.modifyReviewersEmail = modifyReviewersEmail;
this.notifyResolver = notifyResolver;
this.workInProgressOpFactory = workInProgressOpFactory;
@@ -252,6 +282,7 @@
logger.atFine().log("strict label checking is %s", (strictLabels ? "enabled" : "disabled"));
+ metrics.draftHandling.increment(input.drafts == null ? "N/A" : input.drafts.name());
input.drafts = firstNonNull(input.drafts, DraftHandling.KEEP);
logger.atFine().log("draft handling = %s", input.drafts);
diff --git a/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java b/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
index 1d550f1..0634081 100644
--- a/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
+++ b/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
@@ -19,6 +19,7 @@
import static java.util.stream.Collectors.toMap;
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
@@ -266,8 +267,9 @@
return psId().changeId();
}
+ @Memoized
@Override
- public final int hashCode() {
+ public int hashCode() {
return Objects.hash(patchSet().id(), commit());
}
diff --git a/java/com/google/gerrit/server/restapi/project/CheckAccess.java b/java/com/google/gerrit/server/restapi/project/CheckAccess.java
index 37616cd..5c2f932 100644
--- a/java/com/google/gerrit/server/restapi/project/CheckAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/CheckAccess.java
@@ -95,7 +95,6 @@
} catch (AuthException e) {
return Response.ok(
createInfo(
- traceContext,
HttpServletResponse.SC_FORBIDDEN,
String.format("user %s cannot see project %s", match, rsrc.getName())));
}
@@ -126,7 +125,6 @@
} catch (AuthException e) {
return Response.ok(
createInfo(
- traceContext,
HttpServletResponse.SC_FORBIDDEN,
String.format(
"user %s lacks permission %s for %s in project %s",
@@ -141,15 +139,15 @@
}
}
}
- return Response.ok(createInfo(traceContext, HttpServletResponse.SC_OK, message));
+ return Response.ok(createInfo(HttpServletResponse.SC_OK, message));
}
}
- private AccessCheckInfo createInfo(TraceContext traceContext, int statusCode, String message) {
+ private AccessCheckInfo createInfo(int statusCode, String message) {
AccessCheckInfo info = new AccessCheckInfo();
info.status = statusCode;
info.message = message;
- info.debugLogs = traceContext.getAclLogRecords();
+ info.debugLogs = TraceContext.getAclLogRecords();
if (info.debugLogs.isEmpty()) {
info.debugLogs =
ImmutableList.of("Found no rules that apply, so defaulting to no permission");
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 2249b0e..7e6974c 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -121,7 +121,9 @@
@Inject
Metrics(MetricMaker metricMaker) {
Field<String> actionTypeField =
- Field.ofString("action_type", Metadata.Builder::actionType).build();
+ Field.ofString("action_type", Metadata.Builder::actionType)
+ .description("The type of the action that was retried.")
+ .build();
Field<String> operationNameField =
Field.ofString("operation_name", Metadata.Builder::operationName)
.description("The name of the operation that was retried.")
@@ -479,7 +481,7 @@
}
String cause = formatCause(t);
- if (!traceContext.isTracing()) {
+ if (!TraceContext.isTracing()) {
String traceId = "retry-on-failure-" + new RequestId();
traceContext.addTag(RequestId.Type.TRACE_ID, traceId).forceLogging();
logger.atWarning().withCause(t).log(
diff --git a/java/com/google/gerrit/server/util/time/TimeUtil.java b/java/com/google/gerrit/server/util/time/TimeUtil.java
index 639d0a6..54ef305 100644
--- a/java/com/google/gerrit/server/util/time/TimeUtil.java
+++ b/java/com/google/gerrit/server/util/time/TimeUtil.java
@@ -20,6 +20,7 @@
import com.google.gerrit.server.util.git.DelegateSystemReader;
import java.sql.Timestamp;
import java.time.Instant;
+import java.util.concurrent.TimeUnit;
import java.util.function.LongSupplier;
import org.eclipse.jgit.util.SystemReader;
@@ -35,6 +36,10 @@
return currentMillisSupplier.getAsLong();
}
+ public static long nowNanos() {
+ return TimeUnit.NANOSECONDS.convert(TimeUtil.nowMs(), TimeUnit.MILLISECONDS);
+ }
+
public static Instant now() {
return Instant.ofEpochMilli(nowMs());
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 85a7b29..875ce97 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -156,15 +156,15 @@
@UseClockStep
@Test
public void addedRobotCommentsAreLinkedToChangeMessages() throws Exception {
- TestTimeUtil.resetWithClockStep(0, TimeUnit.SECONDS);
- createChange();
- /* Advancing the time after creating the change so that the first robot comment is not in the same timestamp as with the change creation */
+ // Advancing the time after creating the change so that the first robot comment is not in the
+ // same timestamp as with the change creation.
TestTimeUtil.incrementClock(10, TimeUnit.SECONDS);
RobotCommentInput c1 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
RobotCommentInput c2 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
RobotCommentInput c3 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
- /* Give the robot comments identifiable names for testing */
+
+ // Give the robot comments identifiable names for testing
c1.message = "robot comment 1";
c2.message = "robot comment 2";
c3.message = "robot comment 3";
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractForcePush.java b/javatests/com/google/gerrit/acceptance/git/AbstractForcePush.java
index 88d0937..943d990 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractForcePush.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractForcePush.java
@@ -21,6 +21,7 @@
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.OK;
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -116,6 +117,28 @@
assertDeleteRef(OK);
}
+ @Test
+ public void directPushSendsEmail() throws Exception {
+ // create a change
+ PushOneCommit push1 =
+ pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
+ PushOneCommit.Result r = push1.to("refs/for/master");
+ r.assertOkStatus();
+
+ // Add reviewer to receive notifications
+ gApi.changes().id(r.getChangeId()).addReviewer(user.email());
+ sender.clear();
+
+ // direct submit the change
+ PushOneCommit.Result r1 = push1.to("refs/heads/master");
+ r1.assertOkStatus();
+
+ // email received
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(Iterables.getOnlyElement(sender.getMessages()).body())
+ .contains("has submitted this change");
+ }
+
private void assertDeleteRef(RemoteRefUpdate.Status expectedStatus) throws Exception {
BranchInput in = new BranchInput();
in.ref = "refs/heads/test";
diff --git a/javatests/com/google/gerrit/server/CancellationMetricsTest.java b/javatests/com/google/gerrit/server/RequestInfoTest.java
similarity index 96%
rename from javatests/com/google/gerrit/server/CancellationMetricsTest.java
rename to javatests/com/google/gerrit/server/RequestInfoTest.java
index 2343c71..fafe856 100644
--- a/javatests/com/google/gerrit/server/CancellationMetricsTest.java
+++ b/javatests/com/google/gerrit/server/RequestInfoTest.java
@@ -18,7 +18,7 @@
import org.junit.Test;
-public class CancellationMetricsTest {
+public class RequestInfoTest {
@Test
public void redactRequestUri() throws Exception {
// test with valid request URIs
@@ -52,6 +52,6 @@
}
public static String redact(String uri) {
- return CancellationMetrics.redactRequestUri(uri);
+ return RequestInfo.redactRequestUri(uri);
}
}
diff --git a/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java b/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
index ed4325d..fefa066 100644
--- a/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
+++ b/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
@@ -34,6 +34,7 @@
import com.google.inject.Injector;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.eclipse.jgit.lib.Config;
import org.junit.After;
import org.junit.Before;
@@ -110,9 +111,10 @@
@Test
public void
- traceTimersInsidePerformanceLogContextDoNotCreatePerformanceLogIfNoPerformanceLoggers() {
+ traceTimersInsidePerformanceLogContextDoNotCreatePerformanceLogIfNoPerformanceLoggers()
+ throws Exception {
// Remove test performance logger so that there are no registered performance loggers.
- performanceLoggerRegistrationHandle.remove();
+ removeAllPerformanceLoggers();
assertThat(LoggingContext.getInstance().isPerformanceLogging()).isFalse();
assertThat(LoggingContext.getInstance().getPerformanceLogRecords()).isEmpty();
@@ -277,9 +279,10 @@
@Test
public void
- timerMetricssInsidePerformanceLogContextDoNotCreatePerformanceLogIfNoPerformanceLoggers() {
- // Remove test performance logger so that there are no registered performance loggers.
- performanceLoggerRegistrationHandle.remove();
+ timerMetricssInsidePerformanceLogContextDoNotCreatePerformanceLogIfNoPerformanceLoggers()
+ throws Exception {
+ // Remove all performance loggers so that there are no registered performance loggers.
+ removeAllPerformanceLoggers();
assertThat(LoggingContext.getInstance().isPerformanceLogging()).isFalse();
assertThat(LoggingContext.getInstance().getPerformanceLogRecords()).isEmpty();
@@ -369,6 +372,12 @@
}
}
+ private void removeAllPerformanceLoggers() throws Exception {
+ java.lang.reflect.Field itemsField = DynamicSet.class.getDeclaredField("items");
+ itemsField.setAccessible(true);
+ ((CopyOnWriteArrayList<?>) itemsField.get(performanceLoggers)).clear();
+ }
+
@AutoValue
abstract static class PerformanceLogEntry {
static PerformanceLogEntry create(String operation, Metadata metadata) {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 92cbc41..c8949e6 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -3432,6 +3432,7 @@
@Test
public void attentionSetIndexed() throws Exception {
assume().that(getSchema().hasField(ChangeField.ATTENTION_SET_USERS)).isTrue();
+ assume().that(getSchema().hasField(ChangeField.ATTENTION_SET_USERS_COUNT)).isTrue();
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
@@ -3439,8 +3440,18 @@
AttentionSetInput input = new AttentionSetInput(userId.toString(), "some reason");
gApi.changes().id(change1.getChangeId()).addToAttentionSet(input);
+ assertQuery("is:attention", change1);
+ assertQuery("-is:attention", change2);
+ assertQuery("has:attention", change1);
+ assertQuery("-has:attention", change2);
assertQuery("attention:" + user.getUserName().get(), change1);
assertQuery("-attention:" + userId.toString(), change2);
+
+ gApi.changes()
+ .id(change1.getChangeId())
+ .attention(userId.toString())
+ .remove(new AttentionSetInput("removed again"));
+ assertQuery("-is:attention", change1, change2);
}
@Test
diff --git a/package.json b/package.json
index 6ad3ab4..c3dfad0 100644
--- a/package.json
+++ b/package.json
@@ -33,8 +33,8 @@
"eslint": "npm run safe_bazelisk test polygerrit-ui/app:lint_test",
"eslintfix": "npm run safe_bazelisk run polygerrit-ui/app:lint_bin -- -- --fix $(pwd)/polygerrit-ui/app",
"polylint": "npm run safe_bazelisk test polygerrit-ui/app:polylint_test",
- "test:debug": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --browsers ChromeDev --no-single-run --testFiles",
- "test:single": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --testFiles",
+ "test:debug": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --root '.ts-out/polygerrit-ui/app/' --browsers ChromeDev --no-single-run --test-files",
+ "test:single": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --root '.ts-out/polygerrit-ui/app/' --test-files",
"postinstall": "(git apply --reverse --ignore-whitespace twinkie.patch || true) && git apply --ignore-whitespace twinkie.patch",
"polytest": "npm run safe_bazelisk test //polygerrit-ui/app:validate_polymer_templates",
"polytest:dev": "rm -rf ./polygerrit-ui/app/tmpl_out && npm run safe_bazelisk build //polygerrit-ui/app:template_test_tar && mkdir ./polygerrit-ui/app/tmpl_out && tar -xf bazel-bin/polygerrit-ui/app/template_test_tar.tar -C ./polygerrit-ui/app/tmpl_out"
diff --git a/plugins/delete-project b/plugins/delete-project
index 7dce6f7..6202327 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 7dce6f70611cd8dbf1d38628698155258ee8ef82
+Subproject commit 6202327fe2ac6a86c838e624468ab30ee31a4bee
diff --git a/plugins/tsconfig-plugins-base.json b/plugins/tsconfig-plugins-base.json
index b580549..97eae67 100644
--- a/plugins/tsconfig-plugins-base.json
+++ b/plugins/tsconfig-plugins-base.json
@@ -34,6 +34,15 @@
"incremental": true,
"experimentalDecorators": true,
- "allowUmdGlobalAccess": true
+ "allowUmdGlobalAccess": true,
+
+ "typeRoots": [
+ /* typeRoots for Bazel */
+ "../external/ui_dev_npm/node_modules/@types",
+ "../external/plugins_npm/node_modules/@types",
+ /* typeRoots for IDE */
+ "../polygerrit-ui/node_modules/@types",
+ "../plugins/node_modules/@types"
+ ]
},
}
diff --git a/polygerrit-ui/BUILD b/polygerrit-ui/BUILD
index 7bca96d..62d1d92 100644
--- a/polygerrit-ui/BUILD
+++ b/polygerrit-ui/BUILD
@@ -1,5 +1,6 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary")
load("//tools/bzl:genrule2.bzl", "genrule2")
+load("//tools/bzl:js.bzl", "karma_test")
package(default_visibility = ["//visibility:public"])
@@ -33,8 +34,6 @@
],
)
-# Define a karma+plugins binary to run karma-mocha tests.
-# Can be reused multiple time, if there are multiple karma test rules
sh_binary(
name = "karma_bin",
srcs = ["@ui_dev_npm//:node_modules/karma/bin/karma"],
@@ -49,26 +48,8 @@
],
)
-# Run all tests in one.
-# TODO(dmfilippov): allow parallel tests for karma - either on the bazel level
-# or on the karma level. For now single sh_test is enough.
-sh_test(
+karma_test(
name = "karma_test",
- size = "enormous",
srcs = ["karma_test.sh"],
- args = [
- "$(location :karma_bin)",
- "$(location karma.conf.js)",
- ],
- data = [
- "karma.conf.js",
- ":karma_bin",
- "//polygerrit-ui/app:test-srcs-fg",
- ],
- # Should not run sandboxed.
- tags = [
- "karma",
- "local",
- "manual",
- ],
+ data = ["//polygerrit-ui/app:test-srcs-fg"],
)
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 4a186c1..613efd6 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -70,7 +70,6 @@
[
"**/*.js",
"**/*.ts",
- "test/@types/*.d.ts",
],
exclude = [
"node_modules/**",
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 906300f..04326a6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -54,7 +54,7 @@
} from '../../../types/common';
import {ActionType} from '../../../api/change-actions';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
-import {SinonFakeTimers} from 'sinon/pkg/sinon-esm';
+import {SinonFakeTimers} from 'sinon';
import {IronAutogrowTextareaElement} from '@polymer/iron-autogrow-textarea';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index cb4c9a4..422c91b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -57,7 +57,7 @@
LabelValueToDescriptionMap,
Hashtag,
} from '../../../types/common';
-import {SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonStubbedMember} from 'sinon';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
import {GrEditableLabel} from '../../shared/gr-editable-label/gr-editable-label';
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 90b9a2d..831a309 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -30,7 +30,7 @@
someProvidersAreLoadingFirstTime$,
topLevelActionsLatest$,
} from '../../../services/checks/checks-model';
-import {Action, Category, RunStatus} from '../../../api/checks';
+import {Action, Category, Link, RunStatus} from '../../../api/checks';
import {fireShowPrimaryTab} from '../../../utils/event-util';
import '../../shared/gr-avatar/gr-avatar';
import {
@@ -180,6 +180,9 @@
@property()
text = '';
+ @property()
+ links: Link[] = [];
+
static override get styles() {
return [
fontStyles,
@@ -187,6 +190,8 @@
css`
:host {
display: inline-block;
+ position: relative;
+ white-space: nowrap;
}
.checksChip {
color: var(--chip-color);
@@ -202,8 +207,16 @@
position: relative;
top: 2px;
}
- .checksChip:hover .text {
- max-width: 240px;
+ .checksChip.hoverFullLength {
+ position: absolute;
+ z-index: 1;
+ display: none;
+ }
+ .checksChip.hoverFullLength .text {
+ max-width: 400px;
+ }
+ :host(:hover) .checksChip.hoverFullLength {
+ display: inline-block;
}
.checksChip .text {
display: inline-block;
@@ -308,20 +321,51 @@
ariaLabel = `${this.text} ${label} ${type}${plural}`;
}
const chipClass = `checksChip font-small ${icon}`;
+ const chipClassFullLength = `${chipClass} hoverFullLength`;
const grIcon = `gr-icons:${icon}`;
+ // 15 is roughly the number of chars for the chip exceeding its 120px width.
return html`
- <div
- class="${chipClass}"
- role="link"
- tabindex="0"
- aria-label="${ariaLabel}"
- >
- <iron-icon icon="${grIcon}"></iron-icon>
+ ${this.text.length > 15
+ ? html` ${this.renderChip(chipClassFullLength, ariaLabel, grIcon)}`
+ : ''}
+ ${this.renderChip(chipClass, ariaLabel, grIcon)}
+ `;
+ }
+
+ private renderChip(clazz: string, ariaLabel: string, icon: string) {
+ return html`
+ <div class="${clazz}" role="link" tabindex="0" aria-label="${ariaLabel}">
+ <iron-icon icon="${icon}"></iron-icon>
<div class="text">${this.text}</div>
- <slot></slot>
+ ${this.renderLinks()}
</div>
`;
}
+
+ private renderLinks() {
+ return this.links.map(
+ link => html`
+ <a
+ href="${link.url}"
+ target="_blank"
+ @click="${this.onLinkClick}"
+ @keydown="${this.onLinkKeyDown}"
+ aria-label="Link to check details"
+ ><iron-icon class="launch" icon="gr-icons:launch"></iron-icon
+ ></a>
+ `
+ );
+ }
+
+ private onLinkKeyDown(e: KeyboardEvent) {
+ // Prevents onChipKeyDown() from reacting to <a> link keyboard events.
+ e.stopPropagation();
+ }
+
+ private onLinkClick(e: MouseEvent) {
+ // Prevents onChipClick() from reacting to <a> link clicks.
+ e.stopPropagation();
+ }
}
/** What is the maximum number of detailed checks chips? */
@@ -660,21 +704,10 @@
return html`<gr-checks-chip
.statusOrCategory="${statusOrCategory}"
.text="${text}"
+ .links="${links}"
@click="${handler}"
@keydown="${(e: KeyboardEvent) => handleSpaceOrEnter(e, handler)}"
- >${links.map(
- link => html`
- <a
- href="${link.url}"
- target="_blank"
- @click="${this.onLinkClick}"
- @keydown="${this.onLinkKeyDown}"
- aria-label="Link to check details"
- ><iron-icon class="launch" icon="gr-icons:launch"></iron-icon
- ></a>
- `
- )}
- </gr-checks-chip>`;
+ ></gr-checks-chip>`;
}
private onChipClick(state: ChecksTabState) {
@@ -683,16 +716,6 @@
});
}
- private onLinkKeyDown(e: KeyboardEvent) {
- // Prevents onConChipKeyDown() from reacting to <a> link keyboard events.
- e.stopPropagation();
- }
-
- private onLinkClick(e: MouseEvent) {
- // Prevents onChipClick() from reacting to <a> link clicks.
- e.stopPropagation();
- }
-
override render() {
const commentThreads =
this.commentThreads?.filter(t => !isRobotThread(t) || hasHumanReply(t)) ??
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 6b63000..676e066 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -90,7 +90,7 @@
} from '@polymer/iron-test-helpers/mock-interactions';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {AppElementChangeViewParams} from '../../gr-app-types';
-import {SinonFakeTimers, SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {CustomKeyboardEvent} from '../../../types/events';
import {CommentThread, UIRobot} from '../../../utils/comment-util';
@@ -445,7 +445,7 @@
const args = navigateToChangeStub.getCall(0).args;
assert.equal(args[0], element._change);
assert.equal(args[1], 10 as PatchSetNum);
- assert.equal(args[2], 1 as PatchSetNum);
+ assert.equal(args[2], 1 as BasePatchSetNum);
});
test('_handleDiffBaseAgainstLeft', () => {
@@ -483,7 +483,7 @@
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
assert.equal(args[1], 10 as PatchSetNum);
- assert.equal(args[2], 3 as PatchSetNum);
+ assert.equal(args[2], 3 as BasePatchSetNum);
});
test('_handleDiffBaseAgainstLatest', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index e11d3a3..5def25a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -43,7 +43,6 @@
createParsedChange,
createRevision,
} from '../../../test/test-data-generators.js';
-import sinon from 'sinon/pkg/sinon-esm';
import {createDefaultDiffPrefs} from '../../../constants/constants.js';
import {queryAndAssert} from '../../../utils/common-util.js';
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
index 90d2049..f87c4c3 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
@@ -51,7 +51,7 @@
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {CommentSide} from '../../../constants/constants';
-import {SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonStubbedMember} from 'sinon';
const basicFixture = fixtureFromElement('gr-message');
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index e38e0ae..a6dc338f 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-import {SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonStubbedMember} from 'sinon';
import {PluginApi} from '../../../api/plugin';
import {ChangeStatus} from '../../../constants/constants';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 7ca7226..f636df3 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -16,12 +16,14 @@
*/
import '../../../styles/gr-font-styles';
import '../../shared/gr-hovercard/gr-hovercard-shared-style';
+import '../../shared/gr-button/gr-button';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {customElement, property} from '@polymer/decorators';
import {HovercardBehaviorMixin} from '../../shared/gr-hovercard/gr-hovercard-behavior';
import {htmlTemplate} from './gr-submit-requirement-hovercard_html';
import {
AccountInfo,
+ SubmitRequirementExpressionInfo,
SubmitRequirementResultInfo,
SubmitRequirementStatus,
} from '../../../api/rest-api';
@@ -53,6 +55,9 @@
@property({type: Boolean})
mutable?: boolean;
+ @property({type: Boolean})
+ expanded = false;
+
@property({type: Array, computed: 'computeLabels(change, requirement)'})
_labels: Label[] = [];
@@ -82,6 +87,16 @@
computeIcon(status: SubmitRequirementStatus) {
return iconForStatus(status);
}
+
+ renderCondition(expression?: SubmitRequirementExpressionInfo) {
+ if (!expression) return '';
+
+ return expression.expression;
+ }
+
+ _handleShowConditions() {
+ this.expanded = true;
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
index 6adb29e..192a812 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
@@ -43,7 +43,6 @@
}
.row {
display: flex;
- margin-top: var(--spacing-s);
}
.title {
color: var(--deemphasized-text-color);
@@ -53,18 +52,37 @@
margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
display: flex;
}
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
div.sectionIcon iron-icon {
position: relative;
top: 2px;
width: 20px;
height: 20px;
}
+ .condition {
+ background-color: var(--gray-background);
+ padding: var(--spacing-m);
+ flex-grow: 1;
+ }
+ .expression {
+ color: var(--gray-foreground);
+ }
iron-icon.check {
color: var(--success-foreground);
}
iron-icon.close {
color: var(--warning-foreground);
}
+ .showConditions iron-icon {
+ color: inherit;
+ }
+ div.showConditions {
+ border-top: 1px solid var(--border-color);
+ margin-top: var(--spacing-m);
+ padding: var(--spacing-m) var(--spacing-xl) 0;
+ }
</style>
<div id="container" role="tooltip" tabindex="-1">
<div class="section">
@@ -113,5 +131,59 @@
</section>
</template>
</div>
+ <template is="dom-if" if="[[!expanded]]">
+ <div class="showConditions">
+ <gr-button
+ link=""
+ class="showConditions"
+ on-click="_handleShowConditions"
+ >
+ View condition
+ <iron-icon icon="gr-icons:expand-more"></iron-icon
+ ></gr-button>
+ </div>
+ </template>
+ <template is="dom-if" if="[[expanded]]">
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon icon="gr-icons:description"></iron-icon>
+ </div>
+ <div class="sectionContent">[[requirement.description]]</div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon"></div>
+ <div class="sectionContent condition">
+ Blocking condition:<br />
+ <span class="expression">
+ [[renderCondition(requirement.submittability_expression_result)]]
+ </span>
+ </div>
+ </div>
+ <template
+ is="dom-if"
+ if="[[requirement.applicability_expression_result]]"
+ >
+ <div class="section">
+ <div class="sectionIcon"></div>
+ <div class="sectionContent condition">
+ Application condition:<br />
+ <span class="expression">
+ [[renderCondition(requirement.applicability_expression_result)]]
+ </span>
+ </div>
+ </div>
+ </template>
+ <template is="dom-if" if="[[requirement.override_expression_result]]">
+ <div class="section">
+ <div class="sectionIcon"></div>
+ <div class="sectionContent condition">
+ Override condition:<br />
+ <span class="expression">
+ [[renderCondition(requirement.override_expression_result)]]
+ </span>
+ </div>
+ </div>
+ </template>
+ </template>
</div>
`;
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index 2dc737e..7b12b1117 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -67,6 +67,7 @@
'footer:',
'from:',
'has:',
+ 'has:attention',
'has:draft',
'has:edit',
'has:star',
@@ -77,6 +78,7 @@
'is:',
'is:abandoned',
'is:assigned',
+ 'is:attention',
'is:cherrypick',
'is:closed',
'is:ignored',
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 1908df0..6dd67e4 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -41,6 +41,7 @@
import {ParsedChangeInfo} from '../../../types/types';
import {GrButton} from '../../shared/gr-button/gr-button';
import {TokenHighlightLayer} from '../gr-diff-builder/token-highlight-layer';
+import {KnownExperimentId} from '../../../services/flags/flags';
export interface GrApplyFixDialog {
$: {
@@ -98,7 +99,11 @@
})
_disableApplyFixButton = false;
- layers = [new TokenHighlightLayer(this)];
+ layers = appContext.flagsService.isEnabled(
+ KnownExperimentId.TOKEN_HIGHLIGHTING
+ )
+ ? [new TokenHighlightLayer(this)]
+ : [];
private refitOverlay?: () => void;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index 294a78e..663ee7e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -572,6 +572,7 @@
lineLimit: number
): HTMLElement {
const contentText = this._createElement('div', 'contentText');
+ contentText.ariaLabel = text;
const responsive = isResponsive(responsiveMode);
let columnPos = 0;
let textOffset = 0;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
index d606e01..9fc69b5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
@@ -21,7 +21,6 @@
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line.js';
import {HOVER_DELAY_MS, TokenHighlightLayer} from './token-highlight-layer';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
-import sinon from 'sinon/pkg/sinon-esm';
import {html, render} from 'lit-html';
import {_testOnly_allTasks} from '../../../utils/async-util';
import {queryAndAssert} from '../../../test/test-utils';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index fa3ddf4..c4fed53 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -412,7 +412,11 @@
private _getLayers(path: string): DiffLayer[] {
const layers = [];
- layers.push(new TokenHighlightLayer(this));
+ if (
+ appContext.flagsService.isEnabled(KnownExperimentId.TOKEN_HIGHLIGHTING)
+ ) {
+ layers.push(new TokenHighlightLayer(this));
+ }
layers.push(this.syntaxLayer);
// Get layers from plugins (if any).
layers.push(...this.jsAPI.getDiffLayers(path));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index b24b3ba..344f9d8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -1270,7 +1270,7 @@
test('gr-diff-host provides syntax highlighting layer', async () => {
stubRestApi('getDiff').returns(Promise.resolve({content: []}));
await element.reload();
- assert.equal(element.$.diff.layers[1], element.syntaxLayer);
+ assert.equal(element.$.diff.layers[0], element.syntaxLayer);
});
test('rendering normal-sized diff does not disable syntax', () => {
@@ -1324,7 +1324,7 @@
test('gr-diff-host provides syntax highlighting layer', async () => {
stubRestApi('getDiff').returns(Promise.resolve({content: []}));
await element.reload();
- assert.equal(element.$.diff.layers[1], element.syntaxLayer);
+ assert.equal(element.$.diff.layers[0], element.syntaxLayer);
});
test('syntax layer should be disabled', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
index fbdcb69..ba1abd0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
@@ -429,6 +429,11 @@
id="highlight-image"
style="${styleMap({
opacity: this.showHighlight ? '1' : '0',
+ // When the highlight layer is not being shown, saving the image or
+ // opening it in a new tab from the context menu, e.g. for external
+ // comparison, should give back the source image, not the highlight
+ // layer.
+ 'pointer-events': this.showHighlight ? 'auto' : 'none',
})}"
src="${this.diffHighlightSrc}"
/>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index d9c4ba2..a3de30a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -29,7 +29,6 @@
createComment,
} from '../../../test/test-data-generators.js';
import {EditPatchSetNum} from '../../../types/common.js';
-import sinon from 'sinon/pkg/sinon-esm';
import {CursorMoveResult} from '../../../api/core.js';
const basicFixture = fixtureFromElement('gr-diff-view');
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.ts b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.ts
index ab27b1e..f1813a4 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.ts
@@ -27,7 +27,7 @@
createServerInfo,
} from '../../../test/test-data-generators';
import {IronInputElement} from '@polymer/iron-input';
-import {SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonStubbedMember} from 'sinon';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
const basicFixture = fixtureFromElement('gr-account-info');
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
index bb70855..b3c485a 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.ts
@@ -24,6 +24,7 @@
import {
createAccountWithEmail,
createAccountWithId,
+ createServerInfo,
} from '../../../test/test-data-generators';
const basicFixture = fixtureFromElement('gr-avatar');
@@ -116,9 +117,11 @@
suite('config set', () => {
setup(() => {
- stub('gr-avatar', '_getConfig').callsFake(() =>
- Promise.resolve({plugin: {has_avatars: true}})
- );
+ const config = {
+ ...createServerInfo(),
+ plugin: {has_avatars: true, js_resource_paths: []},
+ };
+ stub('gr-avatar', '_getConfig').returns(Promise.resolve(config));
element = basicFixture.instantiate();
});
@@ -154,9 +157,11 @@
let element: GrAvatar;
setup(() => {
- stub('gr-avatar', '_getConfig').callsFake(() =>
- Promise.resolve({plugin: {has_avatars: true}})
- );
+ const config = {
+ ...createServerInfo(),
+ plugin: {has_avatars: true, js_resource_paths: []},
+ };
+ stub('gr-avatar', '_getConfig').returns(Promise.resolve(config));
element = basicFixture.instantiate();
});
@@ -182,7 +187,7 @@
let element: GrAvatar;
setup(() => {
- stub('gr-avatar', '_getConfig').callsFake(() => Promise.resolve({}));
+ stub('gr-avatar', '_getConfig').returns(Promise.resolve(undefined));
element = basicFixture.instantiate();
});
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts
index a56f6f1..39fc7c6 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
-import sinon from 'sinon/pkg/sinon-esm';
import '../../../test/common-test-setup-karma';
import {createChange} from '../../../test/test-data-generators';
import './gr-change-status';
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 39a87a2..d6aae5c 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -54,6 +54,7 @@
import {CustomKeyboardEvent} from '../../../types/events';
import {LineNumber, FILE} from '../../diff/gr-diff/gr-diff-line';
import {GrButton} from '../gr-button/gr-button';
+import {KnownExperimentId} from '../../../services/flags/flags';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {RenderPreferences} from '../../../api/diff';
import {
@@ -211,6 +212,8 @@
private readonly reporting = appContext.reportingService;
+ private readonly flagsService = appContext.flagsService;
+
private readonly commentsService = appContext.commentsService;
readonly storage = appContext.storageService;
@@ -357,7 +360,9 @@
_getLayers(diff?: DiffInfo) {
if (!diff) return [];
const layers = [];
- layers.push(new TokenHighlightLayer(this));
+ if (this.flagsService.isEnabled(KnownExperimentId.TOKEN_HIGHLIGHTING)) {
+ layers.push(new TokenHighlightLayer(this));
+ }
layers.push(this.syntaxLayer);
return layers;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 54d8ee8..31a1614 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -52,7 +52,7 @@
createFixSuggestionInfo,
} from '../../../test/test-data-generators';
import {Timer} from '../../../services/gr-reporting/gr-reporting';
-import {SinonFakeTimers, SinonStubbedMember} from 'sinon/pkg/sinon-esm';
+import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
import {CreateFixCommentEvent} from '../../../types/events';
import {DraftInfo, UIRobot} from '../../../utils/comment-util';
import {MockTimer} from '../../../services/gr-reporting/gr-reporting_mock';
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
index 69573c7..da1a782 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
@@ -158,6 +158,8 @@
<g id="arrow-forward"><path d="M0 0h24v24H0z" fill="none"/><path d="M12 4l-1.41 1.41L16.17 11H4v2h12.17l-5.58 5.59L12 20l8-8z"/></g>
<!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material+Icons:feedback -->
<g id="feedback"><path d="M0 0h24v24H0z" fill="none"/><path d="M20 2H4c-1.1 0-1.99.9-1.99 2L2 22l4-4h14c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zm-7 12h-2v-2h2v2zm0-4h-2V6h2v4z"/></g>
+ <!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material+Icons&icon.query=description -->
+ <g id="description"><path xmlns="http://www.w3.org/2000/svg" d="M0 0h24v24H0V0z" fill="none"/><path xmlns="http://www.w3.org/2000/svg" d="M8 16h8v2H8zm0-4h8v2H8zm6-10H6c-1.1 0-2 .9-2 2v16c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6zm4 18H6V4h7v5h5v11z"/></g>
</defs>
</svg>
</iron-iconset-svg>`;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
index 8ec2607..29db685 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
@@ -23,7 +23,6 @@
import {getPluginLoader} from './gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
import {stubBaseUrl} from '../../../test/test-utils.js';
-import sinon from 'sinon/pkg/sinon-esm';
import {stubRestApi} from '../../../test/test-utils.js';
import {appContext} from '../../../services/app-context.js';
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index f5d6e35..0a5eeca 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -353,7 +353,7 @@
export const fakeRun0: CheckRun = {
pluginName: 'f0',
internalRunId: 'f0',
- checkName: 'FAKE Error Finder Finder Finder',
+ checkName: 'FAKE Error Finder Finder Finder Finder Finder Finder Finder',
labelName: 'Presubmit',
isSingleAttempt: true,
isLatestAttempt: true,
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 21f3aa4..ef5fde2 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -25,6 +25,7 @@
*/
export enum KnownExperimentId {
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
+ TOKEN_HIGHLIGHTING = 'UiFeature__token_highlighting',
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
NEW_REPLY_DIALOG = 'UiFeature__new_reply_dialog',
SUBMIT_REQUIREMENTS_UI = 'UiFeature__submit_requirements_ui',
diff --git a/polygerrit-ui/app/test/@types/sinon-esm.d.ts b/polygerrit-ui/app/test/@types/sinon-esm.d.ts
deleted file mode 100644
index 9074a7a..0000000
--- a/polygerrit-ui/app/test/@types/sinon-esm.d.ts
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * @license
- * 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.
- */
-
-declare module 'sinon/pkg/sinon-esm' {
- // sinon-esm doesn't have it's own d.ts, reexport all types from sinon
- // This is a trick - @types/sinon adds interfaces and sinon instance
- // to a global variables/namespace. We reexport it here, so we
- // can use in our code when importing sinon-esm
- // eslint-disable-next-line import/no-default-export
- export default sinon;
- const sinon: Sinon.SinonStatic;
- export {SinonSpy, SinonFakeTimers, SinonStubbedMember};
-}
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index 5096e09..550d3df 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -33,7 +33,6 @@
TestKeyboardShortcutBinder,
} from './test-utils';
import {_testOnly_getShortcutManagerInstance} from '../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
-import sinon from 'sinon/pkg/sinon-esm';
import {safeTypesBridge} from '../utils/safe-types-util';
import {_testOnly_initGerritPluginApi} from '../elements/shared/gr-js-api-interface/gr-gerrit';
import {initGlobalVariables} from '../elements/gr-app-global-var-init';
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 156217c..a60c1d1 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -23,7 +23,7 @@
} from '../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {appContext} from '../services/app-context';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
-import {SinonSpy} from 'sinon/pkg/sinon-esm';
+import {SinonSpy} from 'sinon';
import {StorageService} from '../services/storage/gr-storage';
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
diff --git a/polygerrit-ui/app/tsconfig.json b/polygerrit-ui/app/tsconfig.json
index 79dae41..9516dce 100644
--- a/polygerrit-ui/app/tsconfig.json
+++ b/polygerrit-ui/app/tsconfig.json
@@ -39,7 +39,13 @@
"incremental": true,
"experimentalDecorators": true,
- "allowUmdGlobalAccess": true
+ "allowUmdGlobalAccess": true,
+
+ /* typeRoots for IDE (see tsconfig_bazel.json for Bazel) */
+ "typeRoots": [
+ "node_modules/@types",
+ "../node_modules/@types"
+ ]
},
// With the * pattern (without an extension), only supported files
// are included. The supported files are .ts, .tsx, .d.ts.
diff --git a/polygerrit-ui/app/tsconfig_bazel_test.json b/polygerrit-ui/app/tsconfig_bazel_test.json
index 9c2ff93..7137e23 100644
--- a/polygerrit-ui/app/tsconfig_bazel_test.json
+++ b/polygerrit-ui/app/tsconfig_bazel_test.json
@@ -2,7 +2,6 @@
"extends": "./tsconfig_bazel.json",
"compilerOptions": {
"typeRoots": [
- "./test/@types",
"../../external/ui_dev_npm/node_modules/@polymer/iron-test-helpers",
"../../external/ui_npm/node_modules/@types",
"../../external/ui_dev_npm/node_modules/@types"
diff --git a/polygerrit-ui/karma.conf.js b/polygerrit-ui/karma.conf.js
index 00ebc63..a3b694f 100644
--- a/polygerrit-ui/karma.conf.js
+++ b/polygerrit-ui/karma.conf.js
@@ -22,6 +22,7 @@
if(runUnderBazel) {
// Run under bazel
return [
+ `external/plugins_npm/node_modules`,
`external/ui_npm/node_modules`,
`external/ui_dev_npm/node_modules`
];
@@ -58,11 +59,11 @@
}
module.exports = function(config) {
- const localDirName = path.resolve(__dirname, '../.ts-out/polygerrit-ui/app');
- const rootDir = runUnderBazel ?
- 'polygerrit-ui/app/_pg_with_tests_out/' : localDirName + '/';
- const testFilesLocationPattern =
- `${rootDir}**/!(template_test_srcs)/`;
+ let root = config.root;
+ if (!root) {
+ console.warn(`--root argument not set. Falling back to __dirname.`)
+ root = path.resolve(__dirname) + '/';
+ }
// Use --test-files to specify pattern for a test files.
// It can be just a file name, without a path:
// --test-files async-foreach-behavior_test.js
@@ -83,7 +84,9 @@
} else {
filePattern = '*_test.js';
}
- const testFilesPattern = testFilesLocationPattern + filePattern;
+ const testFilesPattern = root + '**/' + filePattern;
+
+ console.info(`Karma test file pattern: ${testFilesPattern}`)
// Special patch for grep parameters (see details in the grep-patch-karam.js)
const additionalFiles = runUnderBazel ? [] : ['polygerrit-ui/grep-patch-karma.js'];
config.set({
diff --git a/polygerrit-ui/karma_test.sh b/polygerrit-ui/karma_test.sh
index 5fab442..940b969 100755
--- a/polygerrit-ui/karma_test.sh
+++ b/polygerrit-ui/karma_test.sh
@@ -1,4 +1,6 @@
#!/bin/bash
set -euo pipefail
-./$1 start $2 --single-run
+./$1 start $2 \
+ --root 'polygerrit-ui/app/_pg_with_tests_out/**/' \
+ --test-files '*_test.js'
diff --git a/resources/com/google/gerrit/server/mail/Merged.soy b/resources/com/google/gerrit/server/mail/Merged.soy
index e268a31..b8a19fc 100644
--- a/resources/com/google/gerrit/server/mail/Merged.soy
+++ b/resources/com/google/gerrit/server/mail/Merged.soy
@@ -30,7 +30,7 @@
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
- {$email.stickyApprovalDiff}
+ {if $email.stickyApprovalDiff} ( {$email.stickyApprovalDiff} ){/if}
Change subject: {$change.subject}{\n}
......................................................................{\n}
diff --git a/resources/com/google/gerrit/server/mail/MergedHtml.soy b/resources/com/google/gerrit/server/mail/MergedHtml.soy
index d2f7bfd..ac4afb3 100644
--- a/resources/com/google/gerrit/server/mail/MergedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/MergedHtml.soy
@@ -32,9 +32,11 @@
</p>
{/if}
- {call mailTemplate.UnifiedDiff}
- {param diffLines: $email.stickyApprovalDiffHtml /}
- {/call}
+ {if $email.stickyApprovalDiffHtml}
+ {call mailTemplate.UnifiedDiff}
+ {param diffLines: $email.stickyApprovalDiffHtml /}
+ {/call}
+ {/if}
<div style="white-space:pre-wrap">{$email.approvals}</div>
diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl
index eedf0a1..c8d6e4b 100644
--- a/tools/bzl/js.bzl
+++ b/tools/bzl/js.bzl
@@ -142,3 +142,36 @@
"zip -Drq $$ROOT/$@ -g .",
]),
)
+
+def karma_test(name, srcs, data):
+ """Creates a Karma test target.
+
+ It can be used both for the main Gerrit js bundle, but also for plugins. So
+ it should be extremely easy to add Karma test capabilities for new plugins.
+
+ We are sharing one karma.conf.js file. If you want to customize that, then
+ consider using command line arguments that the config file can process, see
+ the `root` argument for an example.
+
+ Args:
+ name: The name of the test rule.
+ srcs: The shell script to invoke, where you can set command line
+ arguments for Karma and its config.
+ data: The bundle of JavaScript files with the tests included.
+ """
+
+ native.sh_test(
+ name = name,
+ size = "enormous",
+ srcs = srcs,
+ args = [
+ "$(location //polygerrit-ui:karma_bin)",
+ "$(location //polygerrit-ui:karma.conf.js)",
+ ],
+ data = data + [
+ "//polygerrit-ui:karma_bin",
+ "//polygerrit-ui:karma.conf.js",
+ ],
+ # Should not run sandboxed.
+ tags = ["karma", "local", "manual"],
+ )
diff --git a/tools/js/eslint.bzl b/tools/js/eslint.bzl
index b32e2bc..eb4d37a 100644
--- a/tools/js/eslint.bzl
+++ b/tools/js/eslint.bzl
@@ -16,6 +16,34 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test")
+def plugin_eslint():
+ """ Convenience wrapper macro of eslint() for Gerrit js plugins
+
+ Args:
+ name: name of the rule
+ """
+ eslint(
+ name = "lint",
+ srcs = native.glob(["**/*.ts"]),
+ config = ".eslintrc.js",
+ data = [
+ "tsconfig.json",
+ "//plugins:.eslintrc.js",
+ "//plugins:.prettierrc.js",
+ "//plugins:tsconfig-plugins-base.json",
+ ],
+ extensions = [".ts"],
+ ignore = "//plugins:.eslintignore",
+ plugins = [
+ "@npm//eslint-config-google",
+ "@npm//eslint-plugin-html",
+ "@npm//eslint-plugin-import",
+ "@npm//eslint-plugin-jsdoc",
+ "@npm//eslint-plugin-prettier",
+ "@npm//gts",
+ ],
+ )
+
def eslint(name, plugins, srcs, config, ignore, extensions = [".js"], data = []):
""" Macro to define eslint rules for files.
@@ -87,7 +115,7 @@
"*_test_require_patch.js",
"--ignore-pattern",
"*_test_loader.js",
- "./", # Relative to the config file location
+ "./", # Relative to the config file location
],
# Should not run sandboxed.
tags = [
