Merge "Expose auto-value-gson in plugin API"
* submodules:
* Update plugins/replication from branch 'master'
to 93e61dc64debe42eab454e6c268f9c4ee22a78bc
- Bazel: Consume auto-value-gson from plugin API
Depends-On: https://gerrit-review.googlesource.com/c/gerrit/+/301238
Change-Id: I3e0569730b89f80c1209b4370ddb1c8367375e86
diff --git a/.bazelrc b/.bazelrc
index 6e26484..6a3f06e 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -11,6 +11,8 @@
# this flag here once flipped in Bazel again.
build --incompatible_strict_action_env
+build --announce_rc
+
test --build_tests_only
test --test_output=errors
test --java_toolchain=//tools:error_prone_warnings_toolchain_java11
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
index c6d9fb4..8088b66 100644
--- a/Documentation/config-accounts.txt
+++ b/Documentation/config-accounts.txt
@@ -293,12 +293,12 @@
External IDs are stored as Git Notes in the `All-Users` repository. The
name of the notes branch is `refs/meta/external-ids`.
-As note key the SHA1 of the external ID key is used, for example the key
+As note key the SHA-1 of the external ID key is used, for example the key
for the external ID `username:jdoe` is `e0b751ae90ef039f320e097d7d212f490e933706`.
This ensures that an external ID is used only once (e.g. an external ID can
never be assigned to multiple accounts at a point in time).
-The following commands show how to find the SHA1 of an external ID:
+The following commands show how to find the SHA-1 of an external ID:
----
$ echo -n 'gerrit:jdoe' | shasum
@@ -310,7 +310,7 @@
[IMPORTANT]
If the external ID key is changed manually you must adapt the note key
-to the new SHA1, otherwise the external ID becomes inconsistent and is
+to the new SHA-1, otherwise the external ID becomes inconsistent and is
ignored by Gerrit.
The note content is a Git config file:
@@ -322,7 +322,7 @@
password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
----
-Once SHA1 of an external ID is known the following command can be used to
+Once SHA-1 of an external ID is known the following command can be used to
show the content of the note:
----
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4ce5360..3ad4401 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -815,6 +815,7 @@
* `"groups"`: default is unlimited
* `"groups_byname"`: default is unlimited
* `"groups_byuuid"`: default is unlimited
+* `"groups_byuuid_persisted"`: default is `1g` (1 GiB of disk space)
* `"plugin_resources"`: default is 2m (2 MiB of memory)
+
@@ -1038,6 +1039,17 @@
External group membership obtained from LDAP is cached under
`"ldap_groups"`.
+cache `"groups_byuuid_persisted"`::
++
+Caches the basic group information of internal groups by group UUID,
+including the group owner, name, and description.
++
+This is the persisted version of `groups_byuuid` cache. The intention of this
+cache is to have an in-memory size of 0.
++
+External group membership obtained from LDAP is cached under
+`"ldap_groups"`.
+
cache `"groups_bymember"`::
+
Caches the groups which contain a specific member (account). If direct
@@ -2439,7 +2451,7 @@
at a specific commit when `gitweb.type` is set to `custom`.
+
Valid replacements are `${project}` for the project name in Gerrit
-and `${commit}` for the SHA1 hash for the commit.
+and `${commit}` for the SHA-1 hash for the commit.
[[gitweb.project]]gitweb.project::
+
@@ -2471,7 +2483,7 @@
is set to `custom`.
+
Valid replacements are `${project}` for the project name in Gerrit
-and `${commit}` for the SHA1 hash for the commit.
+and `${commit}` for the SHA-1 hash for the commit.
[[gitweb.file]]gitweb.file::
+
@@ -2480,7 +2492,7 @@
set to `custom`.
+
Valid replacements are `${project}` for the project name in Gerrit,
-`${file}` for the file name and `${commit}` for the SHA1 hash for
+`${file}` for the file name and `${commit}` for the SHA-1 hash for
the commit.
[[gitweb.filehistory]]gitweb.filehistory::
@@ -4184,7 +4196,7 @@
very CPU-heavy operation. For non public Gerrit-servers this check may
be overkill.
+
-Only disable this check if you trust the clients not to forge SHA1
+Only disable this check if you trust the clients not to forge SHA-1
references to access commits intended to be hidden from the user.
+
Default is true.
@@ -4825,6 +4837,16 @@
replicate = replication start
----
+[[ssh]]
+=== Section ssh
+
+[[ssh.clientImplementation]]ssh.clientImplementation::
++
+JCraft JSch client is supported in addition to Apache MINA SSH client.
+To use JSch client set the value to `JSCH`.
++
+By default, `APACHE`.
+
[[sshd]]
=== Section sshd
diff --git a/Documentation/config-groups.txt b/Documentation/config-groups.txt
index afabbfc..0917515 100644
--- a/Documentation/config-groups.txt
+++ b/Documentation/config-groups.txt
@@ -64,7 +64,7 @@
The format of this map is as follows:
-* keys are the normal SHA1 of the group name
+* keys are the normal SHA-1 of the group name
* values are blobs that look like
+
----
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 56d26dc..b6184d7 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -353,7 +353,7 @@
If true, all scores for the label are copied forward when a new patch
set is uploaded that has the same parent tree, code delta, and commit
message as the previous patch set. This means that only the patch
-set SHA1 is different. This can be used to enable sticky
+set SHA-1 is different. This can be used to enable sticky
approvals, reducing turn-around for this special case.
It is recommended to leave this enabled for both Verified and
Code-Review labels.
diff --git a/Documentation/cross-repository-changes.txt b/Documentation/cross-repository-changes.txt
index 136219c..53fd5cd 100644
--- a/Documentation/cross-repository-changes.txt
+++ b/Documentation/cross-repository-changes.txt
@@ -82,7 +82,8 @@
To better underestand this behavior, consider this following example.
-==== Example Submission[[example_submission]]
+[[example_submission]]
+=== Example Submission
image::images/cross-repository-changes-example.png[width=600]
@@ -167,7 +168,8 @@
image::images/cross-repository-changes-submit-topic.png[width=600]
-== Reverting a Submission[[reverting]]
+[[reverting]]
+== Reverting a Submission
After a topic is submitted, you can revert all or one of the changes by clicking
the *REVERT* button on any change.
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index adc9be5..747f761 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -318,6 +318,18 @@
bazel test //javatests/com/google/gerrit/acceptance/rest/account:rest_account
----
+To run SSH tests using JSch ssh client:
+
+----
+ bazel test --test_env=SSH_CLIENT_IMPLEMENTATION=JSCH //...
+----
+
+To run SSH tests using Apache MINA ssh client:
+
+----
+ bazel test --test_env=SSH_CLIENT_IMPLEMENTATION=APACHE //...
+----
+
To run only tests that do not use SSH:
----
diff --git a/Documentation/dev-crafting-changes.txt b/Documentation/dev-crafting-changes.txt
index 7935f30..15bf785 100644
--- a/Documentation/dev-crafting-changes.txt
+++ b/Documentation/dev-crafting-changes.txt
@@ -306,6 +306,30 @@
and it also makes `git revert` more useful.
* Use topics to link your separate changes together.
+[[opportunistic-refactoring]]
+== Opportunistic Refactoring
+
+Opportunistic Refactoring is a terminology
+link:https://martinfowler.com/bliki/OpportunisticRefactoring.html[used by Martin Fowler,role=external,window=_blank]
+also known as the "boy scout rule" of the software developer:
+"always leave the code behind in a better state than you found it."
+
+In practice, this rule means you should not add technical debt in the code while
+implementing a new feature or fixing a bug. If you or a reviewer find an
+opportunity to clean up the code during implementation or review of your change,
+take the time to do a little cleanup to improve the overall code base.
+
+When approaching refactoring, keep in mind that changes should do one thing
+(<<change-size,see change size section above>>). If a change you're making
+requires cleanup/refactoring, it is best to do that cleanup in a preparatory and
+separate change. Likewise, if during review for a functional change, an
+opportunity for cleanup/refactoring is discovered, then it is preferable to do
+the cleanup first in a separate change so as to improve the reviewability of the
+functional change.
+
+Reviewers should keep in mind the scope of the change under review and ensure
+suggested refactoring is aligned with that scope.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-design.txt b/Documentation/dev-design.txt
index 203b368..a41c9ea 100644
--- a/Documentation/dev-design.txt
+++ b/Documentation/dev-design.txt
@@ -182,7 +182,7 @@
advertisement if the user lacks read permissions
* Uploads through the git wire protocol must refuse commits that are
- based on SHA1s for data that the user can't see.
+ based on SHA-1s for data that the user can't see.
* Tags are only visible if their commits are visible to user through a
non-tag reference.
@@ -195,7 +195,7 @@
=== Indexing
Almost all data is stored as Git, but Git only supports fast lookup by
-SHA1 or by ref (branch) name. Therefore Gerrit also has an indexing
+SHA-1 or by ref (branch) name. Therefore Gerrit also has an indexing
system (powered by Lucene by default) for other types of queries.
There are 4 indices:
@@ -204,8 +204,8 @@
* Group index - find groups by name, owner, description etc.
* Change index - find changes by file, status, modification date etc.
-The base entities are characterized by SHA1s. Storing the
-characterizing SHA1s allows detection of stale index entries.
+The base entities are characterized by SHA-1s. Storing the
+characterizing SHA-1s allows detection of stale index entries.
== Plug-in architecture
@@ -397,7 +397,7 @@
* Caching: metadata is stored in Git, which is relatively expensive to
access. This is sped up by multiple caches. Metadata entities are
stored in Git, and can therefore be seen as immutable values keyed
- by SHA1, which is very amenable to caching. All SHA1 keyed caches
+ by SHA-1, which is very amenable to caching. All SHA-1 keyed caches
can be persisted on local disk.
The size (memory, disk) of these caches should be adapted to the
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 0db6026..fb17e5c 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2230,7 +2230,7 @@
light-weight plugin that links commits to external
tools (GitBlit, CGit, company specific resources etc).
-PatchSetWebLinks will appear to the right of the commit-SHA1 in the UI.
+PatchSetWebLinks will appear to the right of the commit-SHA-1 in the UI.
[source, java]
----
@@ -2256,7 +2256,7 @@
}
----
-ParentWebLinks will appear to the right of the SHA1 of the parent
+ParentWebLinks will appear to the right of the SHA-1 of the parent
revisions in the UI. The implementation should in most use cases direct
to the same external service as PatchSetWebLink; it is provided as a
separate interface because not all users want to have links for the
diff --git a/Documentation/glossary.txt b/Documentation/glossary.txt
new file mode 100644
index 0000000..2b40b5b
--- /dev/null
+++ b/Documentation/glossary.txt
@@ -0,0 +1,50 @@
+:linkattrs:
+= Glossary
+
+[[event]]
+== Event
+
+It refers to the link:https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/events/Event.java[com.google.gerrit.server.events.Event]
+base abstract class representing any possible action that is generated or
+received in a Gerrit instance. Actions can be associated with change set status
+updates, project creations, indexing of changes, etc.
+
+[[event-broker]]
+== Event broker
+
+Distributes Gerrit Events to listeners if they are allowed to see them.
+
+[[event-dispatcher]]
+== Event dispatcher
+
+Interface for posting events to the Gerrit event system. Implemented by default
+by link:https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/events/EventBroker.java[com.google.gerrit.server.events.EventBroker].
+It can be implemented by plugins and allows to influence how events are managed.
+
+[[event-hierarchy]]
+== Event hierarchy
+
+Hierarchy of events representing anything that can happen in Gerrit.
+
+[[event-listener]]
+== Event listener
+
+API for listening to Gerrit events from plugins, without having any
+visibility restrictions.
+
+[[stream-events]]
+== Stream events
+
+Command that allows a user via CLI or a plugin to receive in a sequential way
+some events that are generated in Gerrit. The consumption of the stream by default
+is available via SSH connection.
+However, plugins can provide an alternative implementation of the event
+brokering by sending them over a reliable messaging queueing system (RabbitMQ)
+or a pub-sub (Kafka).
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 164039b..e56e7ca 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -87,6 +87,7 @@
. link:concept-changes.html[Changes]
. link:concept-refs-for-namespace.html[The refs/for Namespace]
. link:concept-patch-sets.html[Patch Sets]
+. link:glossary.html[Glossary]
== Resources
* link:licenses.html[Licenses and Notices]
diff --git a/Documentation/intro-gerrit-walkthrough-github.txt b/Documentation/intro-gerrit-walkthrough-github.txt
index f16155b..8f3ff88 100644
--- a/Documentation/intro-gerrit-walkthrough-github.txt
+++ b/Documentation/intro-gerrit-walkthrough-github.txt
@@ -206,8 +206,8 @@
When you `git commit --amend` to iterate on your change, you might be worried that
you are changing your previous commit and may thus lose that state of your work.
However, here the Change-Id appended to your commit message comes into play.
-While the SHA1 hash of your change (the commit ID used by Git) might change, the
-Change-Id stays the same (in fact it is the SHA1 hash of the very first version
+While the SHA-1 hash of your change (the commit ID used by Git) might change, the
+Change-Id stays the same (in fact it is the SHA-1 hash of the very first version
of that commit). When this amended commit is uploaded to the Gerrit server,
Gerrit knows that this commit is really an iteration of that previous commit
(and the associated review) and will preserve both, the old and the new state.
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt
index 737f2a4..63601d2 100644
--- a/Documentation/licenses.txt
+++ b/Documentation/licenses.txt
@@ -84,6 +84,7 @@
* mime4j:dom
* mina:core
* mina:sshd
+* mina:sshd-sftp
* openid:consumer
* openid:nekohtml
* openid:xerces
@@ -2348,6 +2349,7 @@
* jgit
* jgit-archive
* jgit-servlet
+* jgit-ssh-apache
[[jgit_license]]
----
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index cde6864..8931348 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -186,7 +186,8 @@
* `git/upload-pack/phase_compressing`: Time spent in the 'Compressing...' phase.
* `git/upload-pack/phase_writing`: Time spent transferring bytes to client.
* `git/upload-pack/pack_bytes`: Distribution of sizes of packs sent to clients.
-* `git/auto-merge/num_operations`: Number of attempted auto merge operations and context.
+* `git/auto-merge/num_operations`: Number of auto merge operations and context.
+* `git/auto-merge/latency`: Latency of auto merge operations and context.
=== BatchUpdate
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ccffa44..516b2fe 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -565,10 +565,73 @@
----
Historical state of the change can be retrieved by specifying the
-`meta=SHA1` parameter. This will use a historical NoteDb snapshot to
-populate ChangeInfo. If the SHA1 is not reachable as a NoteDb state,
+`meta=SHA-1` parameter. This will use a historical NoteDb snapshot to
+populate ChangeInfo. If the SHA-1 is not reachable as a NoteDb state,
status code 412 is returned.
+----
+
+[[get-meta-diff]]
+=== Get Meta Diff
+--
+'GET /changes/link:#change-id[\{change-id\}]/meta_diff/?old=SHA-1&meta=SHA-1'
+--
+
+Retrieves the difference between two historical states of a change
+by specifying the `old=SHA-1` and the `meta=SHA-1` parameters.
+
+If the `old` parameter is not provided, the parent of the `meta`
+SHA-1 is used. If the `meta` parameter is not provided, the current
+state of the change is used. If neither are provided, the
+difference between the current state of the change and its previous
+state is returned.
+
+Additional fields can be obtained by adding `o` parameters, analogous
+to link:#get-change[Get Change], and the same concerns for Get Change hold for
+this endpoint too. Fields are described in link:#list-changes[Query Changes].
+
+.Request
+----
+ GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/meta_diff?old=b083abc34eb6dbdb9e154ba092fc198000e997b4&meta=63b81f2bde703ae07787a940e8fdf9a1828605b1 HTTP/1.0
+----
+
+As a response, two link:#change-info[ChangeInfo] entities are returned
+that describe information added and removed from the `old` change state.
+Only fields that differ between the change's two states are returned.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "added": {
+ "attention_set": [
+ {
+ "account": {
+ "name": "John Doe"
+ },
+ "last_update": "2013-02-21 11:16:36.775000000",
+ "reason": "reviewer or cc replied"
+ }
+ ]
+ "updated": "2013-02-21 11:16:36.775000000",
+ "topic": "new-topic"
+ },
+ "removed": {
+ "updated": "2013-02-20 12:05:34.111000000",
+ "topic": "old-topic"
+ }
+ }
+----
+
+If the provided SHA-1 for `meta` is not reachable as a NoteDb
+state, the status code 412 is returned. If the SHA-1 for `old`
+is not reachable, the difference between the change at state
+`meta` and an empty change is returned.
+----
[[get-change-detail]]
=== Get Change Detail
@@ -1140,7 +1203,7 @@
--
Check if the given change is a pure revert of the change it references in `revertOf`.
-Optionally, the query parameter `o` can be passed in to specify a commit (SHA1 in
+Optionally, the query parameter `o` can be passed in to specify a commit (SHA-1 in
40 digit hex representation) to check against. It takes precedence over `revertOf`.
If the change has no reference in `revertOf`, the parameter is mandatory.
@@ -6478,7 +6541,7 @@
(in which case it will only contain a key for the current revision) or
if link:#all-revisions[all revisions] are requested.
|`meta_rev_id` |optional|
-The SHA1 of the NoteDb meta ref.
+The SHA-1 of the NoteDb meta ref.
|`tracking_ids` |optional|
A list of link:#tracking-id-info[TrackingIdInfo] entities describing
references to external tracking systems. Only set if
@@ -6716,7 +6779,7 @@
Contains the link:rest-api-changes.html#change-message-info[id] of the change
message that this comment is linked to.
|`commit_id` |optional|
-Hex commit SHA1 (40 characters string) of the commit of the patchset to which
+Hex commit SHA-1 (40 characters string) of the commit of the patchset to which
this comment applies.
|`context_lines` |optional|
A list of link:#context-line[ContextLine] containing the lines of the source
@@ -7508,7 +7571,7 @@
|===========================
|Field Name ||Description
|`base` |optional|
-The new parent revision. This can be a ref or a SHA1 to a concrete patchset. +
+The new parent revision. This can be a ref or a SHA-1 to a concrete patchset. +
Alternatively, a change number can be specified, in which case the current
patch set is inferred. +
Empty string is used for rebasing directly on top of the target branch,
diff --git a/Documentation/user-inline-edit.txt b/Documentation/user-inline-edit.txt
index cd26792..4a9d18f 100644
--- a/Documentation/user-inline-edit.txt
+++ b/Documentation/user-inline-edit.txt
@@ -38,11 +38,11 @@
* Select branch for new change: Specify the destination branch of the
change.
- * Provide base commit SHA1 for change: Leave this field blank.
+ * Provide base commit SHA-1 for change: Leave this field blank.
+
-IMPORTANT: Git uses a unique SHA1 value to identify each and every commit (in
-other words, each Git commit generates a new SHA1 hash). This value differs
+IMPORTANT: Git uses a unique SHA-1 value to identify each and every commit (in
+other words, each Git commit generates a new SHA-1 hash). This value differs
from a Gerrit Change-Id, which is used by Gerrit to uniquely identify a
change. The Gerrit Change-Id remains static throughout the life of a Gerrit
change.
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 52c282e..377012a 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -35,7 +35,7 @@
|=============================================================
For change searches (i.e. those using a numerical id, Change-Id, or commit
-SHA1), if the search results in a single change that change will be
+SHA-1), if the search results in a single change that change will be
presented instead of a list.
For more predictable results, use explicit search operators as described
@@ -173,9 +173,9 @@
Changes that have been, or need to be, reviewed by a user in 'GROUP'.
[[commit]]
-commit:'SHA1'::
+commit:'SHA-1'::
+
-Changes where 'SHA1' is one of the patch sets of the change.
+Changes where 'SHA-1' is one of the patch sets of the change.
[[project]]
project:'PROJECT', p:'PROJECT'::
diff --git a/WORKSPACE b/WORKSPACE
index cffbc8d..8dad0f9 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -66,8 +66,8 @@
http_archive(
name = "build_bazel_rules_nodejs",
- sha256 = "fcc6dccb39ca88d481224536eb8f9fa754619676c6163f87aa6af94059b02b12",
- urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.2.0/rules_nodejs-3.2.0.tar.gz"],
+ sha256 = "dd7ea7efda7655c218ca707f55c3e1b9c68055a70c31a98f264b3445bc8f4cb1",
+ urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.2.3/rules_nodejs-3.2.3.tar.gz"],
)
# Golang support for PolyGerrit local dev server.
diff --git a/contrib/find-duplicate-usernames.sh b/contrib/find-duplicate-usernames.sh
new file mode 100755
index 0000000..b59e5be
--- /dev/null
+++ b/contrib/find-duplicate-usernames.sh
@@ -0,0 +1,56 @@
+#!/bin/bash
+# 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.
+usage() {
+ f="$(basename -- $0)"
+ cat <<EOF
+Usage:
+ cd /path/to/All-Users.git
+ "$f [username|gerrit|external]"
+
+This script finds duplicate usernames only differing in case in the given
+account schema ("username", "gerrit" or "external") and their respective accountIds.
+EOF
+ exit 1
+}
+
+if [[ "$#" -ne "1" ]] || ! [[ "$1" =~ ^(gerrit|username|external)$ ]]; then
+ usage
+fi
+
+# 1. find lines with user name and subsequent line in external-ids notes branch
+# example output of git grep -A1 "\[externalId \"username:" refs/meta/external-ids:
+# refs/meta/external-ids:00/1d/abd037e437f71d42134e6ad532a06948a2ba:[externalId "username:johndoe"]
+# refs/meta/external-ids:00/1d/abd037e437f71d42134e6ad532a06948a2ba- accountId = 1000815
+# --
+# refs/meta/external-ids:00/1f/0270fc2a6fc3a2439c454c8ab0c75323fdb0:[externalId "username:JohnDoe"]
+# refs/meta/external-ids:00/1f/0270fc2a6fc3a2439c454c8ab0c75323fdb0- accountId = 1000816
+# --
+# 2. remove group separators
+# 3. remove line break between user name and accountId lines
+# 4. unify separators to ":"
+# 5. cut on ":", select username and accountId fields
+# 6. sort case-insensitive
+# 7. flip columns
+# 8. uniq case-insensitive, only show duplicates, avoid comparing first field
+# 9. flip columns back
+git grep -A1 "\[externalId \"$1:" refs/meta/external-ids \
+ | sed -E "/$1/,/accountId/!d" \
+ | paste -d ' ' - - \
+ | tr \"= : \
+ | cut -d: --output-delimiter="" -f 5,8 \
+ | sort -f \
+ | sed -E "s/(.*) (.*)/\2 \1/" \
+ | uniq -Di -f1 \
+ | sed -E "s/(.*) (.*)/\2 \1/"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 722d9f3..b05050d 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -48,6 +48,7 @@
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.acceptance.testsuite.request.SshSessionFactory;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Account;
@@ -144,7 +145,6 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Provider;
-import com.jcraft.jsch.KeyPair;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
@@ -156,6 +156,7 @@
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.security.KeyPair;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.ArrayList;
@@ -563,7 +564,7 @@
&& (adminSshSession == null || userSshSession == null)) {
// Create Ssh sessions
KeyPair adminKeyPair = sshKeys.getKeyPair(admin);
- GitUtil.initSsh(adminKeyPair);
+ SshSessionFactory.initSsh(adminKeyPair);
Context ctx = newRequestContext(user);
atrScope.set(ctx);
userSshSession = ctx.getSession();
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index 28f67b8..5ee1a08 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -40,6 +40,7 @@
"//lib:guava-retrying",
"//lib:jgit",
"//lib:jgit-ssh-jsch",
+ "//lib:jgit-ssh-apache",
"//lib:jsch",
"//lib/commons:compress",
"//lib/commons:lang",
@@ -52,6 +53,7 @@
"//lib/mina:sshd",
"//lib:guava",
"//lib/bouncycastle:bcpg",
+ "//lib/bouncycastle:bcpkix",
"//lib/bouncycastle:bcprov",
"//prolog:gerrit-prolog-common",
]
diff --git a/java/com/google/gerrit/acceptance/GitUtil.java b/java/com/google/gerrit/acceptance/GitUtil.java
index ae72793..94d329d 100644
--- a/java/com/google/gerrit/acceptance/GitUtil.java
+++ b/java/com/google/gerrit/acceptance/GitUtil.java
@@ -20,17 +20,11 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.primitives.Ints;
-import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Project;
-import com.jcraft.jsch.JSch;
-import com.jcraft.jsch.JSchException;
-import com.jcraft.jsch.KeyPair;
-import com.jcraft.jsch.Session;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
-import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.api.FetchCommand;
@@ -47,41 +41,15 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.FetchResult;
-import org.eclipse.jgit.transport.JschConfigSessionFactory;
-import org.eclipse.jgit.transport.OpenSshConfig.Host;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteRefUpdate;
-import org.eclipse.jgit.transport.SshSessionFactory;
import org.eclipse.jgit.util.FS;
public class GitUtil {
private static final AtomicInteger testRepoCount = new AtomicInteger();
private static final int TEST_REPO_WINDOW_DAYS = 2;
- public static void initSsh(KeyPair keyPair) {
- final Properties config = new Properties();
- config.put("StrictHostKeyChecking", "no");
- JSch.setConfig(config);
-
- // register a JschConfigSessionFactory that adds the private key as identity
- // to the JSch instance of JGit so that SSH communication via JGit can
- // succeed
- SshSessionFactory.setInstance(
- new JschConfigSessionFactory() {
- @Override
- protected void configure(Host hc, Session session) {
- try {
- final JSch jsch = getJSch(hc, FS.DETECTED);
- jsch.addIdentity(
- "KeyPair", TestSshKeys.privateKey(keyPair), keyPair.getPublicKeyBlob(), null);
- } catch (JSchException e) {
- throw new RuntimeException(e);
- }
- }
- });
- }
-
/**
* Create a new {@link TestRepository} with a distinct commit clock.
*
diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index 2a3a35f..83c63f9 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -338,7 +338,7 @@
.project(req.project)
.availableTokens(REPOSITORY_SIZE_GROUP);
availableTokens.throwOnError();
- availableTokens.availableTokens().ifPresent(v -> rp.setMaxObjectSizeLimit(v));
+ availableTokens.availableTokens().ifPresent(rp::setMaxPackSizeLimit);
ImmutableList<PostReceiveHook> hooks =
ImmutableList.<PostReceiveHook>builder()
diff --git a/java/com/google/gerrit/acceptance/SshSession.java b/java/com/google/gerrit/acceptance/SshSession.java
index 6698657..054e523 100644
--- a/java/com/google/gerrit/acceptance/SshSession.java
+++ b/java/com/google/gerrit/acceptance/SshSession.java
@@ -14,27 +14,18 @@
package com.google.gerrit.acceptance;
-import static com.google.common.base.Preconditions.checkState;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.gerrit.acceptance.testsuite.account.TestAccount;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
-import com.jcraft.jsch.ChannelExec;
-import com.jcraft.jsch.JSch;
-import com.jcraft.jsch.KeyPair;
-import com.jcraft.jsch.Session;
-import java.io.InputStream;
import java.net.InetSocketAddress;
-import java.util.Scanner;
-public class SshSession {
- private final TestSshKeys sshKeys;
- private final InetSocketAddress addr;
- private final TestAccount account;
- private Session session;
- private String error;
+public abstract class SshSession {
+ protected final TestSshKeys sshKeys;
+ protected final InetSocketAddress addr;
+ protected final TestAccount account;
+ protected String error;
public SshSession(TestSshKeys sshKeys, InetSocketAddress addr, TestAccount account) {
this.sshKeys = sshKeys;
@@ -42,44 +33,13 @@
this.account = account;
}
- public void open() throws Exception {
- getSession();
- }
+ public abstract void open() throws Exception;
- @SuppressWarnings("resource")
- public String exec(String command) throws Exception {
- ChannelExec channel = (ChannelExec) getSession().openChannel("exec");
- try {
- channel.setCommand(command);
- InputStream in = channel.getInputStream();
- InputStream err = channel.getErrStream();
- channel.connect();
+ public abstract void close();
- Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
- error = s.hasNext() ? s.next() : null;
+ public abstract String exec(String command) throws Exception;
- s = new Scanner(in, UTF_8.name()).useDelimiter("\\A");
- return s.hasNext() ? s.next() : "";
- } finally {
- channel.disconnect();
- }
- }
-
- @SuppressWarnings("resource")
- public int execAndReturnStatus(String command) throws Exception {
- ChannelExec channel = (ChannelExec) getSession().openChannel("exec");
- try {
- channel.setCommand(command);
- InputStream err = channel.getErrStream();
- channel.connect();
-
- Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
- error = s.hasNext() ? s.next() : null;
- return channel.getExitStatus();
- } finally {
- channel.disconnect();
- }
- }
+ public abstract int execAndReturnStatus(String command) throws Exception;
private boolean hasError() {
return error != null;
@@ -102,46 +62,23 @@
assertThat(getError()).contains(error);
}
- public void close() {
- if (session != null) {
- session.disconnect();
- session = null;
- }
- }
-
- private Session getSession() throws Exception {
- if (session == null) {
- KeyPair keyPair = sshKeys.getKeyPair(account);
- JSch jsch = new JSch();
- jsch.addIdentity(
- "KeyPair", TestSshKeys.privateKey(keyPair), keyPair.getPublicKeyBlob(), null);
- String username =
- account
- .username()
- .orElseThrow(
- () ->
- new IllegalStateException(
- "account " + account.accountId() + " must have a username to use SSH"));
- session = jsch.getSession(username, addr.getAddress().getHostAddress(), addr.getPort());
- session.setConfig("StrictHostKeyChecking", "no");
- session.connect();
- }
- return session;
- }
-
public String getUrl() {
- checkState(session != null, "session must be opened");
StringBuilder b = new StringBuilder();
b.append("ssh://");
- b.append(session.getUserName());
+ b.append(account.username().get());
b.append("@");
- b.append(session.getHost());
+ b.append(addr.getAddress().getHostAddress());
b.append(":");
- b.append(session.getPort());
+ b.append(addr.getPort());
return b.toString();
}
- public TestAccount getAccount() {
- return account;
+ protected String getUsername() {
+ return account
+ .username()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "account " + account.accountId() + " must have a username to use SSH"));
}
}
diff --git a/java/com/google/gerrit/acceptance/SshSessionJsch.java b/java/com/google/gerrit/acceptance/SshSessionJsch.java
new file mode 100644
index 0000000..86cc438
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/SshSessionJsch.java
@@ -0,0 +1,156 @@
+// 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;
+
+import static java.nio.charset.StandardCharsets.US_ASCII;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
+import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import com.jcraft.jsch.ChannelExec;
+import com.jcraft.jsch.JSch;
+import com.jcraft.jsch.JSchException;
+import com.jcraft.jsch.Session;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringWriter;
+import java.net.InetSocketAddress;
+import java.security.GeneralSecurityException;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.util.Properties;
+import java.util.Scanner;
+import org.bouncycastle.openssl.jcajce.JcaPEMWriter;
+import org.bouncycastle.openssl.jcajce.JcaPKCS8Generator;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.eclipse.jgit.transport.JschConfigSessionFactory;
+import org.eclipse.jgit.transport.OpenSshConfig.Host;
+import org.eclipse.jgit.transport.SshSessionFactory;
+import org.eclipse.jgit.util.FS;
+
+public class SshSessionJsch extends SshSession {
+
+ private Session session;
+
+ public static void initClient(KeyPair keyPair) {
+ Properties config = new Properties();
+ config.put("StrictHostKeyChecking", "no");
+ JSch.setConfig(config);
+
+ // register a JschConfigSessionFactory that adds the private key as identity
+ // to the JSch instance of JGit so that SSH communication via JGit can
+ // succeed
+ SshSessionFactory.setInstance(
+ new JschConfigSessionFactory() {
+ @Override
+ protected void configure(Host hc, Session session) {
+ try {
+ JSch jsch = getJSch(hc, FS.DETECTED);
+ jsch.addIdentity(
+ "KeyPair", privateKey(keyPair), TestSshKeys.publicKeyBlob(keyPair), null);
+ } catch (JSchException | GeneralSecurityException | IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ });
+ }
+
+ public static KeyPairGenerator initKeyPairGenerator() throws NoSuchAlgorithmException {
+ KeyPairGenerator gen;
+ gen = KeyPairGenerator.getInstance("RSA");
+ gen.initialize(512, new SecureRandom());
+ return gen;
+ }
+
+ public SshSessionJsch(TestSshKeys sshKeys, InetSocketAddress addr, TestAccount account) {
+ super(sshKeys, addr, account);
+ }
+
+ @Override
+ public void open() throws Exception {
+ getJschSession();
+ }
+
+ @Override
+ public void close() {
+ if (session != null) {
+ session.disconnect();
+ session = null;
+ }
+ }
+
+ @SuppressWarnings("resource")
+ @Override
+ public String exec(String command) throws Exception {
+ ChannelExec channel = (ChannelExec) getJschSession().openChannel("exec");
+ try {
+ channel.setCommand(command);
+ InputStream in = channel.getInputStream();
+ InputStream err = channel.getErrStream();
+ channel.connect();
+
+ Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
+ error = s.hasNext() ? s.next() : null;
+
+ s = new Scanner(in, UTF_8.name()).useDelimiter("\\A");
+ return s.hasNext() ? s.next() : "";
+ } finally {
+ channel.disconnect();
+ }
+ }
+
+ @SuppressWarnings("resource")
+ @Override
+ public int execAndReturnStatus(String command) throws Exception {
+ ChannelExec channel = (ChannelExec) getJschSession().openChannel("exec");
+ try {
+ channel.setCommand(command);
+ InputStream err = channel.getErrStream();
+ channel.connect();
+
+ Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
+ error = s.hasNext() ? s.next() : null;
+ return channel.getExitStatus();
+ } finally {
+ channel.disconnect();
+ }
+ }
+
+ private Session getJschSession() throws Exception {
+ if (session == null) {
+ KeyPair keyPair = sshKeys.getKeyPair(account);
+ JSch jsch = new JSch();
+ jsch.addIdentity("KeyPair", privateKey(keyPair), TestSshKeys.publicKeyBlob(keyPair), null);
+ String username = getUsername();
+ session = jsch.getSession(username, addr.getAddress().getHostAddress(), addr.getPort());
+ session.setConfig("StrictHostKeyChecking", "no");
+ session.connect();
+ }
+ return session;
+ }
+
+ private static byte[] privateKey(KeyPair keyPair) throws IOException {
+ // unencrypted form of PKCS#8 file
+ JcaPKCS8Generator gen1 = new JcaPKCS8Generator(keyPair.getPrivate(), null);
+ PemObject obj1 = gen1.generate();
+ StringWriter sw1 = new StringWriter();
+ try (JcaPEMWriter pw = new JcaPEMWriter(sw1)) {
+ pw.writeObject(obj1);
+ }
+ return sw1.toString().getBytes(US_ASCII.name());
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/SshSessionMina.java b/java/com/google/gerrit/acceptance/SshSessionMina.java
new file mode 100644
index 0000000..4514f44
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/SshSessionMina.java
@@ -0,0 +1,170 @@
+// 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;
+
+import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.io.CharSink;
+import com.google.common.io.Files;
+import com.google.common.io.MoreFiles;
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
+import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.security.GeneralSecurityException;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.KeyPairGenerator;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Arrays;
+import java.util.Scanner;
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
+import org.apache.sshd.common.util.security.SecurityUtils;
+import org.eclipse.jgit.transport.SshSessionFactory;
+import org.eclipse.jgit.transport.URIish;
+import org.eclipse.jgit.transport.sshd.DefaultProxyDataFactory;
+import org.eclipse.jgit.transport.sshd.JGitKeyCache;
+import org.eclipse.jgit.transport.sshd.SshdSession;
+import org.eclipse.jgit.transport.sshd.SshdSessionFactory;
+import org.eclipse.jgit.util.FS;
+
+public class SshSessionMina extends SshSession {
+ private static final int TIMEOUT = 100000;
+
+ private SshdSession session;
+
+ public static void initClient() {
+ JGitKeyCache keyCache = new JGitKeyCache();
+ SshdSessionFactory factory = new SshdSessionFactory(keyCache, new DefaultProxyDataFactory());
+ SshSessionFactory.setInstance(factory);
+ }
+
+ public static KeyPairGenerator initKeyPairGenerator()
+ throws GeneralSecurityException, InvalidKeySpecException, InvalidAlgorithmParameterException {
+ int size = 256;
+ KeyPairGenerator gen = SecurityUtils.getKeyPairGenerator(KeyUtils.EC_ALGORITHM);
+ ECCurves curve = ECCurves.fromCurveSize(size);
+ if (curve == null) {
+ throw new InvalidKeySpecException("Unknown curve for key size=" + size);
+ }
+ gen.initialize(curve.getParameters());
+ return gen;
+ }
+
+ public SshSessionMina(TestSshKeys sshKeys, InetSocketAddress addr, TestAccount account) {
+ super(sshKeys, addr, account);
+ }
+
+ @Override
+ public void open() throws Exception {
+ getMinaSession();
+ }
+
+ @Override
+ public void close() {
+ if (session != null) {
+ session.disconnect();
+ session = null;
+ }
+ }
+
+ @SuppressWarnings("resource")
+ @Override
+ public String exec(String command) throws Exception {
+ Process process = getMinaSession().exec(command, TIMEOUT);
+ InputStream in = process.getInputStream();
+ InputStream err = process.getErrorStream();
+
+ Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
+ error = s.hasNext() ? s.next() : null;
+
+ s = new Scanner(in, UTF_8.name()).useDelimiter("\\A");
+ return s.hasNext() ? s.next() : "";
+ }
+
+ @SuppressWarnings("resource")
+ @Override
+ public int execAndReturnStatus(String command) throws Exception {
+ Process process = getMinaSession().exec(command, 0);
+ InputStream in = process.getInputStream();
+ InputStream err = process.getErrorStream();
+
+ Scanner s = new Scanner(err, UTF_8.name()).useDelimiter("\\A");
+ error = s.hasNext() ? s.next() : null;
+
+ s = new Scanner(in, UTF_8.name()).useDelimiter("\\A");
+ try {
+ return process.exitValue();
+ } catch (IllegalThreadStateException e) {
+ // SSH command was interrupted
+ return -1;
+ }
+ }
+
+ private SshdSession getMinaSession() throws Exception {
+ if (session == null) {
+ String username = getUsername();
+
+ URIish uri =
+ new URIish(
+ "ssh://"
+ + username
+ + "@"
+ + addr.getAddress().getHostAddress()
+ + ":"
+ + addr.getPort());
+
+ // TODO(davido): Switch to memory only key resolving mode.
+ File userhome = Files.createTempDir();
+
+ FS fs = FS.DETECTED.setUserHome(userhome);
+ File sshDir = new File(userhome, ".ssh");
+ sshDir.mkdir();
+ OpenSSHKeyPairResourceWriter keyPairWriter = new OpenSSHKeyPairResourceWriter();
+ try (OutputStream out = new FileOutputStream(new File(sshDir, "id_ecdsa"))) {
+ keyPairWriter.writePrivateKey(sshKeys.getKeyPair(account), null, null, out);
+ }
+
+ // TODO(davido): Disable programmatically host key checking: "StrictHostKeyChecking: no" mode.
+ CharSink configFile = Files.asCharSink(new File(sshDir, "config"), UTF_8);
+ configFile.writeLines(Arrays.asList("Host *", "StrictHostKeyChecking no"));
+
+ JGitKeyCache keyCache = new JGitKeyCache();
+ try (SshdSessionFactory factory =
+ new SshdSessionFactory(keyCache, new DefaultProxyDataFactory())) {
+ factory.setHomeDirectory(userhome);
+ factory.setSshDirectory(sshDir);
+
+ session = factory.getSession(uri, null, fs, TIMEOUT);
+
+ session.addCloseListener(
+ future -> {
+ try {
+ MoreFiles.deleteRecursively(userhome.toPath(), ALLOW_INSECURE);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ });
+ }
+ }
+ return session;
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestSshKeys.java b/java/com/google/gerrit/acceptance/testsuite/account/TestSshKeys.java
index 6c95360..277d219 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestSshKeys.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestSshKeys.java
@@ -18,19 +18,20 @@
import static java.nio.charset.StandardCharsets.US_ASCII;
import com.google.gerrit.acceptance.SshEnabled;
+import com.google.gerrit.acceptance.testsuite.request.SshSessionFactory;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import com.jcraft.jsch.JSch;
-import com.jcraft.jsch.JSchException;
-import com.jcraft.jsch.KeyPair;
import java.io.ByteArrayOutputStream;
-import java.io.UnsupportedEncodingException;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.KeyPair;
import java.util.HashMap;
import java.util.Map;
+import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
@Singleton
public class TestSshKeys {
@@ -86,27 +87,26 @@
private KeyPair createKeyPair(Account.Id accountId, String username, @Nullable String email)
throws Exception {
- KeyPair keyPair = genSshKey();
+ KeyPair keyPair = SshSessionFactory.genSshKey();
authorizedKeys.addKey(accountId, publicKey(keyPair, email));
sshKeyCache.evict(username);
return keyPair;
}
- public static KeyPair genSshKey() throws JSchException {
- JSch jsch = new JSch();
- return KeyPair.genKeyPair(jsch, KeyPair.ECDSA, 256);
- }
-
public static String publicKey(KeyPair sshKey, @Nullable String comment)
- throws UnsupportedEncodingException {
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- sshKey.writePublicKey(out, comment);
- return out.toString(US_ASCII.name()).trim();
+ throws IOException, GeneralSecurityException {
+ return preparePublicKey(sshKey, comment).toString(US_ASCII.name()).trim();
}
- public static byte[] privateKey(KeyPair keyPair) {
+ public static byte[] publicKeyBlob(KeyPair sshKey) throws IOException, GeneralSecurityException {
+ return preparePublicKey(sshKey, null).toByteArray();
+ }
+
+ private static ByteArrayOutputStream preparePublicKey(KeyPair sshKey, String comment)
+ throws IOException, GeneralSecurityException {
+ OpenSSHKeyPairResourceWriter keyPairWriter = new OpenSSHKeyPairResourceWriter();
ByteArrayOutputStream out = new ByteArrayOutputStream();
- keyPair.writePrivateKey(out);
- return out.toByteArray();
+ keyPairWriter.writePublicKey(sshKey, comment, out);
+ return out;
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperationsImpl.java
index db730a6..895c7a0 100644
--- a/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperationsImpl.java
@@ -19,7 +19,6 @@
import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
-import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.account.TestAccount;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
@@ -82,7 +81,7 @@
public AcceptanceTestRequestScope.Context setApiUser(TestAccount testAccount) {
return atrScope.set(
atrScope.newContext(
- new SshSession(testSshKeys, sshAddress, testAccount),
+ SshSessionFactory.createSession(testSshKeys, sshAddress, testAccount),
createIdentifiedUser(testAccount.accountId())));
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/request/SshSessionFactory.java b/java/com/google/gerrit/acceptance/testsuite/request/SshSessionFactory.java
new file mode 100644
index 0000000..d5dd28a
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/request/SshSessionFactory.java
@@ -0,0 +1,52 @@
+// 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.testsuite.request;
+
+import static com.google.gerrit.server.config.SshClientImplementation.getFromEnvironment;
+
+import com.google.gerrit.acceptance.SshSession;
+import com.google.gerrit.acceptance.SshSessionJsch;
+import com.google.gerrit.acceptance.SshSessionMina;
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
+import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import java.net.InetSocketAddress;
+import java.security.GeneralSecurityException;
+import java.security.KeyPair;
+
+public class SshSessionFactory {
+ public static SshSession createSession(
+ TestSshKeys testSshKeys, InetSocketAddress sshAddress, TestAccount testAccount) {
+ return getFromEnvironment().isMina()
+ ? new SshSessionMina(testSshKeys, sshAddress, testAccount)
+ : new SshSessionJsch(testSshKeys, sshAddress, testAccount);
+ }
+
+ public static void initSsh(KeyPair keyPair) {
+ if (getFromEnvironment().isMina()) {
+ SshSessionMina.initClient();
+ } else {
+ SshSessionJsch.initClient(keyPair);
+ }
+ }
+
+ private SshSessionFactory() {}
+
+ public static KeyPair genSshKey() throws GeneralSecurityException {
+ return (getFromEnvironment().isMina()
+ ? SshSessionMina.initKeyPairGenerator()
+ : SshSessionJsch.initKeyPairGenerator())
+ .generateKeyPair();
+ }
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 9ea4fe6..7cbfebd 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfoDifference;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitMessageInput;
@@ -32,6 +33,7 @@
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
import java.util.Arrays;
+import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
@@ -260,6 +262,46 @@
EnumSet.complementOf(EnumSet.of(ListChangesOption.CHECK, ListChangesOption.SKIP_DIFFSTAT)));
}
+ default ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId, @Nullable String newMetaRevId) throws RestApiException {
+ return metaDiff(
+ oldMetaRevId,
+ newMetaRevId,
+ EnumSet.noneOf(ListChangesOption.class),
+ ImmutableListMultimap.of());
+ }
+
+ default ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId, @Nullable String newMetaRevId, ListChangesOption... options)
+ throws RestApiException {
+ return metaDiff(oldMetaRevId, newMetaRevId, Arrays.asList(options));
+ }
+
+ default ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId,
+ @Nullable String newMetaRevId,
+ Collection<ListChangesOption> options)
+ throws RestApiException {
+ return metaDiff(
+ oldMetaRevId,
+ newMetaRevId,
+ Sets.newEnumSet(options, ListChangesOption.class),
+ ImmutableListMultimap.of());
+ }
+
+ /**
+ * Gets the diff between a change's metadata with the two given refs.
+ *
+ * @param oldMetaRevId the SHA-1 of the 'before' metadata diffed against {@code newMetaRevId}
+ * @param newMetaRevId the SHA-1 of the 'after' metadata diffed against {@code oldMetaRevId}
+ */
+ ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId,
+ @Nullable String newMetaRevId,
+ EnumSet<ListChangesOption> options,
+ ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException;
+
/** {@link #get(ListChangesOption...)} with no options included. */
default ChangeInfo info() throws RestApiException {
return get(EnumSet.noneOf(ListChangesOption.class));
@@ -628,6 +670,16 @@
}
@Override
+ public ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId,
+ @Nullable String newMetaRevId,
+ EnumSet<ListChangesOption> options,
+ ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void setMessage(CommitMessageInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index b5f40ce..b771255 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -91,7 +91,7 @@
*/
public Boolean containsGitConflicts;
- public int _number;
+ public Integer _number;
public AccountInfo owner;
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD
index ee99702..cd3ebb9 100644
--- a/java/com/google/gerrit/httpd/BUILD
+++ b/java/com/google/gerrit/httpd/BUILD
@@ -32,7 +32,6 @@
"//lib:guava",
"//lib:jgit",
"//lib:jgit-servlet",
- "//lib:jsch",
"//lib:servlet-api",
"//lib:soy",
"//lib/auto:auto-value",
diff --git a/java/com/google/gerrit/httpd/init/BUILD b/java/com/google/gerrit/httpd/init/BUILD
index 37c63a2..adfbdcc 100644
--- a/java/com/google/gerrit/httpd/init/BUILD
+++ b/java/com/google/gerrit/httpd/init/BUILD
@@ -30,6 +30,7 @@
"//java/com/google/gerrit/sshd",
"//lib:guava",
"//lib:jgit",
+ "//lib:jgit-ssh-apache",
"//lib:servlet-api",
"//lib/flogger:api",
"//lib/guice",
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index 2df4739..d03340b 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -103,6 +103,7 @@
import com.google.gerrit.sshd.SshHostKeyModule;
import com.google.gerrit.sshd.SshKeyCacheImpl;
import com.google.gerrit.sshd.SshModule;
+import com.google.gerrit.sshd.SshSessionFactoryInitializer;
import com.google.gerrit.sshd.commands.DefaultCommandModule;
import com.google.gerrit.sshd.commands.IndexCommandsModule;
import com.google.gerrit.sshd.commands.SequenceCommandsModule;
@@ -339,6 +340,7 @@
});
modules.add(new DefaultUrlFormatter.Module());
+ SshSessionFactoryInitializer.init(config);
modules.add(SshKeyCacheImpl.module());
modules.add(
new AbstractModule() {
diff --git a/java/com/google/gerrit/httpd/raw/SshInfoServlet.java b/java/com/google/gerrit/httpd/raw/SshInfoServlet.java
index 1605360..ec67b8b 100644
--- a/java/com/google/gerrit/httpd/raw/SshInfoServlet.java
+++ b/java/com/google/gerrit/httpd/raw/SshInfoServlet.java
@@ -16,11 +16,11 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.gerrit.server.ssh.HostKey;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.util.http.CacheHeaders;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import com.jcraft.jsch.HostKey;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.List;
@@ -59,14 +59,14 @@
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse rsp) throws IOException {
- final List<HostKey> hostKeys = sshd.getHostKeys();
- final String out;
+ List<HostKey> hostKeys = sshd.getHostKeys();
+ String out;
if (!hostKeys.isEmpty()) {
String host = hostKeys.get(0).getHost();
String port = "22";
if (host.contains(":")) {
- final int p = host.lastIndexOf(':');
+ int p = host.lastIndexOf(':');
port = host.substring(p + 1);
host = host.substring(0, p);
}
diff --git a/java/com/google/gerrit/pgm/BUILD b/java/com/google/gerrit/pgm/BUILD
index faedcb7..16eebf2 100644
--- a/java/com/google/gerrit/pgm/BUILD
+++ b/java/com/google/gerrit/pgm/BUILD
@@ -44,6 +44,7 @@
"//lib:args4j",
"//lib:guava",
"//lib:jgit",
+ "//lib:jgit-ssh-apache",
"//lib:protobuf",
"//lib:servlet-api-without-neverlink",
"//lib/auto:auto-value",
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 16c9d27..07bab24 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -114,6 +114,7 @@
import com.google.gerrit.sshd.SshHostKeyModule;
import com.google.gerrit.sshd.SshKeyCacheImpl;
import com.google.gerrit.sshd.SshModule;
+import com.google.gerrit.sshd.SshSessionFactoryInitializer;
import com.google.gerrit.sshd.commands.DefaultCommandModule;
import com.google.gerrit.sshd.commands.IndexCommandsModule;
import com.google.gerrit.sshd.commands.SequenceCommandsModule;
@@ -482,6 +483,7 @@
});
}
modules.add(new DefaultUrlFormatter.Module());
+ SshSessionFactoryInitializer.init(config);
if (sshd) {
modules.add(SshKeyCacheImpl.module());
} else {
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 9fa7456..404906d 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -97,7 +97,6 @@
"//lib:guava-retrying",
"//lib:jgit",
"//lib:jgit-archive",
- "//lib:jsch",
"//lib:juniversalchardet",
"//lib:mime-util",
"//lib:protobuf",
diff --git a/java/com/google/gerrit/server/account/GroupCache.java b/java/com/google/gerrit/server/account/GroupCache.java
index 224ac14..aaae95a 100644
--- a/java/com/google/gerrit/server/account/GroupCache.java
+++ b/java/com/google/gerrit/server/account/GroupCache.java
@@ -16,6 +16,8 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.InternalGroup;
+import java.util.Collection;
+import java.util.Map;
import java.util.Optional;
/** Tracks group objects in memory for efficient access. */
@@ -48,6 +50,18 @@
Optional<InternalGroup> get(AccountGroup.UUID groupUuid);
/**
+ * Returns a {@code Map} of {@code AccountGroup.UUID} to {@code InternalGroup} for the given
+ * groups UUIDs. If not cached yet the groups are loaded. If a group can't be loaded (e.g. because
+ * it is missing), the entry will be missing from the result.
+ *
+ * @param groupUuids UUIDs of the groups that should be retrieved
+ * @return {@code Map} of {@code AccountGroup.UUID} to {@code InternalGroup} instances for the
+ * given group UUIDs, if a group can't be loaded (e.g. because it is missing), the entry will
+ * be missing from the result.
+ */
+ Map<AccountGroup.UUID, InternalGroup> get(Collection<AccountGroup.UUID> groupUuids);
+
+ /**
* Removes the association of the given ID with a group.
*
* <p>The next call to {@link #get(AccountGroup.Id)} won't provide a cached value.
@@ -88,4 +102,7 @@
* @param groupUuid the UUID of a possibly associated group
*/
void evict(AccountGroup.UUID groupUuid);
+
+ /** @see #evict(AccountGroup.UUID); */
+ void evict(Collection<AccountGroup.UUID> groupUuid);
}
diff --git a/java/com/google/gerrit/server/account/GroupCacheImpl.java b/java/com/google/gerrit/server/account/GroupCacheImpl.java
index 1ea4282..eaec9ba 100644
--- a/java/com/google/gerrit/server/account/GroupCacheImpl.java
+++ b/java/com/google/gerrit/server/account/GroupCacheImpl.java
@@ -14,12 +14,27 @@
package com.google.gerrit.server.account;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.proto.Cache;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.cache.serialize.ProtobufSerializer;
+import com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
@@ -31,8 +46,19 @@
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
+import org.bouncycastle.util.Strings;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
/** Tracks group objects in memory for efficient access. */
@Singleton
@@ -42,6 +68,7 @@
private static final String BYID_NAME = "groups";
private static final String BYNAME_NAME = "groups_byname";
private static final String BYUUID_NAME = "groups_byuuid";
+ private static final String BYUUID_NAME_PERSISTED = "groups_byuuid_persisted";
public static Module module() {
return new CacheModule() {
@@ -55,9 +82,35 @@
.maximumWeight(Long.MAX_VALUE)
.loader(ByNameLoader.class);
+ // We split the group cache into two parts for performance reasons:
+ // 1) An in-memory part that has only the group ref uuid as key.
+ // 2) A persisted part that has the group ref uuid and sha1 of the ref as key.
+ //
+ // When loading dashboards or returning change query results we potentially
+ // need to access many groups.
+ // We want the persisted cache to be immutable and we want it to be impossible that a
+ // value for a given key is out of date. We therefore require the sha-1 in the key. That
+ // is in line with the rest of the caches in Gerrit.
+ //
+ // Splitting the cache into two chunks internally in this class allows us to retain
+ // the existing performance guarantees of not requiring reads for the repo for values
+ // cached in-memory but also to persist the cache which leads to a much improved
+ // cold-start behavior and in-memory miss latency.
+
cache(BYUUID_NAME, String.class, new TypeLiteral<Optional<InternalGroup>>() {})
.maximumWeight(Long.MAX_VALUE)
- .loader(ByUUIDLoader.class);
+ .loader(ByUUIDInMemoryLoader.class);
+
+ persist(
+ BYUUID_NAME_PERSISTED,
+ Cache.GroupKeyProto.class,
+ new TypeLiteral<InternalGroup>() {})
+ .loader(PersistedByUUIDLoader.class)
+ .keySerializer(new ProtobufSerializer<>(Cache.GroupKeyProto.parser()))
+ .valueSerializer(PersistedInternalGroupSerializer.INSTANCE)
+ .diskLimit(1 << 30) // 1 GiB
+ .version(1)
+ .maximumWeight(0);
bind(GroupCacheImpl.class);
bind(GroupCache.class).to(GroupCacheImpl.class);
@@ -117,6 +170,20 @@
}
@Override
+ public Map<AccountGroup.UUID, InternalGroup> get(Collection<AccountGroup.UUID> groupUuids) {
+ try {
+ Set<String> groupUuidsStringSet =
+ groupUuids.stream().map(u -> u.get()).collect(toImmutableSet());
+ return byUUID.getAll(groupUuidsStringSet).entrySet().stream()
+ .filter(g -> g.getValue().isPresent())
+ .collect(toImmutableMap(g -> AccountGroup.uuid(g.getKey()), g -> g.getValue().get()));
+ } catch (ExecutionException e) {
+ logger.atWarning().withCause(e).log("Cannot look up groups %s by uuids", groupUuids);
+ return ImmutableMap.of();
+ }
+ }
+
+ @Override
public void evict(AccountGroup.Id groupId) {
if (groupId != null) {
logger.atFine().log("Evict group %s by ID", groupId.get());
@@ -140,6 +207,14 @@
}
}
+ @Override
+ public void evict(Collection<AccountGroup.UUID> groupUuids) {
+ if (groupUuids != null && !groupUuids.isEmpty()) {
+ logger.atFine().log("Evict groups %s by UUID", groupUuids);
+ byUUID.invalidateAll(groupUuids);
+ }
+ }
+
static class ByIdLoader extends CacheLoader<AccountGroup.Id, Optional<InternalGroup>> {
private final Provider<InternalGroupQuery> groupQueryProvider;
@@ -150,7 +225,7 @@
@Override
public Optional<InternalGroup> load(AccountGroup.Id key) throws Exception {
- try (TraceTimer timer =
+ try (TraceTimer ignored =
TraceContext.newTimer(
"Loading group by ID", Metadata.builder().groupId(key.get()).build())) {
return groupQueryProvider.get().byId(key);
@@ -168,7 +243,7 @@
@Override
public Optional<InternalGroup> load(String name) throws Exception {
- try (TraceTimer timer =
+ try (TraceTimer ignored =
TraceContext.newTimer(
"Loading group by name", Metadata.builder().groupName(name).build())) {
return groupQueryProvider.get().byName(AccountGroup.nameKey(name));
@@ -176,21 +251,108 @@
}
}
- static class ByUUIDLoader extends CacheLoader<String, Optional<InternalGroup>> {
- private final Groups groups;
+ static class ByUUIDInMemoryLoader extends CacheLoader<String, Optional<InternalGroup>> {
+ private final LoadingCache<Cache.GroupKeyProto, InternalGroup> persistedCache;
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsersName;
@Inject
- ByUUIDLoader(Groups groups) {
- this.groups = groups;
+ ByUUIDInMemoryLoader(
+ @Named(BYUUID_NAME_PERSISTED)
+ LoadingCache<Cache.GroupKeyProto, InternalGroup> persistedCache,
+ GitRepositoryManager repoManager,
+ AllUsersName allUsersName) {
+ this.persistedCache = persistedCache;
+ this.repoManager = repoManager;
+ this.allUsersName = allUsersName;
}
@Override
public Optional<InternalGroup> load(String uuid) throws Exception {
- try (TraceTimer timer =
- TraceContext.newTimer(
- "Loading group by UUID", Metadata.builder().groupUuid(uuid).build())) {
- return groups.getGroup(AccountGroup.uuid(uuid));
+ return loadAll(ImmutableSet.of(uuid)).get(uuid);
+ }
+
+ @Override
+ public Map<String, Optional<InternalGroup>> loadAll(Iterable<? extends String> uuids)
+ throws Exception {
+ Map<String, Optional<InternalGroup>> toReturn = new HashMap<>();
+ if (Iterables.isEmpty(uuids)) {
+ return toReturn;
}
+ Iterator<? extends String> uuidIterator = uuids.iterator();
+ List<Cache.GroupKeyProto> keyList = new ArrayList<>();
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "Loading group from serialized cache",
+ Metadata.builder().cacheName(BYUUID_NAME_PERSISTED).build());
+ Repository allUsers = repoManager.openRepository(allUsersName)) {
+ while (uuidIterator.hasNext()) {
+ String currentUuid = uuidIterator.next();
+ String ref = RefNames.refsGroups(AccountGroup.uuid(currentUuid));
+ Ref sha1 = allUsers.exactRef(ref);
+ if (sha1 == null) {
+ toReturn.put(currentUuid, Optional.empty());
+ continue;
+ }
+ Cache.GroupKeyProto key =
+ Cache.GroupKeyProto.newBuilder()
+ .setUuid(currentUuid)
+ .setRevision(ObjectIdConverter.create().toByteString(sha1.getObjectId()))
+ .build();
+ keyList.add(key);
+ }
+ }
+ persistedCache.getAll(keyList).entrySet().stream()
+ .forEach(g -> toReturn.put(g.getKey().getUuid(), Optional.of(g.getValue())));
+ return toReturn;
+ }
+ }
+
+ static class PersistedByUUIDLoader extends CacheLoader<Cache.GroupKeyProto, InternalGroup> {
+ private final Groups groups;
+
+ @Inject
+ PersistedByUUIDLoader(Groups groups) {
+ this.groups = groups;
+ }
+
+ @Override
+ public InternalGroup load(Cache.GroupKeyProto key) throws Exception {
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "Loading group by UUID", Metadata.builder().groupUuid(key.getUuid()).build())) {
+ ObjectId sha1 = ObjectIdConverter.create().fromByteString(key.getRevision());
+ Optional<InternalGroup> loadedGroup =
+ groups.getGroup(AccountGroup.uuid(key.getUuid()), sha1);
+ if (!loadedGroup.isPresent()) {
+ throw new IllegalStateException(
+ String.format(
+ "group %s should have the sha-1 %s, but " + "it was not found",
+ key.getUuid(), sha1.getName()));
+ }
+ return loadedGroup.get();
+ }
+ }
+ }
+
+ private enum PersistedInternalGroupSerializer implements CacheSerializer<InternalGroup> {
+ INSTANCE;
+
+ @Override
+ public byte[] serialize(InternalGroup value) {
+ if (value == null) {
+ return new byte[0];
+ }
+ return Protos.toByteArray(InternalGroupSerializer.serialize(value));
+ }
+
+ @Override
+ public InternalGroup deserialize(byte[] in) {
+ if (Strings.fromByteArray(in).isEmpty()) {
+ return null;
+ }
+ return InternalGroupSerializer.deserialize(
+ Protos.parseUnchecked(Cache.InternalGroupProto.parser(), in));
}
}
}
diff --git a/java/com/google/gerrit/server/account/IncludingGroupMembership.java b/java/com/google/gerrit/server/account/IncludingGroupMembership.java
index a277fac..8cec8bf 100644
--- a/java/com/google/gerrit/server/account/IncludingGroupMembership.java
+++ b/java/com/google/gerrit/server/account/IncludingGroupMembership.java
@@ -27,7 +27,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -83,6 +82,9 @@
}
if (tryExpanding) {
+ Set<AccountGroup.UUID> queryIdsSet = new HashSet<>();
+ queryIds.forEach(i -> queryIdsSet.add(i));
+ Map<AccountGroup.UUID, InternalGroup> groups = groupCache.get(queryIdsSet);
for (AccountGroup.UUID id : queryIds) {
if (memberOf.containsKey(id)) {
// Membership was earlier proven to be false.
@@ -90,15 +92,15 @@
}
memberOf.put(id, false);
- Optional<InternalGroup> group = groupCache.get(id);
- if (!group.isPresent()) {
+ InternalGroup group = groups.get(id);
+ if (group == null) {
continue;
}
- if (user.isIdentifiedUser() && group.get().getMembers().contains(user.getAccountId())) {
+ if (user.isIdentifiedUser() && group.getMembers().contains(user.getAccountId())) {
memberOf.put(id, true);
return true;
}
- if (search(group.get().getSubgroups())) {
+ if (search(group.getSubgroups())) {
memberOf.put(id, true);
return true;
}
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index fdd9591..0b340b8 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -46,6 +46,7 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfoDifference;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitMessageInput;
@@ -80,6 +81,7 @@
import com.google.gerrit.server.restapi.change.GetAssignee;
import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.GetHashtags;
+import com.google.gerrit.server.restapi.change.GetMetaDiff;
import com.google.gerrit.server.restapi.change.GetPastAssignees;
import com.google.gerrit.server.restapi.change.GetPureRevert;
import com.google.gerrit.server.restapi.change.GetTopic;
@@ -149,6 +151,7 @@
private final ChangeIncludedIn includedIn;
private final PostReviewers postReviewers;
private final Provider<GetChange> getChangeProvider;
+ private final Provider<GetMetaDiff> getMetaDiffProvider;
private final PostHashtags postHashtags;
private final GetHashtags getHashtags;
private final AttentionSet attentionSet;
@@ -204,6 +207,7 @@
ChangeIncludedIn includedIn,
PostReviewers postReviewers,
Provider<GetChange> getChangeProvider,
+ Provider<GetMetaDiff> getMetaDiffProvider,
PostHashtags postHashtags,
GetHashtags getHashtags,
AttentionSet attentionSet,
@@ -257,6 +261,7 @@
this.includedIn = includedIn;
this.postReviewers = postReviewers;
this.getChangeProvider = getChangeProvider;
+ this.getMetaDiffProvider = getMetaDiffProvider;
this.postHashtags = postHashtags;
this.getHashtags = getHashtags;
this.attentionSet = attentionSet;
@@ -517,6 +522,25 @@
}
@Override
+ public ChangeInfoDifference metaDiff(
+ @Nullable String oldMetaRevId,
+ @Nullable String newMetaRevId,
+ EnumSet<ListChangesOption> options,
+ ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException {
+ try (DynamicOptions dynamicOptions = new DynamicOptions(injector, dynamicBeans)) {
+ GetMetaDiff metaDiff = getMetaDiffProvider.get();
+ metaDiff.setOldMetaRevId(oldMetaRevId);
+ metaDiff.setNewMetaRevId(newMetaRevId);
+ options.forEach(metaDiff::addOption);
+ dynamicOptionParser.parseDynamicOptions(metaDiff, pluginOptions, dynamicOptions);
+ return metaDiff.apply(change).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot retrieve metaDiff", e);
+ }
+ }
+
+ @Override
public ChangeEditApi edit() throws RestApiException {
return changeEditApi.create(change);
}
diff --git a/java/com/google/gerrit/server/audit/BUILD b/java/com/google/gerrit/server/audit/BUILD
index 0870786..3faa259 100644
--- a/java/com/google/gerrit/server/audit/BUILD
+++ b/java/com/google/gerrit/server/audit/BUILD
@@ -46,7 +46,6 @@
"//lib:guava-retrying",
"//lib:jgit",
"//lib:jgit-archive",
- "//lib:jsch",
"//lib:juniversalchardet",
"//lib:mime-util",
"//lib:protobuf",
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializer.java
new file mode 100644
index 0000000..7449917
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializer.java
@@ -0,0 +1,82 @@
+// 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.cache.serialize.entities;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.cache.proto.Cache;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import java.sql.Timestamp;
+
+/** Helper to (de)serialize values for caches. */
+public class InternalGroupSerializer {
+ public static InternalGroup deserialize(Cache.InternalGroupProto proto) {
+ InternalGroup.Builder builder =
+ InternalGroup.builder()
+ .setId(AccountGroup.id(proto.getId()))
+ .setNameKey(AccountGroup.nameKey(proto.getName()))
+ .setOwnerGroupUUID(AccountGroup.uuid(proto.getOwnerGroupUuid()))
+ .setVisibleToAll(proto.getIsVisibleToAll())
+ .setGroupUUID(AccountGroup.uuid(proto.getGroupUuid()))
+ .setCreatedOn(new Timestamp(proto.getCreatedOn()))
+ .setMembers(
+ proto.getMembersIdsList().stream()
+ .map(a -> Account.id(a))
+ .collect(toImmutableSet()))
+ .setSubgroups(
+ proto.getSubgroupUuidsList().stream()
+ .map(s -> AccountGroup.uuid(s))
+ .collect(toImmutableSet()));
+
+ if (!proto.getDescription().isEmpty()) {
+ builder.setDescription(proto.getDescription());
+ }
+
+ if (!proto.getRefState().isEmpty()) {
+ builder.setRefState(ObjectIdConverter.create().fromByteString(proto.getRefState()));
+ }
+
+ return builder.build();
+ }
+
+ public static Cache.InternalGroupProto serialize(InternalGroup autoValue) {
+ Cache.InternalGroupProto.Builder builder =
+ Cache.InternalGroupProto.newBuilder()
+ .setId(autoValue.getId().get())
+ .setName(autoValue.getName())
+ .setOwnerGroupUuid(autoValue.getOwnerGroupUUID().get())
+ .setIsVisibleToAll(autoValue.isVisibleToAll())
+ .setGroupUuid(autoValue.getGroupUUID().get())
+ .setCreatedOn(autoValue.getCreatedOn().getTime());
+
+ autoValue.getMembers().stream().forEach(m -> builder.addMembersIds(m.get()));
+ autoValue.getSubgroups().stream().forEach(s -> builder.addSubgroupUuids(s.get()));
+
+ if (autoValue.getDescription() != null) {
+ builder.setDescription(autoValue.getDescription());
+ }
+
+ if (autoValue.getRefState() != null) {
+ builder.setRefState(ObjectIdConverter.create().toByteString(autoValue.getRefState()));
+ }
+
+ return builder.build();
+ }
+
+ private InternalGroupSerializer() {}
+}
diff --git a/java/com/google/gerrit/server/config/PluginConfigFactory.java b/java/com/google/gerrit/server/config/PluginConfigFactory.java
index a9abd1e..2d0f9a5 100644
--- a/java/com/google/gerrit/server/config/PluginConfigFactory.java
+++ b/java/com/google/gerrit/server/config/PluginConfigFactory.java
@@ -289,7 +289,7 @@
*/
public Config getProjectPluginConfigWithInheritance(
Project.NameKey projectName, String pluginName) throws NoSuchProjectException {
- return getPluginConfig(projectName, pluginName).getWithInheritance(false);
+ return getPluginConfig(projectName, pluginName).getWithInheritance(/* merge= */ false);
}
/**
@@ -311,7 +311,7 @@
*/
public Config getProjectPluginConfigWithInheritance(
ProjectState projectState, String pluginName) {
- return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(false);
+ return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(/* merge= */ false);
}
/**
@@ -336,7 +336,7 @@
*/
public Config getProjectPluginConfigWithMergedInheritance(
Project.NameKey projectName, String pluginName) throws NoSuchProjectException {
- return getPluginConfig(projectName, pluginName).getWithInheritance(true);
+ return getPluginConfig(projectName, pluginName).getWithInheritance(/* merge= */ true);
}
/**
@@ -359,7 +359,7 @@
*/
public Config getProjectPluginConfigWithMergedInheritance(
ProjectState projectState, String pluginName) {
- return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(true);
+ return projectState.getConfig(pluginName + EXTENSION).getWithInheritance(/* merge= */ true);
}
private ProjectLevelConfig getPluginConfig(Project.NameKey projectName, String pluginName)
diff --git a/java/com/google/gerrit/server/config/SshClientImplementation.java b/java/com/google/gerrit/server/config/SshClientImplementation.java
new file mode 100644
index 0000000..5811e4d
--- /dev/null
+++ b/java/com/google/gerrit/server/config/SshClientImplementation.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.config;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Enums;
+import com.google.common.base.Strings;
+
+/* SSH implementation to use by JGit SSH client transport protocol. */
+public enum SshClientImplementation {
+ /** JCraft JSch implementation. */
+ JSCH,
+
+ /** Apache MINA implementation. */
+ APACHE;
+
+ private static final String ENV_VAR = "SSH_CLIENT_IMPLEMENTATION";
+ private static final String SYS_PROP = "gerrit.sshClientImplementation";
+
+ @VisibleForTesting
+ public static SshClientImplementation getFromEnvironment() {
+ String value = System.getenv(ENV_VAR);
+ if (Strings.isNullOrEmpty(value)) {
+ value = System.getProperty(SYS_PROP);
+ }
+ if (Strings.isNullOrEmpty(value)) {
+ return APACHE;
+ }
+ SshClientImplementation client =
+ Enums.getIfPresent(SshClientImplementation.class, value).orNull();
+ if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) {
+ checkArgument(
+ client != null, "Invalid value for env variable %s: %s", ENV_VAR, System.getenv(ENV_VAR));
+ } else {
+ checkArgument(
+ client != null,
+ "Invalid value for system property %s: %s",
+ SYS_PROP,
+ System.getProperty(SYS_PROP));
+ }
+ return client;
+ }
+
+ public boolean isMina() {
+ return this == APACHE;
+ }
+}
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index 2177485..d037994 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -296,7 +296,7 @@
REPOSITORY_SIZE_GROUP, projectName);
throw new RuntimeException(e);
}
- availableTokens.availableTokens().ifPresent(v -> receivePack.setMaxObjectSizeLimit(v));
+ availableTokens.availableTokens().ifPresent(receivePack::setMaxPackSizeLimit);
}
/** Determine if the user can upload commits. */
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 1fde48c..1cb0bea 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -56,11 +56,11 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.ssh.HostKey;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.util.MagicBranch;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import com.jcraft.jsch.HostKey;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java
index a5f5cfc..b2d1849b 100644
--- a/java/com/google/gerrit/server/group/db/GroupConfig.java
+++ b/java/com/google/gerrit/server/group/db/GroupConfig.java
@@ -24,6 +24,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.InternalGroup;
@@ -156,7 +157,7 @@
Project.NameKey projectName,
Repository repository,
AccountGroup.UUID groupUuid,
- ObjectId groupRefObjectId)
+ @Nullable ObjectId groupRefObjectId)
throws IOException, ConfigInvalidException {
GroupConfig groupConfig = new GroupConfig(groupUuid);
if (groupRefObjectId == null) {
diff --git a/java/com/google/gerrit/server/group/db/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java
index 50c339e..30e37cb 100644
--- a/java/com/google/gerrit/server/group/db/Groups.java
+++ b/java/com/google/gerrit/server/group/db/Groups.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.AccountGroupByIdAudit;
import com.google.gerrit.entities.AccountGroupMemberAudit;
@@ -80,6 +81,23 @@
}
/**
+ * Returns the {@code InternalGroup} for the specified UUID and groupRefObjectId
+ *
+ * @param groupUuid the UUID of the group
+ * @param groupRefObjectId the ref revision of this group
+ * @return the found {@code InternalGroup} if it exists, or else an empty {@code Optional}
+ * @throws IOException if the group couldn't be retrieved from NoteDb
+ * @throws ConfigInvalidException if the group couldn't be retrieved from NoteDb
+ */
+ public Optional<InternalGroup> getGroup(
+ AccountGroup.UUID groupUuid, @Nullable ObjectId groupRefObjectId)
+ throws IOException, ConfigInvalidException {
+ try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+ return getGroupFromNoteDb(allUsersName, allUsersRepo, groupUuid, groupRefObjectId);
+ }
+ }
+
+ /**
* Loads an internal group from NoteDb using the group UUID. This method returns the latest state
* of the internal group.
*/
@@ -97,7 +115,7 @@
AllUsersName allUsersName,
Repository allUsersRepository,
AccountGroup.UUID uuid,
- ObjectId groupRefObjectId)
+ @Nullable ObjectId groupRefObjectId)
throws IOException, ConfigInvalidException {
GroupConfig groupConfig =
GroupConfig.loadForGroup(allUsersName, allUsersRepository, uuid, groupRefObjectId);
diff --git a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
index 3edd26b..b3ef679 100644
--- a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
+++ b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
@@ -35,7 +35,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import java.util.Optional;
+import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
@@ -88,16 +88,17 @@
AtomicInteger done = new AtomicInteger();
AtomicInteger failed = new AtomicInteger();
Stopwatch sw = Stopwatch.createStarted();
+ groupCache.evict(uuids);
+ Map<AccountGroup.UUID, InternalGroup> reindexedGroups = groupCache.get(uuids);
for (AccountGroup.UUID uuid : uuids) {
String desc = "group " + uuid;
ListenableFuture<?> future =
executor.submit(
() -> {
try {
- groupCache.evict(uuid);
- Optional<InternalGroup> internalGroup = groupCache.get(uuid);
- if (internalGroup.isPresent()) {
- index.replace(internalGroup.get());
+ InternalGroup internalGroup = reindexedGroups.get(uuid);
+ if (internalGroup != null) {
+ index.replace(internalGroup);
} else {
index.delete(uuid);
diff --git a/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java b/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java
index ffe9fa8..7f9de0d 100644
--- a/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java
+++ b/java/com/google/gerrit/server/notedb/MissingMetaObjectException.java
@@ -16,6 +16,8 @@
/** Separate exception type to throw if requested meta SHA1 is not available. */
public class MissingMetaObjectException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
MissingMetaObjectException(String msg) {
super(msg);
}
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index de5b1da..3f17a2e 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -23,6 +23,7 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.InMemoryInserter;
@@ -86,6 +87,7 @@
}
private final Counter1<OperationType> counter;
+ private final Timer1<OperationType> latency;
private final PersonIdent gerritIdent;
private final boolean save;
private final ThreeWayMergeStrategy configuredMergeStrategy;
@@ -100,6 +102,13 @@
"git/auto-merge/num_operations",
new Description("AutoMerge computations").setRate().setUnit("auto merge computations"),
Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName).build());
+ 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());
this.save = cacheAutomerge(cfg);
this.gerritIdent = gerritIdent;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
@@ -129,8 +138,10 @@
return existingCommit.get();
}
counter.increment(OperationType.IN_MEMORY_WRITE);
- logger.atWarning().log("Computing in-memory AutoMerge for " + merge.name());
- return rw.parseCommit(createAutoMergeCommit(repo.getConfig(), rw, ins, merge, mergeStrategy));
+ logger.atInfo().log("Computing in-memory AutoMerge for " + merge.name());
+ try (Timer1.Context ignored = latency.start(OperationType.IN_MEMORY_WRITE)) {
+ return rw.parseCommit(createAutoMergeCommit(repo.getConfig(), rw, ins, merge, mergeStrategy));
+ }
}
/**
@@ -151,9 +162,12 @@
return Optional.empty();
}
- ObjectId autoMerge =
- createAutoMergeCommit(
- repoView.getConfig(), rw, ins, maybeMergeCommit, configuredMergeStrategy);
+ ObjectId autoMerge;
+ try (Timer1.Context ignored = latency.start(OperationType.ON_DISK_WRITE)) {
+ autoMerge =
+ createAutoMergeCommit(
+ repoView.getConfig(), rw, ins, maybeMergeCommit, configuredMergeStrategy);
+ }
counter.increment(OperationType.ON_DISK_WRITE);
logger.atFine().log("Added %s AutoMerge ref update for commit", autoMerge.name());
return Optional.of(
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 07b01c4..885459a 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -296,15 +296,15 @@
try {
PatchScript patchScript = getPatchScriptWithOldDiffCache(git, aId, bId);
if (areEqualPatchscripts(patchScript, expected)) {
- metrics.diffs.increment(metrics.MATCH);
+ metrics.diffs.increment(Metrics.MATCH);
} else {
- metrics.diffs.increment(metrics.MISMATCH);
+ metrics.diffs.increment(Metrics.MISMATCH);
logger.atWarning().atMostEvery(10, TimeUnit.SECONDS).log(
"Mismatching diff for change %s, old commit ID: %s, new commit ID: %s, file name: %s.",
changeId.toString(), aId, bId, fileName);
}
} catch (PatchListNotAvailableException | IOException e) {
- metrics.diffs.increment(metrics.ERROR);
+ metrics.diffs.increment(Metrics.ERROR);
logger.atSevere().atMostEvery(10, TimeUnit.SECONDS).log(
String.format(
"Error computing new diff for change %s, old commit ID: %s, new commit ID: %s.\n",
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index 8163b19..395312f 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -93,7 +93,7 @@
persist(DIFF, FileDiffCacheKey.class, FileDiffOutput.class)
.maximumWeight(10 << 20)
.weigher(FileDiffWeigher.class)
- .version(3)
+ .version(4)
.keySerializer(FileDiffCacheKey.Serializer.INSTANCE)
.valueSerializer(FileDiffOutput.Serializer.INSTANCE)
.loader(FileDiffLoader.class);
diff --git a/java/com/google/gerrit/server/patch/filediff/FileSizeEvaluator.java b/java/com/google/gerrit/server/patch/filediff/FileSizeEvaluator.java
index 97b55dc..e2c1bc5 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileSizeEvaluator.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileSizeEvaluator.java
@@ -90,6 +90,8 @@
}
private static boolean isBlob(Patch.FileMode mode) {
- return mode.equals(FileMode.REGULAR_FILE) || mode.equals(FileMode.SYMLINK);
+ return mode.equals(FileMode.REGULAR_FILE)
+ || mode.equals(FileMode.EXECUTABLE_FILE)
+ || mode.equals(FileMode.SYMLINK);
}
}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
index a01d447..e1af81d 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
@@ -209,9 +209,6 @@
private static final FieldDescriptor NEW_MODE_DESCRIPTOR =
GitFileDiffProto.getDescriptor().findFieldByNumber(8);
- private static final FieldDescriptor CHANGE_TYPE_DESCRIPTOR =
- GitFileDiffProto.getDescriptor().findFieldByNumber(9);
-
private static final FieldDescriptor PATCH_TYPE_DESCRIPTOR =
GitFileDiffProto.getDescriptor().findFieldByNumber(10);
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 5092e12..10aa9cd 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -148,11 +148,11 @@
// Perform an initial ref filtering with all the refs the caller asked for. If we find tags that
// we have to investigate separately (deferred tags) then perform a reachability check starting
// from all visible branches (refs/heads/*).
- Result initialRefFilter = filterRefs(new ArrayList<>(refs), repo, opts);
+ Result initialRefFilter = filterRefs(new ArrayList<>(refs), opts);
List<Ref> visibleRefs = initialRefFilter.visibleRefs();
if (!initialRefFilter.deferredTags().isEmpty()) {
try (TraceTimer traceTimer = TraceContext.newTimer("Check visibility of deferred tags")) {
- Result allVisibleBranches = filterRefs(getTaggableRefs(repo), repo, opts);
+ Result allVisibleBranches = filterRefs(getTaggableRefs(repo), opts);
checkState(
allVisibleBranches.deferredTags().isEmpty(),
"unexpected tags found when filtering refs/heads/* "
@@ -186,8 +186,7 @@
* separately for later rev-walk-based visibility computation. Tags where visibility is trivial to
* compute will be returned as part of {@link Result#visibleRefs()}.
*/
- Result filterRefs(List<Ref> refs, Repository repo, RefFilterOptions opts)
- throws PermissionBackendException {
+ Result filterRefs(List<Ref> refs, RefFilterOptions opts) throws PermissionBackendException {
logger.atFinest().log("Filter refs (refs = %s)", refs);
// TODO(hiesel): Remove when optimization is done.
diff --git a/java/com/google/gerrit/server/permissions/SectionSortCache.java b/java/com/google/gerrit/server/permissions/SectionSortCache.java
index 6081e9a..d800782 100644
--- a/java/com/google/gerrit/server/permissions/SectionSortCache.java
+++ b/java/com/google/gerrit/server/permissions/SectionSortCache.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.permissions;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.auto.value.AutoValue;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
@@ -28,6 +30,8 @@
import java.util.ArrayList;
import java.util.IdentityHashMap;
import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
/**
* Caches the order AccessSections should be sorted for evaluation.
@@ -60,67 +64,62 @@
this.cache = cache;
}
- // Sorts the given sections, but does not disturb ordering between equally exact sections.
+ /**
+ * Sorts the given sections in-place, but does not disturb ordering between equally exact
+ * sections.
+ */
void sort(String ref, List<AccessSection> sections) {
final int cnt = sections.size();
if (cnt <= 1) {
return;
}
-
EntryKey key = EntryKey.create(ref, sections);
- EntryVal val = cache.getIfPresent(key);
- if (val != null) {
- int[] srcIdx = val.order;
- if (srcIdx != null) {
- AccessSection[] srcList = copy(sections);
- for (int i = 0; i < cnt; i++) {
- sections.set(i, srcList[srcIdx[i]]);
- }
- } else {
- // Identity transform. No sorting is required.
- }
+ EntryVal val;
+ try {
+ val = cache.get(key, new Loader(key, sections));
+ } catch (ExecutionException e) {
+ logger.atWarning().withCause(e).log("Error happened while sorting access sections.");
+ return;
+ }
+ ImmutableList<Integer> order = val.order();
+ List<AccessSection> sorted = new ArrayList<>();
+ for (int i = 0; i < cnt; i++) {
+ sorted.add(sections.get(order.get(i)));
+ }
+ for (int i = 0; i < cnt; i++) {
+ sections.set(i, sorted.get(i));
+ }
+ }
- } else {
- boolean poison = false;
+ private static class Loader implements Callable<EntryVal> {
+ private final List<AccessSection> sections;
+ EntryKey key;
+
+ Loader(EntryKey key, List<AccessSection> sections) {
+ this.key = key;
+ this.sections = sections;
+ }
+
+ @Override
+ public EntryVal call() throws Exception {
+ // We use IdentityHashMap (which uses reference equality for keys/values) to preserve distinct
+ // entries in the map for identical AccessSection keys
IdentityHashMap<AccessSection, Integer> srcMap = new IdentityHashMap<>();
- for (int i = 0; i < cnt; i++) {
- poison |= srcMap.put(sections.get(i), i) != null;
+ for (int i = 0; i < sections.size(); i++) {
+ srcMap.put(sections.get(i), i);
}
-
- sections.sort(new MostSpecificComparator(ref));
-
- int[] srcIdx;
- if (isIdentityTransform(sections, srcMap)) {
- srcIdx = null;
- } else {
- srcIdx = new int[cnt];
- for (int i = 0; i < cnt; i++) {
- srcIdx[i] = srcMap.get(sections.get(i));
- }
+ ImmutableList<AccessSection> sorted =
+ sections.stream()
+ .sorted(new MostSpecificComparator(key.ref()))
+ .collect(toImmutableList());
+ ImmutableList.Builder<Integer> order = ImmutableList.builderWithExpectedSize(sections.size());
+ for (int i = 0; i < sorted.size(); i++) {
+ order.add(srcMap.get(sorted.get(i)));
}
-
- if (poison) {
- logger.atSevere().log("Received duplicate AccessSection instances, not caching sort");
- } else {
- cache.put(key, new EntryVal(srcIdx));
- }
+ return EntryVal.create(order.build());
}
}
- private static AccessSection[] copy(List<AccessSection> sections) {
- return sections.toArray(new AccessSection[sections.size()]);
- }
-
- private static boolean isIdentityTransform(
- List<AccessSection> sections, IdentityHashMap<AccessSection, Integer> srcMap) {
- for (int i = 0; i < sections.size(); i++) {
- if (i != srcMap.get(sections.get(i))) {
- return false;
- }
- }
- return true;
- }
-
@AutoValue
abstract static class EntryKey {
public abstract String ref();
@@ -146,17 +145,18 @@
}
}
- static final class EntryVal {
+ @AutoValue
+ abstract static class EntryVal {
/**
* Maps the input index to the output index.
*
* <p>For {@code x == order[y]} the expression means move the item at source position {@code x}
* to the output position {@code y}.
*/
- final int[] order;
+ abstract ImmutableList<Integer> order();
- EntryVal(int[] order) {
- this.order = order;
+ static EntryVal create(ImmutableList<Integer> order) {
+ return new AutoValue_SectionSortCache_EntryVal(order);
}
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectLevelConfig.java b/java/com/google/gerrit/server/project/ProjectLevelConfig.java
index d82a318..8256198 100644
--- a/java/com/google/gerrit/server/project/ProjectLevelConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectLevelConfig.java
@@ -88,7 +88,7 @@
}
public Config getWithInheritance() {
- return getWithInheritance(false);
+ return getWithInheritance(/* merge= */ false);
}
/**
diff --git a/java/com/google/gerrit/server/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index 2f1c61e..c51bb91 100644
--- a/java/com/google/gerrit/server/restapi/change/GetChange.java
+++ b/java/com/google/gerrit/server/restapi/change/GetChange.java
@@ -62,6 +62,10 @@
@Option(name = "--meta", usage = "NoteDb meta SHA1")
String metaRevId = "";
+ public void setMetaRevId(String metaRevId) {
+ this.metaRevId = metaRevId == null ? "" : metaRevId;
+ }
+
@Option(name = "-O", usage = "Output option flags, in hex")
void setOptionFlagsHex(String hex) {
options.addAll(ListOption.fromBits(ListChangesOption.class, Integer.parseInt(hex, 16)));
diff --git a/java/com/google/gerrit/server/restapi/change/GetMetaDiff.java b/java/com/google/gerrit/server/restapi/change/GetMetaDiff.java
new file mode 100644
index 0000000..af23ba7
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/GetMetaDiff.java
@@ -0,0 +1,161 @@
+// 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.restapi.change;
+
+import static com.google.gerrit.entities.RefNames.changeMetaRef;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ListOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfoDiffer;
+import com.google.gerrit.extensions.common.ChangeInfoDifference;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.DynamicOptions.DynamicBean;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.Map;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.kohsuke.args4j.Option;
+
+/** Gets the diff for a change at two NoteDb meta SHA-1s. */
+public class GetMetaDiff
+ implements RestReadView<ChangeResource>,
+ DynamicOptions.BeanReceiver,
+ DynamicOptions.BeanProvider {
+
+ private final EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
+ private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
+
+ private final Provider<GetChange> getChangeProvider;
+ private final GitRepositoryManager repoManager;
+
+ @Option(name = "-o", usage = "Output options")
+ public void addOption(ListChangesOption o) {
+ options.add(o);
+ }
+
+ @Option(name = "-O", usage = "Output option flags, in hex")
+ void setOptionFlagsHex(String hex) {
+ options.addAll(ListOption.fromBits(ListChangesOption.class, Integer.parseInt(hex, 16)));
+ }
+
+ @Option(name = "--old", usage = "old NoteDb meta SHA-1")
+ String oldMetaRevId = "";
+
+ public void setOldMetaRevId(@Nullable String oldMetaRevId) {
+ this.oldMetaRevId = oldMetaRevId == null ? "" : oldMetaRevId;
+ }
+
+ @Option(name = "--meta", usage = "new NoteDb meta SHA-1")
+ String metaRevId = "";
+
+ public void setNewMetaRevId(@Nullable String metaRevId) {
+ this.metaRevId = metaRevId == null ? "" : metaRevId;
+ }
+
+ @Inject
+ GetMetaDiff(Provider<GetChange> getChangeProvider, GitRepositoryManager repoManager) {
+ this.getChangeProvider = getChangeProvider;
+ this.repoManager = repoManager;
+ }
+
+ @Override
+ public void setDynamicBean(String plugin, DynamicOptions.DynamicBean dynamicBean) {
+ dynamicBeans.put(plugin, dynamicBean);
+ }
+
+ @Override
+ public DynamicBean getDynamicBean(String plugin) {
+ return dynamicBeans.get(plugin);
+ }
+
+ @Override
+ public Response<ChangeInfoDifference> apply(ChangeResource resource)
+ throws BadRequestException, PreconditionFailedException, IOException {
+ return Response.ok(
+ ChangeInfoDiffer.getDifference(getOldChangeInfo(resource), getNewChangeInfo(resource)));
+ }
+
+ private ChangeInfo getOldChangeInfo(ChangeResource resource)
+ throws BadRequestException, IOException, PreconditionFailedException {
+ GetChange getChange = createGetChange();
+ getChange.setMetaRevId(getOldMetaRevId(resource));
+ ChangeInfo oldChangeInfo;
+ try {
+ oldChangeInfo = getChange.apply(resource).value();
+ } catch (PreconditionFailedException e) {
+ oldChangeInfo = new ChangeInfo();
+ }
+ return oldChangeInfo;
+ }
+
+ private String getOldMetaRevId(ChangeResource resource)
+ throws IOException, BadRequestException, PreconditionFailedException {
+ if (!oldMetaRevId.isEmpty()) {
+ return oldMetaRevId;
+ }
+ String newMetaRevId = getNewMetaRevId(resource);
+ try (Repository repo = repoManager.openRepository(resource.getProject());
+ RevWalk rw = new RevWalk(repo)) {
+ ObjectId resourceId = ObjectId.fromString(newMetaRevId);
+ RevCommit commit = rw.parseCommit(resourceId);
+ return commit.getParentCount() == 0
+ ? resourceId.getName()
+ : commit.getParent(0).getId().getName();
+ } catch (InvalidObjectIdException e) {
+ throw new BadRequestException("invalid meta SHA1: " + newMetaRevId, e);
+ } catch (MissingObjectException e) {
+ throw new PreconditionFailedException(e.getMessage());
+ }
+ }
+
+ private ChangeInfo getNewChangeInfo(ChangeResource resource)
+ throws BadRequestException, PreconditionFailedException, IOException {
+ GetChange getChange = createGetChange();
+ getChange.setMetaRevId(getNewMetaRevId(resource));
+ return getChange.apply(resource).value();
+ }
+
+ private String getNewMetaRevId(ChangeResource resource) throws IOException {
+ if (!metaRevId.isEmpty()) {
+ return metaRevId;
+ }
+ try (Repository repo = repoManager.openRepository(resource.getProject())) {
+ return repo.exactRef(changeMetaRef(resource.getId())).getObjectId().getName();
+ }
+ }
+
+ private GetChange createGetChange() {
+ GetChange getChange = getChangeProvider.get();
+ options.forEach(getChange::addOption);
+ dynamicBeans.forEach(getChange::setDynamicBean);
+ return getChange;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index 28f4114..f87c9a1 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -82,6 +82,7 @@
postOnCollection(CHANGE_KIND).to(CreateChange.class);
get(CHANGE_KIND).to(GetChange.class);
+ get(CHANGE_KIND, "meta_diff").to(GetMetaDiff.class);
post(CHANGE_KIND, "merge").to(CreateMergePatchSet.class);
get(CHANGE_KIND, "detail").to(GetDetail.class);
get(CHANGE_KIND, "topic").to(GetTopic.class);
diff --git a/java/com/google/gerrit/server/restapi/group/GetAuditLog.java b/java/com/google/gerrit/server/restapi/group/GetAuditLog.java
index 706a1f4..e3aa0f3 100644
--- a/java/com/google/gerrit/server/restapi/group/GetAuditLog.java
+++ b/java/com/google/gerrit/server/restapi/group/GetAuditLog.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.group;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Comparator.comparing;
import com.google.gerrit.entities.AccountGroup;
@@ -42,7 +43,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import java.util.Optional;
+import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Repository;
@@ -105,14 +106,17 @@
member));
}
}
-
- for (AccountGroupByIdAudit auditEvent :
- groups.getSubgroupsAudit(allUsersRepo, group.getGroupUUID())) {
+ List<AccountGroupByIdAudit> subGroupsAudit =
+ groups.getSubgroupsAudit(allUsersRepo, group.getGroupUUID());
+ Map<AccountGroup.UUID, InternalGroup> groups =
+ groupCache.get(
+ subGroupsAudit.stream().map(a -> a.includeUuid()).collect(toImmutableList()));
+ for (AccountGroupByIdAudit auditEvent : subGroupsAudit) {
AccountGroup.UUID includedGroupUUID = auditEvent.includeUuid();
- Optional<InternalGroup> includedGroup = groupCache.get(includedGroupUUID);
+ InternalGroup includedGroup = groups.get(includedGroupUUID);
GroupInfo member;
- if (includedGroup.isPresent()) {
- member = groupJson.format(new InternalGroupDescription(includedGroup.get()));
+ if (includedGroup != null) {
+ member = groupJson.format(new InternalGroupDescription(includedGroup));
} else {
member = new GroupInfo();
member.id = Url.encode(includedGroupUUID.get());
diff --git a/java/com/google/gerrit/server/restapi/group/ListGroups.java b/java/com/google/gerrit/server/restapi/group/ListGroups.java
index 3e2a577..96402be 100644
--- a/java/com/google/gerrit/server/restapi/group/ListGroups.java
+++ b/java/com/google/gerrit/server/restapi/group/ListGroups.java
@@ -15,12 +15,12 @@
package com.google.gerrit.server.restapi.group;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.toList;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
-import com.google.common.collect.Streams;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
@@ -57,7 +57,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
-import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
@@ -273,10 +272,12 @@
throws IOException, ConfigInvalidException, PermissionBackendException {
Pattern pattern = getRegexPattern();
Stream<GroupDescription.Internal> existingGroups =
- getAllExistingGroups()
- .filter(group -> isRelevant(pattern, group))
- .map(this::loadGroup)
- .flatMap(Streams::stream)
+ loadGroups(
+ getAllExistingGroups()
+ .filter(group -> isRelevant(pattern, group))
+ .map(g -> g.getUUID())
+ .collect(toImmutableSet()))
+ .stream()
.filter(this::isVisible)
.sorted(GROUP_COMPARATOR)
.skip(start);
@@ -359,11 +360,13 @@
throws IOException, ConfigInvalidException, PermissionBackendException {
Pattern pattern = getRegexPattern();
Stream<? extends GroupDescription.Internal> foundGroups =
- groups
- .getAllGroupReferences()
- .filter(group -> isRelevant(pattern, group))
- .map(this::loadGroup)
- .flatMap(Streams::stream)
+ loadGroups(
+ groups
+ .getAllGroupReferences()
+ .filter(group -> isRelevant(pattern, group))
+ .map(g -> g.getUUID())
+ .collect(toImmutableSet()))
+ .stream()
.filter(this::isVisible)
.filter(filter)
.sorted(GROUP_COMPARATOR)
@@ -379,8 +382,10 @@
return groupInfos;
}
- private Optional<GroupDescription.Internal> loadGroup(GroupReference groupReference) {
- return groupCache.get(groupReference.getUUID()).map(InternalGroupDescription::new);
+ private Set<GroupDescription.Internal> loadGroups(Collection<AccountGroup.UUID> groupUuids) {
+ return groupCache.get(groupUuids).values().stream()
+ .map(InternalGroupDescription::new)
+ .collect(toImmutableSet());
}
private List<GroupInfo> getGroupsOwnedBy(String id)
diff --git a/java/com/google/gerrit/server/restapi/group/ListMembers.java b/java/com/google/gerrit/server/restapi/group/ListMembers.java
index cba6408d..218eb98 100644
--- a/java/com/google/gerrit/server/restapi/group/ListMembers.java
+++ b/java/com/google/gerrit/server/restapi/group/ListMembers.java
@@ -144,23 +144,21 @@
private Set<Account.Id> getIndirectMemberIds(
GroupDescription.Internal group, HashSet<AccountGroup.UUID> seenGroups) {
Set<Account.Id> indirectMembers = new HashSet<>();
+ Set<AccountGroup.UUID> subgroupMembersToLoad = new HashSet<>();
for (AccountGroup.UUID subgroupUuid : group.getSubgroups()) {
if (!seenGroups.contains(subgroupUuid)) {
seenGroups.add(subgroupUuid);
-
- Set<Account.Id> subgroupMembers =
- groupCache
- .get(subgroupUuid)
- .map(InternalGroupDescription::new)
- .map(
- subgroup -> {
- GroupControl subgroupControl = groupControlFactory.controlFor(subgroup);
- return getTransitiveMemberIds(subgroup, subgroupControl, seenGroups);
- })
- .orElseGet(ImmutableSet::of);
- indirectMembers.addAll(subgroupMembers);
+ subgroupMembersToLoad.add(subgroupUuid);
}
}
+ groupCache.get(subgroupMembersToLoad).values().stream()
+ .map(InternalGroupDescription::new)
+ .forEach(
+ subgroup -> {
+ GroupControl subgroupControl = groupControlFactory.controlFor(subgroup);
+ indirectMembers.addAll(getTransitiveMemberIds(subgroup, subgroupControl, seenGroups));
+ });
+
return indirectMembers;
}
diff --git a/java/com/google/gerrit/server/schema/MariaDBAccountPatchReviewStore.java b/java/com/google/gerrit/server/schema/MariaDBAccountPatchReviewStore.java
index b0a3370..dd82be2 100644
--- a/java/com/google/gerrit/server/schema/MariaDBAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/schema/MariaDBAccountPatchReviewStore.java
@@ -22,6 +22,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.sql.SQLException;
+import java.sql.Statement;
import org.eclipse.jgit.lib.Config;
@Singleton
@@ -50,4 +51,17 @@
return new StorageException(op + " failure on ACCOUNT_PATCH_REVIEWS", err);
}
}
+
+ @Override
+ protected void doCreateTable(Statement stmt) throws SQLException {
+ stmt.executeUpdate(
+ "CREATE TABLE IF NOT EXISTS account_patch_reviews ("
+ + "account_id INTEGER DEFAULT 0 NOT NULL, "
+ + "change_id INTEGER DEFAULT 0 NOT NULL, "
+ + "patch_set_id INTEGER DEFAULT 0 NOT NULL, "
+ + "file_name VARCHAR(255) DEFAULT '' NOT NULL, "
+ + "CONSTRAINT primary_key_account_patch_reviews "
+ + "PRIMARY KEY (change_id, patch_set_id, account_id, file_name)"
+ + ")");
+ }
}
diff --git a/java/com/google/gerrit/server/ssh/HostKey.java b/java/com/google/gerrit/server/ssh/HostKey.java
new file mode 100644
index 0000000..9397612
--- /dev/null
+++ b/java/com/google/gerrit/server/ssh/HostKey.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.ssh;
+
+public class HostKey {
+ private final String host;
+ private final byte[] key;
+
+ public HostKey(String host, byte[] key) {
+ this.host = host;
+ this.key = key;
+ }
+
+ public String getHost() {
+ return host;
+ }
+
+ public byte[] getKey() {
+ return key;
+ }
+}
diff --git a/java/com/google/gerrit/server/ssh/NoSshInfo.java b/java/com/google/gerrit/server/ssh/NoSshInfo.java
index 91a949b..a716398 100644
--- a/java/com/google/gerrit/server/ssh/NoSshInfo.java
+++ b/java/com/google/gerrit/server/ssh/NoSshInfo.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.ssh;
-import com.jcraft.jsch.HostKey;
import java.util.Collections;
import java.util.List;
diff --git a/java/com/google/gerrit/server/ssh/SshInfo.java b/java/com/google/gerrit/server/ssh/SshInfo.java
index 430846d..ec5a579 100644
--- a/java/com/google/gerrit/server/ssh/SshInfo.java
+++ b/java/com/google/gerrit/server/ssh/SshInfo.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.ssh;
-import com.jcraft.jsch.HostKey;
import java.util.List;
public interface SshInfo {
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index a5b88b4..0668c1e 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -4,6 +4,7 @@
name = "sshd",
srcs = glob(["**/*.java"]),
visibility = ["//visibility:public"],
+ runtime_deps = ["//lib:jsch"],
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
@@ -28,7 +29,7 @@
"//lib:guava",
"//lib:jgit",
"//lib:jgit-archive",
- "//lib:jsch",
+ "//lib:jgit-ssh-apache",
"//lib:servlet-api",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index cd5a511..9ae8660 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -37,6 +37,7 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.ssh.HostKey;
import com.google.gerrit.server.ssh.SshAdvertisedAddresses;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.ssh.SshListenAddresses;
@@ -44,8 +45,6 @@
import com.google.gerrit.server.util.SocketUtil;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import com.jcraft.jsch.HostKey;
-import com.jcraft.jsch.JSchException;
import java.io.File;
import java.io.IOException;
import java.net.InetAddress;
@@ -435,12 +434,7 @@
byte[] keyBin = buf.getCompactData();
for (String addr : advertised) {
- try {
- r.add(new HostKey(addr, keyBin));
- } catch (JSchException e) {
- logger.atWarning().log(
- "Cannot format SSHD host key [%s]: %s", pub.getAlgorithm(), e.getMessage());
- }
+ r.add(new HostKey(addr, keyBin));
}
}
diff --git a/java/com/google/gerrit/sshd/SshSessionFactoryInitializer.java b/java/com/google/gerrit/sshd/SshSessionFactoryInitializer.java
new file mode 100644
index 0000000..1cdf923
--- /dev/null
+++ b/java/com/google/gerrit/sshd/SshSessionFactoryInitializer.java
@@ -0,0 +1,37 @@
+// 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.sshd;
+
+import static com.google.gerrit.server.config.SshClientImplementation.APACHE;
+
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.SshSessionFactory;
+import org.eclipse.jgit.transport.sshd.DefaultProxyDataFactory;
+import org.eclipse.jgit.transport.sshd.JGitKeyCache;
+import org.eclipse.jgit.transport.sshd.SshdSessionFactory;
+import org.eclipse.jgit.util.FS;
+
+public class SshSessionFactoryInitializer {
+ public static void init(Config config) {
+ if (APACHE == config.getEnum("ssh", null, "clientImplementation", APACHE)) {
+ SshdSessionFactory factory =
+ new SshdSessionFactory(new JGitKeyCache(), new DefaultProxyDataFactory());
+ factory.setHomeDirectory(FS.DETECTED.userHome());
+ SshSessionFactory.setInstance(factory);
+ }
+ }
+
+ private SshSessionFactoryInitializer() {}
+}
diff --git a/java/com/google/gerrit/testing/TestLoggingActivator.java b/java/com/google/gerrit/testing/TestLoggingActivator.java
index a766429..6b5d8fd 100644
--- a/java/com/google/gerrit/testing/TestLoggingActivator.java
+++ b/java/com/google/gerrit/testing/TestLoggingActivator.java
@@ -31,6 +31,7 @@
// Silence non-critical messages from MINA SSHD.
.put("org.apache.mina", Level.WARN)
+ .put("org.apache.sshd.client", Level.WARN)
.put("org.apache.sshd.common", Level.WARN)
.put("org.apache.sshd.server", Level.WARN)
.put("org.apache.sshd.common.keyprovider.FileKeyPairProvider", Level.INFO)
@@ -61,6 +62,8 @@
// Silence non-critical messages from JGit.
.put("org.eclipse.jgit.transport.PacketLineIn", Level.WARN)
.put("org.eclipse.jgit.transport.PacketLineOut", Level.WARN)
+ .put("org.eclipse.jgit.internal.transport.sshd", Level.WARN)
+ .put("org.eclipse.jgit.util.FileUtils", Level.WARN)
.put("org.eclipse.jgit.internal.storage.file.FileSnapshot", Level.WARN)
.put("org.eclipse.jgit.util.FS", Level.WARN)
.put("org.eclipse.jgit.util.SystemReader", Level.WARN)
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 1b55652..7495e63 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -75,6 +75,7 @@
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.acceptance.testsuite.request.SshSessionFactory;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.AccessSection;
@@ -150,9 +151,9 @@
import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.jcraft.jsch.KeyPair;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.security.KeyPair;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -2001,7 +2002,7 @@
// Add a new key
sender.clear();
- String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
+ String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), admin.email());
gApi.accounts().self().addSshKey(newKey);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(2);
@@ -2023,7 +2024,7 @@
// Add another new key
sender.clear();
- String newKey2 = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
+ String newKey2 = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), admin.email());
gApi.accounts().self().addSshKey(newKey2);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(3);
@@ -2074,7 +2075,7 @@
// Add a new key
sender.clear();
- String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), user.email());
+ String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), user.email());
gApi.accounts().id(user.username()).addSshKey(newKey);
info = gApi.accounts().id(user.username()).listSshKeys();
assertThat(info).hasSize(2);
@@ -2103,7 +2104,7 @@
@Test
@UseSsh
public void userCannotAddSshKeyToOtherAccount() throws Exception {
- String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
+ String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), admin.email());
requestScopeOperations.setApiUser(user.id());
assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).addSshKey(newKey));
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
index e025c52..2cd04ed 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMetaIT.java
@@ -104,7 +104,7 @@
protected static class PluginDefinedSimpleAttributeModule extends AbstractModule {
static class MyMetaHash extends PluginDefinedInfo {
String myMetaRef;
- };
+ }
static PluginDefinedInfo newMyMetaHash(ChangeData cd) {
MyMetaHash mmh = new MyMetaHash();
@@ -125,6 +125,7 @@
}
@Test
+ @SuppressWarnings("unchecked")
public void pluginDefinedAttribute() throws Exception {
try (AutoCloseable ignored =
installPlugin("my-plugin", PluginDefinedSimpleAttributeModule.class)) {
@@ -133,7 +134,6 @@
gApi.changes().id(changeId).setMessage("before\n\n" + "Change-Id: " + result.getChangeId());
ChangeInfo before = gApi.changes().id(changeId).get();
gApi.changes().id(changeId).setMessage("after\n\n" + "Change-Id: " + result.getChangeId());
- ChangeInfo after = gApi.changes().id(changeId).get();
RestResponse resp =
adminRestSession.get("/changes/" + changeId + "/?meta=" + before.metaRevId);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/GetMetaDiffIT.java b/javatests/com/google/gerrit/acceptance/rest/change/GetMetaDiffIT.java
new file mode 100644
index 0000000..2cb96e8
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/change/GetMetaDiffIT.java
@@ -0,0 +1,200 @@
+// 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.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.HashtagsInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfoDifference;
+import org.junit.Test;
+
+public class GetMetaDiffIT extends AbstractDaemonTest {
+
+ private static final String UNSAVED_REV_ID = "0000000000000000000000000000000000000001";
+ private static final String TOPIC = "topic";
+ private static final String HASHTAG = "hashtag";
+
+ @Test
+ public void metaDiff() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeApi chApi = gApi.changes().id(ch.getChangeId());
+ chApi.topic(TOPIC);
+ ChangeInfo oldInfo = chApi.get();
+ chApi.topic(TOPIC + "-2");
+ chApi.setHashtags(new HashtagsInput(ImmutableSet.of(HASHTAG)));
+ ChangeInfo newInfo = chApi.get();
+
+ ChangeInfoDifference difference = chApi.metaDiff(oldInfo.metaRevId, newInfo.metaRevId);
+
+ assertThat(difference.added().topic).isEqualTo(newInfo.topic);
+ assertThat(difference.added().hashtags).isNotNull();
+ assertThat(difference.added().hashtags).containsExactly(HASHTAG);
+ assertThat(difference.removed().topic).isEqualTo(oldInfo.topic);
+ assertThat(difference.removed().hashtags).isNull();
+ }
+
+ @Test
+ public void metaDiffReturnsSuccessful() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeInfo info = gApi.changes().id(ch.getChangeId()).get();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + ch.getChangeId() + "/meta_diff/?meta=" + info.metaRevId);
+
+ resp.assertOK();
+ }
+
+ @Test
+ public void metaDiffUnreachableNewSha1() throws Exception {
+ PushOneCommit.Result ch1 = createChange();
+ PushOneCommit.Result ch2 = createChange();
+
+ ChangeInfo info2 = gApi.changes().id(ch2.getChangeId()).get();
+
+ RestResponse resp =
+ adminRestSession.get(
+ "/changes/" + ch1.getChangeId() + "/meta_diff/?meta=" + info2.metaRevId);
+
+ resp.assertStatus(412);
+ }
+
+ @Test
+ public void metaDiffInvalidNewSha1() throws Exception {
+ PushOneCommit.Result ch = createChange();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + ch.getChangeId() + "/meta_diff/?meta=invalid");
+
+ resp.assertBadRequest();
+ }
+
+ @Test
+ public void metaDiffInvalidOldSha1() throws Exception {
+ PushOneCommit.Result ch = createChange();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + ch.getChangeId() + "/meta_diff/?old=invalid");
+
+ resp.assertBadRequest();
+ }
+
+ @Test
+ public void metaDiffWithNewSha1NotInRepo() throws Exception {
+ PushOneCommit.Result ch = createChange();
+
+ RestResponse resp =
+ adminRestSession.get("/changes/" + ch.getChangeId() + "/meta_diff/?meta=" + UNSAVED_REV_ID);
+
+ resp.assertStatus(412);
+ }
+
+ @Test
+ public void metaDiffUnreachableOldSha1UsesDefault() throws Exception {
+ PushOneCommit.Result ch1 = createChange();
+ PushOneCommit.Result ch2 = createChange();
+ gApi.changes().id(ch1.getChangeId()).topic("intermediate-topic");
+ gApi.changes().id(ch1.getChangeId()).topic(TOPIC);
+ ChangeInfo info1 = gApi.changes().id(ch1.getChangeId()).get();
+ ChangeInfo info2 = gApi.changes().id(ch2.getChangeId()).get();
+
+ ChangeInfoDifference difference =
+ gApi.changes().id(ch1.getChangeId()).metaDiff(info2.metaRevId, info1.metaRevId);
+
+ assertThat(difference.added().topic).isEqualTo(TOPIC);
+ assertThat(difference.removed().topic).isNull();
+ }
+
+ @Test
+ public void metaDiffWithOldSha1NotInRepoUsesDefault() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ gApi.changes().id(ch.getChangeId()).topic("intermediate-topic");
+ gApi.changes().id(ch.getChangeId()).topic(TOPIC);
+ ChangeInfo info = gApi.changes().id(ch.getChangeId()).get();
+
+ ChangeInfoDifference difference =
+ gApi.changes().id(ch.getChangeId()).metaDiff(UNSAVED_REV_ID, info.metaRevId);
+
+ assertThat(difference.added().topic).isEqualTo(TOPIC);
+ assertThat(difference.removed().topic).isNull();
+ }
+
+ @Test
+ public void metaDiffNoOldMetaGivenUsesPatchSetBeforeNew() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeApi chApi = gApi.changes().id(ch.getChangeId());
+ chApi.topic(TOPIC);
+ ChangeInfo newInfo = chApi.get();
+ chApi.topic(TOPIC + "2");
+
+ ChangeInfoDifference difference = chApi.metaDiff(null, newInfo.metaRevId);
+
+ assertThat(difference.added().topic).isEqualTo(TOPIC);
+ assertThat(difference.removed().topic).isNull();
+ }
+
+ @Test
+ public void metaDiffNoNewMetaGivenUsesCurrentPatchSet() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeApi chApi = gApi.changes().id(ch.getChangeId());
+ ChangeInfo oldInfo = chApi.get();
+ chApi.topic(TOPIC);
+
+ ChangeInfoDifference difference = chApi.metaDiff(oldInfo.metaRevId, null);
+
+ assertThat(difference.added().topic).isEqualTo(TOPIC);
+ assertThat(difference.removed().topic).isNull();
+ }
+
+ @Test
+ public void metaDiffWithoutOptionDoesNotIncludeExtraInformation() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeApi chApi = gApi.changes().id(ch.getChangeId());
+ ChangeInfo oldInfo = chApi.get();
+ amendChange(ch.getChangeId());
+ ChangeInfo newInfo = chApi.get();
+
+ ChangeInfoDifference difference = chApi.metaDiff(oldInfo.metaRevId, newInfo.metaRevId);
+
+ assertThat(difference.added().currentRevision).isNull();
+ assertThat(difference.removed().currentRevision).isNull();
+ }
+
+ @Test
+ public void metaDiffWithOptionIncludesExtraInformation() throws Exception {
+ PushOneCommit.Result ch = createChange();
+ ChangeApi chApi = gApi.changes().id(ch.getChangeId());
+ ChangeInfo oldInfo = chApi.get(ListChangesOption.CURRENT_REVISION);
+ amendChange(ch.getChangeId());
+ ChangeInfo newInfo = chApi.get(ListChangesOption.CURRENT_REVISION);
+
+ ChangeInfoDifference difference =
+ chApi.metaDiff(
+ oldInfo.metaRevId,
+ newInfo.metaRevId,
+ ImmutableSet.of(ListChangesOption.CURRENT_REVISION));
+
+ assertThat(newInfo.currentRevision).isNotNull();
+ assertThat(oldInfo.currentRevision).isNotNull();
+ assertThat(difference.added().currentRevision).isEqualTo(newInfo.currentRevision);
+ assertThat(difference.removed().currentRevision).isEqualTo(oldInfo.currentRevision);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
index ffdbd8e..83d2256 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java
@@ -175,7 +175,7 @@
expectedCfg.setString("s3", null, "k5", "childValue3");
expectedCfg.setString("s3", "ss", "k6", "childValue4");
- assertThat(state.getConfig(configName).getWithInheritance(true).toText())
+ assertThat(state.getConfig(configName).getWithInheritance(/* merge= */ true).toText())
.isEqualTo(expectedCfg.toText());
assertThat(state.getConfig(configName).get().toText()).isEqualTo(cfg.toText());
diff --git a/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
index 801288a..2692584 100644
--- a/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
@@ -34,7 +34,7 @@
import com.google.gerrit.server.quota.QuotaResponse;
import com.google.inject.Module;
import java.util.Collections;
-import org.eclipse.jgit.api.errors.TooLargeObjectInPackException;
+import org.eclipse.jgit.api.errors.TooLargePackException;
import org.eclipse.jgit.api.errors.TransportException;
import org.junit.Before;
import org.junit.Test;
@@ -77,7 +77,7 @@
@Test
public void pushWithAvailableTokens() throws Exception {
when(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP))
- .thenReturn(singletonAggregation(ok(276L)));
+ .thenReturn(singletonAggregation(ok(277L)));
when(quotaBackendWithResource.requestTokens(eq(REPOSITORY_SIZE_GROUP), anyLong()))
.thenReturn(singletonAggregation(ok()));
when(quotaBackendWithUser.project(project)).thenReturn(quotaBackendWithResource);
@@ -91,12 +91,10 @@
when(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP))
.thenReturn(singletonAggregation(ok(availableTokens)));
when(quotaBackendWithUser.project(project)).thenReturn(quotaBackendWithResource);
- TooLargeObjectInPackException thrown =
- assertThrows(TooLargeObjectInPackException.class, () -> pushCommit());
- assertThat(thrown).hasMessageThat().contains("Object too large");
- assertThat(thrown)
- .hasMessageThat()
- .contains(String.format("Max object size limit is %d bytes.", availableTokens));
+ assertThat(assertThrows(TooLargePackException.class, () -> pushCommit()).getMessage())
+ .contains(
+ String.format(
+ "Pack exceeds the limit of %d bytes, rejecting the pack", availableTokens));
}
@Test
diff --git a/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java b/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java
index a41d63b..09b0438 100644
--- a/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java
+++ b/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java
@@ -37,6 +37,7 @@
ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(new ChangeInfo(), new ChangeInfo());
// Spot check a few fields, including collections and maps.
+ assertThat(diff.added()._number).isNull();
assertThat(diff.added().branch).isNull();
assertThat(diff.added().project).isNull();
assertThat(diff.added().currentRevision).isNull();
@@ -44,6 +45,7 @@
assertThat(diff.added().messages).isNull();
assertThat(diff.added().reviewers).isNull();
assertThat(diff.added().hashtags).isNull();
+ assertThat(diff.removed()._number).isNull();
assertThat(diff.removed().branch).isNull();
assertThat(diff.removed().project).isNull();
assertThat(diff.removed().currentRevision).isNull();
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializerTest.java
new file mode 100644
index 0000000..8d301e4
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializerTest.java
@@ -0,0 +1,61 @@
+// 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.cache.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer.deserialize;
+import static com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer.serialize;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class InternalGroupSerializerTest {
+ static final InternalGroup MINIMAL_VALUES_SET =
+ InternalGroup.builder()
+ .setId(AccountGroup.id(123456))
+ .setNameKey(AccountGroup.nameKey("group name"))
+ .setOwnerGroupUUID(AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"))
+ .setVisibleToAll(false)
+ .setGroupUUID(AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeef12345678"))
+ .setCreatedOn(TimeUtil.nowTs())
+ .setMembers(ImmutableSet.of(Account.id(123), Account.id(321)))
+ .setSubgroups(
+ ImmutableSet.of(
+ AccountGroup.uuid("87654321deadbeefdeadbeefdeadbeefdeadbeef"),
+ AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeef87654321")))
+ .build();
+
+ static final InternalGroup ALL_VALUES_SET =
+ MINIMAL_VALUES_SET
+ .toBuilder()
+ .setDescription("description")
+ .setRefState(ObjectId.fromString("12345678deadbeefdeadbeefdeadbeefdeadbeef"))
+ .build();
+
+ @Test
+ public void roundTrip() {
+ assertThat(deserialize(serialize(ALL_VALUES_SET))).isEqualTo(ALL_VALUES_SET);
+ }
+
+ @Test
+ public void roundTripWithMinimalValues() {
+ assertThat(deserialize(serialize(MINIMAL_VALUES_SET))).isEqualTo(MINIMAL_VALUES_SET);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/permissions/SectionSortCacheTest.java b/javatests/com/google/gerrit/server/permissions/SectionSortCacheTest.java
new file mode 100644
index 0000000..9ec1625
--- /dev/null
+++ b/javatests/com/google/gerrit/server/permissions/SectionSortCacheTest.java
@@ -0,0 +1,84 @@
+// 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.permissions;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.gerrit.entities.AccessSection;
+import com.google.gerrit.server.permissions.SectionSortCache.EntryKey;
+import com.google.gerrit.server.permissions.SectionSortCache.EntryVal;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Test for {@link SectionSortCache} */
+public class SectionSortCacheTest {
+ private SectionSortCache sectionSortCache;
+ private Cache<EntryKey, EntryVal> cache;
+
+ private static final AccessSection sectionA = AccessSection.create("refs/heads/branch_1");
+ private static final AccessSection sectionB = AccessSection.create("refs/base/branch_2");
+ private static final String REF_BASE = "refs/base";
+
+ @Before
+ public void setup() {
+ cache = CacheBuilder.newBuilder().build();
+ sectionSortCache = new SectionSortCache(cache);
+ }
+
+ @Test
+ public void sortSingleElement() {
+ List<AccessSection> input = new ArrayList<>();
+ input.add(sectionA);
+ sectionSortCache.sort(REF_BASE, input);
+ assertThat(input).containsExactly(sectionA);
+ }
+
+ @Test
+ public void sortMultiElements() {
+ List<AccessSection> input = new ArrayList<>();
+ input.add(sectionA);
+ input.add(sectionB);
+ sectionSortCache.sort(REF_BASE, input);
+ assertThat(input).containsExactly(sectionB, sectionA).inOrder();
+ }
+
+ @Test
+ public void sortMultiElementsWhenAlreadyOrdered() {
+ List<AccessSection> input = new ArrayList<>();
+ input.add(sectionB);
+ input.add(sectionA);
+ sectionSortCache.sort(REF_BASE, input);
+ assertThat(input).containsExactly(sectionB, sectionA).inOrder();
+ }
+
+ @Test
+ public void sortMultiElementsWithDuplicates() {
+ AccessSection sectionAClone = sectionA.toBuilder().build();
+ AccessSection sectionBClone = sectionB.toBuilder().build();
+ AccessSection[] input = {sectionBClone, sectionA, sectionAClone, sectionA, sectionB};
+ List<AccessSection> sorted = Arrays.asList(input);
+ sectionSortCache.sort(REF_BASE, sorted);
+ // Cache preserves relative order (reference equality) for identical elements
+ AccessSection[] expected = {sectionBClone, sectionB, sectionA, sectionAClone, sectionA};
+ for (int i = 0; i < sorted.size(); i++) {
+ assert (sorted.get(i) == expected[i]);
+ }
+ }
+}
diff --git a/lib/BUILD b/lib/BUILD
index 0110047..f924e4ca 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -54,6 +54,16 @@
)
java_library(
+ name = "jgit-ssh-apache",
+ data = ["//lib:LICENSE-jgit"],
+ visibility = ["//visibility:public"],
+ exports = ["@jgit//org.eclipse.jgit.ssh.apache:ssh-apache"],
+ runtime_deps = [
+ "//lib/mina:sshd-sftp",
+ ],
+)
+
+java_library(
name = "jgit-archive",
data = ["//lib:LICENSE-jgit"],
visibility = ["//visibility:public"],
diff --git a/lib/mina/BUILD b/lib/mina/BUILD
index 70e7c1d..3f23263 100644
--- a/lib/mina/BUILD
+++ b/lib/mina/BUILD
@@ -13,6 +13,13 @@
)
java_library(
+ name = "sshd-sftp",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@sshd-sftp//jar"],
+)
+
+java_library(
name = "eddsa",
data = ["//lib:LICENSE-CC0-1.0"],
visibility = ["//visibility:public"],
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index f596164..272cfa9 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -40,6 +40,7 @@
soy
sshd-mina
sshd-osgi
+sshd-sftp
testcontainers
truth
truth-java8-extension
diff --git a/modules/jgit b/modules/jgit
index 9bfb0f3..c82818e 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 9bfb0f3a4ec856dcbebb477a1ee8803a3c47c194
+Subproject commit c82818e0e02a9d1bd979d27bd342bb05661150d4
diff --git a/package.json b/package.json
index fc4161b..7f4ac44 100644
--- a/package.json
+++ b/package.json
@@ -3,9 +3,9 @@
"version": "3.1.0-SNAPSHOT",
"description": "Gerrit Code Review",
"dependencies": {
- "@bazel/rollup": "^3.2.0",
- "@bazel/terser": "^3.2.0",
- "@bazel/typescript": "^3.2.0"
+ "@bazel/rollup": "^3.2.3",
+ "@bazel/terser": "^3.2.3",
+ "@bazel/typescript": "^3.2.3"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^4.11.0",
diff --git a/plugins/replication b/plugins/replication
index bf19013..93e61dc 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit bf19013d9d4b93558eaa015fd3c14c01490e4915
+Subproject commit 93e61dc64debe42eab454e6c268f9c4ee22a78bc
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index a177a24..6797994 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -284,7 +284,6 @@
annotate(
textElement: HTMLElement,
lineNumberElement: HTMLElement,
- line: GrDiffLine,
- side: Side
+ line: GrDiffLine
): void;
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index a862b27..da63314 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -54,6 +54,7 @@
import {CustomKeyboardEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
import {windowLocationReload} from '../../../utils/dom-util';
+import {ScrollMode} from '../../../constants/constants';
const NUMBER_FIXED_COLUMNS = 3;
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
@@ -66,9 +67,7 @@
results: ChangeInfo[];
}
export interface GrChangeList {
- $: {
- cursor: GrCursorManager;
- };
+ $: {};
}
@customElement('gr-change-list')
export class GrChangeList extends ChangeTableMixin(
@@ -157,6 +156,15 @@
};
}
+ private cursor = new GrCursorManager();
+
+ constructor() {
+ super();
+ this.cursor.scrollMode = ScrollMode.KEEP_VISIBLE;
+ this.cursor.focusOnMove = true;
+ this.addEventListener('keydown', e => this._scopedKeydownHandler(e));
+ }
+
/** @override */
ready() {
super.ready();
@@ -168,7 +176,6 @@
/** @override */
connectedCallback() {
super.connectedCallback();
- this.addEventListener('keydown', e => this._scopedKeydownHandler(e));
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -180,7 +187,7 @@
/** @override */
disconnectedCallback() {
- this.$.cursor.unsetCursor();
+ this.cursor.unsetCursor();
super.disconnectedCallback();
}
@@ -392,8 +399,8 @@
}
e.preventDefault();
- this.$.cursor.next();
- this.selectedIndex = this.$.cursor.index;
+ this.cursor.next();
+ this.selectedIndex = this.cursor.index;
}
_prevChange(e: CustomKeyboardEvent) {
@@ -402,8 +409,8 @@
}
e.preventDefault();
- this.$.cursor.previous();
- this.selectedIndex = this.$.cursor.index;
+ this.cursor.previous();
+ this.selectedIndex = this.cursor.index;
}
_openChange(e: CustomKeyboardEvent) {
@@ -516,10 +523,9 @@
_sectionsChanged() {
// Flush DOM operations so that the list item elements will be loaded.
afterNextRender(this, () => {
- this.$.cursor.stops = this._getListItems();
- this.$.cursor.moveToStart();
- if (this.selectedIndex)
- this.$.cursor.setCursorAtIndex(this.selectedIndex);
+ this.cursor.stops = this._getListItems();
+ this.cursor.moveToStart();
+ if (this.selectedIndex) this.cursor.setCursorAtIndex(this.selectedIndex);
});
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.ts
index 0313e0b..b6b4160 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.ts
@@ -161,9 +161,4 @@
</tbody>
</template>
</table>
- <gr-cursor-manager
- id="cursor"
- scroll-mode="keep-visible"
- focus-on-move=""
- ></gr-cursor-manager>
`;
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 c4c2b4e..aae8bb6 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
@@ -124,14 +124,10 @@
render() {
const chipClass = `summaryChip font-small ${this.styleType}`;
const grIcon = this.icon ? `gr-icons:${this.icon}` : '';
- return html`<div
- class="${chipClass}"
- role="button"
- @click="${this.handleClick}"
- >
+ return html`<button class="${chipClass}" @click="${this.handleClick}">
${this.icon && html`<iron-icon icon="${grIcon}"></iron-icon>`}
<slot></slot>
- </div>`;
+ </button>`;
}
private handleClick(e: MouseEvent) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index a0bea33..a9f8860 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -197,13 +197,11 @@
_computeExpandedClass(filesExpanded: FilesExpandedState) {
const classes = [];
if (filesExpanded === FilesExpandedState.ALL) {
- classes.push('expanded');
- }
- if (
- filesExpanded === FilesExpandedState.SOME ||
- filesExpanded === FilesExpandedState.ALL
- ) {
classes.push('openFile');
+ classes.push('allExpanded');
+ } else if (filesExpanded === FilesExpandedState.SOME) {
+ classes.push('openFile');
+ classes.push('someExpanded');
}
return classes.join(' ');
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index af72b67..e0ee812 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -82,17 +82,18 @@
justify-content: flex-end;
}
#collapseBtn,
- .expanded #expandBtn,
+ .allExpanded #expandBtn,
.fileViewActions {
display: none;
}
- .expanded #expandBtn {
- display: none;
+ .someExpanded #expandBtn {
+ margin-right: 8px;
}
gr-linked-chip {
--linked-chip-text-color: var(--primary-text-color);
}
- .expanded #collapseBtn,
+ .someExpanded #collapseBtn,
+ .allExpanded #collapseBtn,
.openFile .fileViewActions {
align-items: center;
display: flex;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
index f877527..a9349fb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -161,7 +161,7 @@
assert.isTrue(element._expandAllDiffs.called);
});
- test('collapseAllDiffs called when expand button clicked', () => {
+ test('collapseAllDiffs called when collapse button clicked', () => {
element.shownFileCount = 1;
flush();
sinon.stub(element, '_collapseAllDiffs');
@@ -203,20 +203,29 @@
});
test('expand/collapse buttons are toggled correctly', () => {
+ // Only the expand button should be visible in the initial state when
+ // NO files are expanded.
element.shownFileCount = 10;
flush();
const expandBtn = element.shadowRoot.querySelector('#expandBtn');
const collapseBtn = element.shadowRoot.querySelector('#collapseBtn');
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
+
+ // Both expand and collapse buttons should be visible when SOME files are
+ // expanded.
element.filesExpanded = FilesExpandedState.SOME;
flush();
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
- assert.equal(getComputedStyle(collapseBtn).display, 'none');
+ assert.notEqual(getComputedStyle(collapseBtn).display, 'none');
+
+ // Only the collapse button should be visible when ALL files are expanded.
element.filesExpanded = FilesExpandedState.ALL;
flush();
assert.equal(getComputedStyle(expandBtn).display, 'none');
assert.notEqual(getComputedStyle(collapseBtn).display, 'none');
+
+ // Only the expand button should be visible when NO files are expanded.
element.filesExpanded = FilesExpandedState.NONE;
flush();
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index a7894af..4b5525a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -42,7 +42,11 @@
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {appContext} from '../../../services/app-context';
-import {DiffViewMode, SpecialFilePath} from '../../../constants/constants';
+import {
+ DiffViewMode,
+ ScrollMode,
+ SpecialFilePath,
+} from '../../../constants/constants';
import {descendedFromClass, toggleClass} from '../../../utils/dom-util';
import {
addUnmodifiedFiles,
@@ -88,7 +92,6 @@
$: {
diffPreferencesDialog: GrDiffPreferencesDialog;
diffCursor: GrDiffCursor;
- fileCursor: GrCursorManager;
};
}
@@ -341,10 +344,19 @@
};
}
+ private fileCursor = new GrCursorManager();
+
+ constructor() {
+ super();
+ this.fileCursor.scrollMode = ScrollMode.KEEP_VISIBLE;
+ this.fileCursor.cursorTargetClass = 'selected';
+ this.fileCursor.focusOnMove = true;
+ this.addEventListener('keydown', e => this._scopedKeydownHandler(e));
+ }
+
/** @override */
connectedCallback() {
super.connectedCallback();
- this.addEventListener('keydown', e => this._scopedKeydownHandler(e));
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -393,7 +405,7 @@
/** @override */
disconnectedCallback() {
- this.$.fileCursor.unsetCursor();
+ this.fileCursor.unsetCursor();
this._cancelDiffs();
this.loadingTask?.cancel();
super.disconnectedCallback();
@@ -777,7 +789,7 @@
e.preventDefault();
// Prevent _handleFileListClick handler call
e.stopPropagation();
- this.$.fileCursor.setCursor(fileRow.element);
+ this.fileCursor.setCursor(fileRow.element);
fileAction(fileRow.file);
}
}
@@ -817,7 +829,7 @@
}
e.preventDefault();
- this.$.fileCursor.setCursor(fileRow.element);
+ this.fileCursor.setCursor(fileRow.element);
this._toggleFileExpanded(file);
}
@@ -874,13 +886,13 @@
if (
this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e) ||
- this.$.fileCursor.index === -1
+ this.fileCursor.index === -1
) {
return;
}
e.preventDefault();
- this._toggleFileExpandedByIndex(this.$.fileCursor.index);
+ this._toggleFileExpandedByIndex(this.fileCursor.index);
}
_handleToggleAllInlineDiffs(e: CustomKeyboardEvent) {
@@ -916,8 +928,8 @@
return;
}
e.preventDefault();
- this.$.fileCursor.next();
- this.selectedIndex = this.$.fileCursor.index;
+ this.fileCursor.next();
+ this.selectedIndex = this.fileCursor.index;
}
}
@@ -936,8 +948,8 @@
return;
}
e.preventDefault();
- this.$.fileCursor.previous();
- this.selectedIndex = this.$.fileCursor.index;
+ this.fileCursor.previous();
+ this.selectedIndex = this.fileCursor.index;
}
}
@@ -1032,10 +1044,10 @@
}
e.preventDefault();
- if (!this._files[this.$.fileCursor.index]) {
+ if (!this._files[this.fileCursor.index]) {
return;
}
- this._reviewFile(this._files[this.$.fileCursor.index].__path);
+ this._reviewFile(this._files[this.fileCursor.index].__path);
}
_handleToggleLeftPane(e: CustomKeyboardEvent) {
@@ -1072,9 +1084,9 @@
_openSelectedFile(index?: number) {
if (index !== undefined) {
- this.$.fileCursor.setCursorAtIndex(index);
+ this.fileCursor.setCursorAtIndex(index);
}
- if (!this._files[this.$.fileCursor.index]) {
+ if (!this._files[this.fileCursor.index]) {
return;
}
if (!this.change || !this.patchRange) {
@@ -1082,7 +1094,7 @@
}
GerritNav.navigateToDiff(
this.change,
- this._files[this.$.fileCursor.index].__path,
+ this._files[this.fileCursor.index].__path,
this.patchRange.patchNum,
this.patchRange.basePatchNum
);
@@ -1284,10 +1296,10 @@
_filesChanged() {
if (this._files && this._files.length > 0) {
flush();
- this.$.fileCursor.stops = Array.from(
+ this.fileCursor.stops = Array.from(
this.root!.querySelectorAll(`.${FILE_ROW_CLASS}`)
);
- this.$.fileCursor.setCursorAtIndex(this.selectedIndex, true);
+ this.fileCursor.setCursorAtIndex(this.selectedIndex, true);
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
index 2078d1a..89983ad 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
@@ -737,10 +737,4 @@
>
</gr-diff-preferences-dialog>
<gr-diff-cursor id="diffCursor"></gr-diff-cursor>
- <gr-cursor-manager
- id="fileCursor"
- scroll-mode="keep-visible"
- focus-on-move=""
- cursor-target-class="selected"
- ></gr-cursor-manager>
`;
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 4cd12cf..b8ba86c 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
@@ -480,7 +480,7 @@
patchNum: 2,
};
element.change = {_number: 42};
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.setCursorAtIndex(0);
});
test('toggle left diff via shortcut', () => {
@@ -498,38 +498,38 @@
flush();
const items = [...element.root.querySelectorAll('.file-row')];
- element.$.fileCursor.stops = items;
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.stops = items;
+ element.fileCursor.setCursorAtIndex(0);
assert.equal(items.length, 3);
assert.isTrue(items[0].classList.contains('selected'));
assert.isFalse(items[1].classList.contains('selected'));
assert.isFalse(items[2].classList.contains('selected'));
// j with a modifier should not move the cursor.
MockInteractions.pressAndReleaseKeyOn(element, 74, 'shift', 'j');
- assert.equal(element.$.fileCursor.index, 0);
+ assert.equal(element.fileCursor.index, 0);
// down should not move the cursor.
MockInteractions.pressAndReleaseKeyOn(element, 40, null, 'down');
- assert.equal(element.$.fileCursor.index, 0);
+ assert.equal(element.fileCursor.index, 0);
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
- assert.equal(element.$.fileCursor.index, 1);
+ assert.equal(element.fileCursor.index, 1);
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
const navStub = sinon.stub(GerritNav, 'navigateToDiff');
- assert.equal(element.$.fileCursor.index, 2);
+ assert.equal(element.fileCursor.index, 2);
assert.equal(element.selectedIndex, 2);
// k with a modifier should not move the cursor.
MockInteractions.pressAndReleaseKeyOn(element, 75, 'shift', 'k');
- assert.equal(element.$.fileCursor.index, 2);
+ assert.equal(element.fileCursor.index, 2);
// up should not move the cursor.
MockInteractions.pressAndReleaseKeyOn(element, 38, null, 'down');
- assert.equal(element.$.fileCursor.index, 2);
+ assert.equal(element.fileCursor.index, 2);
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
- assert.equal(element.$.fileCursor.index, 1);
+ assert.equal(element.fileCursor.index, 1);
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 79, null, 'o');
@@ -540,7 +540,7 @@
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
- assert.equal(element.$.fileCursor.index, 0);
+ assert.equal(element.fileCursor.index, 0);
assert.equal(element.selectedIndex, 0);
const createCommentInPlaceStub = sinon.stub(element.$.diffCursor,
@@ -554,8 +554,8 @@
sinon.stub(element, '_expandedFilesChanged');
flush();
const files = [...element.root.querySelectorAll('.file-row')];
- element.$.fileCursor.stops = files;
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.stops = files;
+ element.fileCursor.setCursorAtIndex(0);
assert.equal(element.diffs.length, 0);
assert.equal(element._expandedFiles.length, 0);
@@ -571,7 +571,7 @@
assert.equal(element.diffs.length, 0);
assert.equal(element._expandedFiles.length, 0);
- element.$.fileCursor.setCursorAtIndex(1);
+ element.fileCursor.setCursorAtIndex(1);
MockInteractions.keyUpOn(element, 73, null, 'i');
flush();
assert.equal(element.diffs.length, 1);
@@ -698,7 +698,7 @@
basePatchNum: 'PARENT',
patchNum: 2,
};
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.setCursorAtIndex(0);
const reviewSpy = sinon.spy(element, '_reviewFile');
const toggleExpandSpy = sinon.spy(element, '_toggleFileExpanded');
@@ -804,7 +804,7 @@
basePatchNum: 'PARENT',
patchNum: 2,
};
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.setCursorAtIndex(0);
sinon.stub(element, '_expandedFilesChanged');
flush();
const fileRows =
@@ -832,7 +832,7 @@
patchNum: 2,
};
sinon.spy(element, '_updateDiffPreferences');
- element.$.fileCursor.setCursorAtIndex(0);
+ element.fileCursor.setCursorAtIndex(0);
flush();
// Tap on a file to generate the diff.
@@ -1483,7 +1483,7 @@
assert.isFalse(diffStops[11].classList.contains('target-row'));
// The file cursor is now at 1.
- assert.equal(element.$.fileCursor.index, 1);
+ assert.equal(element.fileCursor.index, 1);
MockInteractions.keyUpOn(element, 73, null, 'i');
flush();
@@ -1525,7 +1525,7 @@
assert.isTrue(diffStops[11].classList.contains('target-row'));
// The file cursor is still at 0.
- assert.equal(element.$.fileCursor.index, 0);
+ assert.equal(element.fileCursor.index, 0);
});
suite('n key presses', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
index 6d678eb..c53f386 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
@@ -62,16 +62,40 @@
}
gr-button.iron-selected[vote='positive'] {
--button-background-color: var(--vote-color-recommended);
- border-radius: 12px;
- border: 1px solid var(--vote-outline-recommended);
+ --gr-button: {
+ padding: 0 var(--spacing-m);
+ border-style: solid;
+ border-color: var(--vote-outline-recommended);
+ border-top-left-radius: 1em;
+ border-top-right-radius: 1em;
+ border-bottom-right-radius: 1em;
+ border-bottom-left-radius: 1em;
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+ color: var(--chip-color);
+ }
}
gr-button.iron-selected[vote='min'] {
--button-background-color: var(--vote-color-rejected);
}
gr-button.iron-selected[vote='negative'] {
--button-background-color: var(--vote-color-disliked);
- border-radius: 12px;
- border: 1px solid var(--vote-outline-disliked);
+ --gr-button: {
+ padding: 0 var(--spacing-m);
+ border-style: solid;
+ border-color: var(--vote-outline-disliked);
+ border-top-left-radius: 1em;
+ border-top-right-radius: 1em;
+ border-bottom-right-radius: 1em;
+ border-bottom-left-radius: 1em;
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+ color: var(--chip-color);
+ }
}
gr-button.iron-selected[vote='neutral'] {
--button-background-color: var(--vote-color-neutral);
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index ed1c981..4a41316 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -171,18 +171,28 @@
.score.negative {
background-color: var(--vote-color-disliked);
border: 1px solid var(--vote-outline-disliked);
+ line-height: calc(var(--line-height-normal) - 2px);
+ color: var(--chip-color);
}
.score.negative.min {
background-color: var(--vote-color-rejected);
border: none;
+ padding-top: 1px;
+ padding-bottom: 1px;
+ color: var(--vote-text-color);
}
.score.positive {
background-color: var(--vote-color-recommended);
border: 1px solid var(--vote-outline-recommended);
+ line-height: calc(var(--line-height-normal) - 2px);
+ color: var(--chip-color);
}
.score.positive.max {
background-color: var(--vote-color-approved);
border: none;
+ padding-top: 1px;
+ padding-bottom: 1px;
+ color: var(--vote-text-color);
}
gr-account-label {
--gr-account-label-text-style: {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 1cedf55..99abf89 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -508,19 +508,29 @@
oldValue?: CommentTabState
) {
if (!newValue || newValue === oldValue) return;
+ let focusTo: string | undefined;
switch (newValue) {
case CommentTabState.UNRESOLVED:
this._handleOnlyUnresolved();
+ // input is null because it's not rendered yet.
+ focusTo = '#unresolvedRadio';
break;
case CommentTabState.DRAFTS:
this._handleOnlyDrafts();
+ focusTo = '#draftsRadio';
break;
case CommentTabState.SHOW_ALL:
this._handleAllComments();
+ focusTo = '#allRadio';
break;
default:
assertNever(newValue, 'Unsupported preferred state');
}
+ const selector = focusTo;
+ window.setTimeout(() => {
+ const input = this.shadowRoot?.querySelector<HTMLInputElement>(selector);
+ input?.focus();
+ }, 0);
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
index 7b9329d..a674a8a 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
@@ -236,7 +236,7 @@
items.forEach(link => {
topMenuLinks[m.name].push(link);
});
- } else {
+ } else if (items.length > 0) {
links.push({
title: m.name,
links: topMenuLinks[m.name] = items,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
index c51edf3..d12fa92 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -46,9 +46,8 @@
import {GrDiffLine, LineNumber} from '../gr-diff/gr-diff-line';
import {GrDiffGroup} from '../gr-diff/gr-diff-group';
import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
-import {getLineNumber, getSideByLineEl} from '../gr-diff/gr-diff-utils';
+import {getLineNumber} from '../gr-diff/gr-diff-utils';
import {fireAlert, fireEvent} from '../../../utils/event-util';
-import {TokenHighlightLayer} from './token-highlight-layer';
const TRAILING_WHITESPACE_PATTERN = /\s+$/;
@@ -246,10 +245,10 @@
this._createTrailingWhitespaceLayer(),
this._createIntralineLayer(),
this._createTabIndicatorLayer(),
+ this._createSpecialCharacterIndicatorLayer(),
this.$.rangeLayer,
this.$.coverageLayerLeft,
this.$.coverageLayerRight,
- new TokenHighlightLayer(),
];
if (this.layers) {
@@ -258,6 +257,26 @@
this._layers = layers;
}
+ getLineElByChild(node?: Node): HTMLElement | null {
+ while (node) {
+ if (node instanceof Element) {
+ if (node.classList.contains('lineNum')) {
+ return node as HTMLElement;
+ }
+ if (node.classList.contains('section')) {
+ return null;
+ }
+ }
+ node = node.previousSibling ?? node.parentElement ?? undefined;
+ }
+ return null;
+ }
+
+ getLineNumberByChild(node: Node) {
+ const lineEl = this.getLineElByChild(node);
+ return getLineNumber(lineEl);
+ }
+
getContentTdByLine(lineNumber: LineNumber, side?: Side, root?: Element) {
if (!this._builder) return null;
return this._builder.getContentTdByLine(lineNumber, side, root);
@@ -274,7 +293,7 @@
if (!lineEl) return null;
const line = getLineNumber(lineEl);
if (!line) return null;
- const side = getSideByLineEl(lineEl);
+ const side = this.getSideByLineEl(lineEl);
// Performance optimization because we already have an element in the
// correct row
const row = this._getDiffRowByChild(lineEl);
@@ -288,6 +307,10 @@
);
}
+ getSideByLineEl(lineEl: Element) {
+ return lineEl.classList.contains(Side.RIGHT) ? Side.RIGHT : Side.LEFT;
+ }
+
emitGroup(group: GrDiffGroup, sectionEl: HTMLElement) {
if (!this._builder) return;
this._builder.emitGroup(group, sectionEl);
@@ -466,6 +489,31 @@
};
}
+ _createSpecialCharacterIndicatorLayer(): DiffLayer {
+ return {
+ annotate(contentEl: HTMLElement, _: HTMLElement, line: GrDiffLine) {
+ // Find and annotate the locations of soft hyphen.
+ const split = line.text.split('\u00AD'); // \u00AD soft hyphen
+ if (!split || split.length < 2) {
+ return;
+ }
+ for (let i = 0, pos = 0; i < split.length - 1; i++) {
+ // Skip forward by the length of the content
+ pos += split[i].length;
+
+ GrAnnotation.annotateElement(
+ contentEl,
+ pos,
+ 1,
+ 'style-scope gr-diff special-char-indicator'
+ );
+
+ pos++;
+ }
+ },
+ };
+ }
+
_createTrailingWhitespaceLayer(): DiffLayer {
const show = () => {
return this._showTrailingWhitespace;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
index 30b9291..650a9e7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
@@ -104,7 +104,7 @@
private _getImageSrc(image: ImageInfo | null): string {
return image && IMAGE_MIME_PATTERN.test(image.type)
- ? `data:${image.type};base64, ${image.body}`
+ ? `data:${image.type};base64,${image.body}`
: '';
}
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 2ab35c0..24a7d78 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
@@ -611,14 +611,14 @@
contentText.setAttribute('data-side', side);
}
- if (lineNumberEl && side) {
+ if (lineNumberEl) {
for (const layer of this.layers) {
if (typeof layer.annotate === 'function') {
- layer.annotate(contentText, lineNumberEl, line, side);
+ layer.annotate(contentText, lineNumberEl, line);
}
}
} else {
- console.error('lineNumberEl or side not set, skipping layer.annotate');
+ console.error('The lineNumberEl is null, skipping layer annotations.');
}
td.appendChild(contentText);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
deleted file mode 100644
index d0b3d3c..0000000
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
+++ /dev/null
@@ -1,259 +0,0 @@
-/**
- * @license
- * 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.
- */
-import {DiffLayer, DiffLayerListener} from '../../../types/types';
-import {GrDiffLine, Side} from '../../../api/diff';
-import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
-import {debounce, DelayedTask} from '../../../utils/async-util';
-import {
- getLineNumberByChild,
- lineNumberToNumber,
-} from '../gr-diff/gr-diff-utils';
-import {appContext} from '../../../services/app-context';
-import {KnownExperimentId} from '../../../services/flags/flags';
-
-const tokenMatcher = new RegExp(/[a-zA-Z0-9_-]+/g);
-
-/** CSS class for all tokens. */
-const CSS_TOKEN = 'token';
-
-/** CSS class for the currently hovered token. */
-const CSS_HIGHLIGHT = 'token-highlight';
-
-const UPDATE_TOKEN_TASK_DELAY_MS = 50;
-
-const LINE_LENGTH_LIMIT = 500;
-
-const TOKEN_LENGTH_LIMIT = 100;
-
-const TOKEN_COUNT_LIMIT = 10000;
-
-const TOKEN_OCCURRENCES_LIMIT = 1000;
-
-/**
- * Token highlighting is only useful for code on-screen, so don't bother
- * highlighting tokens that are further away than this threshold from where the
- * user is hovering.
- */
-const LINE_DISTANCE_THRESHOLD = 100;
-
-/**
- * When a user hovers over a token in the diff, then this layer makes sure that
- * all occurrences of this token are annotated with the 'token-highlight' css
- * class. And removes that class when the user moves the mouse away from the
- * token.
- *
- * The layer does not react to mouse events directly by adding a css class to
- * the appropriate elements, but instead it just sets the currently highlighted
- * token and notifies the diff renderer that certain lines must be re-rendered.
- * And when that re-rendering happens the appropriate css class is added.
- */
-export class TokenHighlightLayer implements DiffLayer {
- /** The only listener is typically the renderer of gr-diff. */
- private listeners: DiffLayerListener[] = [];
-
- /** The currently highlighted token. */
- private currentHighlight?: string;
-
- /**
- * The line of the currently highlighted token. We store this in order to
- * re-render only relevant lines of the diff. Only lines visible on the screen
- * need a highlight. For example in a file with 10,000 lines it is sufficient
- * to just re-render the ~100 lines that are visible to the user.
- *
- * It is a known issue that we are only storing the line number on the side of
- * where the user is hovering and we use that also to determine which line
- * numbers to re-render on the other side, but it is non-trivial to look up or
- * store a reliable mapping of line numbers, so we just accept this
- * shortcoming with the reasoning that the user is mostly interested in the
- * tokens on the side where they are hovering anyway.
- *
- * Another known issue is that we are not able to see past collapsed lines
- * with the current implementation.
- */
- private currentHighlightLineNumber = 0;
-
- /**
- * Keeps track of where tokens occur in a file during rendering, so that it is
- * easy to look up when processing mouse events.
- */
- private tokenToLinesLeft = new Map<string, Set<number>>();
-
- private tokenToLinesRight = new Map<string, Set<number>>();
-
- private updateTokenTask?: DelayedTask;
-
- private readonly enabled = appContext.flagsService.isEnabled(
- KnownExperimentId.TOKEN_HIGHLIGHTING
- );
-
- annotate(
- el: HTMLElement,
- _: HTMLElement,
- line: GrDiffLine,
- side: Side
- ): void {
- if (!this.enabled) return;
- const text = el.textContent;
- if (!text) return;
- // Binary files encoded as text for example can have super long lines
- // with super long tokens. Let's guard against against this scenario.
- if (text.length > LINE_LENGTH_LIMIT) return;
- let match;
- let atLeastOneTokenMatched = false;
- while ((match = tokenMatcher.exec(text))) {
- const token = match[0];
- const index = match.index;
- const length = token.length;
- // Binary files encoded as text for example can have super long lines
- // with super long tokens. Let's guard against this scenario.
- if (length > TOKEN_LENGTH_LIMIT) continue;
- atLeastOneTokenMatched = true;
- const css = token === this.currentHighlight ? CSS_HIGHLIGHT : CSS_TOKEN;
- // We add the tk-* class so that we can look up the token later easily
- // even if the token element was split up into multiple smaller nodes.
- GrAnnotation.annotateElement(el, index, length, `tk-${token} ${css}`);
- // We could try to detect whether we are re-rendering instead of initially
- // rendering the line. Then we would not have to call storeLineForToken()
- // again. But since the Set swallows the duplicates we don't care.
- this.storeLineForToken(token, line, side);
- }
- if (atLeastOneTokenMatched) {
- // These listeners do not have to be cleaned, because listeners are
- // garbage collected along with the element itself once it is not attached
- // to the DOM anymore and no references exist anymore.
- el.addEventListener('mouseover', this.handleMouseOver);
- el.addEventListener('mouseout', this.handleMouseOut);
- }
- }
-
- private storeLineForToken(token: string, line: GrDiffLine, side: Side) {
- const tokenToLines =
- side === Side.LEFT ? this.tokenToLinesLeft : this.tokenToLinesRight;
- // Just to make sure that we don't break down on large files.
- if (tokenToLines.size > TOKEN_COUNT_LIMIT) return;
- let numbers = tokenToLines.get(token);
- if (!numbers) {
- numbers = new Set<number>();
- tokenToLines.set(token, numbers);
- }
- // Just to make sure that we don't break down on large files.
- if (numbers.size > TOKEN_OCCURRENCES_LIMIT) return;
- const lineNumber =
- side === Side.LEFT ? line.beforeNumber : line.afterNumber;
- numbers.add(Number(lineNumber));
- }
-
- private readonly handleMouseOut = (e: MouseEvent) => {
- if (!this.currentHighlight) return;
- if (this.interferesWithSelection(e)) return;
- const el = this.findTokenAncestor(e?.target);
- if (!el) return;
- this.updateTokenHighlight(undefined, undefined);
- };
-
- private readonly handleMouseOver = (e: MouseEvent) => {
- if (this.interferesWithSelection(e)) return;
- const {line, token} = this.findTokenAncestor(e?.target);
- if (!token) return;
- const oldHighlight = this.currentHighlight;
- const newHighlight = token;
- if (!newHighlight || newHighlight === oldHighlight) return;
- if (this.countOccurrences(newHighlight) <= 1) return;
- this.updateTokenHighlight(line, newHighlight);
- };
-
- private interferesWithSelection(e: MouseEvent) {
- if (e.buttons > 0) return true;
- if (window.getSelection()?.type === 'Range') return true;
- return false;
- }
-
- private updateTokenHighlight(
- newLineNumber: number | undefined,
- newHighlight: string | undefined
- ) {
- this.updateTokenTask = debounce(
- this.updateTokenTask,
- () => {
- const oldHighlight = this.currentHighlight;
- const oldLineNumber = this.currentHighlightLineNumber;
- this.currentHighlight = newHighlight;
- this.currentHighlightLineNumber = newLineNumber ?? 0;
- this.notifyForToken(oldHighlight, oldLineNumber);
- this.notifyForToken(newHighlight, newLineNumber ?? 0);
- },
- UPDATE_TOKEN_TASK_DELAY_MS
- );
- }
-
- findTokenAncestor(
- el?: EventTarget | Element | null
- ): {
- token?: string;
- line: number;
- } {
- if (!(el instanceof Element)) return {line: 0, token: undefined};
- if (
- el.classList.contains(CSS_TOKEN) ||
- el.classList.contains(CSS_HIGHLIGHT)
- ) {
- const tkClass = [...el.classList].find(c => c.startsWith('tk-'));
- const line = lineNumberToNumber(getLineNumberByChild(el));
- if (!line || !tkClass) return {line: 0, token: undefined};
- return {line, token: tkClass.substring(3)};
- }
- if (el.tagName === 'TD') return {line: 0, token: undefined};
- return this.findTokenAncestor(el.parentElement);
- }
-
- countOccurrences(token: string | undefined) {
- if (!token) return 0;
- const linesLeft = this.tokenToLinesLeft.get(token);
- const linesRight = this.tokenToLinesRight.get(token);
- return (linesLeft?.size ?? 0) + (linesRight?.size ?? 0);
- }
-
- notifyForToken(token: string | undefined, lineNumber: number) {
- if (!token) return;
- const linesLeft = this.tokenToLinesLeft.get(token);
- linesLeft?.forEach(line => {
- if (Math.abs(line - lineNumber) < LINE_DISTANCE_THRESHOLD) {
- this.notifyListeners(line, Side.LEFT);
- }
- });
- const linesRight = this.tokenToLinesRight.get(token);
- linesRight?.forEach(line => {
- if (Math.abs(line - lineNumber) < LINE_DISTANCE_THRESHOLD) {
- this.notifyListeners(line, Side.RIGHT);
- }
- });
- }
-
- addListener(listener: DiffLayerListener) {
- this.listeners.push(listener);
- }
-
- removeListener(listener: DiffLayerListener) {
- this.listeners = this.listeners.filter(f => f !== listener);
- }
-
- notifyListeners(line: number, side: Side) {
- for (const listener of this.listeners) {
- listener(line, line, side);
- }
- }
-}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 17026fc..9b7c25a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -36,6 +36,7 @@
import {GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {GrDiff} from '../gr-diff/gr-diff';
import {fireAlert, fireEvent} from '../../../utils/event-util';
+import {Subscription} from 'rxjs';
import {toggleClass} from '../../../utils/dom-util';
type GrDiffRowType = GrDiffLineType | GrDiffGroupType;
@@ -47,9 +48,7 @@
const NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS = 5000;
export interface GrDiffCursor {
- $: {
- cursorManager: GrCursorManager;
- };
+ $: {};
}
@customElement('gr-diff-cursor')
@@ -82,20 +81,18 @@
@property({type: Number})
initialLineNumber: number | null = null;
- /**
- * The scroll behavior for the cursor. Values are 'never' and
- * 'keep-visible'. 'keep-visible' will only scroll if the cursor is beyond
- * the viewport.
- */
- @property({type: String})
- _scrollMode = ScrollMode.KEEP_VISIBLE;
-
- @property({type: Boolean})
- _focusOnMove = true;
-
@property({type: Boolean})
_listeningForScroll = false;
+ private cursorManager = new GrCursorManager();
+
+ constructor() {
+ super();
+ this.cursorManager.cursorTargetClass = 'target-row';
+ this.cursorManager.scrollMode = ScrollMode.KEEP_VISIBLE;
+ this.cursorManager.focusOnMove = true;
+ }
+
/** @override */
ready() {
super.ready();
@@ -119,28 +116,34 @@
});
}
+ private targetSubscription?: Subscription;
+
/** @override */
connectedCallback() {
super.connectedCallback();
// Catch when users are scrolling as the view loads.
window.addEventListener('scroll', this._boundHandleWindowScroll);
+ this.targetSubscription = this.cursorManager.target$.subscribe(target => {
+ this.diffRow = target || undefined;
+ });
}
/** @override */
disconnectedCallback() {
+ if (this.targetSubscription) this.targetSubscription.unsubscribe();
window.removeEventListener('scroll', this._boundHandleWindowScroll);
- this.$.cursorManager.unsetCursor();
+ this.cursorManager.unsetCursor();
super.disconnectedCallback();
}
// Don't remove - used by clients embedding gr-diff outside of Gerrit.
isAtStart() {
- return this.$.cursorManager.isAtStart();
+ return this.cursorManager.isAtStart();
}
// Don't remove - used by clients embedding gr-diff outside of Gerrit.
isAtEnd() {
- return this.$.cursorManager.isAtEnd();
+ return this.cursorManager.isAtEnd();
}
moveLeft() {
@@ -159,31 +162,31 @@
moveDown() {
if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- return this.$.cursorManager.next({
+ return this.cursorManager.next({
filter: (row: Element) => this._rowHasSide(row),
});
} else {
- return this.$.cursorManager.next();
+ return this.cursorManager.next();
}
}
moveUp() {
if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- return this.$.cursorManager.previous({
+ return this.cursorManager.previous({
filter: (row: Element) => this._rowHasSide(row),
});
} else {
- return this.$.cursorManager.previous();
+ return this.cursorManager.previous();
}
}
moveToVisibleArea() {
if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- this.$.cursorManager.moveToVisibleArea((row: Element) =>
+ this.cursorManager.moveToVisibleArea((row: Element) =>
this._rowHasSide(row)
);
} else {
- this.$.cursorManager.moveToVisibleArea();
+ this.cursorManager.moveToVisibleArea();
}
}
@@ -214,7 +217,7 @@
clipToTop?: boolean,
navigateToNextFile?: boolean
): CursorMoveResult {
- const result = this.$.cursorManager.next({
+ const result = this.cursorManager.next({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
getTargetHeight: target =>
(target?.parentNode as HTMLElement)?.scrollHeight || 0,
@@ -233,7 +236,7 @@
}
moveToPreviousChunk(navigateToPreviousFile?: boolean): CursorMoveResult {
- const result = this.$.cursorManager.previous({
+ const result = this.cursorManager.previous({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
});
if (navigateToPreviousFile && this.isAtStart()) {
@@ -248,7 +251,7 @@
fireEvent(this, 'navigate-to-next-file-with-comments');
return;
}
- const result = this.$.cursorManager.next({
+ const result = this.cursorManager.next({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
this._fixSide();
@@ -256,7 +259,7 @@
}
moveToPreviousCommentThread(): CursorMoveResult {
- const result = this.$.cursorManager.previous({
+ const result = this.cursorManager.previous({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
this._fixSide();
@@ -267,7 +270,7 @@
const row = this._findRowByNumberAndFile(number, side, path);
if (row) {
this.side = side;
- this.$.cursorManager.setCursor(row);
+ this.cursorManager.setCursor(row);
}
}
@@ -299,7 +302,7 @@
}
moveToFirstChunk() {
- this.$.cursorManager.moveToStart();
+ this.cursorManager.moveToStart();
if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
this.moveToNextChunk(true);
} else {
@@ -308,7 +311,7 @@
}
moveToLastChunk() {
- this.$.cursorManager.moveToEnd();
+ this.cursorManager.moveToEnd();
if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
this.moveToPreviousChunk();
} else {
@@ -328,7 +331,7 @@
reInitCursor() {
if (!this.diffRow) {
// does not scroll during init unless requested
- this._scrollMode = this.initialLineNumber
+ this.cursorManager.scrollMode = this.initialLineNumber
? ScrollMode.KEEP_VISIBLE
: ScrollMode.NEVER;
if (this.initialLineNumber) {
@@ -342,13 +345,13 @@
}
reInit() {
- this._scrollMode = ScrollMode.KEEP_VISIBLE;
+ this.cursorManager.scrollMode = ScrollMode.KEEP_VISIBLE;
}
private _boundHandleWindowScroll = () => {
if (this.preventAutoScrollOnManualScroll) {
- this._scrollMode = ScrollMode.NEVER;
- this._focusOnMove = false;
+ this.cursorManager.scrollMode = ScrollMode.NEVER;
+ this.cursorManager.focusOnMove = false;
this.preventAutoScrollOnManualScroll = false;
}
};
@@ -374,7 +377,7 @@
private _boundHandleDiffRenderContent = () => {
this._updateStops();
// When done rendering, turn focus on move and automatic scrolling back on
- this._focusOnMove = true;
+ this.cursorManager.focusOnMove = true;
this.preventAutoScrollOnManualScroll = false;
};
@@ -533,7 +536,7 @@
}
_updateStops() {
- this.$.cursorManager.stops = this.diffs.reduce(
+ this.cursorManager.stops = this.diffs.reduce(
(stops: Stop[], diff) => stops.concat(diff.getCursorStops()),
[]
);
@@ -612,7 +615,7 @@
const diff = this.diffs.filter(diff => diff.path === path)[0];
stops = diff.getCursorStops();
} else {
- stops = this.$.cursorManager.stops;
+ stops = this.cursorManager.stops;
}
// Sadly needed for type narrowing to understand that the result is always
// targetable.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_html.ts
index 1539a22..1489006 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_html.ts
@@ -16,12 +16,4 @@
*/
import {html} from '@polymer/polymer/lib/utils/html-tag';
-export const htmlTemplate = html`
- <gr-cursor-manager
- id="cursorManager"
- scroll-mode="[[_scrollMode]]"
- cursor-target-class="target-row"
- focus-on-move="[[_focusOnMove]]"
- target="{{diffRow}}"
- ></gr-cursor-manager>
-`;
+export const htmlTemplate = html``;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 60b82da..75439c8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -187,20 +187,20 @@
});
test('cursor scroll behavior', () => {
- assert.equal(cursorElement._scrollMode, 'keep-visible');
+ assert.equal(cursorElement.cursorManager.scrollMode, 'keep-visible');
diffElement.dispatchEvent(new Event('render-start'));
- assert.isTrue(cursorElement._focusOnMove);
+ assert.isTrue(cursorElement.cursorManager.focusOnMove);
window.dispatchEvent(new Event('scroll'));
- assert.equal(cursorElement._scrollMode, 'never');
- assert.isFalse(cursorElement._focusOnMove);
+ assert.equal(cursorElement.cursorManager.scrollMode, 'never');
+ assert.isFalse(cursorElement.cursorManager.focusOnMove);
diffElement.dispatchEvent(new Event('render-content'));
- assert.isTrue(cursorElement._focusOnMove);
+ assert.isTrue(cursorElement.cursorManager.focusOnMove);
cursorElement.reInitCursor();
- assert.equal(cursorElement._scrollMode, 'keep-visible');
+ assert.equal(cursorElement.cursorManager.scrollMode, 'keep-visible');
});
test('moves to selected line', () => {
@@ -266,7 +266,7 @@
// to the right side.
assert.equal(cursorElement.side, 'right');
assert.equal(cursorElement.diffRow, firstDeltaRow);
- const firstIndex = cursorElement.$.cursorManager.index;
+ const firstIndex = cursorElement.cursorManager.index;
// Move the side to the left. Because this delta only has a right side, we
// should be moved up to the previous line where there is content on the
@@ -275,7 +275,7 @@
assert.equal(cursorElement.side, 'left');
assert.notEqual(cursorElement.diffRow, firstDeltaRow);
- assert.equal(cursorElement.$.cursorManager.index, firstIndex - 1);
+ assert.equal(cursorElement.cursorManager.index, firstIndex - 1);
assert.equal(cursorElement.diffRow.parentElement,
firstDeltaSection.previousSibling);
@@ -285,7 +285,7 @@
assert.equal(cursorElement.side, 'left');
assert.notEqual(cursorElement.diffRow, firstDeltaRow);
- assert.isTrue(cursorElement.$.cursorManager.index > firstIndex);
+ assert.isTrue(cursorElement.cursorManager.index > firstIndex);
assert.equal(cursorElement.diffRow.parentElement,
firstDeltaSection.nextSibling);
});
@@ -446,8 +446,7 @@
});
test('navigate to next unreviewed file via moveToNextChunk', () => {
- const cursorManager =
- cursorElement.shadowRoot.querySelector('#cursorManager');
+ const cursorManager = cursorElement.cursorManager;
cursorManager.index = cursorManager.stops.length - 1;
const dispatchEventStub = sinon.stub(cursorElement, 'dispatchEvent');
cursorElement.moveToNextChunk(/* opt_clipToTop = */false,
@@ -465,8 +464,9 @@
let scrollBehaviorDuringMove;
const moveToNumStub = sinon.stub(cursorElement, 'moveToLineNumber');
const moveToChunkStub = sinon.stub(cursorElement, 'moveToFirstChunk')
- .callsFake(
- () => { scrollBehaviorDuringMove = cursorElement._scrollMode; });
+ .callsFake(() => {
+ scrollBehaviorDuringMove = cursorElement.cursorManager.scrollMode;
+ });
function renderHandler() {
diffElement.removeEventListener('render', renderHandler);
@@ -474,7 +474,7 @@
assert.isFalse(moveToNumStub.called);
assert.isTrue(moveToChunkStub.called);
assert.equal(scrollBehaviorDuringMove, 'never');
- assert.equal(cursorElement._scrollMode, 'keep-visible');
+ assert.equal(cursorElement.cursorManager.scrollMode, 'keep-visible');
done();
}
diffElement.addEventListener('render', renderHandler);
@@ -484,8 +484,9 @@
test('initialLineNumber provided', done => {
let scrollBehaviorDuringMove;
const moveToNumStub = sinon.stub(cursorElement, 'moveToLineNumber')
- .callsFake(
- () => { scrollBehaviorDuringMove = cursorElement._scrollMode; });
+ .callsFake(() => {
+ scrollBehaviorDuringMove = cursorElement.cursorManager.scrollMode;
+ });
const moveToChunkStub = sinon.stub(cursorElement, 'moveToFirstChunk');
function renderHandler() {
diffElement.removeEventListener('render', renderHandler);
@@ -495,7 +496,7 @@
assert.equal(moveToNumStub.lastCall.args[0], 10);
assert.equal(moveToNumStub.lastCall.args[1], 'right');
assert.equal(scrollBehaviorDuringMove, 'keep-visible');
- assert.equal(cursorElement._scrollMode, 'keep-visible');
+ assert.equal(cursorElement.cursorManager.scrollMode, 'keep-visible');
done();
}
diffElement.addEventListener('render', renderHandler);
@@ -600,7 +601,7 @@
{leftSide: true, number: 10});
// Should be null if there is no selection.
- cursorElement.$.cursorManager.unsetCursor();
+ cursorElement.cursorManager.unsetCursor();
assert.isNotOk(cursorElement.getAddress());
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index ca003f3..76e02f0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -28,13 +28,7 @@
import {GrSelectionActionBox} from '../gr-selection-action-box/gr-selection-action-box';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
import {FILE} from '../gr-diff/gr-diff-line';
-import {
- getLineElByChild,
- getLineNumberByChild,
- getRange,
- getSide,
- getSideByLineEl,
-} from '../gr-diff/gr-diff-utils';
+import {getRange, getSide} from '../gr-diff/gr-diff-utils';
import {debounce, DelayedTask} from '../../../utils/async-util';
interface SidedRange {
@@ -362,11 +356,11 @@
): NormalizedPosition | null {
let column;
if (!node || !this.contains(node)) return null;
- const lineEl = getLineElByChild(node);
+ const lineEl = this.diffBuilder.getLineElByChild(node);
if (!lineEl) return null;
- const side = getSideByLineEl(lineEl);
+ const side = this.diffBuilder.getSideByLineEl(lineEl);
if (!side) return null;
- const line = getLineNumberByChild(lineEl);
+ const line = this.diffBuilder.getLineNumberByChild(lineEl);
if (!line || line === FILE || line === 'LOST') return null;
const contentTd = this.diffBuilder.getContentTdByLineEl(lineEl);
if (!contentTd) return null;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.js
index 18fbe9a..07e83a8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.js
@@ -438,7 +438,7 @@
const contentText = stubContent(140, 'left');
const contentTd = contentText.parentElement;
- emulateSelection(contentTd.parentElement, 0,
+ emulateSelection(contentTd.previousElementSibling, 0,
contentText.firstChild, 2);
assert.isFalse(!!element.selectedRange);
});
@@ -584,6 +584,21 @@
});
assert.equal(side, 'right');
});
+
+ test('_fixTripleClickSelection empty line', () => {
+ const startContent = stubContent(146, 'right');
+ const endContent = stubContent(165, 'left');
+ emulateSelection(startContent.firstChild, 0,
+ endContent.parentElement.previousElementSibling, 0);
+ const {range, side} = element.selectedRange;
+ assert.deepEqual(range, {
+ start_line: 146,
+ start_character: 0,
+ end_line: 146,
+ end_character: 84,
+ });
+ assert.equal(side, 'right');
+ });
});
});
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 d31d934..5e1b5ff 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
@@ -404,7 +404,7 @@
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile1.body);
+ 'data:image/bmp;base64,' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
leftLoaded = true;
if (rightLoaded) {
@@ -416,7 +416,7 @@
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile2.body);
+ 'data:image/bmp;base64,' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
rightLoaded = true;
@@ -495,7 +495,7 @@
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile1.body);
+ 'data:image/bmp;base64,' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
leftLoaded = true;
if (rightLoaded) {
@@ -507,7 +507,7 @@
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile2.body);
+ 'data:image/bmp;base64,' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
rightLoaded = true;
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 a650f68..77fe299 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
@@ -36,7 +36,9 @@
import {classMap} from 'lit-html/directives/class-map';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
-import {Dimensions, fitToFrame, FrameConstrainer, Rect} from './util';
+import {Dimensions, fitToFrame, FrameConstrainer, Point, Rect} from './util';
+
+const DRAG_DEAD_ZONE_PIXELS = 5;
/**
* This components allows the user to rapidly switch between two given images
@@ -51,10 +53,6 @@
// URL for the image to use as revision.
@property({type: String}) revisionUrl = '';
- @internalProperty() protected baseImage?: HTMLImageElement;
-
- @internalProperty() protected revisionImage?: HTMLImageElement;
-
@internalProperty() protected baseSelected = true;
@internalProperty() protected scaledSelected = true;
@@ -99,6 +97,14 @@
2,
];
+ @internalProperty() protected grabbing = false;
+
+ private ownsMouseDown = false;
+
+ private centerOnDown: Point = {x: 0, y: 0};
+
+ private pointerOnDown: Point = {x: 0, y: 0};
+
private readonly frameConstrainer = new FrameConstrainer();
private readonly resizeObserver = new ResizeObserver(
@@ -321,18 +327,6 @@
></div>
`;
- const shapeLeft = `
- border-radius: 4px 0 0 4px;
- border-left-width: 1px;
- border-right-width: 0;
- `;
- const shapeRight = `
- border-radius: 0 4px 4px 0;
- border-left-width: 0;
- border-right-width: 1px;
- `;
- const outlinedBorder =
- 'border: 1px solid var(--primary-button-background-color);';
// To pass CSS mixins for @apply to Polymer components, they need to be
// wrapped in a <custom-style>.
const customStyle = html`
@@ -340,24 +334,30 @@
<style>
paper-button.left {
--paper-button: {
- ${shapeLeft}
+ border-radius: 4px 0 0 4px;
+ border-width: 1px 0 1px 1px;
}
}
paper-button.left[outlined] {
--paper-button: {
- ${outlinedBorder}
- ${shapeLeft}
+ border-radius: 4px 0 0 4px;
+ border-width: 1px 0 1px 1px;
+ border-style: solid;
+ border-color: var(--primary-button-background-color);
}
}
paper-button.right {
--paper-button: {
- ${shapeRight}
+ border-radius: 0 4px 4px 0;
+ border-width: 1px 1px 1px 0;
}
}
paper-button.right[outlined] {
--paper-button: {
- ${outlinedBorder}
- ${shapeRight}
+ border-radius: 0 4px 4px 0;
+ border-width: 1px 1px 1px 0;
+ border-style: solid;
+ border-color: var(--primary-button-background-color);
}
}
paper-item {
@@ -390,11 +390,17 @@
base: this.baseSelected,
revision: !this.baseSelected,
})}"
- style="${styleMap(this.zoomedImageStyle)}"
+ style="${styleMap({
+ ...this.zoomedImageStyle,
+ cursor: this.grabbing ? 'grabbing' : 'pointer',
+ })}"
.scale="${this.scale}"
.frameRect="${this.magnifierFrame}"
- @click="${this.toggleImage}"
+ @mousedown="${this.mousedownMagnifier}"
+ @mouseup="${this.mouseupMagnifier}"
@mousemove="${this.mousemoveMagnifier}"
+ @mouseleave="${this.mouseleaveMagnifier}"
+ @dragstart="${this.dragstartMagnifier}"
>
${sourceImage}
</gr-zoomed-image>
@@ -463,8 +469,53 @@
this.followMouse = !this.followMouse;
}
+ mousedownMagnifier(event: MouseEvent) {
+ if (event.buttons === 1) {
+ this.ownsMouseDown = true;
+ this.centerOnDown = this.frameConstrainer.getCenter();
+ this.pointerOnDown = {
+ x: event.clientX,
+ y: event.clientY,
+ };
+ }
+ }
+
+ mouseupMagnifier(event: MouseEvent) {
+ const offsetX = event.clientX - this.pointerOnDown.x;
+ const offsetY = event.clientY - this.pointerOnDown.y;
+ const distance = Math.max(Math.abs(offsetX), Math.abs(offsetY));
+ // Consider very short drags as clicks. These tend to happen more often on
+ // external mice.
+ if (this.ownsMouseDown && distance < DRAG_DEAD_ZONE_PIXELS) {
+ this.toggleImage();
+ }
+ this.grabbing = false;
+ this.ownsMouseDown = false;
+ }
+
mousemoveMagnifier(event: MouseEvent) {
- if (!this.followMouse) return;
+ if (event.buttons === 1 && this.ownsMouseDown) {
+ this.handleMagnifierDrag(event);
+ return;
+ }
+ if (this.followMouse) {
+ this.handleFollowMouse(event);
+ return;
+ }
+ }
+
+ private handleMagnifierDrag(event: MouseEvent) {
+ this.grabbing = true;
+ const offsetX = event.clientX - this.pointerOnDown.x;
+ const offsetY = event.clientY - this.pointerOnDown.y;
+ this.frameConstrainer.requestCenter({
+ x: this.centerOnDown.x - offsetX / this.scale,
+ y: this.centerOnDown.y - offsetY / this.scale,
+ });
+ this.updateFrames();
+ }
+
+ private handleFollowMouse(event: MouseEvent) {
const rect = this.imageArea!.getBoundingClientRect();
const offsetX = event.clientX - rect.left;
const offsetY = event.clientY - rect.top;
@@ -477,6 +528,15 @@
this.updateFrames();
}
+ mouseleaveMagnifier() {
+ this.grabbing = false;
+ this.ownsMouseDown = false;
+ }
+
+ dragstartMagnifier(event: DragEvent) {
+ event.preventDefault();
+ }
+
onOverviewCenterUpdated(event: CustomEvent) {
this.frameConstrainer.requestCenter({
x: event.detail.x as number,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
index 621d28c..55f83d6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
@@ -184,6 +184,7 @@
}
clickOverview(event: MouseEvent) {
+ if (event.buttons !== 1) return;
event.preventDefault();
this.updateOverlaySize();
@@ -197,6 +198,7 @@
}
grabFrame(event: MouseEvent) {
+ if (event.buttons !== 1) return;
event.preventDefault();
// Do not bubble up into clickOverview().
event.stopPropagation();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
index b64f61d..3df2e18 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.ts
@@ -28,12 +28,7 @@
import {DiffInfo} from '../../../types/diff';
import {Side} from '../../../constants/constants';
import {GrDiffBuilderElement} from '../gr-diff-builder/gr-diff-builder-element';
-import {
- getLineElByChild,
- getSide,
- getSideByLineEl,
- isThreadEl,
-} from '../gr-diff/gr-diff-utils';
+import {getSide, isThreadEl} from '../gr-diff/gr-diff-utils';
/**
* Possible CSS classes indicating the state of selection. Dynamically added/
@@ -115,7 +110,7 @@
// Handle the down event on comment thread in Polymer 2
const handled = this._handleDownOnRangeComment(target);
if (handled) return;
- const lineEl = getLineElByChild(target);
+ const lineEl = this.diffBuilder.getLineElByChild(target);
const blameSelected = this._elementDescendedFromClass(target, 'blame');
if (!lineEl && !blameSelected) {
return;
@@ -130,7 +125,7 @@
target,
'gr-comment'
);
- const side = getSideByLineEl(lineEl);
+ const side = this.diffBuilder.getSideByLineEl(lineEl);
targetClasses.push(
side === 'left' ? SelectionClass.LEFT : SelectionClass.RIGHT
@@ -184,9 +179,9 @@
if (this.classList.contains(SelectionClass.COMMENT)) {
commentSelected = true;
}
- const lineEl = getLineElByChild(target);
+ const lineEl = this.diffBuilder.getLineElByChild(target);
if (!lineEl) return;
- const side = getSideByLineEl(lineEl);
+ const side = this.diffBuilder.getSideByLineEl(lineEl);
const text = this._getSelectedText(side, commentSelected);
if (text && e.clipboardData) {
e.clipboardData.setData('Text', text);
@@ -229,9 +224,9 @@
return this._getCommentLines(sel, side);
}
const range = normalize(sel.getRangeAt(0));
- const startLineEl = getLineElByChild(range.startContainer);
+ const startLineEl = this.diffBuilder.getLineElByChild(range.startContainer);
if (!startLineEl) return;
- const endLineEl = getLineElByChild(range.endContainer);
+ const endLineEl = this.diffBuilder.getLineElByChild(range.endContainer);
// Happens when triple click in side-by-side mode with other side empty.
const endsAtOtherEmptySide =
!endLineEl &&
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.js b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.js
index 8d7264c..5c9fe3f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.js
@@ -143,8 +143,8 @@
test('applies selected-left on left side click', () => {
element.classList.add('selected-right');
- const lineNumberEl = element.querySelector('.lineNum.left');
- MockInteractions.down(lineNumberEl);
+ element._cachedDiffBuilder.getSideByLineEl.returns('left');
+ MockInteractions.down(element);
assert.isTrue(
element.classList.contains('selected-left'), 'adds selected-left');
assert.isFalse(
@@ -154,8 +154,8 @@
test('applies selected-right on right side click', () => {
element.classList.add('selected-left');
- const lineNumberEl = element.querySelector('.lineNum.right');
- MockInteractions.down(lineNumberEl);
+ element._cachedDiffBuilder.getSideByLineEl.returns('right');
+ MockInteractions.down(element);
assert.isTrue(
element.classList.contains('selected-right'), 'adds selected-right');
assert.isFalse(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index 96fbd8d..0ca929a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -43,36 +43,6 @@
return range.end_line - range.start_line > 10;
}
-export function getLineNumberByChild(node?: Node) {
- return getLineNumber(getLineElByChild(node));
-}
-
-export function lineNumberToNumber(lineNumber?: LineNumber | null): number {
- if (!lineNumber) return 0;
- if (lineNumber === 'LOST') return 0;
- if (lineNumber === 'FILE') return 0;
- return lineNumber;
-}
-
-export function getLineElByChild(node?: Node): HTMLElement | null {
- while (node) {
- if (node instanceof Element) {
- if (node.classList.contains('lineNum')) {
- return node as HTMLElement;
- }
- if (node.classList.contains('section')) {
- return null;
- }
- }
- node = node.previousSibling ?? node.parentElement ?? undefined;
- }
- return null;
-}
-
-export function getSideByLineEl(lineEl: Element) {
- return lineEl.classList.contains(Side.RIGHT) ? Side.RIGHT : Side.LEFT;
-}
-
export function getLineNumber(lineEl?: Element | null): LineNumber | null {
if (!lineEl) return null;
const lineNumberStr = lineEl.getAttribute('data-value');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 81be4f5..5a3cdcd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -29,7 +29,6 @@
import {LineNumber} from './gr-diff-line';
import {
getLine,
- getLineElByChild,
getLineNumber,
getRange,
getSide,
@@ -254,7 +253,7 @@
@property({type: Boolean})
showNewlineWarningRight = false;
- @property({type: String})
+ @property({type: String, observer: '_useNewImageDiffUiObserver'})
useNewImageDiffUi = false;
@property({
@@ -547,7 +546,7 @@
el.classList.contains('content') ||
el.classList.contains('contentText')
) {
- const target = getLineElByChild(el);
+ const target = this.$.diffBuilder.getLineElByChild(el);
if (target) {
this._selectLine(target);
}
@@ -709,6 +708,10 @@
this._prefsChanged(this.prefs);
}
+ _useNewImageDiffUiObserver() {
+ this._prefsChanged(this.prefs);
+ }
+
_prefsChanged(prefs?: DiffPreferencesInfo) {
if (!prefs) return;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 572df0c..3aa4db9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -348,9 +348,16 @@
transform: translateY(-50%);
--gr-button: {
color: var(--diff-context-control-color);
- border: solid var(--border-color);
- border-width: 1px;
- border-radius: var(--border-radius);
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ border-bottom-right-radius: var(--border-radius);
+ border-bottom-left-radius: var(--border-radius);
padding: var(--spacing-s) var(--spacing-l);
}
}
@@ -368,9 +375,16 @@
transform: translateY(-100%);
--gr-button: {
color: var(--diff-context-control-color);
- border: solid var(--border-color);
- border-width: 1px 1px 0 1px;
- border-radius: var(--border-radius) var(--border-radius) 0 0;
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 0;
+ border-left-width: 1px;
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ border-bottom-right-radius: 0;
+ border-bottom-left-radius: 0;
padding: var(--spacing-xxs) var(--spacing-l);
}
}
@@ -379,9 +393,16 @@
top: calc(100% + var(--divider-border));
--gr-button: {
color: var(--diff-context-control-color);
- border: solid var(--border-color);
- border-width: 0 1px 1px 1px;
- border-radius: 0 0 var(--border-radius) var(--border-radius);
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 0;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+ border-top-left-radius: 0;
+ border-top-right-radius: 0;
+ border-bottom-right-radius: var(--border-radius);
+ border-bottom-left-radius: var(--border-radius);
padding: var(--spacing-xxs) var(--spacing-l);
}
}
@@ -402,6 +423,15 @@
content: '\\00BB';
position: absolute;
}
+ .special-char-indicator {
+ /* spacing so elements don't collide */
+ padding-right: var(--spacing-m);
+ }
+ .special-char-indicator:before {
+ color: var(--diff-tab-indicator-color);
+ content: '•';
+ position: absolute;
+ }
/* Is defined after other background-colors, such that this
rule wins in case of same specificity. */
.trailing-whitespace,
@@ -572,10 +602,6 @@
border: 1px solid var(--diff-context-control-border-color);
text-align: center;
}
-
- .token-highlight {
- background-color: var(--token-highlighting-color, #fffd54);
- }
</style>
<style include="gr-syntax-theme">
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
index 48e009b..86946a6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
@@ -208,7 +208,7 @@
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile1.body);
+ 'data:image/bmp;base64,' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1\u00d71 image/bmp');// \u00d7 - '×'
leftLoaded = true;
if (rightLoaded) {
@@ -220,7 +220,7 @@
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile2.body);
+ 'data:image/bmp;base64,' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1\u00d71 image/bmp');// \u00d7 - '×'
rightLoaded = true;
@@ -302,7 +302,7 @@
leftImage.addEventListener('load', () => {
assert.isOk(leftImage);
assert.equal(leftImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile1.body);
+ 'data:image/bmp;base64,' + mockFile1.body);
assert.equal(leftLabelContent.textContent, '1\u00d71 image/bmp');// \u00d7 - '×'
leftLoaded = true;
if (rightLoaded) {
@@ -314,7 +314,7 @@
rightImage.addEventListener('load', () => {
assert.isOk(rightImage);
assert.equal(rightImage.getAttribute('src'),
- 'data:image/bmp;base64, ' + mockFile2.body);
+ 'data:image/bmp;base64,' + mockFile2.body);
assert.equal(rightLabelContent.textContent, '1\u00d71 image/bmp');// \u00d7 - '×'
rightLoaded = true;
@@ -472,12 +472,10 @@
test('_handleTap content', done => {
const content = document.createElement('div');
const lineEl = document.createElement('div');
- lineEl.className = 'lineNum';
- const row = document.createElement('div');
- row.appendChild(lineEl);
- row.appendChild(content);
const selectStub = sinon.stub(element, '_selectLine');
+ sinon.stub(element.$.diffBuilder, 'getLineElByChild')
+ .callsFake(() => lineEl);
content.className = 'content';
content.addEventListener('click', e => {
diff --git a/polygerrit-ui/app/elements/gr-app.ts b/polygerrit-ui/app/elements/gr-app.ts
index 463fab9..2d3289d 100644
--- a/polygerrit-ui/app/elements/gr-app.ts
+++ b/polygerrit-ui/app/elements/gr-app.ts
@@ -45,7 +45,7 @@
installPolymerResin(safeTypesBridge);
@customElement('gr-app')
-export class GrApp extends PolymerElement {
+class GrApp extends PolymerElement {
static get template() {
return htmlTemplate;
}
diff --git a/polygerrit-ui/app/elements/gr-app_test.js b/polygerrit-ui/app/elements/gr-app_test.js
new file mode 100644
index 0000000..8178c89
--- /dev/null
+++ b/polygerrit-ui/app/elements/gr-app_test.js
@@ -0,0 +1,77 @@
+/**
+ * @license
+ * Copyright (C) 2016 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.
+ */
+
+import '../test/common-test-setup-karma.js';
+import './gr-app.js';
+import {appContext} from '../services/app-context.js';
+import {GerritNav} from './core/gr-navigation/gr-navigation.js';
+import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+import {stubRestApi} from '../test/test-utils.js';
+
+const basicFixture = fixtureFromTemplate(html`<gr-app id="app"></gr-app>`);
+
+suite('gr-app tests', () => {
+ let element;
+ let configStub;
+
+ setup(done => {
+ sinon.stub(appContext.reportingService, 'appStarted');
+ stub('gr-account-dropdown', '_getTopContent');
+ stub('gr-router', 'start');
+ stubRestApi('getAccount').returns(Promise.resolve({}));
+ stubRestApi('getAccountCapabilities').returns(Promise.resolve({}));
+ configStub = stubRestApi('getConfig').returns(Promise.resolve({
+ plugin: {},
+ auth: {
+ auth_type: undefined,
+ },
+ }));
+ stubRestApi('getPreferences').returns(Promise.resolve({my: []}));
+ stubRestApi('getVersion').returns(Promise.resolve(42));
+ stubRestApi('probePath').returns(Promise.resolve(42));
+
+ element = basicFixture.instantiate();
+ flush(done);
+ });
+
+ const appElement = () => element.$['app-element'];
+
+ test('reporting', () => {
+ assert.isTrue(appElement().reporting.appStarted.calledOnce);
+ });
+
+ test('reporting called before router start', () => {
+ const element = appElement();
+ const appStartedStub = element.reporting.appStarted;
+ const routerStartStub = element.$.router.start;
+ sinon.assert.callOrder(appStartedStub, routerStartStub);
+ });
+
+ test('passes config to gr-plugin-host', () =>
+ configStub.lastCall.returnValue.then(config => {
+ assert.deepEqual(appElement().$.plugins.config, config);
+ })
+ );
+
+ test('_paramsChanged sets search page', () => {
+ appElement()._paramsChanged({base: {view: GerritNav.View.CHANGE}});
+ assert.notOk(appElement()._lastSearchPage);
+ appElement()._paramsChanged({base: {view: GerritNav.View.SEARCH}});
+ assert.ok(appElement()._lastSearchPage);
+ });
+});
+
diff --git a/polygerrit-ui/app/elements/gr-app_test.ts b/polygerrit-ui/app/elements/gr-app_test.ts
deleted file mode 100644
index 3583a6a..0000000
--- a/polygerrit-ui/app/elements/gr-app_test.ts
+++ /dev/null
@@ -1,82 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 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.
- */
-
-import '../test/common-test-setup-karma';
-import {GrApp} from './gr-app';
-import {appContext} from '../services/app-context';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-import {queryAndAssert} from '../test/test-utils';
-import {createServerInfo} from '../test/test-data-generators';
-import {GrAppElement} from './gr-app-element';
-import {GrPluginHost} from './plugins/gr-plugin-host/gr-plugin-host';
-import {GerritView} from '../services/router/router-model';
-import {
- AppElementChangeViewParams,
- AppElementSearchParam,
-} from './gr-app-types';
-import {GrRouter} from './core/gr-router/gr-router';
-import {ReportingService} from '../services/gr-reporting/gr-reporting';
-
-const basicFixture = fixtureFromTemplate(html`<gr-app id="app"></gr-app>`);
-
-suite('gr-app tests', () => {
- let element: GrApp;
- let appStartedStub: sinon.SinonStubbedMember<ReportingService['appStarted']>;
- let routerStartStub: sinon.SinonStubbedMember<GrRouter['start']>;
-
- setup(done => {
- appStartedStub = sinon.stub(appContext.reportingService, 'appStarted');
- routerStartStub = stub('gr-router', 'start');
- stub('gr-account-dropdown', '_getTopContent');
-
- element = basicFixture.instantiate() as GrApp;
- flush(done);
- });
-
- const appElement = () =>
- queryAndAssert<GrAppElement>(element, '#app-element');
-
- test('reporting', () => {
- assert.isTrue(appStartedStub.calledOnce);
- });
-
- test('reporting called before router start', () => {
- sinon.assert.callOrder(appStartedStub, routerStartStub);
- });
-
- test('passes config to gr-plugin-host', () => {
- assert.deepEqual(
- queryAndAssert<GrPluginHost>(appElement(), 'gr-plugin-host').config,
- createServerInfo()
- );
- });
-
- test('_paramsChanged sets search page', () => {
- appElement()._paramsChanged({
- path: '',
- value: undefined,
- base: {view: GerritView.CHANGE} as AppElementChangeViewParams,
- });
- assert.notOk(appElement()._lastSearchPage);
- appElement()._paramsChanged({
- path: '',
- value: undefined,
- base: {view: GerritView.SEARCH} as AppElementSearchParam,
- });
- assert.ok(appElement()._lastSearchPage);
- });
-});
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
index 651eac4..ac493a2 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
@@ -20,7 +20,7 @@
import {ServerInfo} from '../../../types/common';
@customElement('gr-plugin-host')
-export class GrPluginHost extends PolymerElement {
+class GrPluginHost extends PolymerElement {
@property({type: Object, observer: '_configChanged'})
config?: ServerInfo;
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts
index 4e6dd1a..396c9f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts
@@ -42,7 +42,10 @@
}
gr-button.remove {
--gr-remove-button-style: {
- border: 0;
+ border-top-width: 0;
+ border-right-width: 0;
+ border-bottom-width: 0;
+ border-left-width: 0;
color: var(--deemphasized-text-color);
font-weight: var(--font-weight-normal);
height: 0.6em;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
index 3c0073e..fdc72ce 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
@@ -29,7 +29,6 @@
export interface GrAutocompleteDropdown {
$: {
- cursor: GrCursorManager;
suggestions: Element;
};
}
@@ -84,9 +83,6 @@
@property({type: Array})
suggestions: Item[] = [];
- @property({type: Array})
- _suggestionEls: Element[] = [];
-
get keyBindings() {
return {
up: '_handleUp',
@@ -97,9 +93,17 @@
};
}
+ private cursor = new GrCursorManager();
+
+ constructor() {
+ super();
+ this.cursor.cursorTargetClass = 'selected';
+ this.cursor.focusOnMove = true;
+ }
+
/** @override */
disconnectedCallback() {
- this.$.cursor.unsetCursor();
+ this.cursor.unsetCursor();
super.disconnectedCallback();
}
@@ -136,13 +140,13 @@
cursorDown() {
if (!this.isHidden) {
- this.$.cursor.next();
+ this.cursor.next();
}
}
cursorUp() {
if (!this.isHidden) {
- this.$.cursor.previous();
+ this.cursor.previous();
}
}
@@ -153,7 +157,7 @@
new CustomEvent('item-selected', {
detail: {
trigger: 'tab',
- selected: this.$.cursor.target,
+ selected: this.cursor.target,
},
composed: true,
bubbles: true,
@@ -168,7 +172,7 @@
new CustomEvent('item-selected', {
detail: {
trigger: 'enter',
- selected: this.$.cursor.target,
+ selected: this.cursor.target,
},
composed: true,
bubbles: true,
@@ -208,7 +212,7 @@
}
getCursorTarget() {
- return this.$.cursor.target;
+ return this.cursor.target;
}
@observe('suggestions')
@@ -216,18 +220,23 @@
if (this.suggestions.length > 0) {
if (!this.isHidden) {
flush();
- this._suggestionEls = Array.from(
+ this.cursor.stops = Array.from(
this.$.suggestions.querySelectorAll('li')
);
this._resetCursorIndex();
}
} else {
- this._suggestionEls = [];
+ this.cursor.stops = [];
}
}
+ @observe('index')
+ _setIndex() {
+ this.cursor.index = this.index || -1;
+ }
+
_resetCursorIndex() {
- this.$.cursor.setCursorAtIndex(0);
+ this.cursor.setCursorAtIndex(0);
}
_computeLabelClass(item: Item) {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_html.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_html.ts
index 7e00e47..b86e8ec 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_html.ts
@@ -91,12 +91,4 @@
</template>
</ul>
</div>
- <gr-cursor-manager
- id="cursor"
- index="[[index]]"
- cursor-target-class="selected"
- scroll-mode="never"
- focus-on-move=""
- stops="[[_suggestionEls]]"
- ></gr-cursor-manager>
`;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.js b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.js
index ad06649..200fddc 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.js
@@ -55,7 +55,7 @@
element.addEventListener('item-selected', itemSelectedStub);
MockInteractions.pressAndReleaseKeyOn(element, 9);
assert.isTrue(handleTabSpy.called);
- assert.equal(element.$.cursor.index, 0);
+ assert.equal(element.cursor.index, 0);
assert.isTrue(itemSelectedStub.called);
assert.deepEqual(itemSelectedStub.lastCall.args[0].detail, {
trigger: 'tab',
@@ -69,7 +69,7 @@
element.addEventListener('item-selected', itemSelectedStub);
MockInteractions.pressAndReleaseKeyOn(element, 13);
assert.isTrue(handleEnterSpy.called);
- assert.equal(element.$.cursor.index, 0);
+ assert.equal(element.cursor.index, 0);
assert.deepEqual(itemSelectedStub.lastCall.args[0].detail, {
trigger: 'enter',
selected: element.getCursorTarget(),
@@ -78,28 +78,28 @@
test('down key', () => {
element.isHidden = true;
- const nextSpy = sinon.spy(element.$.cursor, 'next');
+ const nextSpy = sinon.spy(element.cursor, 'next');
MockInteractions.pressAndReleaseKeyOn(element, 40);
assert.isFalse(nextSpy.called);
- assert.equal(element.$.cursor.index, 0);
+ assert.equal(element.cursor.index, 0);
element.isHidden = false;
MockInteractions.pressAndReleaseKeyOn(element, 40);
assert.isTrue(nextSpy.called);
- assert.equal(element.$.cursor.index, 1);
+ assert.equal(element.cursor.index, 1);
});
test('up key', () => {
element.isHidden = true;
- const prevSpy = sinon.spy(element.$.cursor, 'previous');
+ const prevSpy = sinon.spy(element.cursor, 'previous');
MockInteractions.pressAndReleaseKeyOn(element, 38);
assert.isFalse(prevSpy.called);
- assert.equal(element.$.cursor.index, 0);
+ assert.equal(element.cursor.index, 0);
element.isHidden = false;
- element.$.cursor.setCursorAtIndex(1);
- assert.equal(element.$.cursor.index, 1);
+ element.cursor.setCursorAtIndex(1);
+ assert.equal(element.cursor.index, 1);
MockInteractions.pressAndReleaseKeyOn(element, 38);
assert.isTrue(prevSpy.called);
- assert.equal(element.$.cursor.index, 0);
+ assert.equal(element.cursor.index, 0);
});
test('tapping selects item', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
index aa7486e..b8c2c21 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -25,7 +25,6 @@
import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {property, customElement, observe} from '@polymer/decorators';
import {GrAutocompleteDropdown} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
-import {GrCursorManager} from '../gr-cursor-manager/gr-cursor-manager';
import {PaperInputElementExt} from '../../../types/types';
import {CustomKeyboardEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
@@ -38,7 +37,6 @@
$: {
input: PaperInputElementExt;
suggestions: GrAutocompleteDropdown;
- cursor: GrCursorManager;
};
}
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.js
index 2c00a56..d72007e 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.js
@@ -46,7 +46,7 @@
]));
element.query = queryStub;
assert.isTrue(element.$.suggestions.isHidden);
- assert.equal(element.$.suggestions.$.cursor.index, -1);
+ assert.equal(element.$.suggestions.cursor.index, -1);
focusOnInput(element);
element.text = 'blah';
@@ -64,7 +64,7 @@
assert.equal(suggestions[i].innerText.trim(), 'blah ' + i);
}
- assert.notEqual(element.$.suggestions.$.cursor.index, -1);
+ assert.notEqual(element.$.suggestions.cursor.index, -1);
});
});
@@ -121,7 +121,7 @@
element.query = queryStub;
assert.isTrue(element.$.suggestions.isHidden);
- assert.equal(element.$.suggestions.$.cursor.index, -1);
+ assert.equal(element.$.suggestions.cursor.index, -1);
element._focused = true;
element.text = 'blah';
@@ -131,21 +131,21 @@
const commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
- assert.equal(element.$.suggestions.$.cursor.index, 0);
+ assert.equal(element.$.suggestions.cursor.index, 0);
MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null,
'down');
- assert.equal(element.$.suggestions.$.cursor.index, 1);
+ assert.equal(element.$.suggestions.cursor.index, 1);
MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null,
'down');
- assert.equal(element.$.suggestions.$.cursor.index, 2);
+ assert.equal(element.$.suggestions.cursor.index, 2);
MockInteractions.pressAndReleaseKeyOn(element.$.input, 38, null, 'up');
- assert.equal(element.$.suggestions.$.cursor.index, 1);
+ assert.equal(element.$.suggestions.cursor.index, 1);
MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
'enter');
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button_html.ts b/polygerrit-ui/app/elements/shared/gr-button/gr-button_html.ts
index b272951..64cdb9c 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button_html.ts
@@ -53,7 +53,10 @@
font: inherit;
text-transform: uppercase;
outline-width: 0;
- border-radius: var(--border-radius);
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ border-bottom-right-radius: var(--border-radius);
+ border-bottom-left-radius: var(--border-radius);
-moz-user-select: none;
-ms-user-select: none;
-webkit-user-select: none;
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
index 411dac4..8408c78 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
@@ -14,21 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-cursor-manager_html';
+import {BehaviorSubject} from 'rxjs';
import {ScrollMode} from '../../../constants/constants';
-import {customElement, property, observe} from '@polymer/decorators';
-
-export interface GrCursorManager {
- $: {};
-}
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-cursor-manager': GrCursorManager;
- }
-}
/**
* Return type for cursor moves, that indicate whether a move was possible.
@@ -60,31 +47,33 @@
return !(stop instanceof AbortStop);
}
-@customElement('gr-cursor-manager')
-export class GrCursorManager extends LegacyElementMixin(PolymerElement) {
- static get template() {
- return htmlTemplate;
+export class GrCursorManager {
+ get target(): HTMLElement | null {
+ return this.targetSubject.getValue();
}
- @property({type: Object, notify: true})
- target: HTMLElement | null = null;
+ set target(target: HTMLElement | null) {
+ this.targetSubject.next(target);
+ this._scrollToTarget();
+ }
+
+ private targetSubject = new BehaviorSubject<HTMLElement | null>(null);
+
+ target$ = this.targetSubject.asObservable();
/**
* The height of content intended to be included with the target.
*/
- @property({type: Number})
_targetHeight: number | null = null;
/**
* The index of the current target (if any). -1 otherwise.
*/
- @property({type: Number})
index = -1;
/**
* The class to apply to the current target. Use null for no class.
*/
- @property({type: String})
cursorTargetClass: string | null = null;
/**
@@ -95,17 +84,23 @@
*
* @type {string|undefined}
*/
- @property({type: String})
scrollMode: string = ScrollMode.NEVER;
/**
* When true, will call element.focus() during scrolling.
*/
- @property({type: Boolean})
focusOnMove = false;
- @property({type: Array})
- stops: Stop[] = [];
+ set stops(stops: Stop[]) {
+ this.stopsInternal = stops;
+ this._updateIndex();
+ }
+
+ get stops(): Stop[] {
+ return this.stopsInternal;
+ }
+
+ private stopsInternal: Stop[] = [];
/** Only non-AbortStop stops. */
get targetableStops(): HTMLElement[] {
@@ -377,7 +372,6 @@
}
}
- @observe('stops')
_updateIndex() {
if (!this.target) {
this.index = -1;
@@ -424,7 +418,6 @@
return top + -dims.innerHeight / 3 + target.offsetHeight / 2;
}
- @observe('target')
_scrollToTarget() {
if (!this.target || this.scrollMode === ScrollMode.NEVER) {
return;
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_html.ts b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_html.ts
deleted file mode 100644
index 1489006..0000000
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_html.ts
+++ /dev/null
@@ -1,19 +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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html``;
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
index 6f74d6b..e51d190 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
@@ -18,10 +18,9 @@
import '../../../test/common-test-setup-karma.js';
import './gr-cursor-manager.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-import {AbortStop, CursorMoveResult} from './gr-cursor-manager.js';
+import {AbortStop, CursorMoveResult, GrCursorManager} from './gr-cursor-manager.js';
const basicTestFixutre = fixtureFromTemplate(html`
- <gr-cursor-manager cursor-target-class="targeted"></gr-cursor-manager>
<ul>
<li>A</li>
<li>B</li>
@@ -31,180 +30,180 @@
`);
suite('gr-cursor-manager tests', () => {
- let element;
+ let cursor;
let list;
setup(() => {
- const fixtureElements = basicTestFixutre.instantiate();
- element = fixtureElements[0];
- list = fixtureElements[1];
+ list = basicTestFixutre.instantiate();
+ cursor = new GrCursorManager();
+ cursor.cursorTargetClass = 'targeted';
});
test('core cursor functionality', () => {
// The element is initialized into the proper state.
- assert.isArray(element.stops);
- assert.equal(element.stops.length, 0);
- assert.equal(element.index, -1);
- assert.isNotOk(element.target);
+ assert.isArray(cursor.stops);
+ assert.equal(cursor.stops.length, 0);
+ assert.equal(cursor.index, -1);
+ assert.isNotOk(cursor.target);
// Initialize the cursor with its stops.
- element.stops = [...list.querySelectorAll('li')];
+ cursor.stops = [...list.querySelectorAll('li')];
// It should have the stops but it should not be targeting any of them.
- assert.isNotNull(element.stops);
- assert.equal(element.stops.length, 4);
- assert.equal(element.index, -1);
- assert.isNotOk(element.target);
+ assert.isNotNull(cursor.stops);
+ assert.equal(cursor.stops.length, 4);
+ assert.equal(cursor.index, -1);
+ assert.isNotOk(cursor.target);
// Select the third stop.
- element.setCursor(list.children[2]);
+ cursor.setCursor(list.children[2]);
// It should update its internal state and update the element's class.
- assert.equal(element.index, 2);
- assert.equal(element.target, list.children[2]);
+ assert.equal(cursor.index, 2);
+ assert.equal(cursor.target, list.children[2]);
assert.isTrue(list.children[2].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isFalse(element.isAtEnd());
+ assert.isFalse(cursor.isAtStart());
+ assert.isFalse(cursor.isAtEnd());
// Progress the cursor.
- let result = element.next();
+ let result = cursor.next();
// Confirm that the next stop is selected and that the previous stop is
// unselected.
assert.equal(result, CursorMoveResult.MOVED);
- assert.equal(element.index, 3);
- assert.equal(element.target, list.children[3]);
- assert.isTrue(element.isAtEnd());
+ assert.equal(cursor.index, 3);
+ assert.equal(cursor.target, list.children[3]);
+ assert.isTrue(cursor.isAtEnd());
assert.isFalse(list.children[2].classList.contains('targeted'));
assert.isTrue(list.children[3].classList.contains('targeted'));
// Progress the cursor.
- result = element.next();
+ result = cursor.next();
// We should still be at the end.
assert.equal(result, CursorMoveResult.CLIPPED);
- assert.equal(element.index, 3);
- assert.equal(element.target, list.children[3]);
- assert.isTrue(element.isAtEnd());
+ assert.equal(cursor.index, 3);
+ assert.equal(cursor.target, list.children[3]);
+ assert.isTrue(cursor.isAtEnd());
// Wind the cursor all the way back to the first stop.
- result = element.previous();
+ result = cursor.previous();
assert.equal(result, CursorMoveResult.MOVED);
- result = element.previous();
+ result = cursor.previous();
assert.equal(result, CursorMoveResult.MOVED);
- result = element.previous();
+ result = cursor.previous();
assert.equal(result, CursorMoveResult.MOVED);
// The element state should reflect the start of the list.
- assert.equal(element.index, 0);
- assert.equal(element.target, list.children[0]);
- assert.isTrue(element.isAtStart());
+ assert.equal(cursor.index, 0);
+ assert.equal(cursor.target, list.children[0]);
+ assert.isTrue(cursor.isAtStart());
assert.isTrue(list.children[0].classList.contains('targeted'));
const newLi = document.createElement('li');
newLi.textContent = 'Z';
list.insertBefore(newLi, list.children[0]);
- element.stops = [...list.querySelectorAll('li')];
+ cursor.stops = [...list.querySelectorAll('li')];
- assert.equal(element.index, 1);
+ assert.equal(cursor.index, 1);
// De-select all targets.
- element.unsetCursor();
+ cursor.unsetCursor();
// There should now be no cursor target.
assert.isFalse(list.children[1].classList.contains('targeted'));
- assert.isNotOk(element.target);
- assert.equal(element.index, -1);
+ assert.isNotOk(cursor.target);
+ assert.equal(cursor.index, -1);
});
test('isAtStart() returns true when there are no stops', () => {
- element.stops = [];
- assert.isTrue(element.isAtStart());
+ cursor.stops = [];
+ assert.isTrue(cursor.isAtStart());
});
test('isAtEnd() returns true when there are no stops', () => {
- element.stops = [];
- assert.isTrue(element.isAtEnd());
+ cursor.stops = [];
+ assert.isTrue(cursor.isAtEnd());
});
test('next() goes to first element when no cursor is set', () => {
- element.stops = [...list.querySelectorAll('li')];
- const result = element.next();
+ cursor.stops = [...list.querySelectorAll('li')];
+ const result = cursor.next();
assert.equal(result, CursorMoveResult.MOVED);
- assert.equal(element.index, 0);
- assert.equal(element.target, list.children[0]);
+ assert.equal(cursor.index, 0);
+ assert.equal(cursor.target, list.children[0]);
assert.isTrue(list.children[0].classList.contains('targeted'));
- assert.isTrue(element.isAtStart());
- assert.isFalse(element.isAtEnd());
+ assert.isTrue(cursor.isAtStart());
+ assert.isFalse(cursor.isAtEnd());
});
test('next() resets the cursor when there are no stops', () => {
- element.stops = [];
- const result = element.next();
+ cursor.stops = [];
+ const result = cursor.next();
assert.equal(result, CursorMoveResult.NO_STOPS);
- assert.equal(element.index, -1);
- assert.isNotOk(element.target);
+ assert.equal(cursor.index, -1);
+ assert.isNotOk(cursor.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
});
test('previous() goes to last element when no cursor is set', () => {
- element.stops = [...list.querySelectorAll('li')];
- const result = element.previous();
+ cursor.stops = [...list.querySelectorAll('li')];
+ const result = cursor.previous();
assert.equal(result, CursorMoveResult.MOVED);
const lastIndex = list.children.length - 1;
- assert.equal(element.index, lastIndex);
- assert.equal(element.target, list.children[lastIndex]);
+ assert.equal(cursor.index, lastIndex);
+ assert.equal(cursor.target, list.children[lastIndex]);
assert.isTrue(list.children[lastIndex].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isTrue(element.isAtEnd());
+ assert.isFalse(cursor.isAtStart());
+ assert.isTrue(cursor.isAtEnd());
});
test('previous() resets the cursor when there are no stops', () => {
- element.stops = [];
- const result = element.previous();
+ cursor.stops = [];
+ const result = cursor.previous();
assert.equal(result, CursorMoveResult.NO_STOPS);
- assert.equal(element.index, -1);
- assert.isNotOk(element.target);
+ assert.equal(cursor.index, -1);
+ assert.isNotOk(cursor.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
});
test('_moveCursor', () => {
// Initialize the cursor with its stops.
- element.stops = [...list.querySelectorAll('li')];
+ cursor.stops = [...list.querySelectorAll('li')];
// Select the first stop.
- element.setCursor(list.children[0]);
+ cursor.setCursor(list.children[0]);
const getTargetHeight = sinon.stub();
// Move the cursor without an optional get target height function.
- element._moveCursor(1);
+ cursor._moveCursor(1);
assert.isFalse(getTargetHeight.called);
// Move the cursor with an optional get target height function.
- element._moveCursor(1, {getTargetHeight});
+ cursor._moveCursor(1, {getTargetHeight});
assert.isTrue(getTargetHeight.called);
});
test('_moveCursor from for invalid index does not check height', () => {
- element.stops = [];
+ cursor.stops = [];
const getTargetHeight = sinon.stub();
- element._moveCursor(1, () => false, {getTargetHeight});
+ cursor._moveCursor(1, () => false, {getTargetHeight});
assert.isFalse(getTargetHeight.called);
});
test('setCursorAtIndex with noScroll', () => {
- sinon.stub(element, '_targetIsVisible').callsFake(() => false);
+ sinon.stub(cursor, '_targetIsVisible').callsFake(() => false);
const scrollStub = sinon.stub(window, 'scrollTo');
- element.stops = [...list.querySelectorAll('li')];
- element.scrollMode = 'keep-visible';
+ cursor.stops = [...list.querySelectorAll('li')];
+ cursor.scrollMode = 'keep-visible';
- element.setCursorAtIndex(1, true);
+ cursor.setCursorAtIndex(1, true);
assert.isFalse(scrollStub.called);
- element.setCursorAtIndex(2);
+ cursor.setCursorAtIndex(2);
assert.isTrue(scrollStub.called);
});
@@ -212,30 +211,30 @@
const isLetterB = function(row) {
return row.textContent === 'B';
};
- element.stops = [...list.querySelectorAll('li')];
+ cursor.stops = [...list.querySelectorAll('li')];
// Start cursor at the first stop.
- element.setCursor(list.children[0]);
+ cursor.setCursor(list.children[0]);
// Move forward to meet the next condition.
- element.next({filter: isLetterB});
- assert.equal(element.index, 1);
+ cursor.next({filter: isLetterB});
+ assert.equal(cursor.index, 1);
// Nothing else meets the condition, should be at last stop.
- element.next({filter: isLetterB});
- assert.equal(element.index, 3);
+ cursor.next({filter: isLetterB});
+ assert.equal(cursor.index, 3);
// Should stay at last stop if try to proceed.
- element.next({filter: isLetterB});
- assert.equal(element.index, 3);
+ cursor.next({filter: isLetterB});
+ assert.equal(cursor.index, 3);
// Go back to the previous condition met. Should be back at.
// stop 1.
- element.previous({filter: isLetterB});
- assert.equal(element.index, 1);
+ cursor.previous({filter: isLetterB});
+ assert.equal(cursor.index, 1);
// Go back. No more meet the condition. Should be at stop 0.
- element.previous({filter: isLetterB});
- assert.equal(element.index, 0);
+ cursor.previous({filter: isLetterB});
+ assert.equal(cursor.index, 0);
});
test('focusOnMove prop', () => {
@@ -243,129 +242,129 @@
for (let i = 0; i < listEls.length; i++) {
sinon.spy(listEls[i], 'focus');
}
- element.stops = listEls;
- element.setCursor(list.children[0]);
+ cursor.stops = listEls;
+ cursor.setCursor(list.children[0]);
- element.focusOnMove = false;
- element.next();
- assert.isFalse(element.target.focus.called);
+ cursor.focusOnMove = false;
+ cursor.next();
+ assert.isFalse(cursor.target.focus.called);
- element.focusOnMove = true;
- element.next();
- assert.isTrue(element.target.focus.called);
+ cursor.focusOnMove = true;
+ cursor.next();
+ assert.isTrue(cursor.target.focus.called);
});
suite('_scrollToTarget', () => {
let scrollStub;
setup(() => {
- element.stops = [...list.querySelectorAll('li')];
- element.scrollMode = 'keep-visible';
+ cursor.stops = [...list.querySelectorAll('li')];
+ cursor.scrollMode = 'keep-visible';
// There is a target which has a targetNext
- element.setCursor(list.children[0]);
- element._moveCursor(1);
+ cursor.setCursor(list.children[0]);
+ cursor._moveCursor(1);
scrollStub = sinon.stub(window, 'scrollTo');
window.innerHeight = 60;
});
test('Called when top and bottom not visible', () => {
- sinon.stub(element, '_targetIsVisible').returns(false);
- element._scrollToTarget();
+ sinon.stub(cursor, '_targetIsVisible').returns(false);
+ cursor._scrollToTarget();
assert.isTrue(scrollStub.called);
});
test('Not called when top and bottom visible', () => {
- sinon.stub(element, '_targetIsVisible').returns(true);
- element._scrollToTarget();
+ sinon.stub(cursor, '_targetIsVisible').returns(true);
+ cursor._scrollToTarget();
assert.isFalse(scrollStub.called);
});
test('Called when top is visible, bottom is not, scroll is lower', () => {
- const visibleStub = sinon.stub(element, '_targetIsVisible').callsFake(
+ const visibleStub = sinon.stub(cursor, '_targetIsVisible').callsFake(
() => visibleStub.callCount === 2);
- sinon.stub(element, '_getWindowDims').returns({
+ sinon.stub(cursor, '_getWindowDims').returns({
scrollX: 123,
scrollY: 15,
innerHeight: 1000,
pageYOffset: 0,
});
- sinon.stub(element, '_calculateScrollToValue').returns(20);
- element._scrollToTarget();
+ sinon.stub(cursor, '_calculateScrollToValue').returns(20);
+ cursor._scrollToTarget();
assert.isTrue(scrollStub.called);
assert.isTrue(scrollStub.calledWithExactly(123, 20));
assert.equal(visibleStub.callCount, 2);
});
test('Called when top is visible, bottom not, scroll is higher', () => {
- const visibleStub = sinon.stub(element, '_targetIsVisible').callsFake(
+ const visibleStub = sinon.stub(cursor, '_targetIsVisible').callsFake(
() => visibleStub.callCount === 2);
- sinon.stub(element, '_getWindowDims').returns({
+ sinon.stub(cursor, '_getWindowDims').returns({
scrollX: 123,
scrollY: 25,
innerHeight: 1000,
pageYOffset: 0,
});
- sinon.stub(element, '_calculateScrollToValue').returns(20);
- element._scrollToTarget();
+ sinon.stub(cursor, '_calculateScrollToValue').returns(20);
+ cursor._scrollToTarget();
assert.isFalse(scrollStub.called);
assert.equal(visibleStub.callCount, 2);
});
test('_calculateScrollToValue', () => {
- sinon.stub(element, '_getWindowDims').returns({
+ sinon.stub(cursor, '_getWindowDims').returns({
scrollX: 123,
scrollY: 25,
innerHeight: 300,
pageYOffset: 0,
});
- assert.equal(element._calculateScrollToValue(1000, {offsetHeight: 10}),
+ assert.equal(cursor._calculateScrollToValue(1000, {offsetHeight: 10}),
905);
});
});
suite('AbortStops', () => {
test('next() does not skip AbortStops', () => {
- element.stops = [
+ cursor.stops = [
document.createElement('li'),
new AbortStop(),
document.createElement('li'),
];
- element.setCursorAtIndex(0);
+ cursor.setCursorAtIndex(0);
- const result = element.next();
+ const result = cursor.next();
assert.equal(result, CursorMoveResult.ABORTED);
- assert.equal(element.index, 0);
+ assert.equal(cursor.index, 0);
});
test('setCursorAtIndex() does not target AbortStops', () => {
- element.stops = [
+ cursor.stops = [
document.createElement('li'),
new AbortStop(),
document.createElement('li'),
];
- element.setCursorAtIndex(1);
- assert.equal(element.index, -1);
+ cursor.setCursorAtIndex(1);
+ assert.equal(cursor.index, -1);
});
test('moveToStart() does not target AbortStop', () => {
- element.stops = [
+ cursor.stops = [
new AbortStop(),
document.createElement('li'),
document.createElement('li'),
];
- element.moveToStart();
- assert.equal(element.index, -1);
+ cursor.moveToStart();
+ assert.equal(cursor.index, -1);
});
test('moveToEnd() does not target AbortStop', () => {
- element.stops = [
+ cursor.stops = [
document.createElement('li'),
document.createElement('li'),
new AbortStop(),
];
- element.moveToEnd();
- assert.equal(element.index, -1);
+ cursor.moveToEnd();
+ assert.equal(cursor.index, -1);
});
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
index 3d2b7b8..4e61bcd 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
@@ -40,7 +40,6 @@
export interface GrDropdown {
$: {
dropdown: IronDropdownElement;
- cursor: GrCursorManager;
};
}
@@ -110,12 +109,6 @@
@property({type: Array})
disabledIds: string[] = [];
- /**
- * The elements of the list.
- */
- @property({type: Array})
- _listElements: Element[] = [];
-
get keyBindings() {
return {
down: '_handleDown',
@@ -125,9 +118,17 @@
};
}
+ private cursor = new GrCursorManager();
+
+ constructor() {
+ super();
+ this.cursor.cursorTargetClass = 'selected';
+ this.cursor.focusOnMove = true;
+ }
+
/** @override */
disconnectedCallback() {
- this.$.cursor.unsetCursor();
+ this.cursor.unsetCursor();
super.disconnectedCallback();
}
@@ -138,7 +139,7 @@
if (this.$.dropdown.opened) {
e.preventDefault();
e.stopPropagation();
- this.$.cursor.previous();
+ this.cursor.previous();
} else {
this._open();
}
@@ -151,7 +152,7 @@
if (this.$.dropdown.opened) {
e.preventDefault();
e.stopPropagation();
- this.$.cursor.next();
+ this.cursor.next();
} else {
this._open();
}
@@ -178,8 +179,8 @@
// TODO(milutin): This solution is not particularly robust in general.
// Since gr-tooltip-content click on shadow dom is not propagated down,
// we have to target `a` inside it.
- if (this.$.cursor.target !== null) {
- const el = this.$.cursor.target.querySelector(':not([hidden]) a');
+ if (this.cursor.target !== null) {
+ const el = this.cursor.target.querySelector(':not([hidden]) a');
if (el) {
(el as HTMLElement).click();
}
@@ -215,8 +216,8 @@
_open() {
this.$.dropdown.open();
this._resetCursorStops();
- this.$.cursor.setCursorAtIndex(0);
- if (this.$.cursor.target !== null) this.$.cursor.target.focus();
+ this.cursor.setCursorAtIndex(0);
+ if (this.cursor.target !== null) this.cursor.target.focus();
}
_close() {
@@ -331,7 +332,7 @@
_resetCursorStops() {
if (this.items && this.items.length > 0 && this.$.dropdown.opened) {
flush();
- this._listElements =
+ this.cursor.stops =
this.root !== null ? Array.from(this.root.querySelectorAll('li')) : [];
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
index b1fa7bd..aaeb47c 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
@@ -159,11 +159,4 @@
</ul>
</div>
</iron-dropdown>
- <gr-cursor-manager
- id="cursor"
- cursor-target-class="selected"
- scroll-mode="never"
- focus-on-move=""
- stops="[[_listElements]]"
- ></gr-cursor-manager>
`;
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.js
index fea4a08..c271b41 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.js
@@ -153,7 +153,7 @@
});
test('down', () => {
- const stub = sinon.stub(element.$.cursor, 'next');
+ const stub = sinon.stub(element.cursor, 'next');
assert.isFalse(element.$.dropdown.opened);
MockInteractions.pressAndReleaseKeyOn(element, 40);
assert.isTrue(element.$.dropdown.opened);
@@ -162,7 +162,7 @@
});
test('up', () => {
- const stub = sinon.stub(element.$.cursor, 'previous');
+ const stub = sinon.stub(element.cursor, 'previous');
assert.isFalse(element.$.dropdown.opened);
MockInteractions.pressAndReleaseKeyOn(element, 38);
assert.isTrue(element.$.dropdown.opened);
@@ -177,7 +177,7 @@
MockInteractions.pressAndReleaseKeyOn(element, 32); // Space
assert.isTrue(element.$.dropdown.opened);
- const el = element.$.cursor.target.querySelector(':not([hidden]) a');
+ const el = element.cursor.target.querySelector(':not([hidden]) a');
const stub = sinon.stub(el, 'click');
MockInteractions.pressAndReleaseKeyOn(element, 32); // Space
assert.isTrue(stub.called);
diff --git a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status-chip_html.ts b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status-chip_html.ts
index 9e49868..6c7f3e0a 100644
--- a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status-chip_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status-chip_html.ts
@@ -25,7 +25,7 @@
padding: 0 var(--spacing-m);
color: var(--primary-text-color);
font-size: var(--font-size-small);
- background-color: var(--dark-add-highlight-color);
+ background-color: var(--file-status-added);
}
.status.invisible,
.status.M {
@@ -34,10 +34,10 @@
.status.D,
.status.R,
.status.W {
- background-color: var(--dark-remove-highlight-color);
+ background-color: var(--file-status-changed);
}
.status.U {
- background-color: var(--comment-background-color);
+ background-color: var(--file-status-unchanged);
}
</style>
<span
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
index 7ad7223..48de78e 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
@@ -35,6 +35,26 @@
const HIDE_CLASS = 'hide';
/**
+ * ID for the container element.
+ */
+const containerId = 'gr-hovercard-container';
+
+export function getHovercardContainer(
+ options: {createIfNotExists: boolean} = {createIfNotExists: false}
+): HTMLElement | null {
+ let container = getRootElement().querySelector<HTMLElement>(
+ `#${containerId}`
+ );
+ if (!container && options.createIfNotExists) {
+ // If it does not exist, create and initialize the hovercard container.
+ container = document.createElement('div');
+ container.setAttribute('id', containerId);
+ getRootElement().appendChild(container);
+ }
+ return container;
+}
+
+/**
* How long should we wait before showing the hovercard when the user hovers
* over the element?
*/
@@ -99,12 +119,6 @@
@property({type: Object})
container: HTMLElement | null = null;
- /**
- * ID for the container element.
- */
- @property({type: String})
- containerId = 'gr-hovercard-container';
-
private hideTask?: DelayedTask;
private showTask?: DelayedTask;
@@ -147,16 +161,7 @@
ready() {
super.ready();
// First, check to see if the container has already been created.
- this.container = getRootElement().querySelector('#' + this.containerId);
-
- if (this.container) {
- return;
- }
-
- // If it does not exist, create and initialize the hovercard container.
- this.container = document.createElement('div');
- this.container.setAttribute('id', this.containerId);
- getRootElement().appendChild(this.container);
+ this.container = getHovercardContainer({createIfNotExists: true});
}
removeListeners() {
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
index 3fa0da7..56ab8c6 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
@@ -45,11 +45,13 @@
background-color: var(--vote-color-recommended);
border-radius: 12px;
border: 1px solid var(--vote-outline-recommended);
+ color: var(--chip-color);
}
.negative {
background-color: var(--vote-color-disliked);
border-radius: 12px;
border: 1px solid var(--vote-outline-disliked);
+ color: var(--chip-color);
}
.hidden {
display: none;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_html.ts b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_html.ts
index 8581a0c..cac88b7 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_html.ts
@@ -31,7 +31,10 @@
}
gr-button.remove {
--gr-remove-button-style: {
- border: 0;
+ border-top-width: 0;
+ border-right-width: 0;
+ border-bottom-width: 0;
+ border-left-width: 0;
color: var(--deemphasized-text-color);
font-weight: var(--font-weight-normal);
height: 0.6em;
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index 100d5cc..2c48ec0f 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -22,6 +22,7 @@
import {IronOverlayBehavior} from '@polymer/iron-overlay-behavior/iron-overlay-behavior';
import {findActiveElement} from '../../../utils/dom-util';
import {fireEvent} from '../../../utils/event-util';
+import {getHovercardContainer} from '../gr-hovercard/gr-hovercard-behavior';
const AWAIT_MAX_ITERS = 10;
const AWAIT_STEP = 5;
@@ -118,6 +119,20 @@
}
}
+ _onCaptureFocus(e: Event) {
+ const hovercardContainer = getHovercardContainer();
+ if (hovercardContainer) {
+ // Hovercard container is not a child of an overlay.
+ // When an overlay is opened and a user clicks inside hovercard,
+ // the IronOverlayBehavior doesn't allow to set focus inside a hovercard.
+ // As a result, user can't select a text (username) in the hovercard
+ // in a dialog. We should skip default _onCaptureFocus for hovercards.
+ const path = e.composedPath();
+ if (path.indexOf(hovercardContainer) >= 0) return;
+ }
+ super._onCaptureFocus(e);
+ }
+
/**
* Override the focus stops that iron-overlay-behavior tries to find.
*/
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts
index 1960718..18d53612 100644
--- a/polygerrit-ui/app/services/checks/checks-service.ts
+++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -131,7 +131,14 @@
}
assertIsDefined(change.revisions, 'change.revisions');
const patchsetSha = getShaByPatchNum(change.revisions, patchNum);
- assertIsDefined(patchsetSha, 'patchsetSha');
+ // Sometimes patchNum is updated earlier than change, so change
+ // revisions don't have patchNum yet
+ if (!patchsetSha) {
+ return of({
+ responseCode: ResponseCode.OK,
+ runs: [],
+ });
+ }
const data: ChangeData = {
changeNumber: changeNum,
patchsetNumber: patchNum,
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 6f94b8d..ff950b8 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -31,5 +31,4 @@
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
COMMENT_CONTEXT = 'UiFeature__comment_context',
- TOKEN_HIGHLIGHTING = 'UiFeature__token_highlighting',
}
diff --git a/polygerrit-ui/app/styles/gr-voting-styles.ts b/polygerrit-ui/app/styles/gr-voting-styles.ts
index c1989de..b50aee6 100644
--- a/polygerrit-ui/app/styles/gr-voting-styles.ts
+++ b/polygerrit-ui/app/styles/gr-voting-styles.ts
@@ -27,8 +27,16 @@
<style>
:host {
--vote-chip-styles: {
- border: 1px solid var(--border-color);
- border-radius: 1em;
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-left-radius: 1em;
+ border-top-right-radius: 1em;
+ border-bottom-right-radius: 1em;
+ border-bottom-left-radius: 1em;
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
box-shadow: none;
box-sizing: border-box;
min-width: 3em;
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 6c40707..43ec286 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -257,6 +257,11 @@
--status-ready: var(--pink-800);
--status-custom: var(--purple-900);
+ /* file status colors */
+ --file-status-added: var(--green-300);
+ --file-status-changed: var(--red-200);
+ --file-status-unchanged: var(--grey-300);
+
/* fonts */
--font-family: 'Roboto', -apple-system, BlinkMacSystemFont, 'Segoe UI', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
--header-font-family: 'Open Sans', 'Roboto', -apple-system, BlinkMacSystemFont, 'Segoe UI', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
@@ -331,7 +336,6 @@
--coverage-covered: #e0f2f1;
--coverage-not-covered: #ffd1a4;
--ranged-comment-hint-text-color: var(--orange-900);
- --token-highlighting-color: #fffd54;
/* syntax colors */
--syntax-attr-color: #219;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index f5a4b02..f877771 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -128,7 +128,7 @@
--vote-outline-disliked: var(--red-200);
--vote-color-neutral: var(--gray-700);
--vote-color-recommended: var(--green-tonal);
- --vote-outline-recommended: var(--green-700);
+ --vote-outline-recommended: var(--green-200);
--vote-color-rejected: var(--red-200);
--outline-color-focus: var(--gray-100);
@@ -155,6 +155,11 @@
--status-ready: var(--pink-500);
--status-custom: var(--purple-400);
+ /* file status colors */
+ --file-status-added: var(--green-tonal);
+ --file-status-changed: var(--red-tonal);
+ --file-status-unchanged: var(--grey-700);
+
/* fonts */
--font-weight-bold: 700; /* 700 is the same as 'bold' */
@@ -193,7 +198,6 @@
--coverage-covered: #112826;
--coverage-not-covered: #6b3600;
--ranged-comment-hint-text-color: var(--blue-50);
- --token-highlighting-color: var(--yellow-tonal);
/* syntax colors */
--syntax-attr-color: #80cbbf;
diff --git a/polygerrit-ui/app/utils/access-util.ts b/polygerrit-ui/app/utils/access-util.ts
index 4af5533..44830e2 100644
--- a/polygerrit-ui/app/utils/access-util.ts
+++ b/polygerrit-ui/app/utils/access-util.ts
@@ -32,7 +32,6 @@
FORGE_COMMITTER = 'forgeCommitter',
FORGE_SERVER_AS_COMMITTER = 'forgeServerAsCommitter',
OWNER = 'owner',
- PUBLISH_DRAFTS = 'publishDrafts',
PUSH = 'push',
PUSH_MERGE = 'pushMerge',
READ = 'read',
@@ -108,10 +107,6 @@
id: AccessPermissionId.OWNER,
name: 'Owner',
},
- [AccessPermissionId.PUBLISH_DRAFTS]: {
- id: AccessPermissionId.PUBLISH_DRAFTS,
- name: 'Publish Drafts',
- },
[AccessPermissionId.PUSH]: {
id: AccessPermissionId.PUSH,
name: 'Push',
diff --git a/proto/cache.proto b/proto/cache.proto
index 874e60a..292a225 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -286,6 +286,29 @@
repeated ExternalGroupProto external_group = 1;
}
+// Serialized key for com.google.gerrit.server.account.GroupCacheImpl.
+// Next ID: 3
+message GroupKeyProto {
+ string uuid = 1;
+ bytes revision = 2;
+}
+
+
+// Serialized form of com.google.gerrit.entities.InternalGroup.
+// Next ID: 11
+message InternalGroupProto {
+ int32 id = 1;
+ string name = 2;
+ string description = 3;
+ string owner_group_uuid = 4;
+ bool is_visible_to_all = 5;
+ string group_uuid = 6;
+ int64 created_on = 7;
+ repeated int32 members_ids = 8;
+ repeated string subgroup_uuids = 9;
+ bytes ref_state = 10;
+}
+
// Key for com.google.gerrit.server.git.PureRevertCache.
// Next ID: 4
message PureRevertKeyProto {
diff --git a/resources/log4j.properties b/resources/log4j.properties
index 28c0ee4..39246b3 100644
--- a/resources/log4j.properties
+++ b/resources/log4j.properties
@@ -21,6 +21,7 @@
# Silence non-critical messages from MINA SSHD.
#
log4j.logger.org.apache.mina=WARN
+log4j.logger.org.apache.sshd.client=WARN
log4j.logger.org.apache.sshd.common=WARN
log4j.logger.org.apache.sshd.server=WARN
log4j.logger.org.apache.sshd.common.keyprovider.FileKeyPairProvider=INFO
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index acb5346..c7398a8 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -188,6 +188,8 @@
classpathentry('src', 'modules/jgit/org.eclipse.jgit.junit/src')
classpathentry('src', 'modules/jgit/org.eclipse.jgit.ssh.jsch/src')
classpathentry('src', 'modules/jgit/org.eclipse.jgit.ssh.jsch/resources')
+ classpathentry('src', 'modules/jgit/org.eclipse.jgit.ssh.apache/src')
+ classpathentry('src', 'modules/jgit/org.eclipse.jgit.ssh.apache/resources')
def classpathentry(kind, path, src=None, out=None, exported=None, excluding=None):
e = doc.createElement('classpathentry')
diff --git a/tools/node_tools/package.json b/tools/node_tools/package.json
index 9acbd07..bfc5191 100644
--- a/tools/node_tools/package.json
+++ b/tools/node_tools/package.json
@@ -3,8 +3,8 @@
"description": "Gerrit Build Tools",
"browser": false,
"dependencies": {
- "@bazel/rollup": "^3.2.0",
- "@bazel/typescript": "^3.2.0",
+ "@bazel/rollup": "^3.2.3",
+ "@bazel/typescript": "^3.2.3",
"@types/node": "^10.17.12",
"@types/parse5": "^4.0.0",
"@types/parse5-html-rewriting-stream": "^5.1.2",
diff --git a/tools/node_tools/yarn.lock b/tools/node_tools/yarn.lock
index 45a0c89..767f285 100644
--- a/tools/node_tools/yarn.lock
+++ b/tools/node_tools/yarn.lock
@@ -492,15 +492,15 @@
lodash "^4.17.13"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.2.0":
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.0.tgz#4241a5767e12e57b01279a539af2537c2d01924a"
- integrity sha512-Wkw6L+hor/+FzpDswri7IlWAbKyShnUZRx59fG06+qqVhpNaS3V3lnZqVytMlLLT4oSP8YSIzoXC5GkXgLI2/Q==
+"@bazel/rollup@^3.2.3":
+ version "3.2.3"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.3.tgz#eb6b17f89da5d38b035a6d59a99000f2d5ada667"
+ integrity sha512-7PmJqekk92OaiwwEN3dYsp5qbZX8cqr0WkFUiaM3FBsNYVoN/rdh+VxQJWTuDGLjWiHxLwE95YwVHcvwerSrCg==
-"@bazel/typescript@^3.2.0":
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.0.tgz#299bd173fe04f98407ab9be4f654662c1c28470e"
- integrity sha512-RKdy9ThbcUAqZR3AJK7AR/nxbJqdHi7pPayIGUSMIpxVkeTxVRQpf1aGe2H02HdZ9fR/uk1xXhO/Ff9TLvTgHQ==
+"@bazel/typescript@^3.2.3":
+ version "3.2.3"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.3.tgz#6e40bdb7c5294e588bac3b7d1269e58b98a1856c"
+ integrity sha512-Q1Yin/AYdh9yrkSJo3H6nVn6mMaohr5syjLd0Df0w7WI4zerdJTxrY5nhoWZwO/S1rPj8/MedDwZudCqPDeDMA==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 11710f0..3d04592 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -44,6 +44,12 @@
)
maven_jar(
+ name = "sshd-sftp",
+ artifact = "org.apache.sshd:sshd-sftp:" + SSHD_VERS,
+ sha1 = "6eddfe8fdf59a3d9a49151e4177f8c1bebeb30c9",
+ )
+
+ maven_jar(
name = "eddsa",
artifact = "net.i2p.crypto:eddsa:0.3.0",
sha1 = "1901c8d4d8bffb7d79027686cfb91e704217c3e1",
diff --git a/tools/remote-bazelrc b/tools/remote-bazelrc
index bbfe3a8..78b86d2 100644
--- a/tools/remote-bazelrc
+++ b/tools/remote-bazelrc
@@ -57,7 +57,7 @@
# Enable authentication. This will pick up application default credentials by
# default. You can use --auth_credentials=some_file.json to use a service
# account credential instead.
-build:remote --auth_enabled=true
+build:remote --google_default_credentials
# The following flags enable the remote cache so action results can be shared
# across machines, developers, and workspaces.
diff --git a/yarn.lock b/yarn.lock
index a424d79..adb731c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -485,20 +485,20 @@
lodash "^4.17.11"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.2.0":
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.0.tgz#4241a5767e12e57b01279a539af2537c2d01924a"
- integrity sha512-Wkw6L+hor/+FzpDswri7IlWAbKyShnUZRx59fG06+qqVhpNaS3V3lnZqVytMlLLT4oSP8YSIzoXC5GkXgLI2/Q==
+"@bazel/rollup@^3.2.3":
+ version "3.2.3"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.3.tgz#eb6b17f89da5d38b035a6d59a99000f2d5ada667"
+ integrity sha512-7PmJqekk92OaiwwEN3dYsp5qbZX8cqr0WkFUiaM3FBsNYVoN/rdh+VxQJWTuDGLjWiHxLwE95YwVHcvwerSrCg==
-"@bazel/terser@^3.2.0":
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.2.0.tgz#e53ad32733a0b231323b9eb55ebc2a3c65b10223"
- integrity sha512-/yq4gST3t1mETkP6NjC05yEyIIL//4mbfLI56hE3CC/mm/xJ6UeooFVpUdlJREQEDRAdNWoiMesQ1ZtgpNPzFg==
+"@bazel/terser@^3.2.3":
+ version "3.2.3"
+ resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.2.3.tgz#18849cb01f6a9fd840955547b0a9dbf63f02c720"
+ integrity sha512-SzX3ytgt8+i36sODPO9ju0LbjKXB8w9zi00S+XNManQTRilNrr/NODTYbBzKrjuebA/o52NMpvRGigRd1Z2RGg==
-"@bazel/typescript@^3.2.0":
- version "3.2.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.0.tgz#299bd173fe04f98407ab9be4f654662c1c28470e"
- integrity sha512-RKdy9ThbcUAqZR3AJK7AR/nxbJqdHi7pPayIGUSMIpxVkeTxVRQpf1aGe2H02HdZ9fR/uk1xXhO/Ff9TLvTgHQ==
+"@bazel/typescript@^3.2.3":
+ version "3.2.3"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.3.tgz#6e40bdb7c5294e588bac3b7d1269e58b98a1856c"
+ integrity sha512-Q1Yin/AYdh9yrkSJo3H6nVn6mMaohr5syjLd0Df0w7WI4zerdJTxrY5nhoWZwO/S1rPj8/MedDwZudCqPDeDMA==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
