Merge "Change checks filter to also match tag names"
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index feafe59..0bbf4fb 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -12,41 +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
@@ -64,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
@@ -93,66 +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/post_review/draft_handling`: Total number of draft handling option (KEEP, PUBLISH, PUBLISH_ALL_REVISIONS) selected by users while posting a review.
+* `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
@@ -171,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
@@ -187,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
@@ -204,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
@@ -230,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
@@ -275,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/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 487a748..f534ccb 100644
--- a/java/com/google/gerrit/server/CancellationMetrics.java
+++ b/java/com/google/gerrit/server/CancellationMetrics.java
@@ -43,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) -> {})
@@ -59,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(
diff --git a/java/com/google/gerrit/server/PerformanceMetrics.java b/java/com/google/gerrit/server/PerformanceMetrics.java
index fa6e22c..85452ce 100644
--- a/java/com/google/gerrit/server/PerformanceMetrics.java
+++ b/java/com/google/gerrit/server/PerformanceMetrics.java
@@ -32,9 +32,12 @@
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) {
@@ -42,13 +45,29 @@
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) -> {}).build();
+ 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) -> {}).build();
+ 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) -> {}).build();
+ 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(
@@ -66,6 +85,13 @@
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
@@ -91,6 +117,9 @@
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) {
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 8bf095c..30f4094 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -191,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());
@@ -226,7 +227,8 @@
}
@Override
- public final int hashCode() {
+ @Memoized
+ public int hashCode() {
return Objects.hash(sha1());
}
@@ -322,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/change/FileInfoJsonComparingImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonComparingImpl.java
deleted file mode 100644
index a926147..0000000
--- a/java/com/google/gerrit/server/change/FileInfoJsonComparingImpl.java
+++ /dev/null
@@ -1,167 +0,0 @@
-// 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.change;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.common.FileInfo;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
-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.logging.Metadata;
-import com.google.gerrit.server.patch.DiffExecutor;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import org.eclipse.jgit.lib.ObjectId;
-
-/**
- * Implementation of FileInfoJson which uses {@link FileInfoJsonOldImpl}, but also runs {@link
- * FileInfoJsonNewImpl} asynchronously and compares the results. This implementation is temporary
- * and will be used to verify that the results are the same.
- */
-public class FileInfoJsonComparingImpl implements FileInfoJson {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private final FileInfoJsonOldImpl oldImpl;
- private final FileInfoJsonNewImpl newImpl;
- private final ExecutorService executor;
- private final Metrics metrics;
-
- /**
- * TODO(ghareeb): These metrics are temporary for launching the new diff cache redesign and are
- * not documented. These will be removed soon.
- */
- @VisibleForTesting
- @Singleton
- static class Metrics {
- private enum Status {
- MATCH,
- MISMATCH,
- ERROR
- }
-
- final Counter1<Status> diffs;
-
- @Inject
- Metrics(MetricMaker metricMaker) {
- diffs =
- metricMaker.newCounter(
- "diff/list_files/dark_launch",
- new Description(
- "Total number of matching, non-matching, or error in list-files diffs in the old and new diff cache implementations.")
- .setRate()
- .setUnit("count"),
- Field.ofEnum(Status.class, "type", Metadata.Builder::eventType).build());
- }
- }
-
- @Inject
- public FileInfoJsonComparingImpl(
- FileInfoJsonOldImpl oldImpl,
- FileInfoJsonNewImpl newImpl,
- @DiffExecutor ExecutorService executor,
- Metrics metrics) {
- this.oldImpl = oldImpl;
- this.newImpl = newImpl;
- this.executor = executor;
- this.metrics = metrics;
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Change change, ObjectId objectId, @Nullable PatchSet base)
- throws ResourceConflictException, PatchListNotAvailableException {
- Map<String, FileInfo> result = oldImpl.getFileInfoMap(change, objectId, base);
- @SuppressWarnings("unused")
- Future<?> ignored =
- executor.submit(
- () -> {
- try {
- Map<String, FileInfo> fileInfoNew = newImpl.getFileInfoMap(change, objectId, base);
- compareAndLogMetrics(
- result,
- fileInfoNew,
- String.format(
- "Mismatch comparing old and new diff implementations for change: %s, objectId: %s and base: %s",
- change, objectId, base == null ? "none" : base.id()));
- } catch (ResourceConflictException | PatchListNotAvailableException e) {
- // If an exception happens while evaluating the new diff, increment the non-matching
- // counter
- metrics.diffs.increment(Metrics.Status.ERROR);
- logger.atWarning().withCause(e).log(
- "Error comparing old and new diff implementations.");
- }
- });
- return result;
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Project.NameKey project, ObjectId objectId, int parentNum)
- throws ResourceConflictException, PatchListNotAvailableException {
- Map<String, FileInfo> result = oldImpl.getFileInfoMap(project, objectId, parentNum);
- @SuppressWarnings("unused")
- Future<?> ignored =
- executor.submit(
- () -> {
- try {
- Map<String, FileInfo> resultNew =
- newImpl.getFileInfoMap(project, objectId, parentNum);
- compareAndLogMetrics(
- result,
- resultNew,
- String.format(
- "Mismatch comparing old and new diff implementations for project: %s, objectId: %s and parentNum: %d",
- project, objectId, parentNum));
- } catch (ResourceConflictException | PatchListNotAvailableException e) {
- // If an exception happens while evaluating the new diff, increment the non-matching
- // ctr
- metrics.diffs.increment(Metrics.Status.ERROR);
- logger.atWarning().withCause(e).log(
- "Error comparing old and new diff implementations.");
- }
- });
- return result;
- }
-
- private void compareAndLogMetrics(
- Map<String, FileInfo> fileInfoMapOld,
- Map<String, FileInfo> fileInfoMapNew,
- String warningMessage) {
- if (fileInfoMapOld.equals(fileInfoMapNew)) {
- metrics.diffs.increment(Metrics.Status.MATCH);
- return;
- }
- metrics.diffs.increment(Metrics.Status.MISMATCH);
- logger.atWarning().log(
- warningMessage
- + "\n"
- + "Result using old impl: "
- + fileInfoMapOld
- + "\n"
- + "Result using new impl: "
- + fileInfoMapNew);
- }
-}
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonExperimentImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonExperimentImpl.java
deleted file mode 100644
index 81f014d..0000000
--- a/java/com/google/gerrit/server/change/FileInfoJsonExperimentImpl.java
+++ /dev/null
@@ -1,72 +0,0 @@
-// 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.change;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.common.FileInfo;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.server.experiments.ExperimentFeatures;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import java.util.Map;
-import javax.inject.Inject;
-import org.eclipse.jgit.lib.ObjectId;
-
-/**
- * An experimental implementation of FileInfoJson that uses {@link FileInfoJsonNewImpl} if the
- * experiment flag "GerritBackendRequestFeature__use_new_diff_cache" is enabled, or {@link
- * FileInfoJsonOldImpl} otherwise. This would enable a gradual rollout of {@link
- * FileInfoJsonNewImpl}.
- */
-public class FileInfoJsonExperimentImpl implements FileInfoJson {
- @VisibleForTesting
- public static final String NEW_DIFF_CACHE_FEATURE =
- "GerritBackendRequestFeature__use_new_diff_cache";
-
- private final FileInfoJsonOldImpl oldImpl;
- private final FileInfoJsonNewImpl newImpl;
- private final ExperimentFeatures experimentFeatures;
-
- @Inject
- public FileInfoJsonExperimentImpl(
- FileInfoJsonOldImpl oldImpl,
- FileInfoJsonNewImpl newImpl,
- ExperimentFeatures experimentFeatures) {
- this.oldImpl = oldImpl;
- this.newImpl = newImpl;
- this.experimentFeatures = experimentFeatures;
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Change change, ObjectId objectId, @Nullable PatchSet base)
- throws ResourceConflictException, PatchListNotAvailableException {
- return experimentFeatures.isFeatureEnabled(NEW_DIFF_CACHE_FEATURE)
- ? newImpl.getFileInfoMap(change, objectId, base)
- : oldImpl.getFileInfoMap(change, objectId, base);
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Project.NameKey project, ObjectId objectId, int parentNum)
- throws ResourceConflictException, PatchListNotAvailableException {
- return experimentFeatures.isFeatureEnabled(NEW_DIFF_CACHE_FEATURE)
- ? newImpl.getFileInfoMap(project, objectId, parentNum)
- : oldImpl.getFileInfoMap(project, objectId, parentNum);
- }
-}
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonImpl.java
similarity index 95%
rename from java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
rename to java/com/google/gerrit/server/change/FileInfoJsonImpl.java
index 7277404..b729c11 100644
--- a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
+++ b/java/com/google/gerrit/server/change/FileInfoJsonImpl.java
@@ -32,12 +32,12 @@
import org.eclipse.jgit.errors.NoMergeBaseException;
import org.eclipse.jgit.lib.ObjectId;
-/** Implementation of {@link FileInfoJson} using the new diff cache {@link DiffOperations}. */
-public class FileInfoJsonNewImpl implements FileInfoJson {
+/** Implementation of {@link FileInfoJson} using {@link DiffOperations}. */
+public class FileInfoJsonImpl implements FileInfoJson {
private final DiffOperations diffs;
@Inject
- FileInfoJsonNewImpl(DiffOperations diffOperations) {
+ FileInfoJsonImpl(DiffOperations diffOperations) {
this.diffs = diffOperations;
}
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonModule.java b/java/com/google/gerrit/server/change/FileInfoJsonModule.java
index 952b503..b8e05f0 100644
--- a/java/com/google/gerrit/server/change/FileInfoJsonModule.java
+++ b/java/com/google/gerrit/server/change/FileInfoJsonModule.java
@@ -20,7 +20,6 @@
@Override
public void configure() {
- // Binding to the experimental implementation to enable gradual rollout of the new diff cache.
- bind(FileInfoJson.class).to(FileInfoJsonExperimentImpl.class);
+ bind(FileInfoJson.class).to(FileInfoJsonImpl.class);
}
}
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonOldImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonOldImpl.java
deleted file mode 100644
index 0570296..0000000
--- a/java/com/google/gerrit/server/change/FileInfoJsonOldImpl.java
+++ /dev/null
@@ -1,128 +0,0 @@
-// 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.change;
-
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.extensions.common.FileInfo;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.server.patch.PatchList;
-import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListEntry;
-import com.google.gerrit.server.patch.PatchListKey;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.Map;
-import java.util.TreeMap;
-import java.util.concurrent.ExecutionException;
-import org.eclipse.jgit.errors.NoMergeBaseException;
-import org.eclipse.jgit.lib.ObjectId;
-
-/** Implementation of {@link FileInfoJson} using the old diff cache {@link PatchListCache}. */
-@Deprecated
-@Singleton
-class FileInfoJsonOldImpl implements FileInfoJson {
- private final PatchListCache patchListCache;
-
- @Inject
- FileInfoJsonOldImpl(PatchListCache patchListCache) {
- this.patchListCache = patchListCache;
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Change change, ObjectId objectId, @Nullable PatchSet base)
- throws ResourceConflictException, PatchListNotAvailableException {
- ObjectId a = base != null ? base.commitId() : null;
- return toFileInfoMap(change, PatchListKey.againstCommit(a, objectId, Whitespace.IGNORE_NONE));
- }
-
- @Override
- public Map<String, FileInfo> getFileInfoMap(
- Project.NameKey project, ObjectId objectId, int parentNum)
- throws ResourceConflictException, PatchListNotAvailableException {
- PatchListKey key =
- parentNum == 0
- ? PatchListKey.againstDefaultBase(objectId, Whitespace.IGNORE_NONE)
- : PatchListKey.againstParentNum(
- parentNum, objectId, DiffPreferencesInfo.Whitespace.IGNORE_NONE);
- return toFileInfoMap(project, key);
- }
-
- private Map<String, FileInfo> toFileInfoMap(Change change, PatchListKey key)
- throws ResourceConflictException, PatchListNotAvailableException {
- return toFileInfoMap(change.getProject(), key);
- }
-
- Map<String, FileInfo> toFileInfoMap(Project.NameKey project, PatchListKey key)
- throws ResourceConflictException, PatchListNotAvailableException {
- PatchList list;
- try {
- list = patchListCache.get(key, project);
- } catch (PatchListNotAvailableException e) {
- Throwable cause = e.getCause();
- if (cause instanceof ExecutionException) {
- cause = cause.getCause();
- }
- if (cause instanceof NoMergeBaseException) {
- throw new ResourceConflictException(
- String.format("Cannot create auto merge commit: %s", e.getMessage()), e);
- }
- throw e;
- }
-
- Map<String, FileInfo> files = new TreeMap<>();
- for (PatchListEntry e : list.getPatches()) {
- FileInfo fileInfo = new FileInfo();
- fileInfo.status =
- e.getChangeType() != Patch.ChangeType.MODIFIED ? e.getChangeType().getCode() : null;
- fileInfo.oldPath = e.getOldName();
- fileInfo.sizeDelta = e.getSizeDelta();
- fileInfo.size = e.getSize();
- if (e.getPatchType() == Patch.PatchType.BINARY) {
- fileInfo.binary = true;
- } else {
- fileInfo.linesInserted = e.getInsertions() > 0 ? e.getInsertions() : null;
- fileInfo.linesDeleted = e.getDeletions() > 0 ? e.getDeletions() : null;
- }
-
- FileInfo o = files.put(e.getNewName(), fileInfo);
- if (o != null) {
- // This should only happen on a delete-add break created by JGit
- // when the file was rewritten and too little content survived. Write
- // a single record with data from both sides.
- fileInfo.status = Patch.ChangeType.REWRITE.getCode();
- fileInfo.sizeDelta = o.sizeDelta;
- fileInfo.size = o.size;
- if (o.binary != null && o.binary) {
- fileInfo.binary = true;
- }
- if (o.linesInserted != null) {
- fileInfo.linesInserted = o.linesInserted;
- }
- if (o.linesDeleted != null) {
- fileInfo.linesDeleted = o.linesDeleted;
- }
- }
- }
- return files;
- }
-}
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/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index e333824..487e0af 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -277,6 +277,10 @@
.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();
}
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/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 6fa14e1..96b23c8 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -26,21 +26,13 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.restapi.AuthException;
-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.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.change.FileInfoJsonExperimentImpl;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
-import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculatorResult;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
@@ -51,9 +43,7 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
@@ -88,27 +78,6 @@
CurrentUser currentUser);
}
- /** These metrics are temporary for launching the new redesigned diff cache. */
- @Singleton
- static class Metrics {
- final Counter1<String> diffs;
- static final String MATCH = "match";
- static final String MISMATCH = "mismatch";
- static final String ERROR = "error";
-
- @Inject
- Metrics(MetricMaker metricMaker) {
- diffs =
- metricMaker.newCounter(
- "diff/get_diff/dark_launch",
- new Description(
- "Total number of matching, non-matching, or error in diffs in the old and new diff cache implementations.")
- .setRate()
- .setUnit("count"),
- Field.ofString("type", Metadata.Builder::eventType).build());
- }
- }
-
private final GitRepositoryManager repoManager;
private final PatchSetUtil psUtil;
private final Provider<PatchScriptBuilder> builderFactory;
@@ -130,8 +99,6 @@
private ChangeNotes notes;
- private final boolean runNewDiffCache;
-
@AssistedInject
PatchScriptFactory(
GitRepositoryManager grm,
@@ -139,7 +106,6 @@
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ChangeEditUtil editReader,
- ExperimentFeatures experimentFeatures,
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
@@ -165,10 +131,6 @@
this.psb = patchSetB;
this.diffPrefs = diffPrefs;
this.currentUser = currentUser;
-
- this.runNewDiffCache =
- experimentFeatures.isFeatureEnabled(FileInfoJsonExperimentImpl.NEW_DIFF_CACHE_FEATURE);
-
changeId = patchSetB.changeId();
}
@@ -179,7 +141,6 @@
Provider<PatchScriptBuilder> builderFactory,
PatchListCache patchListCache,
ChangeEditUtil editReader,
- ExperimentFeatures experimentFeatures,
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
@@ -205,10 +166,6 @@
this.psb = patchSetB;
this.diffPrefs = diffPrefs;
this.currentUser = currentUser;
-
- this.runNewDiffCache =
- experimentFeatures.isFeatureEnabled(FileInfoJsonExperimentImpl.NEW_DIFF_CACHE_FEATURE);
-
changeId = patchSetB.changeId();
checkArgument(parentNum > 0, "parentNum must be > 0");
}
@@ -246,11 +203,7 @@
}
bId = edit.get().getEditCommit();
}
- return runNewDiffCache
- ? getPatchScriptWithNewDiffCache(git, aId, bId)
- : getPatchScriptWithOldDiffCache(git, aId, bId);
- } catch (PatchListNotAvailableException e) {
- throw new NoSuchChangeException(changeId, e);
+ return getPatchScript(git, aId, bId);
} catch (DiffNotAvailableException e) {
throw new StorageException(e);
} catch (IOException e) {
@@ -268,15 +221,7 @@
}
}
- private PatchScript getPatchScriptWithOldDiffCache(Repository git, ObjectId aId, ObjectId bId)
- throws IOException, PatchListNotAvailableException {
- PatchScriptBuilder patchScriptBuilder = newBuilder();
- PatchList list = listFor(keyFor(aId, bId, diffPrefs.ignoreWhitespace));
- PatchListEntry content = list.get(fileName);
- return patchScriptBuilder.toPatchScriptOld(git, list, content);
- }
-
- private PatchScript getPatchScriptWithNewDiffCache(Repository git, ObjectId aId, ObjectId bId)
+ private PatchScript getPatchScript(Repository git, ObjectId aId, ObjectId bId)
throws IOException, DiffNotAvailableException {
FileDiffOutput fileDiffOutput =
aId == null
@@ -304,17 +249,6 @@
return Optional.of(getCommitId(psb));
}
- private PatchListKey keyFor(ObjectId aId, ObjectId bId, Whitespace whitespace) {
- if (parentNum == 0) {
- return PatchListKey.againstCommit(aId, bId, whitespace);
- }
- return PatchListKey.againstParentNum(parentNum, bId, whitespace);
- }
-
- private PatchList listFor(PatchListKey key) throws PatchListNotAvailableException {
- return patchListCache.get(key, notes.getProjectName());
- }
-
private PatchScriptBuilder newBuilder() {
final PatchScriptBuilder b = builderFactory.get();
b.setDiffPrefs(diffPrefs);
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/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 94dc21f..5002a82 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -170,7 +170,11 @@
+ "selected by users while posting a review.")
.setRate()
.setUnit("count"),
- Field.ofString("type", Metadata.Builder::eventType).build());
+ Field.ofString("type", Metadata.Builder::eventType)
+ .description(
+ "The type of the draft handling option"
+ + " (KEEP, PUBLISH, PUBLISH_ALL_REVISIONS).")
+ .build());
}
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 94becc7..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.")
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index eadd259..58dc0b0 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -50,7 +50,6 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.webui.EditWebLink;
-import com.google.gerrit.server.change.FileInfoJsonExperimentImpl;
import com.google.gerrit.server.patch.DiffOperations;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.inject.Inject;
@@ -93,7 +92,6 @@
@Inject private ProjectOperations projectOperations;
private boolean intraline;
- private boolean useNewDiffCache;
private ObjectId initialCommit;
private ObjectId commit1;
@@ -108,9 +106,6 @@
baseConfig.setString("cache", "diff_intraline", "timeout", "1 minute");
intraline = baseConfig.getBoolean(TEST_PARAMETER_MARKER, "intraline", false);
- useNewDiffCache =
- Arrays.asList(baseConfig.getStringList("experiments", null, "enabled"))
- .contains(FileInfoJsonExperimentImpl.NEW_DIFF_CACHE_FEATURE);
ObjectId headCommit = testRepo.getRepository().resolve("HEAD");
initialCommit = headCommit;
@@ -1358,10 +1353,10 @@
}
@Test
+ @Ignore
public void renamedUnrelatedFileIsIgnored_forPatchSetDiffWithRebase_whenEquallyModifiedInBoth()
throws Exception {
// TODO(ghareeb): fix this test for the new diff cache implementation
- assume().that(useNewDiffCache).isFalse();
Function<String, String> contentModification =
fileContent -> fileContent.replace("1st line\n", "First line\n");
@@ -1452,9 +1447,9 @@
}
@Test
+ @Ignore
public void filesTouchedByPatchSetsAndContainingOnlyRebaseHunksAreIgnored() throws Exception {
// TODO(ghareeb): fix this test for the new diff cache implementation
- assume().that(useNewDiffCache).isFalse();
addModifiedPatchSet(
changeId, FILE_NAME, fileContent -> fileContent.replace("Line 50\n", "Line fifty\n"));
@@ -2865,13 +2860,11 @@
DiffInfo diffInfo = gApi.changes().id(cId).current().file(symlink).diff(initialRev);
assertThat(diffInfo.content).hasSize(1);
assertThat(diffInfo).content().element(0).linesOfB().containsExactly("new content");
- // TODO(ghareeb): remove conditional assertion when new diff cache is fully rolled out.
- assertThat(diffInfo.changeType)
- .isEqualTo(useNewDiffCache ? ChangeType.REWRITE : ChangeType.ADDED);
+ assertThat(diffInfo.changeType).isEqualTo(ChangeType.REWRITE);
}
@Test
- public void renameDeleteByJgit_isIdentifiedAsRewritten6() throws Exception {
+ public void renameDeleteByJgit_isIdentifiedAsRewritten() throws Exception {
String target = "file.txt";
String symlink = "link.lnk";
PushOneCommit push =
@@ -2908,9 +2901,7 @@
"rename to link.lnk");
assertThat(diffInfo.content).hasSize(1);
assertThat(diffInfo).content().element(0).commonLines().containsExactly("content");
- // TODO(ghareeb): remove conditional assertion when new diff cache is fully rolled out.
- assertThat(diffInfo.changeType)
- .isEqualTo(useNewDiffCache ? ChangeType.REWRITE : ChangeType.RENAMED);
+ assertThat(diffInfo.changeType).isEqualTo(ChangeType.REWRITE);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionNewDiffCacheIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionNewDiffCacheIT.java
deleted file mode 100644
index 714bd78..0000000
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionNewDiffCacheIT.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// 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.acceptance.api.revision;
-
-import com.google.gerrit.server.change.FileInfoJsonExperimentImpl;
-import com.google.gerrit.testing.ConfigSuite;
-import org.eclipse.jgit.lib.Config;
-
-/**
- * Runs the {@link RevisionDiffIT} with the list files endpoint using the new diff cache. This is
- * temporary until the new diff cache is fully deployed. The new diff cache will become the default
- * in the future.
- */
-public class RevisionNewDiffCacheIT extends RevisionDiffIT {
- @ConfigSuite.Default
- public static Config newDiffCacheConfig() {
- Config config = new Config();
- config.setString(
- "experiments", null, "enabled", FileInfoJsonExperimentImpl.NEW_DIFF_CACHE_FEATURE);
- return config;
- }
-}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java b/javatests/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java
deleted file mode 100644
index b23f9a3..0000000
--- a/javatests/com/google/gerrit/acceptance/server/change/PatchListCacheIT.java
+++ /dev/null
@@ -1,326 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.acceptance.server.change;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.getChangeId;
-import static com.google.gerrit.acceptance.GitUtil.pushHead;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.cache.Cache;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.Patch.ChangeType;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.server.patch.IntraLineDiff;
-import com.google.gerrit.server.patch.IntraLineDiffArgs;
-import com.google.gerrit.server.patch.IntraLineDiffKey;
-import com.google.gerrit.server.patch.PatchList;
-import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListCacheImpl;
-import com.google.gerrit.server.patch.PatchListEntry;
-import com.google.gerrit.server.patch.PatchListKey;
-import com.google.gerrit.server.patch.Text;
-import com.google.inject.Inject;
-import com.google.inject.name.Named;
-import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
-import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.junit.Test;
-
-@NoHttpd
-public class PatchListCacheIT extends AbstractDaemonTest {
- private static String SUBJECT_1 = "subject 1";
- private static String SUBJECT_2 = "subject 2";
- private static String SUBJECT_3 = "subject 3";
- private static String FILE_A = "a.txt";
- private static String FILE_B = "b.txt";
- private static String FILE_C = "c.txt";
- private static String FILE_D = "d.txt";
-
- @Inject private PatchListCache patchListCache;
-
- @Inject
- @Named("diff")
- private Cache<PatchListKey, PatchList> abstractPatchListCache;
-
- @Test
- public void ensureLegacyBackendIsUsedForFileCacheBackend() throws Exception {
- Field fileCacheField = patchListCache.getClass().getDeclaredField("fileCache");
- fileCacheField.setAccessible(true);
- // Use the reflection to access "localCache" field that is only present in Guava backend.
- assertThat(
- Arrays.stream(fileCacheField.get(patchListCache).getClass().getDeclaredFields())
- .anyMatch(f -> f.getName().equals("localCache")))
- .isTrue();
-
- // intraCache (and all other cache backends) should use Caffeine backend.
- Field intraCacheField = patchListCache.getClass().getDeclaredField("intraCache");
- intraCacheField.setAccessible(true);
- assertThat(
- Arrays.stream(intraCacheField.get(patchListCache).getClass().getDeclaredFields())
- .noneMatch(f -> f.getName().equals("localCache")))
- .isTrue();
- }
-
- @Test
- public void listPatchesAgainstBase() throws Exception {
- commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
- pushHead(testRepo, "refs/heads/master", false);
-
- // Change 1, 1 (+FILE_A, -FILE_D)
- RevCommit c =
- commitBuilder().add(FILE_A, "1").rm(FILE_D).message(SUBJECT_2).insertChangeId().create();
- String id = getChangeId(testRepo, c).get();
- pushHead(testRepo, "refs/for/master", false);
-
- // Compare Change 1,1 with Base (+FILE_A, -FILE_D)
- List<PatchListEntry> entries = getCurrentPatches(id);
- assertThat(entries).hasSize(3);
- assertAdded(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_A, entries.get(1));
- assertDeleted(FILE_D, entries.get(2));
-
- // Change 1,2 (+FILE_A, +FILE_B, -FILE_D)
- amendBuilder().add(FILE_B, "2").create();
- pushHead(testRepo, "refs/for/master", false);
- entries = getCurrentPatches(id);
-
- // Compare Change 1,2 with Base (+FILE_A, +FILE_B, -FILE_D)
- assertThat(entries).hasSize(4);
- assertAdded(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_A, entries.get(1));
- assertAdded(FILE_B, entries.get(2));
- assertDeleted(FILE_D, entries.get(3));
- }
-
- @Test
- public void listPatchesAgainstBaseWithRebase() throws Exception {
- commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
- pushHead(testRepo, "refs/heads/master", false);
-
- // Change 1,1 (+FILE_A, -FILE_D)
- RevCommit c = commitBuilder().add(FILE_A, "1").rm(FILE_D).message(SUBJECT_2).create();
- String id = getChangeId(testRepo, c).get();
- pushHead(testRepo, "refs/for/master", false);
- List<PatchListEntry> entries = getCurrentPatches(id);
- assertThat(entries).hasSize(3);
- assertAdded(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_A, entries.get(1));
- assertDeleted(FILE_D, entries.get(2));
-
- // Change 2,1 (+FILE_B)
- testRepo.reset("HEAD~1");
- commitBuilder().add(FILE_B, "2").message(SUBJECT_3).create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Change 1,2 (+FILE_A, -FILE_D))
- testRepo.cherryPick(c);
- pushHead(testRepo, "refs/for/master", false);
-
- // Compare Change 1,2 with Base (+FILE_A, -FILE_D))
- entries = getCurrentPatches(id);
- assertThat(entries).hasSize(3);
- assertAdded(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_A, entries.get(1));
- assertDeleted(FILE_D, entries.get(2));
- }
-
- @Test
- public void listPatchesAgainstOtherPatchSet() throws Exception {
- commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
- pushHead(testRepo, "refs/heads/master", false);
-
- // Change 1,1 (+FILE_A, +FILE_C, -FILE_D)
- RevCommit a =
- commitBuilder().add(FILE_A, "1").add(FILE_C, "3").rm(FILE_D).message(SUBJECT_2).create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Change 1,2 (+FILE_A, +FILE_B, -FILE_D)
- RevCommit b = amendBuilder().add(FILE_B, "2").rm(FILE_C).create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Compare Change 1,1 with Change 1,2 (+FILE_B, -FILE_C)
- List<PatchListEntry> entries = getPatches(a, b);
- assertThat(entries).hasSize(3);
- assertModified(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_B, entries.get(1));
- assertDeleted(FILE_C, entries.get(2));
-
- // Compare Change 1,2 with Change 1,1 (-FILE_B, +FILE_C)
- List<PatchListEntry> entriesReverse = getPatches(b, a);
- assertThat(entriesReverse).hasSize(3);
- assertModified(Patch.COMMIT_MSG, entriesReverse.get(0));
- assertDeleted(FILE_B, entriesReverse.get(1));
- assertAdded(FILE_C, entriesReverse.get(2));
- }
-
- @Test
- public void listPatchesAgainstOtherPatchSetWithRebase() throws Exception {
- commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
- pushHead(testRepo, "refs/heads/master", false);
-
- // Change 1,1 (+FILE_A, -FILE_D)
- RevCommit a = commitBuilder().add(FILE_A, "1").rm(FILE_D).message(SUBJECT_2).create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Change 2,1 (+FILE_B)
- testRepo.reset("HEAD~1");
- commitBuilder().add(FILE_B, "2").message(SUBJECT_3).create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Change 1,2 (+FILE_A, +FILE_C, -FILE_D)
- testRepo.cherryPick(a);
- RevCommit b = amendBuilder().add(FILE_C, "2").create();
- pushHead(testRepo, "refs/for/master", false);
-
- // Compare Change 1,1 with Change 1,2 (+FILE_C)
- List<PatchListEntry> entries = getPatches(a, b);
- assertThat(entries).hasSize(2);
- assertModified(Patch.COMMIT_MSG, entries.get(0));
- assertAdded(FILE_C, entries.get(1));
-
- // Compare Change 1,2 with Change 1,1 (-FILE_C)
- List<PatchListEntry> entriesReverse = getPatches(b, a);
- assertThat(entriesReverse).hasSize(2);
- assertModified(Patch.COMMIT_MSG, entriesReverse.get(0));
- assertDeleted(FILE_C, entriesReverse.get(1));
- }
-
- @Test
- public void harmfulMutationsOfEditsAreNotPossibleForPatchListEntry() throws Exception {
- RevCommit commit =
- commitBuilder().add("a.txt", "First line\nSecond line\n").message(SUBJECT_1).create();
- pushHead(testRepo, "refs/heads/master", false);
-
- PatchListKey diffKey = PatchListKey.againstDefaultBase(commit.copy(), Whitespace.IGNORE_NONE);
- PatchList patchList = patchListCache.get(diffKey, project);
-
- PatchListEntry patchListEntry = getEntryFor(patchList, "a.txt");
- Edit outputEdit = Iterables.getOnlyElement(patchListEntry.getEdits());
- Edit originalEdit =
- new Edit(
- outputEdit.getBeginA(),
- outputEdit.getEndA(),
- outputEdit.getBeginB(),
- outputEdit.getEndB());
-
- outputEdit.shift(5);
-
- assertThat(patchListEntry.getEdits()).containsExactly(originalEdit);
- }
-
- @Test
- public void harmfulMutationsOfEditsAreNotPossibleForIntraLineDiffArgsAndCachedValue() {
- String a = "First line\nSecond line\n";
- String b = "1st line\n2nd line\n";
- Text aText = new Text(a.getBytes(UTF_8));
- Text bText = new Text(b.getBytes(UTF_8));
- Edit inputEdit = new Edit(0, 2, 0, 2);
- List<Edit> inputEdits = new ArrayList<>(ImmutableList.of(inputEdit));
- Set<Edit> inputEditsDueToRebase = new HashSet<>(ImmutableSet.of(inputEdit));
-
- IntraLineDiffKey diffKey =
- IntraLineDiffKey.create(ObjectId.zeroId(), ObjectId.zeroId(), Whitespace.IGNORE_NONE);
- IntraLineDiffArgs diffArgs =
- IntraLineDiffArgs.create(
- aText,
- bText,
- inputEdits,
- inputEditsDueToRebase,
- project,
- ObjectId.zeroId(),
- "file.txt");
- IntraLineDiff intraLineDiff = patchListCache.getIntraLineDiff(diffKey, diffArgs);
-
- Edit outputEdit = Iterables.getOnlyElement(intraLineDiff.getEdits());
-
- outputEdit.shift(5);
- inputEdit.shift(7);
- inputEdits.add(new Edit(43, 47, 50, 51));
- inputEditsDueToRebase.add(new Edit(53, 57, 60, 61));
-
- Edit originalEdit = new Edit(0, 2, 0, 2);
- assertThat(diffArgs.edits()).containsExactly(originalEdit);
- assertThat(diffArgs.editsDueToRebase()).containsExactly(originalEdit);
- assertThat(intraLineDiff.getEdits()).containsExactly(originalEdit);
- }
-
- @Test
- public void largeObjectTombstoneGetsCached() {
- PatchListKey key = PatchListKey.againstDefaultBase(ObjectId.zeroId(), Whitespace.IGNORE_ALL);
- PatchListCacheImpl.LargeObjectTombstone tombstone =
- new PatchListCacheImpl.LargeObjectTombstone();
- abstractPatchListCache.put(key, tombstone);
- assertThat(abstractPatchListCache.getIfPresent(key)).isSameInstanceAs(tombstone);
- }
-
- private static void assertAdded(String expectedNewName, PatchListEntry e) {
- assertName(expectedNewName, e);
- assertThat(e.getChangeType()).isEqualTo(ChangeType.ADDED);
- }
-
- private static void assertModified(String expectedNewName, PatchListEntry e) {
- assertName(expectedNewName, e);
- assertThat(e.getChangeType()).isEqualTo(ChangeType.MODIFIED);
- }
-
- private static void assertDeleted(String expectedNewName, PatchListEntry e) {
- assertName(expectedNewName, e);
- assertThat(e.getChangeType()).isEqualTo(ChangeType.DELETED);
- }
-
- private static void assertName(String expectedNewName, PatchListEntry e) {
- assertThat(e.getNewName()).isEqualTo(expectedNewName);
- assertThat(e.getOldName()).isNull();
- }
-
- private List<PatchListEntry> getCurrentPatches(String changeId) throws Exception {
- return patchListCache.get(getKey(null, getCurrentRevisionId(changeId)), project).getPatches();
- }
-
- private List<PatchListEntry> getPatches(ObjectId revisionIdA, ObjectId revisionIdB)
- throws Exception {
- return patchListCache.get(getKey(revisionIdA, revisionIdB), project).getPatches();
- }
-
- private PatchListKey getKey(ObjectId revisionIdA, ObjectId revisionIdB) {
- return PatchListKey.againstCommit(revisionIdA, revisionIdB, Whitespace.IGNORE_NONE);
- }
-
- private ObjectId getCurrentRevisionId(String changeId) throws Exception {
- return ObjectId.fromString(gApi.changes().id(changeId).get().currentRevision);
- }
-
- private static PatchListEntry getEntryFor(PatchList patchList, String filePath) {
- Optional<PatchListEntry> patchListEntry =
- patchList.getPatches().stream()
- .filter(entry -> entry.getNewName().equals(filePath))
- .findAny();
- return patchListEntry.orElseThrow(
- () -> new IllegalStateException("No PatchListEntry for " + filePath + " exists"));
- }
-}
