Merge "Revert "Remove legacy-data-mixin""
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index dd31dd8..f51fdd1 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2338,57 +2338,122 @@
[[httpd.listenUrl]]httpd.listenUrl::
+
-Specifies the URLs the internal HTTP daemon should listen for
-connections on. The special hostname '*' may be used to listen
-on all local addresses. A context path may optionally be included,
-placing Gerrit Code Review's web address within a subdirectory of
-the server.
+Configuration for the listening sockets of the internal HTTP daemon.
+Each entry of `listenUrl` combines the following options for a
+listening socket: protocol, network address, port and context path.
+
-Multiple protocol schemes are supported:
+_Protocol_ can be either `http://`, `https://`, `proxy-http://` or
+`proxy-https://`. The latter two are special forms of `http://` with
+awareness of a reverse proxy (see below). _Network address_ selects
+the interface and/or scope of the listening socket. For notes
+examples, see below. _Port_ is the TCP port number and is optional
+(default value depends on the protocol). _Context path_ is the
+optional "base URI" for the Gerrit Code Review as application to
+serve on.
+
-* `http://`'hostname'`:`'port'
+**Protocol** schemes:
++
+* `http://`
+
Plain-text HTTP protocol. If port is not supplied, defaults to 80,
the standard HTTP port.
+
-* `https://`'hostname'`:`'port'
+* `https://`
+
SSL encrypted HTTP protocol. If port is not supplied, defaults to
443, the standard HTTPS port.
+
-Externally facing production sites are encouraged to use a reverse
-proxy configuration and `proxy-https://` (below), rather than using
-the embedded servlet container to implement the SSL processing.
-The proxy server with SSL support is probably easier to configure,
-provides more configuration options to control cipher usage, and
-is likely using natively compiled encryption algorithms, resulting
-in higher throughput.
+For configuration of the certificate and private key, see
+<<httpd.sslKeyStore,httpd.sslKeyStore>>.
+
-* `proxy-http://`'hostname'`:`'port'
+[NOTE]
+SSL/TLS configuration capabilities of Gerrit internal HTTP daemon
+are very limited. Externally facing production sites are strongly
+encouraged to use a reverse proxy configuration to handle SSL/TLS
+and use a `proxy-https://` scheme here (below) for security and
+performance reasons.
++
+* `proxy-http://`
+
Plain-text HTTP relayed from a reverse proxy. If port is not
supplied, defaults to 8080.
+
-Like http, but additional header parsing features are
-enabled to honor X-Forwarded-For, X-Forwarded-Host and
-X-Forwarded-Server. These headers are typically set by Apache's
-link:http://httpd.apache.org/docs/2.2/mod/mod_proxy.html#x-headers[mod_proxy].
+Like `http://`, but additional header parsing features are
+enabled to honor `X-Forwarded-For`, `X-Forwarded-Host` and
+`X-Forwarded-Server`. These headers are typically set by Apache's
+link:https://httpd.apache.org/docs/2.4/mod/mod_proxy.html#x-headers[mod_proxy].
+
-* `proxy-https://`'hostname'`:`'port'
+[NOTE]
+--
+For secruity reasons, make sure to only allow connections from a
+trusted reverse proxy in your network, as clients could otherwise
+easily spoof these headers and thus spoof their originating IP
+address effectively. If the reverse proxy is running on the same
+machine as Gerrit daemon, the use of a _loopback_ network address
+to bind to (see below) is strongly recommended to mitigate this.
+
+If not using Apache's mod_proxy, validate that your reverse proxy
+sets these headers on all requests. If not, either configure it to
+sanitize them from the origin, or use the `http://` scheme instead.
+--
+
-Plain text HTTP relayed from a reverse proxy that has already
+* `proxy-https://`
++
+Plain-text HTTP relayed from a reverse proxy that has already
handled the SSL encryption/decryption. If port is not supplied,
defaults to 8080.
+
-Behaves exactly like proxy-http, but also sets the scheme to assume
-'https://' is the proper URL back to the server.
+Behaves exactly like `proxy-http://`, but also sets the scheme to
+assume `https://` is the proper URL back to the server.
+
--
+**Network address** forms:
+
+* Loopback (localhost): `127.0.0.1` (IPv4) or `[::1]` (IPv6).
+* All (unspecified): `0.0.0.0` (IPv4), `[::]` (IPv6) or `*`
+ (IPv4 and IPv6)
+* Interface IP address, e.g. `1.2.3.4` (IPv4) or
+ `[2001:db8::a00:20ff:fea7:ccea]` (IPv6)
+* Hostname, resolved at startup time to an address.
+
+**Context path** is the local part of the URL to be used to access
+Gerrit on ('base URL'). E.g. `/gerrit/` to serve Gerrit on that URI
+as base. If set, consider to align this with the
+<<gerrit.canonicalWebUrl,gerrit.canonicalWebUrl>> setting. Correct
+settings may depend on the reverse proxy configuration as well. By
+default, this is `/` so that Gerrit serves requests on the root.
+
If multiple values are supplied, the daemon will listen on all
of them.
-By default, http://*:8080.
+Examples:
+
+----
+[httpd]
+ listenUrl = proxy-https://127.0.0.1:9999/gerrit/
+[gerrit]
+ # Reverse proxy is configured to serve with SSL/TLS on
+ # example.com and to relay requests on /gerrit/ onto
+ # http://127.0.0.1:9999/gerrit/
+ canonicalWebUrl = https://example.com/gerrit/
+----
+
+----
+[httpd]
+ # Listen on specific external interface with plaintext
+ # HTTP on IPv6.
+ listenUrl = http://[2001:db8::a00:20ff:fea7:ccea]
+
+ # Also listen on specific internal interface for use with
+ # reverse proxy run on another host.
+ listenUrl = proxy-https://192.168.100.123
+----
+
+See also the page on link:config-reverseproxy.html[reverse proxy]
+configuration.
+
+By default, `\http://*:8080`.
--
[[httpd.reuseAddress]]httpd.reuseAddress::
diff --git a/Documentation/dev-community.txt b/Documentation/dev-community.txt
index 0656090..591451b 100644
--- a/Documentation/dev-community.txt
+++ b/Documentation/dev-community.txt
@@ -49,6 +49,7 @@
[[plugin-development]]
== Plugin Development
+* link:dev-plugins-lifecycle.html[Plugin Lifecycle]
* link:dev-plugins.html[Developing Plugins]
* link:dev-build-plugins.html[Building Gerrit plugins]
* link:js-api.html[JavaScript Plugin API]
diff --git a/Documentation/dev-plugins-lifecycle.txt b/Documentation/dev-plugins-lifecycle.txt
new file mode 100644
index 0000000..b552472
--- /dev/null
+++ b/Documentation/dev-plugins-lifecycle.txt
@@ -0,0 +1,254 @@
+= Plugin Lifecycle
+
+Most of the plugins are hosted on the same instance as the
+link:https://gerrit-review.googlesource.com[Gerrit project itself] to make them
+more discoverable and have more chances to be reviewed by the whole community.
+
+[[hosting_lifecycle]]
+== Hosting Lifecycle
+
+The process of writing a new plugin goes through different phases:
+
+- Ideation and Discussion:
++
+The idea of creating a new plugin is posted and discussed on the
+link:https://groups.google.com/d/forum/repo-discuss[repo-discuss] mailing list.
++
+Also see section link#ideation_discussion[Ideation and discussion] below.
+
+- Prototyping (optional):
++
+The author of the plugin creates a working prototype on a public repository
+accessible to the community.
++
+Also see section link#plugin_prototyping[Plugin Prototyping] below.
+
+- Proposal and Hosting:
++
+The author proposes to release the plugin under the
+link:https://www.apache.org/licenses/LICENSE-2.0.html[Apache 2.0 OpenSource
+license] and requests the plugin to be hosted on
+link:https://gerrit-review.googlesource.com[the Gerrit project site]. The
+proposal must be accepted by at least one Gerrit maintainer. In case of
+disagreement between maintainers, the issue can be escalated to the
+link:dev-processes.html#steering-committee[Engineering Steering Committee]. If
+the plugin is accepted, the Gerrit maintainer creates the project under the
+plugins path on link:https://gerrit-review.googlesource.com[the Gerrit project
+site].
++
+Also see section link#plugin_proposal[Plugin Proposal] below.
+
+- Build:
++
+To make the consumption of the plugin easy and to notice plugin breakages early
+the plugin author should setup build jobs on
+link:https://gerrit-ci.gerritforge.com[the GerritForge CI] that build the
+plugin for each Gerrit version that it supports.
++
+Also see section link#build[Build] below.
+
+- Development and Contribution:
++
+The author develops a production-ready code base of the plugin, with
+contributions, reviews, and help from the Gerrit community.
++
+Also see section link#development_contribution[Development and contribution]
+below.
+
+- Release:
++
+The author releases the plugin by creating a Git tag and announcing the plugin
+on the link:https://groups.google.com/d/forum/repo-discuss[repo-discuss]
+mailing list.
++
+Also see section link#plugin_release[Plugin release] below.
+
+- Maintenance:
++
+The author maintains their plugins as new Gerrit versions are released, updates
+them when necessary, develops further existing or new features and reviews
+incoming contributions.
+
+- Deprecation:
++
+The author declares that the plugin is not maintained anymore or is deprecated
+and should not be used anymore.
++
+Also see section link#plugin_deprecation[Plugin deprecation] below.
+
+[[ideation_discussion]]
+== Ideation and Discussion
+
+Starting a new plugin project is a community effort: it starts with the
+identification of a gap in the Gerrit Code Review product but evolves with the
+contribution of ideas and suggestions by the whole community.
+
+The ideator of the plugin starts with an RFC (Request For Comments) post on the
+link:https://groups.google.com/d/forum/repo-discuss[repo-discuss] mailing list
+with a description of the main reasons for starting a new plugin.
+
+Example of a post:
+
+----
+ [RFC] Code-Formatter plugin
+
+ Hello, community,
+ I am proposing to create a new plugin for Gerrit called 'Code-Formatter', see
+ the details below.
+
+ *The gap*
+ Often, when I post a new change to Gerrit, I forget to run the common code
+ formatting tool (e.g. Google-Java-Format for the Gerrit project). I would
+ like Gerrit to be in charge of highlighting these issues to me and save many
+ people's time.
+
+ *The proposal*
+ The Code-Formatter plugin reads the formatting rules in the project config
+ and applies them automatically to every patch-set. Any issue is reported as a
+ regular review comment to the patchset, highlighting the part of the code to
+ be changed.
+
+ What do you think? Did anyone have the same idea or need?
+----
+
+The idea is discussed on the mailing list and can evolve based on the needs and
+inputs from the entire community.
+
+After the discussion, the ideator of the plugin can decide to start prototyping
+on it or park the proposal, if the feedback provided an alternative solution to
+the problem. The prototype phase can be optionally skipped if the idea is clear
+enough and receives a general agreement from the Gerrit maintainers. The author
+can be given a "leap of faith" and can go directly to the format plugin
+proposal (see below) and the creation of the plugin repository.
+
+[[plugin_prototyping]]
+== Plugin Prototyping
+
+The initial idea is translated to code by the plugin author. The development
+can happen on any public or private source code repository and can involve one
+or more contributors. The purpose of prototyping is to verify that the idea can
+be implemented and provides the expected benefits.
+
+Once a working prototype is ready, it can be announced as a follow-up to the
+initial RFC proposal so that other members of the community can see the code
+and try the plugin themselves.
+
+[[plugin_proposal]]
+== Plugin Proposal
+
+The author decides that the plugin prototype makes sense as a general purpose
+plugin and decides to release the code with the same
+link:https://www.apache.org/licenses/LICENSE-2.0.html[Apache 2.0 license]
+as the Gerrit Code Review project and have it hosted on
+link:https://gerrit-review.googlesource.com[the Gerrit project site].
+
+The plugin author formalizes the proposal with a follow-up of the initial RFC
+post and asks for public opinion on it.
+
+Example:
+
+----
+ Re - [RFC] Code-Formatter plugin
+
+ Hello, community,
+ thanks for your feedback on the prototype. I have now decided to donate the
+ project to the Gerrit Code Review project and make it a plugin:
+
+ Plugin name:
+ /plugins/code-formatter
+
+ Plugin description:
+ Plugin to allow automatic posting review based on code-formatting rules
+----
+
+The community discusses the proposal and the value of the plugin for the whole
+project; the result of the discussion can end up in one of the following cases:
+
+- The plugin's project request is widely appreciated and formally accepted by
+ at least one Gerrit maintainer who creates the repository as child project of
+ 'Public-Projects' on link:https://gerrit-review.googlesource.com[the Gerrit
+ project site], creates an associated plugin owners group with "Owner"
+ permissions for the plugin and adds the plugin's author as member of it.
+- The plugin's project is widely appreciated; however, another existing plugin
+ already partially covers the same use-case and thus it would make more sense
+ to have the features integrated into the existing plugin. The new plugin's
+ author contributes his prototype commits refactored to be included as change
+ into the existing plugin.
+- The plugin's project is found useful; however, it is too specific to the
+ author's use-case and would not make sense outside of it. The plugin remains
+ in a public repository, widely accessible and OpenSource, but not hosted on
+ link:https://gerrit-review.googlesource.com[the Gerrit project site].
+
+[[build]]
+== Build
+
+The plugin's maintainer creates a job on the
+link:https://gerrit-ci.gerritforge.com[GerritForge CI] by creating a new YAML
+definition in the link:https://gerrit.googlesource.com/gerrit-ci-scripts[Gerrit
+CI Scripts] repository.
+
+Example of a YAML CI job for plugins:
+
+----
+ - project:
+ name: code-formatter
+ jobs:
+ - 'plugin-{name}-bazel-{branch}':
+ branch:
+ - master
+----
+
+[[development_contribution]]
+== Development and Contribution
+
+The plugin follows the same lifecycle as Gerrit Code Review and needs to be
+kept up-to-date with the current active branches, according to the
+link:https://www.gerritcodereview.com/#support[current support policy].
+During the development, the plugin's maintainer can reward contributors
+requesting to be more involved and making them maintainers of his plugin,
+adding them to the list of the project owners.
+
+[[plugin_release]]
+== Plugin Release
+
+The plugin's maintainer is the only person responsible for making and
+announcing the official releases, typically, but not limited to, in conjunction
+with the major releases of Gerrit Code Review. The plugin's maintainer may tag
+his plugin and follow the notation and semantics of the Gerrit Code Review
+project; however it is not mandatory and many of the plugins do not have any
+tags or releases.
+
+Example of a YAML CI job for a plugin compatible with multiple Gerrit versions:
+
+----
+ - project:
+ name: code-formatter
+ jobs:
+ - 'plugin-{name}-bazel-{branch}-{gerrit-branch}':
+ branch:
+ - master
+ gerrit-branch:
+ - master
+ - stable-3.0
+ - stable-2.16
+----
+
+[[plugin_deprecation]]
+== Plugin Deprecation
+
+The plugin's maintainer and the community have agreed that the plugin is not
+useful anymore or there isn't anyone willing to contribute to bringing it
+forward and keeping it up-to-date with the recent versions of Gerrit Code
+Review.
+
+The plugin's maintainer puts a deprecation notice in the README.md of the
+plugin and pushes it for review. If nobody is willing to bring the code
+forward, the change gets merged, and the master branch is removed from the list
+of branches to be built on the GerritFoge CI.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 7975575..0fbfa24 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2861,202 +2861,6 @@
`com.google.gerrit.server.RequestListener` is an extension point that is
invoked each time the server executes a request from a user.
-[[plugins_hosting]]
-== Plugins source code hosting
-
-Most of the plugins are hosted on the same instance as the
-link:https://gerrit-review.googlesource.com[Gerrit project itself] to make them
-more discoverable and have more chances to be reviewed by the whole community.
-
-[[hosting_lifecycle]]
-=== Hosting lifecycle
-
-The process of writing a new plugin goes through different phases:
-
-- Ideation and discussion
- The idea of creating a new plugin is posted and discussed on the
- link:https://groups.google.com/d/forum/repo-discuss[repo-discuss] mailing list.
-- Prototyping (optional)
- The author of the plugin creates a working prototype on a public repository
- accessible to the community.
-- Proposal and Hosting
- The author proposes to release the plugin under the
- link:https://www.apache.org/licenses/LICENSE-2.0.html[Apache 2.0 OpenSource license]
- and request to be hosted on
- link:https://gerrit-review.googlesource.com[the Gerrit project site]. The proposal must be
- accepted by at least one Gerrit maintainer. In case of disagreement between maintainers, the
- issue can be escalated to the Engineering Steering Committee. If the plugin is accepted,
- the Gerrit maintainer creates the project under the plugins path on
- link:https://gerrit-review.googlesource.com[the Gerrit project site].
-- Development and contribution
- The author develops a production-ready code base of the plugin, with contributions, reviews,
- and help from the Gerrit community.
-- Release
- The author releases the plugin by tagging and announcing on the
- link:https://groups.google.com/d/forum/repo-discuss[repo-discuss] mailing list.
-- Maintenance
- The author maintains their plugins as new Gerrit versions are released, updates them when necessary,
- develops further existing or new features and reviews incoming new contributions.
- A plugin should declare and build on
- link:https://gerrit-ci.gerritforge.com[the GerritForge CI] for the Gerrit versions it supports.
-- Deprecation
- The author declares that the plugin is not maintained anymore or is deprecated and should
- not be used anymore.
-
-[[ideation_discussion]]
-=== Ideation and discussion
-
-Starting a new plugin project is a community effort: it starts with the identification of a gap
-in the Gerrit Code Review product but evolves with the contribution of ideas and suggestions
-by the whole community.
-
-The ideator of the plugin starts with an RFC (Request For Comments) post on the
-link:https://groups.google.com/d/forum/repo-discuss[repo-discuss] mailing list with a description
-of the main reasons for starting a new plugin.
-
-Example of a post:
-
- [RFC] Code-Formatter plugin
-
- Hello, community,
- I am proposing to create a new plugin for Gerrit called 'Code-Formatter', see the
- details below.
-
- *The gap*
- Often, when I post a new change to Gerrit, I forget to run the common code formatting
- tool (e.g. Google-Java-Format for the Gerrit project). I would like Gerrit to be in charge
- of highlighting these issues to me and save many people's time.
-
- *The proposal*
- The Code-Formatter plugin reads the formatting rules in the project config and applies
- them automatically to every patch-set. Any issue is reported as a regular review comment
- to the patchset, highlighting the part of the code to be changed.
-
- What do you think? Did anyone have the same idea or need?
-
-The idea is discussed on the mailing list and can evolve based on the needs and inputs from
-the entire community.
-
-After the discussion, the ideator of the plugin can decide to start prototyping on it or park the
-proposal, if the feedback provided an alternative solution to the problem.
-The prototype phase can be optionally skipped if the idea is clear enough and receives a general
-agreement from the Gerrit maintainers. The author can be given a "leap of faith" and can go
-directly to the format plugin proposal (see below) and the creation of the plugin repository.
-
-[[plugin_prototyping]]
-=== Plugin Prototyping
-
-The initial idea is translated to code by the plugin author. The development can happen on any
-public or private source code repository and can involve one or more contributors.
-The purpose of prototyping is to verify that the idea can be implemented and provides the expected
-benefits.
-
-Once a working prototype is ready, it can be announced as a follow-up to the initial RFC proposal
-so that other members of the community can see the code and try the plugin themselves.
-
-[[plugin_proposal]]
-==== Plugin Proposal
-
-The author decides that the plugin prototype makes sense as a general purpose plugin and decides
-to release the code with the same
-link:https://www.apache.org/licenses/LICENSE-2.0.html[Apache 2.0 license]
-as the Gerrit Code Review project and have it hosted on
-link:https://gerrit-review.googlesource.com[the Gerrit project site].
-
-The plugin author formalizes the proposal with a follow-up of the initial RFC post and asks
-for public opinion on it.
-
-Example:
-
-----
- Re - [RFC] Code-Formatter plugin
-
- Hello, community,
- thanks for your feedback on the prototype. I have now decided to donate the project to the
- Gerrit Code Review project and make it a plugin:
-
- Plugin name:
- /plugins/code-formatter
-
- Plugin description:
- Plugin to allow automatic posting review based on code-formatting rules
-----
-
-The community discusses the proposal and the value of the plugin for the whole project; the result
-of the discussion can end up in one of the following cases:
-
-- The plugin's project request is widely appreciated and formally accepted by at least one
- Gerrit maintainer who creates the repository as child project of 'Public-Projects' on
- link:https://gerrit-review.googlesource.com[the Gerrit project site], creates an associated
- plugin owners group with "Owner" permissions for the plugin and adds the plugin's
- author as member of it.
-- The plugin's project is widely appreciated; however, another existing plugin already
- partially covers the same use-case and thus it would make more sense to have the features
- integrated into the existing plugin. The new plugin's author contributes his prototype commits
- refactored to be included as change into the existing plugin.
-- The plugin's project is found useful; however, it is too specific to the author's use-case
- and would not make sense outside of it. The plugin remains in a public repository, widely
- accessible and OpenSource, but not hosted on link:https://gerrit-review.googlesource.com[the Gerrit project site].
-
-[[development_contribution]]
-== Development and contribution
-
-The plugin's maintainer creates a job on the link:https://gerrit-ci.gerritforge.com[GerritForge CI]
-by creating a new YAML definition in the
-link:https://gerrit.googlesource.com/gerrit-ci-scripts[Gerrit CI Scripts] repository.
-
-Example of a YAML CI job for plugins:
-
-----
- - project:
- name: code-formatter
- jobs:
- - 'plugin-{name}-bazel-{branch}':
- branch:
- - master
-----
-
-The plugin follows the same lifecycle as Gerrit Code Review and needs to be kept up-to-date with
-the current active branches, according to the
-link:https://www.gerritcodereview.com/#support[current support policy].
-During the development, the plugin's maintainer can reward contributors requesting to be more
-involved and making them maintainers of his plugin, adding them to the list of the project owners.
-
-[[plugin_release]]
-== Plugin release
-
-The plugin's maintainer is the only person responsible for making and announcing the official
-releases, typically, but not limited to, in conjunction with the major releases of Gerrit Code Review.
-The plugin's maintainer may tag his plugin and follow the notation and semantics of the
-Gerrit Code Review project; however it is not mandatory and many of the plugins do not have any
-tags or releases.
-
-Example of a YAML CI job for a plugin compatible with multiple Gerrit versions:
-
-----
- - project:
- name: code-formatter
- jobs:
- - 'plugin-{name}-bazel-{branch}-{gerrit-branch}':
- branch:
- - master
- gerrit-branch:
- - master
- - stable-3.0
- - stable-2.16
-----
-
-[[plugin_deprecation]]
-=== Plugin deprecation
-
-The plugin's maintainer and the community have agreed that the plugin is not useful anymore or there
-isn't anyone willing to contribute to bringing it forward and keeping it up-to-date with the recent
-versions of Gerrit Code Review.
-
-The plugin's maintainer puts a deprecation notice in the README.md of the plugin and pushes it for
-review. If nobody is willing to bring the code forward, the change gets merged, and the master branch is
-removed from the list of branches to be built on the GerritFoge CI.
-
== SEE ALSO
* link:js-api.html[JavaScript API]
diff --git a/Documentation/index.txt b/Documentation/index.txt
index d02570c..77e0ed4 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -10,6 +10,7 @@
. link:intro-how-gerrit-works.html[How Gerrit Works]
. link:intro-gerrit-walkthrough.html[Basic Gerrit Walkthrough]
. link:dev-community.html[Gerrit Community]
+.. link:dev-contributing.html[Contributor Guide]
== Guides
. link:intro-user.html[User Guide]
diff --git a/WORKSPACE b/WORKSPACE
index 530a4a7..2801f5f 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -784,28 +784,36 @@
sha1 = "89bb3aa5b98b48e584eee2a7401b7682a46779b4",
)
+SSHD_VERS = "2.3.0"
+
maven_jar(
name = "sshd",
- artifact = "org.apache.sshd:sshd-core:2.0.0",
- sha1 = "f4275079a2463cfd2bf1548a80e1683288a8e86b",
+ artifact = "org.apache.sshd:sshd-core:" + SSHD_VERS,
+ sha1 = "21aeea9deba96c9b81ea0935fa4fac61aa3cf646",
)
maven_jar(
- name = "eddsa",
- artifact = "net.i2p.crypto:eddsa:0.2.0",
- sha1 = "0856a92559c4daf744cb27c93cd8b7eb1f8c4780",
-)
-
-maven_jar(
- name = "mina-core",
- artifact = "org.apache.mina:mina-core:2.0.17",
- sha1 = "7e10ec974760436d931f3e58be507d1957bcc8db",
+ name = "sshd-common",
+ artifact = "org.apache.sshd:sshd-common:" + SSHD_VERS,
+ sha1 = "8b6e3baaa0d35b547696965eef3e62477f5e74c9",
)
maven_jar(
name = "sshd-mina",
- artifact = "org.apache.sshd:sshd-mina:2.0.0",
- sha1 = "50f2669312494f6c1996d8bd0d266c1fca7be6f6",
+ artifact = "org.apache.sshd:sshd-mina:" + SSHD_VERS,
+ sha1 = "55dc0830dfcbceba01f9460812ee454978a15fe8",
+)
+
+maven_jar(
+ name = "eddsa",
+ artifact = "net.i2p.crypto:eddsa:0.3.0",
+ sha1 = "1901c8d4d8bffb7d79027686cfb91e704217c3e1",
+)
+
+maven_jar(
+ name = "mina-core",
+ artifact = "org.apache.mina:mina-core:2.0.21",
+ sha1 = "e1a317689ecd438f54e863747e832f741ef8e092",
)
maven_jar(
@@ -903,12 +911,6 @@
)
maven_jar(
- name = "easymock",
- artifact = "org.easymock:easymock:3.1",
- sha1 = "3e127311a86fc2e8f550ef8ee4abe094bbcf7e7e",
-)
-
-maven_jar(
name = "cglib-3_2",
artifact = "cglib:cglib-nodep:3.2.6",
sha1 = "92bf48723d277d6efd1150b2f7e9e1e92cb56caf",
@@ -1015,18 +1017,18 @@
sha1 = "0f5a654e4675769c716e5b387830d19b501ca191",
)
-TESTCONTAINERS_VERSION = "1.12.1"
+TESTCONTAINERS_VERSION = "1.12.2"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "1dc8666ead914c5515d087f75ffe92629414caf6",
+ sha1 = "660d2fab2021154b98ce91d3104ff673a7ab9348",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "2491f792627a1f15d341bfcd6dd0ea7e3541d82f",
+ sha1 = "88c751b2d787dfc19a91a7ee6fb623b881ffba5a",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index d0ed673..2b6cb00 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.RequestCleanup;
import com.google.gerrit.server.config.GerritRequestModule;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
+import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
@@ -247,12 +248,15 @@
if (projectState == null) {
throw new RuntimeException("can't load project state for " + req.project.get());
}
- UploadPack up = new UploadPack(repo);
+ Repository permissionAwareRepository = PermissionAwareRepositoryManager.wrap(repo, perm);
+ UploadPack up = new UploadPack(permissionAwareRepository);
up.setPackConfig(transferConfig.getPackConfig());
up.setTimeout(transferConfig.getTimeout());
up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
- hooks.add(uploadValidatorsFactory.create(projectState.getProject(), repo, "localhost-test"));
+ hooks.add(
+ uploadValidatorsFactory.create(
+ projectState.getProject(), permissionAwareRepository, "localhost-test"));
up.setPreUploadHook(PreUploadHookChain.newChain(hooks));
uploadPackInitializers.runEach(initializer -> initializer.init(req.project, up));
return up;
diff --git a/java/com/google/gerrit/acceptance/SshdModule.java b/java/com/google/gerrit/acceptance/SshdModule.java
index 185d6e2..873ba177 100644
--- a/java/com/google/gerrit/acceptance/SshdModule.java
+++ b/java/com/google/gerrit/acceptance/SshdModule.java
@@ -34,7 +34,7 @@
if (keys == null) {
keys = new SimpleGeneratorHostKeyProvider();
keys.setAlgorithm("RSA");
- keys.loadKeys();
+ keys.loadKeys(null);
}
return keys;
}
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index d66e8ac..755bf7a 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
@@ -310,27 +311,33 @@
private final DynamicSet<PreUploadHook> preUploadHooks;
private final DynamicSet<PostUploadHook> postUploadHooks;
private final PluginSetContext<UploadPackInitializer> uploadPackInitializers;
+ private final PermissionBackend permissionBackend;
@Inject
UploadFactory(
TransferConfig tc,
DynamicSet<PreUploadHook> preUploadHooks,
DynamicSet<PostUploadHook> postUploadHooks,
- PluginSetContext<UploadPackInitializer> uploadPackInitializers) {
+ PluginSetContext<UploadPackInitializer> uploadPackInitializers,
+ PermissionBackend permissionBackend) {
this.config = tc;
this.preUploadHooks = preUploadHooks;
this.postUploadHooks = postUploadHooks;
this.uploadPackInitializers = uploadPackInitializers;
+ this.permissionBackend = permissionBackend;
}
@Override
public UploadPack create(HttpServletRequest req, Repository repo) {
- UploadPack up = new UploadPack(repo);
+ ProjectState state = (ProjectState) req.getAttribute(ATT_STATE);
+ UploadPack up =
+ new UploadPack(
+ PermissionAwareRepositoryManager.wrap(
+ repo, permissionBackend.currentUser().project(state.getNameKey())));
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
up.setPreUploadHook(PreUploadHookChain.newChain(Lists.newArrayList(preUploadHooks)));
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));
- ProjectState state = (ProjectState) req.getAttribute(ATT_STATE);
uploadPackInitializers.runEach(initializer -> initializer.init(state.getNameKey(), up));
return up;
}
diff --git a/java/com/google/gerrit/index/IndexConfig.java b/java/com/google/gerrit/index/IndexConfig.java
index d8a1a89..807c78c 100644
--- a/java/com/google/gerrit/index/IndexConfig.java
+++ b/java/com/google/gerrit/index/IndexConfig.java
@@ -91,7 +91,6 @@
checkLimit(cfg.maxLimit(), "maxLimit");
checkLimit(cfg.maxPages(), "maxPages");
checkLimit(cfg.maxTerms(), "maxTerms");
- checkTypeLimit(cfg);
return cfg;
}
}
@@ -100,12 +99,6 @@
checkArgument(limit > 0, "%s must be positive: %s", name, limit);
}
- private static void checkTypeLimit(IndexConfig cfg) {
- String limit = cfg.type();
- boolean known = IndexType.getKnownTypes().asList().contains(limit);
- checkArgument(known, "type must be known: %s", limit);
- }
-
/**
* @return maximum limit supported by the underlying index, or limited for performance reasons.
*/
@@ -123,7 +116,7 @@
*/
public abstract int maxTerms();
- /** @return index type, limited to be either one of the known types. */
+ /** @return index type. */
public abstract String type();
/**
diff --git a/java/com/google/gerrit/pgm/init/InitHttpd.java b/java/com/google/gerrit/pgm/init/InitHttpd.java
index 6b4c7ca..d08bca0 100644
--- a/java/com/google/gerrit/pgm/init/InitHttpd.java
+++ b/java/com/google/gerrit/pgm/init/InitHttpd.java
@@ -112,11 +112,10 @@
urlbuf.append(port);
}
urlbuf.append(context);
- httpd.set("listenUrl", urlbuf.toString());
URI uri;
try {
- uri = toURI(httpd.get("listenUrl"));
+ uri = toURI(urlbuf.toString());
if (uri.getScheme().startsWith("proxy-")) {
// If its a proxy URL, assume the reverse proxy is on our system
// at the protocol standard ports (so omit the ports from the URL).
@@ -127,6 +126,7 @@
} catch (URISyntaxException e) {
throw die("invalid httpd.listenUrl");
}
+ httpd.set("listenUrl", urlbuf.toString());
gerrit.string("Canonical URL", "canonicalWebUrl", uri.toString());
generateSslCertificate();
}
diff --git a/java/com/google/gerrit/server/ApprovalCopier.java b/java/com/google/gerrit/server/ApprovalCopier.java
deleted file mode 100644
index 85a6079..0000000
--- a/java/com/google/gerrit/server/ApprovalCopier.java
+++ /dev/null
@@ -1,167 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static java.util.Objects.requireNonNull;
-
-import com.google.common.collect.HashBasedTable;
-import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Table;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.LabelType;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.client.ChangeKind;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.change.ChangeKindCache;
-import com.google.gerrit.server.change.LabelNormalizer;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.io.IOException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.TreeMap;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.revwalk.RevWalk;
-
-/**
- * Copies approvals between patch sets.
- *
- * <p>The result of a copy may either be stored, as when stamping approvals in the database at
- * submit time, or refreshed on demand, as when reading approvals from the NoteDb.
- */
-@Singleton
-public class ApprovalCopier {
- private final ProjectCache projectCache;
- private final ChangeKindCache changeKindCache;
- private final LabelNormalizer labelNormalizer;
- private final ChangeData.Factory changeDataFactory;
- private final PatchSetUtil psUtil;
-
- @Inject
- ApprovalCopier(
- ProjectCache projectCache,
- ChangeKindCache changeKindCache,
- LabelNormalizer labelNormalizer,
- ChangeData.Factory changeDataFactory,
- PatchSetUtil psUtil) {
- this.projectCache = projectCache;
- this.changeKindCache = changeKindCache;
- this.labelNormalizer = labelNormalizer;
- this.changeDataFactory = changeDataFactory;
- this.psUtil = psUtil;
- }
-
- Iterable<PatchSetApproval> getForPatchSet(
- ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) {
-
- PatchSet ps = psUtil.get(notes, psId);
- if (ps == null) {
- return Collections.emptyList();
- }
-
- ChangeData cd = changeDataFactory.create(notes);
- try {
- ProjectState project = projectCache.checkedGet(cd.change().getDest().project());
- ListMultimap<PatchSet.Id, PatchSetApproval> all = cd.approvals();
- requireNonNull(all, "all should not be null");
-
- Table<String, Account.Id, PatchSetApproval> byUser = HashBasedTable.create();
- for (PatchSetApproval psa : all.get(ps.id())) {
- byUser.put(psa.label(), psa.accountId(), psa);
- }
-
- TreeMap<Integer, PatchSet> patchSets = getPatchSets(cd);
-
- Table<String, Account.Id, PatchSetApproval> wontCopy = HashBasedTable.create();
-
- // Walk patch sets strictly less than current in descending order.
- Collection<PatchSet> allPrior =
- patchSets.descendingMap().tailMap(ps.id().get(), false).values();
- for (PatchSet priorPs : allPrior) {
- List<PatchSetApproval> priorApprovals = all.get(priorPs.id());
- if (priorApprovals.isEmpty()) {
- continue;
- }
-
- ChangeKind kind =
- changeKindCache.getChangeKind(
- project.getNameKey(), rw, repoConfig, priorPs.commitId(), ps.commitId());
-
- for (PatchSetApproval psa : priorApprovals) {
- if (wontCopy.contains(psa.label(), psa.accountId())) {
- continue;
- }
- if (byUser.contains(psa.label(), psa.accountId())) {
- continue;
- }
- if (!canCopy(project, psa, ps.id(), kind)) {
- wontCopy.put(psa.label(), psa.accountId(), psa);
- continue;
- }
- byUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
- }
- }
- return labelNormalizer.normalize(notes, byUser.values()).getNormalized();
- } catch (IOException e) {
- throw new StorageException(e);
- }
- }
-
- private static TreeMap<Integer, PatchSet> getPatchSets(ChangeData cd) {
- Collection<PatchSet> patchSets = cd.patchSets();
- TreeMap<Integer, PatchSet> result = new TreeMap<>();
- for (PatchSet ps : patchSets) {
- result.put(ps.id().get(), ps);
- }
- return result;
- }
-
- private static boolean canCopy(
- ProjectState project, PatchSetApproval psa, PatchSet.Id psId, ChangeKind kind) {
- int n = psa.key().patchSetId().get();
- checkArgument(n != psId.get());
- LabelType type = project.getLabelTypes().byLabel(psa.labelId());
- if (type == null) {
- return false;
- } else if ((type.isCopyMinScore() && type.isMaxNegative(psa))
- || (type.isCopyMaxScore() && type.isMaxPositive(psa))) {
- return true;
- }
- switch (kind) {
- case MERGE_FIRST_PARENT_UPDATE:
- return type.isCopyAllScoresOnMergeFirstParentUpdate();
- case NO_CODE_CHANGE:
- return type.isCopyAllScoresIfNoCodeChange();
- case TRIVIAL_REBASE:
- return type.isCopyAllScoresOnTrivialRebase();
- case NO_CHANGE:
- return type.isCopyAllScoresIfNoChange()
- || type.isCopyAllScoresOnTrivialRebase()
- || type.isCopyAllScoresOnMergeFirstParentUpdate()
- || type.isCopyAllScoresIfNoCodeChange();
- case REWORK:
- default:
- return false;
- }
- }
-}
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/ApprovalInference.java
new file mode 100644
index 0000000..4cdb7d9
--- /dev/null
+++ b/java/com/google/gerrit/server/ApprovalInference.java
@@ -0,0 +1,181 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Table;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.server.change.ChangeKindCache;
+import com.google.gerrit.server.change.LabelNormalizer;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/**
+ * Computes approvals for a given patch set by looking at approvals applied to the given patch set
+ * and by additionally inferring approvals from the patch set's parents. The latter is done by
+ * asserting a change's kind and checking the project config for allowed forward-inference.
+ *
+ * <p>The result of a copy may either be stored, as when stamping approvals in the database at
+ * submit time, or refreshed on demand, as when reading approvals from the NoteDb.
+ */
+@Singleton
+public class ApprovalInference {
+ private final ProjectCache projectCache;
+ private final ChangeKindCache changeKindCache;
+ private final LabelNormalizer labelNormalizer;
+ private final ChangeData.Factory changeDataFactory;
+ private final PatchSetUtil psUtil;
+
+ @Inject
+ ApprovalInference(
+ ProjectCache projectCache,
+ ChangeKindCache changeKindCache,
+ LabelNormalizer labelNormalizer,
+ ChangeData.Factory changeDataFactory,
+ PatchSetUtil psUtil) {
+ this.projectCache = projectCache;
+ this.changeKindCache = changeKindCache;
+ this.labelNormalizer = labelNormalizer;
+ this.changeDataFactory = changeDataFactory;
+ this.psUtil = psUtil;
+ }
+
+ /**
+ * Returns all approvals that apply to the given patch set. Honors direct and indirect (approval
+ * on parents) approvals.
+ */
+ Iterable<PatchSetApproval> forPatchSet(
+ ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) {
+ Collection<PatchSetApproval> approvals =
+ getForPatchSetWithoutNormalization(notes, psId, rw, repoConfig);
+ try {
+ return labelNormalizer.normalize(notes, approvals).getNormalized();
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ private static boolean canCopy(
+ ProjectState project, PatchSetApproval psa, PatchSet.Id psId, ChangeKind kind) {
+ int n = psa.key().patchSetId().get();
+ checkArgument(n != psId.get());
+ LabelType type = project.getLabelTypes().byLabel(psa.labelId());
+ if (type == null) {
+ return false;
+ } else if ((type.isCopyMinScore() && type.isMaxNegative(psa))
+ || (type.isCopyMaxScore() && type.isMaxPositive(psa))) {
+ return true;
+ }
+ switch (kind) {
+ case MERGE_FIRST_PARENT_UPDATE:
+ return type.isCopyAllScoresOnMergeFirstParentUpdate();
+ case NO_CODE_CHANGE:
+ return type.isCopyAllScoresIfNoCodeChange();
+ case TRIVIAL_REBASE:
+ return type.isCopyAllScoresOnTrivialRebase();
+ case NO_CHANGE:
+ return type.isCopyAllScoresIfNoChange()
+ || type.isCopyAllScoresOnTrivialRebase()
+ || type.isCopyAllScoresOnMergeFirstParentUpdate()
+ || type.isCopyAllScoresIfNoCodeChange();
+ case REWORK:
+ default:
+ return false;
+ }
+ }
+
+ private Collection<PatchSetApproval> getForPatchSetWithoutNormalization(
+ ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) {
+ PatchSet ps = psUtil.get(notes, psId);
+ if (ps == null) {
+ return Collections.emptyList();
+ }
+
+ ChangeData cd = changeDataFactory.create(notes);
+ ProjectState project;
+ try {
+ project = projectCache.checkedGet(cd.change().getDest().project());
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
+
+ // Start by collecting all current approvals
+ Table<String, Account.Id, PatchSetApproval> byUser = HashBasedTable.create();
+ ListMultimap<PatchSet.Id, PatchSetApproval> all = cd.approvals();
+ requireNonNull(all, "all should not be null");
+ all.get(ps.id()).forEach(psa -> byUser.put(psa.label(), psa.accountId(), psa));
+
+ // Bail out immediately if this is the first patch set
+ if (psId.get() == 1) {
+ return byUser.values();
+ }
+
+ // Call this algorithm recursively to check if the prior patch set had approvals. This has the
+ // advantage that all caches - most importantly ChangeKindCache - have values cached for what we
+ // need for this computation.
+ // The way this algorithm is written is that any approval will be copied forward by one patch
+ // set at a time if configs and change kind allow so. Once an approval is held back - for
+ // example because the patch set is a REWORK - it will not be picked up again in a future
+ // patch set.
+ PatchSet priorPatchSet = notes.load().getPatchSets().lowerEntry(psId).getValue();
+ if (priorPatchSet == null) {
+ return byUser.values();
+ }
+
+ Iterable<PatchSetApproval> priorApprovals =
+ getForPatchSetWithoutNormalization(notes, priorPatchSet.id(), rw, repoConfig);
+ if (!priorApprovals.iterator().hasNext()) {
+ return byUser.values();
+ }
+
+ Table<String, Account.Id, PatchSetApproval> wontCopy = HashBasedTable.create();
+ ChangeKind kind =
+ changeKindCache.getChangeKind(
+ project.getNameKey(), rw, repoConfig, priorPatchSet.commitId(), ps.commitId());
+ for (PatchSetApproval psa : priorApprovals) {
+ if (wontCopy.contains(psa.label(), psa.accountId())) {
+ continue;
+ }
+ if (byUser.contains(psa.label(), psa.accountId())) {
+ continue;
+ }
+ if (!canCopy(project, psa, ps.id(), kind)) {
+ wontCopy.put(psa.label(), psa.accountId(), psa);
+ continue;
+ }
+ byUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
+ }
+ return byUser.values();
+ }
+}
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java
index c4f13f1..1374e74 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -91,14 +91,14 @@
return Iterables.filter(psas, a -> Objects.equals(a.accountId(), accountId));
}
- private final ApprovalCopier copier;
+ private final ApprovalInference copier;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@VisibleForTesting
@Inject
public ApprovalsUtil(
- ApprovalCopier copier, PermissionBackend permissionBackend, ProjectCache projectCache) {
+ ApprovalInference copier, PermissionBackend permissionBackend, ProjectCache projectCache) {
this.copier = copier;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
@@ -340,7 +340,7 @@
public Iterable<PatchSetApproval> byPatchSet(
ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) {
- return copier.getForPatchSet(notes, psId, rw, repoConfig);
+ return copier.forPatchSet(notes, psId, rw, repoConfig);
}
public Iterable<PatchSetApproval> byPatchSetUser(
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index c5d291e..b2ade77 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -227,7 +227,7 @@
if (newEmail != null && !newEmail.equals(oldEmail)) {
ExternalId extIdWithNewEmail =
ExternalId.create(extId.key(), extId.accountId(), newEmail, extId.password());
- checkEmailNotUsed(extIdWithNewEmail);
+ checkEmailNotUsed(extId.accountId(), extIdWithNewEmail);
accountUpdates.add(u -> u.replaceExternalId(extId, extIdWithNewEmail));
if (oldEmail != null && oldEmail.equals(user.getAccount().preferredEmail())) {
@@ -279,7 +279,7 @@
ExternalId extId =
ExternalId.createWithEmail(who.getExternalIdKey(), newId, who.getEmailAddress());
logger.atFine().log("Created external Id: %s", extId);
- checkEmailNotUsed(extId);
+ checkEmailNotUsed(newId, extId);
ExternalId userNameExtId =
who.getUserName().isPresent() ? createUsername(newId, who.getUserName().get()) : null;
@@ -354,7 +354,8 @@
return ExternalId.create(SCHEME_USERNAME, username, accountId);
}
- private void checkEmailNotUsed(ExternalId extIdToBeCreated) throws IOException, AccountException {
+ private void checkEmailNotUsed(Account.Id accountId, ExternalId extIdToBeCreated)
+ throws IOException, AccountException {
String email = extIdToBeCreated.email();
if (email == null) {
return;
@@ -365,14 +366,18 @@
return;
}
- logger.atWarning().log(
- "Email %s is already assigned to account %s;"
- + " cannot create external ID %s with the same email for account %s.",
- email,
- existingExtIdsWithEmail.iterator().next().accountId().get(),
- extIdToBeCreated.key().get(),
- extIdToBeCreated.accountId().get());
- throw new AccountException("Email '" + email + "' in use by another account");
+ for (ExternalId externalId : existingExtIdsWithEmail) {
+ if (externalId.accountId().get() != accountId.get()) {
+ logger.atWarning().log(
+ "Email %s is already assigned to account %s;"
+ + " cannot create external ID %s with the same email for account %s.",
+ email,
+ externalId.accountId().get(),
+ extIdToBeCreated.key().get(),
+ extIdToBeCreated.accountId().get());
+ throw new AccountException("Email '" + email + "' in use by another account");
+ }
+ }
}
private void addGroupMember(AccountGroup.UUID groupUuid, IdentifiedUser user)
@@ -413,7 +418,7 @@
} else {
ExternalId newExtId =
ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress());
- checkEmailNotUsed(newExtId);
+ checkEmailNotUsed(to, newExtId);
accountsUpdateProvider
.get()
.update(
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java b/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
index 8d6e8a8..815f7d0 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
@@ -72,8 +72,7 @@
private List<ConsistencyProblemInfo> check(ExternalIdNotes extIdNotes) throws IOException {
List<ConsistencyProblemInfo> problems = new ArrayList<>();
- ListMultimap<String, ExternalId.Key> emails =
- MultimapBuilder.hashKeys().arrayListValues().build();
+ ListMultimap<String, ExternalId> emails = MultimapBuilder.hashKeys().arrayListValues().build();
try (RevWalk rw = new RevWalk(extIdNotes.getRepository())) {
NoteMap noteMap = extIdNotes.getNoteMap();
@@ -84,7 +83,11 @@
problems.addAll(validateExternalId(extId));
if (extId.email() != null) {
- emails.put(extId.email(), extId.key());
+ String email = extId.email();
+ if (emails.get(email).stream()
+ .noneMatch(e -> e.accountId().get() == extId.accountId().get())) {
+ emails.put(email, extId);
+ }
}
} catch (ConfigInvalidException e) {
addError(String.format(e.getMessage()), problems);
@@ -101,7 +104,7 @@
"Email '%s' is not unique, it's used by the following external IDs: %s",
e.getKey(),
e.getValue().stream()
- .map(k -> "'" + k.get() + "'")
+ .map(k -> "'" + k.key().get() + "'")
.sorted()
.collect(joining(", "))),
problems));
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index a3c2e92..d50e740 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -418,6 +418,7 @@
for (ChangeData cd : changes) {
ChangeInfo i = cache.get(cd.getId());
if (i != null) {
+ changeInfos.add(i);
continue;
}
try {
diff --git a/java/com/google/gerrit/server/git/DelegateRefDatabase.java b/java/com/google/gerrit/server/git/DelegateRefDatabase.java
new file mode 100644
index 0000000..34dd6a9
--- /dev/null
+++ b/java/com/google/gerrit/server/git/DelegateRefDatabase.java
@@ -0,0 +1,86 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.RefRename;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Wrapper around {@link RefDatabase} that delegates all calls to the wrapped {@link RefDatabase}.
+ */
+public class DelegateRefDatabase extends RefDatabase {
+
+ private Repository delegate;
+
+ DelegateRefDatabase(Repository delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void create() throws IOException {
+ delegate.getRefDatabase().create();
+ }
+
+ @Override
+ public void close() {
+ delegate.close();
+ }
+
+ @Override
+ public boolean isNameConflicting(String name) throws IOException {
+ return delegate.getRefDatabase().isNameConflicting(name);
+ }
+
+ @Override
+ public RefUpdate newUpdate(String name, boolean detach) throws IOException {
+ return delegate.getRefDatabase().newUpdate(name, detach);
+ }
+
+ @Override
+ public RefRename newRename(String fromName, String toName) throws IOException {
+ return delegate.getRefDatabase().newRename(fromName, toName);
+ }
+
+ @Override
+ public Ref exactRef(String name) throws IOException {
+ return delegate.getRefDatabase().exactRef(name);
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ public Map<String, Ref> getRefs(String prefix) throws IOException {
+ return delegate.getRefDatabase().getRefs(prefix);
+ }
+
+ @Override
+ public List<Ref> getAdditionalRefs() throws IOException {
+ return delegate.getRefDatabase().getAdditionalRefs();
+ }
+
+ @Override
+ public Ref peel(Ref ref) throws IOException {
+ return delegate.getRefDatabase().peel(ref);
+ }
+
+ Repository getDelegate() {
+ return delegate;
+ }
+}
diff --git a/java/com/google/gerrit/server/git/DelegateRepository.java b/java/com/google/gerrit/server/git/DelegateRepository.java
new file mode 100644
index 0000000..800490d
--- /dev/null
+++ b/java/com/google/gerrit/server/git/DelegateRepository.java
@@ -0,0 +1,89 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import java.io.IOException;
+import org.eclipse.jgit.attributes.AttributesNodeProvider;
+import org.eclipse.jgit.lib.BaseRepositoryBuilder;
+import org.eclipse.jgit.lib.ObjectDatabase;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.ReflogReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+
+/** Wrapper around {@link Repository} that delegates all calls to the wrapped {@link Repository}. */
+class DelegateRepository extends Repository {
+
+ private final Repository delegate;
+
+ DelegateRepository(Repository delegate) {
+ super(toBuilder(delegate));
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void create(boolean bare) throws IOException {
+ delegate.create(bare);
+ }
+
+ @Override
+ public String getIdentifier() {
+ return delegate.getIdentifier();
+ }
+
+ @Override
+ public ObjectDatabase getObjectDatabase() {
+ return delegate.getObjectDatabase();
+ }
+
+ @Override
+ public RefDatabase getRefDatabase() {
+ return delegate.getRefDatabase();
+ }
+
+ @Override
+ public StoredConfig getConfig() {
+ return delegate.getConfig();
+ }
+
+ @Override
+ public AttributesNodeProvider createAttributesNodeProvider() {
+ return delegate.createAttributesNodeProvider();
+ }
+
+ @Override
+ public void scanForRepoChanges() throws IOException {
+ delegate.scanForRepoChanges();
+ }
+
+ @Override
+ public void notifyIndexChanged(boolean internal) {
+ delegate.notifyIndexChanged(internal);
+ }
+
+ @Override
+ public ReflogReader getReflogReader(String refName) throws IOException {
+ return delegate.getReflogReader(refName);
+ }
+
+ @SuppressWarnings("rawtypes")
+ private static BaseRepositoryBuilder toBuilder(Repository repo) {
+ if (!repo.isBare()) {
+ throw new IllegalArgumentException("non-bare repository is not supported");
+ }
+
+ return new BaseRepositoryBuilder<>().setFS(repo.getFS()).setGitDir(repo.getDirectory());
+ }
+}
diff --git a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
new file mode 100644
index 0000000..8f7e684
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
@@ -0,0 +1,159 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.annotations.Nullable;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefRename;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Wrapper around {@link DelegateRefDatabase} that filters all refs using {@link
+ * com.google.gerrit.server.permissions.PermissionBackend}.
+ */
+public class PermissionAwareReadOnlyRefDatabase extends DelegateRefDatabase {
+
+ private final PermissionBackend.ForProject forProject;
+
+ @Inject
+ PermissionAwareReadOnlyRefDatabase(
+ Repository delegateRepository, PermissionBackend.ForProject forProject) {
+ super(delegateRepository);
+ this.forProject = forProject;
+ }
+
+ @Override
+ public boolean isNameConflicting(String name) {
+ throw new UnsupportedOperationException("PermissionAwareReadOnlyRefDatabase is read-only");
+ }
+
+ @Override
+ public RefUpdate newUpdate(String name, boolean detach) {
+ throw new UnsupportedOperationException("PermissionAwareReadOnlyRefDatabase is read-only");
+ }
+
+ @Override
+ public RefRename newRename(String fromName, String toName) {
+ throw new UnsupportedOperationException("PermissionAwareReadOnlyRefDatabase is read-only");
+ }
+
+ @Override
+ public Ref exactRef(String name) throws IOException {
+ Ref ref = getDelegate().getRefDatabase().exactRef(name);
+ if (ref == null) {
+ return null;
+ }
+
+ Map<String, Ref> result;
+ try {
+ result =
+ forProject.filter(ImmutableMap.of(name, ref), getDelegate(), RefFilterOptions.defaults());
+ } catch (PermissionBackendException e) {
+ if (e.getCause() instanceof IOException) {
+ throw (IOException) e.getCause();
+ }
+ throw new IOException(e);
+ }
+ if (result.isEmpty()) {
+ return null;
+ }
+
+ Preconditions.checkState(
+ result.size() == 1, "Only one element expected, but was: " + result.size());
+ return Iterables.getOnlyElement(result.values());
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ public Map<String, Ref> getRefs(String prefix) throws IOException {
+ Map<String, Ref> refs = getDelegate().getRefDatabase().getRefs(prefix);
+ if (refs.isEmpty()) {
+ return refs;
+ }
+
+ Map<String, Ref> result;
+ try {
+ result = forProject.filter(refs, getDelegate(), RefFilterOptions.defaults());
+ } catch (PermissionBackendException e) {
+ throw new IOException("");
+ }
+ return result;
+ }
+
+ @Override
+ public List<Ref> getRefsByPrefix(String prefix) throws IOException {
+ Map<String, Ref> coarseRefs;
+ int lastSlash = prefix.lastIndexOf('/');
+ if (lastSlash == -1) {
+ coarseRefs = getRefs(ALL);
+ } else {
+ coarseRefs = getRefs(prefix.substring(0, lastSlash + 1));
+ }
+
+ List<Ref> result;
+ if (lastSlash + 1 == prefix.length()) {
+ result = coarseRefs.values().stream().collect(toList());
+ } else {
+ String p = prefix.substring(lastSlash + 1);
+ result =
+ coarseRefs.entrySet().stream()
+ .filter(e -> e.getKey().startsWith(p))
+ .map(e -> e.getValue())
+ .collect(toList());
+ }
+ return Collections.unmodifiableList(result);
+ }
+
+ @Override
+ @NonNull
+ public Map<String, Ref> exactRef(String... refs) throws IOException {
+ Map<String, Ref> result = new HashMap<>(refs.length);
+ for (String name : refs) {
+ Ref ref = exactRef(name);
+ if (ref != null) {
+ result.put(name, ref);
+ }
+ }
+ return result;
+ }
+
+ @Override
+ @Nullable
+ public Ref firstExactRef(String... refs) throws IOException {
+ for (String name : refs) {
+ Ref ref = exactRef(name);
+ if (ref != null) {
+ return ref;
+ }
+ }
+ return null;
+ }
+}
diff --git a/java/com/google/gerrit/server/git/PermissionAwareRepository.java b/java/com/google/gerrit/server/git/PermissionAwareRepository.java
new file mode 100644
index 0000000..bb80cb5
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PermissionAwareRepository.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import com.google.gerrit.server.permissions.PermissionBackend;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Wrapper around {@link DelegateRepository} that overwrites {@link #getRefDatabase()} to return a
+ * {@link PermissionAwareReadOnlyRefDatabase}.
+ */
+public class PermissionAwareRepository extends DelegateRepository {
+
+ private final PermissionAwareReadOnlyRefDatabase permissionAwareReadOnlyRefDatabase;
+
+ public PermissionAwareRepository(Repository delegate, PermissionBackend.ForProject forProject) {
+ super(delegate);
+ this.permissionAwareReadOnlyRefDatabase =
+ new PermissionAwareReadOnlyRefDatabase(delegate, forProject);
+ }
+
+ @Override
+ public RefDatabase getRefDatabase() {
+ return permissionAwareReadOnlyRefDatabase;
+ }
+}
diff --git a/java/com/google/gerrit/server/git/PermissionAwareRepositoryManager.java b/java/com/google/gerrit/server/git/PermissionAwareRepositoryManager.java
new file mode 100644
index 0000000..b11aa49
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PermissionAwareRepositoryManager.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import com.google.common.base.Preconditions;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import org.eclipse.jgit.lib.Repository;
+
+/**
+ * Wraps and unwraps existing repositories and makes them permission-aware by returning a {@link
+ * PermissionAwareReadOnlyRefDatabase}.
+ */
+public class PermissionAwareRepositoryManager {
+ public static Repository wrap(Repository delegate, PermissionBackend.ForProject forProject) {
+ Preconditions.checkState(
+ !(delegate instanceof PermissionAwareRepository),
+ "Cannot wrap PermissionAwareRepository instance");
+ return new PermissionAwareRepository(delegate, forProject);
+ }
+}
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index b8a2aed..a644b52 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.ReceiveCommitsExecutor;
import com.google.gerrit.server.git.MultiProgressMonitor;
+import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
import com.google.gerrit.server.git.ProjectRunnable;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.logging.Metadata;
@@ -281,9 +282,11 @@
this.user = user;
this.repo = repo;
this.metrics = metrics;
-
+ // If the user lacks READ permission, some references may be filtered and hidden from view.
+ // Check objects mentioned inside the incoming pack file are reachable from visible refs.
Project.NameKey projectName = projectState.getNameKey();
- receivePack = new ReceivePack(repo);
+ this.perm = permissionBackend.user(user).project(projectName);
+ receivePack = new ReceivePack(PermissionAwareRepositoryManager.wrap(repo, perm));
receivePack.setAllowCreates(true);
receivePack.setAllowDeletes(true);
receivePack.setAllowNonFastForwards(true);
@@ -296,9 +299,6 @@
receivePack.setPreReceiveHook(this);
receivePack.setPostReceiveHook(lazyPostReceive.create(user, projectName));
- // If the user lacks READ permission, some references may be filtered and hidden from view.
- // Check objects mentioned inside the incoming pack file are reachable from visible refs.
- this.perm = permissionBackend.user(user).project(projectName);
try {
projectState.checkStatePermitsRead();
this.perm.check(ProjectPermission.READ);
@@ -314,7 +314,7 @@
resultChangeIds = new ResultChangeIds();
receiveCommits =
factory.create(
- projectState, user, receivePack, allRefsWatcher, messageSender, resultChangeIds);
+ projectState, user, receivePack, repo, allRefsWatcher, messageSender, resultChangeIds);
receiveCommits.init();
QuotaResponse.Aggregated availableTokens =
quotaBackend.user(user).project(projectName).availableTokens(REPOSITORY_SIZE_GROUP);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 6859999..c05ef0c 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -248,6 +248,7 @@
ProjectState projectState,
IdentifiedUser user,
ReceivePack receivePack,
+ Repository repository,
AllRefsWatcher allRefsWatcher,
MessageSender messageSender,
ResultChangeIds resultChangeIds);
@@ -422,6 +423,7 @@
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted ReceivePack rp,
+ @Assisted Repository repository,
@Assisted AllRefsWatcher allRefsWatcher,
@Nullable @Assisted MessageSender messageSender,
@Assisted ResultChangeIds resultChangeIds)
@@ -471,13 +473,15 @@
this.projectState = projectState;
this.user = user;
this.receivePack = rp;
+ // This repository instance in unwrapped, while the repository instance in
+ // receivePack.getRepo() is wrapped in PermissionAwareRepository instance.
+ this.repo = repository;
// Immutable fields derived from constructor arguments.
- repo = rp.getRepository();
project = projectState.getProject();
labelTypes = projectState.getLabelTypes();
permissions = permissionBackend.user(user).project(project.getNameKey());
- rejectCommits = BanCommit.loadRejectCommitsMap(rp.getRepository(), rp.getRevWalk());
+ rejectCommits = BanCommit.loadRejectCommitsMap(repo, rp.getRevWalk());
// Collections populated during processing.
errors = MultimapBuilder.linkedHashKeys().arrayListValues().build();
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 3e32628..95083d9 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -19,6 +19,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.exceptions.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
@@ -319,7 +320,7 @@
}
/** Lookup a human readable name for an account, usually the "full name". */
- protected String getNameFor(Account.Id accountId) {
+ protected String getNameFor(@Nullable Account.Id accountId) {
if (accountId == null) {
return args.gerritPersonIdent.getName();
}
@@ -345,7 +346,14 @@
* @param accountId user to fetch.
* @return name/email of account, or Anonymous Coward if unset.
*/
- protected String getNameEmailFor(Account.Id accountId) {
+ protected String getNameEmailFor(@Nullable Account.Id accountId) {
+ if (accountId == null) {
+ return args.gerritPersonIdent.getName()
+ + " <"
+ + args.gerritPersonIdent.getEmailAddress()
+ + ">";
+ }
+
Optional<Account> account = args.accountCache.get(accountId).map(AccountState::account);
if (account.isPresent()) {
String name = account.get().fullName();
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index b8054cd..65237ac 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -399,6 +399,10 @@
private Map<String, Ref> addUsersSelfSymref(Repository repo, Map<String, Ref> refs)
throws PermissionBackendException {
if (user.isIdentifiedUser()) {
+ // User self symref is already there
+ if (refs.containsKey(REFS_USERS_SELF)) {
+ return refs;
+ }
String refName = RefNames.refsUsers(user.getAccountId());
try {
Ref r = repo.exactRef(refName);
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index bf920e1..11ca6bf 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -134,6 +134,24 @@
this.notifyResolver = notifyResolver;
}
+ /**
+ * This function is used for cherry picking a change.
+ *
+ * @param batchUpdateFactory Used for applying changes to the database.
+ * @param change Change to cherry pick.
+ * @param patch The patch of that change.
+ * @param input Input object for different configurations of cherry pick.
+ * @param dest Destination branch for the cherry pick.
+ * @return Result object that describes the cherry pick.
+ * @throws IOException Unable to open repository or read from the database.
+ * @throws InvalidChangeOperationException Parent or branch don't exist, or two changes with same
+ * key exist in the branch.
+ * @throws IntegrationException Merge conflict or trees are identical after cherry pick.
+ * @throws UpdateException Problem updating the database using batchUpdateFactory.
+ * @throws RestApiException Error such as invalid SHA1
+ * @throws ConfigInvalidException Can't find account to notify.
+ * @throws NoSuchProjectException Can't find project state.
+ */
public Result cherryPick(
BatchUpdate.Factory batchUpdateFactory,
Change change,
@@ -146,6 +164,27 @@
batchUpdateFactory, change, change.getProject(), patch.commitId(), input, dest);
}
+ /**
+ * This function is called directly to cherry pick a commit. Also, it is used to cherry pick a
+ * change as well as long as sourceChange is not null.
+ *
+ * @param batchUpdateFactory Used for applying changes to the database.
+ * @param sourceChange Change to cherry pick. Can be null, and then the function will only cherry
+ * pick a commit.
+ * @param project Project name
+ * @param sourceCommit Id of the commit to be cherry picked.
+ * @param input Input object for different configurations of cherry pick.
+ * @param dest Destination branch for the cherry pick.
+ * @return Result object that describes the cherry pick.
+ * @throws IOException Unable to open repository or read from the database.
+ * @throws InvalidChangeOperationException Parent or branch don't exist, or two changes with same
+ * key exist in the branch.
+ * @throws IntegrationException Merge conflict or trees are identical after cherry pick.
+ * @throws UpdateException Problem updating the database using batchUpdateFactory.
+ * @throws RestApiException Error such as invalid SHA1
+ * @throws ConfigInvalidException Can't find account to notify.
+ * @throws NoSuchProjectException Can't find project state.
+ */
public Result cherryPick(
BatchUpdate.Factory batchUpdateFactory,
@Nullable Change sourceChange,
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
index 469894a..76c1529 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -21,14 +21,14 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.client.ReviewerState;
-import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.FanOutExecutor;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestedReviewer;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -45,11 +45,11 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -64,13 +64,6 @@
public class ReviewerRecommender {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static final double BASE_REVIEWER_WEIGHT = 10;
- private static final double BASE_OWNER_WEIGHT = 1;
- private static final double BASE_COMMENT_WEIGHT = 0.5;
- private static final double[] WEIGHTS =
- new double[] {
- BASE_REVIEWER_WEIGHT, BASE_OWNER_WEIGHT, BASE_COMMENT_WEIGHT,
- };
private static final long PLUGIN_QUERY_TIMEOUT = 500; // ms
private final ChangeQueryBuilder changeQueryBuilder;
@@ -79,6 +72,7 @@
private final Provider<InternalChangeQuery> queryProvider;
private final ExecutorService executor;
private final ApprovalsUtil approvalsUtil;
+ private final AccountCache accountCache;
@Inject
ReviewerRecommender(
@@ -87,13 +81,15 @@
Provider<InternalChangeQuery> queryProvider,
@FanOutExecutor ExecutorService executor,
ApprovalsUtil approvalsUtil,
- @GerritServerConfig Config config) {
+ @GerritServerConfig Config config,
+ AccountCache accountCache) {
this.changeQueryBuilder = changeQueryBuilder;
this.config = config;
this.queryProvider = queryProvider;
this.reviewerSuggestionPluginMap = reviewerSuggestionPluginMap;
this.executor = executor;
this.approvalsUtil = approvalsUtil;
+ this.accountCache = accountCache;
}
public List<Account.Id> suggestReviewers(
@@ -111,12 +107,7 @@
double baseWeight = config.getInt("addReviewer", "baseWeight", 1);
logger.atFine().log("base weight: %s", baseWeight);
- Map<Account.Id, MutableDouble> reviewerScores;
- if (Strings.isNullOrEmpty(query)) {
- reviewerScores = baseRankingForEmptyQuery(baseWeight);
- } else {
- reviewerScores = baseRankingForCandidateList(candidateList, projectState, baseWeight);
- }
+ Map<Account.Id, MutableDouble> reviewerScores = baseRanking(baseWeight, query, candidateList);
logger.atFine().log("Base reviewer scores: %s", reviewerScores);
// Send the query along with a candidate list to all plugins and merge the
@@ -198,7 +189,18 @@
return sortedSuggestions;
}
- private Map<Account.Id, MutableDouble> baseRankingForEmptyQuery(double baseWeight)
+ /**
+ * @param baseWeight The weight applied to the ordering of the reviewers.
+ * @param query Query to match. For example, it can try to match all users that start with "Ab".
+ * @param candidateList The list of candidates based on the query. If query is empty, this list is
+ * also empty.
+ * @return Map of account ids that match the query and their appropriate ranking (the better the
+ * ranking, the better it is to suggest them as reviewers).
+ * @throws IOException Can't find owner="self" account.
+ * @throws ConfigInvalidException Can't find owner="self" account.
+ */
+ private Map<Account.Id, MutableDouble> baseRanking(
+ double baseWeight, String query, List<Account.Id> candidateList)
throws IOException, ConfigInvalidException {
// Get the user's last 25 changes, check approvals
try {
@@ -208,14 +210,15 @@
.setLimit(25)
.setRequestedFields(ChangeField.APPROVAL)
.query(changeQueryBuilder.owner("self"));
- Map<Account.Id, MutableDouble> suggestions = new HashMap<>();
+ Map<Account.Id, MutableDouble> suggestions = new LinkedHashMap<>();
+ // Put those candidates at the bottom of the list
+ candidateList.stream().forEach(id -> suggestions.put(id, new MutableDouble(0)));
+
for (ChangeData cd : result) {
for (PatchSetApproval approval : cd.currentApprovals()) {
Account.Id id = approval.accountId();
- if (suggestions.containsKey(id)) {
- suggestions.get(id).add(baseWeight);
- } else {
- suggestions.put(id, new MutableDouble(baseWeight));
+ if (Strings.isNullOrEmpty(query) || accountMatchesQuery(id, query)) {
+ suggestions.computeIfAbsent(id, (ignored) -> new MutableDouble(0)).add(baseWeight);
}
}
}
@@ -227,63 +230,15 @@
}
}
- private Map<Account.Id, MutableDouble> baseRankingForCandidateList(
- List<Account.Id> candidates, ProjectState projectState, double baseWeight)
- throws IOException, ConfigInvalidException {
- // Get each reviewer's activity based on number of applied labels
- // (weighted 10d), number of comments (weighted 0.5d) and number of owned
- // changes (weighted 1d).
- Map<Account.Id, MutableDouble> reviewers = new LinkedHashMap<>();
- if (candidates.size() == 0) {
- return reviewers;
- }
- List<Predicate<ChangeData>> predicates = new ArrayList<>();
- for (Account.Id id : candidates) {
- try {
- Predicate<ChangeData> projectQuery = changeQueryBuilder.project(projectState.getName());
-
- // Get all labels for this project and create a compound OR query to
- // fetch all changes where users have applied one of these labels
- List<LabelType> labelTypes = projectState.getLabelTypes().getLabelTypes();
- List<Predicate<ChangeData>> labelPredicates = new ArrayList<>(labelTypes.size());
- for (LabelType type : labelTypes) {
- labelPredicates.add(changeQueryBuilder.label(type.getName() + ",user=" + id));
- }
- Predicate<ChangeData> reviewerQuery =
- Predicate.and(projectQuery, Predicate.or(labelPredicates));
-
- Predicate<ChangeData> ownerQuery =
- Predicate.and(projectQuery, changeQueryBuilder.owner(id.toString()));
- Predicate<ChangeData> commentedByQuery =
- Predicate.and(projectQuery, changeQueryBuilder.commentby(id.toString()));
-
- predicates.add(reviewerQuery);
- predicates.add(ownerQuery);
- predicates.add(commentedByQuery);
- reviewers.put(id, new MutableDouble());
- } catch (QueryParseException e) {
- // Unhandled: If an exception is thrown, we won't increase the
- // candidates's score
- logger.atSevere().withCause(e).log("Exception while suggesting reviewers");
+ private boolean accountMatchesQuery(Account.Id id, String query) {
+ Optional<Account> account = accountCache.get(id).map(AccountState::account);
+ if (account.isPresent() && account.get().isActive()) {
+ if ((account.get().fullName() != null && account.get().fullName().startsWith(query))
+ || (account.get().preferredEmail() != null
+ && account.get().preferredEmail().startsWith(query))) {
+ return true;
}
}
-
- List<List<ChangeData>> result = queryProvider.get().setLimit(25).noFields().query(predicates);
-
- Iterator<List<ChangeData>> queryResultIterator = result.iterator();
- Iterator<Account.Id> reviewersIterator = reviewers.keySet().iterator();
-
- int i = 0;
- Account.Id currentId = null;
- while (queryResultIterator.hasNext()) {
- List<ChangeData> currentResult = queryResultIterator.next();
- if (i % WEIGHTS.length == 0) {
- currentId = reviewersIterator.next();
- }
-
- reviewers.get(currentId).add(WEIGHTS[i % WEIGHTS.length] * baseWeight * currentResult.size());
- i++;
- }
- return reviewers;
+ return false;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index 1aeb1a7..562fe846 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -118,10 +118,6 @@
}
}
- // Generate a candidate list at 3x the size of what the user wants to see to
- // give the ranking algorithm a good set of candidates it can work with
- private static final int CANDIDATE_LIST_MULTIPLIER = 3;
-
private final AccountLoader.Factory accountLoaderFactory;
private final AccountQueryBuilder accountQueryBuilder;
private final AccountIndexRewriter accountIndexRewriter;
@@ -251,7 +247,7 @@
QueryOptions.create(
indexConfig,
0,
- suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER,
+ suggestReviewers.getLimit(),
ImmutableSet.of(AccountField.ID.getName())))
.readRaw();
List<Account.Id> matches =
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 6844cac..8b81f10 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -179,6 +179,7 @@
if (input.pluginConfigValues != null) {
ConfigInput in = new ConfigInput();
in.pluginConfigValues = input.pluginConfigValues;
+ in.description = args.projectDescription;
putConfig.get().apply(projectState, in);
}
return Response.created(json.format(projectState));
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 813f9ab0..d310fe8 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -932,8 +932,8 @@
change.currentPatchSetId(),
internalUserFactory.create(),
change.getLastUpdatedOn(),
- ChangeMessagesUtil.TAG_MERGED,
- "Project was deleted.");
+ "Project was deleted.",
+ ChangeMessagesUtil.TAG_MERGED);
cmUtil.addChangeMessage(ctx.getUpdate(change.currentPatchSetId()), msg);
return true;
diff --git a/java/com/google/gerrit/sshd/AbstractGitCommand.java b/java/com/google/gerrit/sshd/AbstractGitCommand.java
index c49ae82..f3ba99b 100644
--- a/java/com/google/gerrit/sshd/AbstractGitCommand.java
+++ b/java/com/google/gerrit/sshd/AbstractGitCommand.java
@@ -24,6 +24,7 @@
import com.google.inject.Inject;
import java.io.IOException;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository;
import org.kohsuke.args4j.Argument;
@@ -47,7 +48,7 @@
protected Project project;
@Override
- public void start(Environment env) {
+ public void start(ChannelSession channel, Environment env) {
Context ctx = context.subContext(newSession(), context.getCommandLine());
final Context old = sshScope.set(ctx);
try {
diff --git a/java/com/google/gerrit/sshd/AliasCommand.java b/java/com/google/gerrit/sshd/AliasCommand.java
index 567cf00..bf0dd91 100644
--- a/java/com/google/gerrit/sshd/AliasCommand.java
+++ b/java/com/google/gerrit/sshd/AliasCommand.java
@@ -27,6 +27,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
/** Command that executes some other command. */
@@ -47,9 +48,9 @@
}
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
try {
- begin(env);
+ begin(channel, env);
} catch (Failure e) {
String msg = e.getMessage();
if (!msg.endsWith("\n")) {
@@ -61,7 +62,7 @@
}
}
- private void begin(Environment env) throws IOException, Failure {
+ private void begin(ChannelSession channel, Environment env) throws IOException, Failure {
Map<String, CommandProvider> map = root.getMap();
for (String name : chain(command)) {
CommandProvider p = map.get(name);
@@ -90,15 +91,15 @@
}
provideStateTo(cmd);
atomicCmd.set(cmd);
- cmd.start(env);
+ cmd.start(channel, env);
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
Command cmd = atomicCmd.getAndSet(null);
if (cmd != null) {
try {
- cmd.destroy();
+ cmd.destroy(channel);
} catch (Exception e) {
Throwables.throwIfUnchecked(e);
throw new RuntimeException(e);
diff --git a/java/com/google/gerrit/sshd/BaseCommand.java b/java/com/google/gerrit/sshd/BaseCommand.java
index 2081967..d326237 100644
--- a/java/com/google/gerrit/sshd/BaseCommand.java
+++ b/java/com/google/gerrit/sshd/BaseCommand.java
@@ -57,6 +57,7 @@
import org.apache.sshd.common.SshException;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.ExitCallback;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
@@ -182,7 +183,7 @@
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
Future<?> future = task.getAndSet(null);
if (future != null && !future.isDone()) {
future.cancel(true);
@@ -264,7 +265,8 @@
/**
* Spawn a function into its own thread.
*
- * <p>Typically this should be invoked within {@link Command#start(Environment)}, such as:
+ * <p>Typically this should be invoked within {@link Command#start(ChannelSession, Environment)},
+ * such as:
*
* <pre>
* startThread(new CommandRunnable() {
diff --git a/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java b/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java
new file mode 100644
index 0000000..f8ab90e
--- /dev/null
+++ b/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+/**
+ * This file is based on sshd-contrib Apache SSHD Mina project. Original commit:
+ * https://github.com/apache/mina-sshd/commit/11b33dee37b5b9c71a40a8a98a42007e3687131e
+ */
+package com.google.gerrit.sshd;
+
+import com.google.common.flogger.FluentLogger;
+import java.io.IOException;
+import org.apache.sshd.common.AttributeRepository.AttributeKey;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.Channel;
+import org.apache.sshd.common.channel.ChannelListener;
+import org.apache.sshd.common.channel.exception.SshChannelNotFoundException;
+import org.apache.sshd.common.session.ConnectionService;
+import org.apache.sshd.common.session.Session;
+import org.apache.sshd.common.session.helpers.DefaultUnknownChannelReferenceHandler;
+import org.apache.sshd.common.util.buffer.Buffer;
+
+/**
+ * Makes sure that the referenced "unknown" channel identifier is one that was assigned in
+ * the past. <B>Note:</B> it relies on the fact that the default {@code ConnectionService}
+ * implementation assigns channels identifiers in ascending order.
+ *
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class ChannelIdTrackingUnknownChannelReferenceHandler
+ extends DefaultUnknownChannelReferenceHandler implements ChannelListener {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ public static final AttributeKey<Integer> LAST_CHANNEL_ID_KEY = new AttributeKey<>();
+
+ public static final ChannelIdTrackingUnknownChannelReferenceHandler TRACKER =
+ new ChannelIdTrackingUnknownChannelReferenceHandler();
+
+ public ChannelIdTrackingUnknownChannelReferenceHandler() {
+ super();
+ }
+
+ @Override
+ public void channelInitialized(Channel channel) {
+ int channelId = channel.getId();
+ Session session = channel.getSession();
+ Integer lastTracked = session.setAttribute(LAST_CHANNEL_ID_KEY, channelId);
+ logger.atFine().log(
+ "channelInitialized(%s) updated last tracked channel ID %s => %s",
+ channel, lastTracked, channelId);
+ }
+
+ @Override
+ public Channel handleUnknownChannelCommand(
+ ConnectionService service, byte cmd, int channelId, Buffer buffer) throws IOException {
+ Session session = service.getSession();
+ Integer lastTracked = session.getAttribute(LAST_CHANNEL_ID_KEY);
+ if ((lastTracked != null) && (channelId <= lastTracked.intValue())) {
+ // Use TRACE level in order to avoid messages flooding
+ logger.atFinest().log(
+ "handleUnknownChannelCommand(%s) apply default handling for %s on channel=%s (lastTracked=%s)",
+ session, SshConstants.getCommandMessageName(cmd), channelId, lastTracked);
+ return super.handleUnknownChannelCommand(service, cmd, channelId, buffer);
+ }
+
+ throw new SshChannelNotFoundException(
+ channelId,
+ "Received "
+ + SshConstants.getCommandMessageName(cmd)
+ + " on unassigned channel "
+ + channelId
+ + " (last assigned="
+ + lastTracked
+ + ")");
+ }
+}
diff --git a/java/com/google/gerrit/sshd/CommandFactoryProvider.java b/java/com/google/gerrit/sshd/CommandFactoryProvider.java
index 245dd60..19f3386 100644
--- a/java/com/google/gerrit/sshd/CommandFactoryProvider.java
+++ b/java/com/google/gerrit/sshd/CommandFactoryProvider.java
@@ -40,6 +40,7 @@
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.ExitCallback;
import org.apache.sshd.server.SessionAware;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.command.CommandFactory;
import org.apache.sshd.server.session.ServerSession;
@@ -91,13 +92,13 @@
@Override
public CommandFactory get() {
- return requestCommand -> {
- String c = requestCommand;
+ return (channelSession, requestCommand) -> {
+ String command = requestCommand;
SshCreateCommandInterceptor interceptor = createCommandInterceptor.get();
if (interceptor != null) {
- c = interceptor.intercept(c);
+ command = interceptor.intercept(command);
}
- return new Trampoline(c);
+ return new Trampoline(command);
};
}
@@ -148,7 +149,7 @@
}
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
this.env = env;
final Context ctx = this.ctx;
task.set(
@@ -157,7 +158,7 @@
@Override
public void run() {
try {
- onStart();
+ onStart(channel);
} catch (Exception e) {
logger.atWarning().withCause(e).log(
"Cannot start command \"%s\" for user %s",
@@ -172,7 +173,7 @@
}));
}
- private void onStart() throws IOException {
+ private void onStart(ChannelSession channel) throws IOException {
synchronized (this) {
final Context old = sshScope.set(ctx);
try {
@@ -195,7 +196,7 @@
log(rc);
}
});
- cmd.start(env);
+ cmd.start(channel, env);
} finally {
sshScope.set(old);
}
@@ -225,20 +226,20 @@
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
Future<?> future = task.getAndSet(null);
if (future != null) {
future.cancel(true);
- destroyExecutor.execute(this::onDestroy);
+ destroyExecutor.execute(() -> onDestroy(channel));
}
}
- private void onDestroy() {
+ private void onDestroy(ChannelSession channel) {
synchronized (this) {
if (cmd != null) {
final Context old = sshScope.set(ctx);
try {
- cmd.destroy();
+ cmd.destroy(channel);
log(BaseCommand.STATUS_CANCEL);
} finally {
ctx = null;
diff --git a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
index 1e32e1b..6c0f3af 100644
--- a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
+++ b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
@@ -31,6 +31,7 @@
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.PublicKey;
import java.util.Collection;
@@ -80,19 +81,24 @@
}
private static Set<PublicKey> myHostKeys(KeyPairProvider p) {
- final Set<PublicKey> keys = new HashSet<>(6);
- addPublicKey(keys, p, KeyPairProvider.SSH_ED25519);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP256);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP384);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP521);
- addPublicKey(keys, p, KeyPairProvider.SSH_RSA);
- addPublicKey(keys, p, KeyPairProvider.SSH_DSS);
+ Set<PublicKey> keys = new HashSet<>(6);
+ try {
+ addPublicKey(keys, p, KeyPairProvider.SSH_ED25519);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP256);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP384);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP521);
+ addPublicKey(keys, p, KeyPairProvider.SSH_RSA);
+ addPublicKey(keys, p, KeyPairProvider.SSH_DSS);
+ } catch (IOException | GeneralSecurityException e) {
+ throw new IllegalStateException("Cannot load SSHD host key", e);
+ }
+
return keys;
}
- private static void addPublicKey(
- final Collection<PublicKey> out, KeyPairProvider p, String type) {
- final KeyPair pair = p.loadKey(type);
+ private static void addPublicKey(Collection<PublicKey> out, KeyPairProvider p, String type)
+ throws IOException, GeneralSecurityException {
+ KeyPair pair = p.loadKey(null, type);
if (pair != null && pair.getPublic() != null) {
out.add(pair.getPublic());
}
diff --git a/java/com/google/gerrit/sshd/DispatchCommand.java b/java/com/google/gerrit/sshd/DispatchCommand.java
index 68962db..7db65bd 100644
--- a/java/com/google/gerrit/sshd/DispatchCommand.java
+++ b/java/com/google/gerrit/sshd/DispatchCommand.java
@@ -33,6 +33,7 @@
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.kohsuke.args4j.Argument;
@@ -69,7 +70,7 @@
}
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
try {
parseCommandLine();
if (Strings.isNullOrEmpty(commandName)) {
@@ -115,7 +116,7 @@
provideStateTo(cmd);
atomicCmd.set(cmd);
- cmd.start(env);
+ cmd.start(channel, env);
} catch (UnloggedFailure e) {
String msg = e.getMessage();
@@ -145,11 +146,11 @@
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
Command cmd = atomicCmd.getAndSet(null);
if (cmd != null) {
try {
- cmd.destroy();
+ cmd.destroy(channel);
} catch (Exception e) {
Throwables.throwIfUnchecked(e);
throw new RuntimeException(e);
diff --git a/java/com/google/gerrit/sshd/HostKeyProvider.java b/java/com/google/gerrit/sshd/HostKeyProvider.java
index bffcfcd..3578fb9 100644
--- a/java/com/google/gerrit/sshd/HostKeyProvider.java
+++ b/java/com/google/gerrit/sshd/HostKeyProvider.java
@@ -18,7 +18,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
-import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -44,21 +43,21 @@
Path ecdsaKey_521 = site.ssh_ecdsa_521;
Path ed25519Key = site.ssh_ed25519;
- final List<File> stdKeys = new ArrayList<>(6);
+ final List<Path> stdKeys = new ArrayList<>(6);
if (Files.exists(rsaKey)) {
- stdKeys.add(rsaKey.toAbsolutePath().toFile());
+ stdKeys.add(rsaKey);
}
if (Files.exists(ecdsaKey_256)) {
- stdKeys.add(ecdsaKey_256.toAbsolutePath().toFile());
+ stdKeys.add(ecdsaKey_256);
}
if (Files.exists(ecdsaKey_384)) {
- stdKeys.add(ecdsaKey_384.toAbsolutePath().toFile());
+ stdKeys.add(ecdsaKey_384);
}
if (Files.exists(ecdsaKey_521)) {
- stdKeys.add(ecdsaKey_521.toAbsolutePath().toFile());
+ stdKeys.add(ecdsaKey_521);
}
if (Files.exists(ed25519Key)) {
- stdKeys.add(ed25519Key.toAbsolutePath().toFile());
+ stdKeys.add(ed25519Key);
}
if (Files.exists(objKey)) {
@@ -70,14 +69,14 @@
// Both formats of host key exist, we don't know which format
// should be authoritative. Complain and abort.
//
- stdKeys.add(objKey.toAbsolutePath().toFile());
+ stdKeys.add(objKey);
throw new ProvisionException("Multiple host keys exist: " + stdKeys);
}
if (stdKeys.isEmpty()) {
throw new ProvisionException("No SSH keys under " + site.etc_dir);
}
FileKeyPairProvider kp = new FileKeyPairProvider();
- kp.setFiles(stdKeys);
+ kp.setPaths(stdKeys);
return kp;
}
}
diff --git a/java/com/google/gerrit/sshd/NoShell.java b/java/com/google/gerrit/sshd/NoShell.java
index d61d6f7..680dc34 100644
--- a/java/com/google/gerrit/sshd/NoShell.java
+++ b/java/com/google/gerrit/sshd/NoShell.java
@@ -27,12 +27,13 @@
import java.io.OutputStream;
import java.net.MalformedURLException;
import java.net.URL;
-import org.apache.sshd.common.Factory;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.ExitCallback;
import org.apache.sshd.server.SessionAware;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.session.ServerSession;
+import org.apache.sshd.server.shell.ShellFactory;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.util.SystemReader;
@@ -42,7 +43,7 @@
* <p>This implementation is used to ensure clients who try to SSH directly to this server without
* supplying a command will get a reasonable error message, but cannot continue further.
*/
-class NoShell implements Factory<Command> {
+class NoShell implements ShellFactory {
private final Provider<SendMessage> shell;
@Inject
@@ -51,7 +52,7 @@
}
@Override
- public Command create() {
+ public Command createShell(ChannelSession channel) {
return shell.get();
}
@@ -98,7 +99,7 @@
}
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
Context old = sshScope.set(context);
String message;
try {
@@ -116,7 +117,7 @@
}
@Override
- public void destroy() {}
+ public void destroy(ChannelSession channel) {}
}
static class MessageFactory {
diff --git a/java/com/google/gerrit/sshd/SshCommand.java b/java/com/google/gerrit/sshd/SshCommand.java
index 2590188..e60ba6d 100644
--- a/java/com/google/gerrit/sshd/SshCommand.java
+++ b/java/com/google/gerrit/sshd/SshCommand.java
@@ -27,6 +27,7 @@
import java.io.IOException;
import java.io.PrintWriter;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.eclipse.jgit.lib.Config;
import org.kohsuke.args4j.Option;
@@ -45,7 +46,7 @@
protected PrintWriter stderr;
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
startThread(
() -> {
parseCommandLine();
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index 69176a2..7512b3e 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -51,6 +51,7 @@
import java.nio.file.WatchService;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.nio.file.spi.FileSystemProvider;
+import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.KeyPair;
import java.security.PublicKey;
@@ -208,6 +209,7 @@
final boolean enableCompression = cfg.getBoolean("sshd", "enableCompression", false);
SshSessionBackend backend = cfg.getEnum("sshd", null, "backend", SshSessionBackend.NIO2);
+ boolean channelIdTracking = cfg.getBoolean("sshd", "enableChannelIdTracking", true);
System.setProperty(
IoServiceFactoryFactory.class.getName(),
@@ -221,7 +223,7 @@
initMacs(cfg);
initSignatures();
initChannels();
- initUnknownChannelReferenceHandler();
+ initUnknownChannelReferenceHandler(channelIdTracking);
initForwarding();
initFileSystemFactory();
initSubsystems();
@@ -381,12 +383,12 @@
return Collections.emptyList();
}
- final List<PublicKey> keys = myHostKeys();
- final List<HostKey> r = new ArrayList<>();
+ List<HostKey> r = new ArrayList<>();
+ List<PublicKey> keys = myHostKeys();
for (PublicKey pub : keys) {
- final Buffer buf = new ByteArrayBuffer();
+ Buffer buf = new ByteArrayBuffer();
buf.putRawPublicKey(pub);
- final byte[] keyBin = buf.getCompactData();
+ byte[] keyBin = buf.getCompactData();
for (String addr : advertised) {
try {
@@ -397,24 +399,29 @@
}
}
}
+
return Collections.unmodifiableList(r);
}
private List<PublicKey> myHostKeys() {
- final KeyPairProvider p = getKeyPairProvider();
- final List<PublicKey> keys = new ArrayList<>(6);
- addPublicKey(keys, p, KeyPairProvider.SSH_ED25519);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP256);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP384);
- addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP521);
- addPublicKey(keys, p, KeyPairProvider.SSH_RSA);
- addPublicKey(keys, p, KeyPairProvider.SSH_DSS);
+ KeyPairProvider p = getKeyPairProvider();
+ List<PublicKey> keys = new ArrayList<>(6);
+ try {
+ addPublicKey(keys, p, KeyPairProvider.SSH_ED25519);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP256);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP384);
+ addPublicKey(keys, p, KeyPairProvider.ECDSA_SHA2_NISTP521);
+ addPublicKey(keys, p, KeyPairProvider.SSH_RSA);
+ addPublicKey(keys, p, KeyPairProvider.SSH_DSS);
+ } catch (IOException | GeneralSecurityException e) {
+ throw new IllegalStateException("Cannot load SSHD host key", e);
+ }
return keys;
}
- private static void addPublicKey(
- final Collection<PublicKey> out, KeyPairProvider p, String type) {
- final KeyPair pair = p.loadKey(type);
+ private static void addPublicKey(final Collection<PublicKey> out, KeyPairProvider p, String type)
+ throws IOException, GeneralSecurityException {
+ final KeyPair pair = p.loadKey(null, type);
if (pair != null && pair.getPublic() != null) {
out.add(pair.getPublic());
}
@@ -514,14 +521,14 @@
@SuppressWarnings("unchecked")
private void initCiphers(Config cfg) {
- final List<NamedFactory<Cipher>> a = BaseBuilder.setUpDefaultCiphers(true);
+ List<NamedFactory<Cipher>> a = BaseBuilder.setUpDefaultCiphers(true);
for (Iterator<NamedFactory<Cipher>> i = a.iterator(); i.hasNext(); ) {
- final NamedFactory<Cipher> f = i.next();
+ NamedFactory<Cipher> f = i.next();
try {
- final Cipher c = f.create();
- final byte[] key = new byte[c.getBlockSize()];
- final byte[] iv = new byte[c.getIVSize()];
+ Cipher c = f.create();
+ byte[] key = new byte[c.getKdfSize()];
+ byte[] iv = new byte[c.getIVSize()];
c.init(Cipher.Mode.Encrypt, key, iv);
} catch (InvalidKeyException e) {
logger.atWarning().log(
@@ -614,7 +621,8 @@
}
private void initSignatures() {
- setSignatureFactories(BaseBuilder.setUpDefaultSignatures(true));
+ setSignatureFactories(
+ NamedFactory.setUpBuiltinFactories(false, ServerBuilder.DEFAULT_SIGNATURE_PREFERENCE));
}
private void initCompression(boolean enableCompression) {
@@ -646,8 +654,11 @@
setChannelFactories(ServerBuilder.DEFAULT_CHANNEL_FACTORIES);
}
- private void initUnknownChannelReferenceHandler() {
- setUnknownChannelReferenceHandler(DefaultUnknownChannelReferenceHandler.INSTANCE);
+ private void initUnknownChannelReferenceHandler(boolean enableChannelIdTracking) {
+ setUnknownChannelReferenceHandler(
+ enableChannelIdTracking
+ ? ChannelIdTrackingUnknownChannelReferenceHandler.TRACKER
+ : DefaultUnknownChannelReferenceHandler.INSTANCE);
}
private void initSubsystems() {
diff --git a/java/com/google/gerrit/sshd/SshSession.java b/java/com/google/gerrit/sshd/SshSession.java
index 1a60a20..d6ecc73 100644
--- a/java/com/google/gerrit/sshd/SshSession.java
+++ b/java/com/google/gerrit/sshd/SshSession.java
@@ -19,7 +19,7 @@
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
-import org.apache.sshd.common.AttributeStore.AttributeKey;
+import org.apache.sshd.common.AttributeRepository.AttributeKey;
/** Global data related to an active SSH connection. */
public class SshSession {
diff --git a/java/com/google/gerrit/sshd/SuExec.java b/java/com/google/gerrit/sshd/SuExec.java
index 7053a0d..a126250 100644
--- a/java/com/google/gerrit/sshd/SuExec.java
+++ b/java/com/google/gerrit/sshd/SuExec.java
@@ -35,6 +35,7 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
@@ -90,7 +91,7 @@
}
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
try {
checkCanRunAs();
parseCommandLine();
@@ -102,7 +103,7 @@
cmd.setArguments(args.toArray(new String[args.size()]));
provideStateTo(cmd);
atomicCmd.set(cmd);
- cmd.start(env);
+ cmd.start(channel, env);
} finally {
sshScope.set(old);
}
@@ -158,11 +159,11 @@
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
Command cmd = atomicCmd.getAndSet(null);
if (cmd != null) {
try {
- cmd.destroy();
+ cmd.destroy(channel);
} catch (Exception e) {
Throwables.throwIfUnchecked(e);
throw new RuntimeException(e);
diff --git a/java/com/google/gerrit/sshd/commands/ScpCommand.java b/java/com/google/gerrit/sshd/commands/ScpCommand.java
index 5122b35..6912795 100644
--- a/java/com/google/gerrit/sshd/commands/ScpCommand.java
+++ b/java/com/google/gerrit/sshd/commands/ScpCommand.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
final class ScpCommand extends BaseCommand {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -81,7 +82,7 @@
}
@Override
- public void start(Environment env) {
+ public void start(ChannelSession channel, Environment env) {
startThread(this::runImp, AccessPath.SSH_COMMAND);
}
diff --git a/java/com/google/gerrit/sshd/commands/ShowCaches.java b/java/com/google/gerrit/sshd/commands/ShowCaches.java
index cee06e1..db0a481 100644
--- a/java/com/google/gerrit/sshd/commands/ShowCaches.java
+++ b/java/com/google/gerrit/sshd/commands/ShowCaches.java
@@ -51,6 +51,7 @@
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.mina.MinaSession;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.kohsuke.args4j.Option;
/** Show the current cache states. */
@@ -97,7 +98,7 @@
private int nw;
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
String s = env.getEnv().get(Environment.ENV_COLUMNS);
if (s != null && !s.isEmpty()) {
try {
@@ -106,7 +107,7 @@
columns = 80;
}
}
- super.start(env);
+ super.start(channel, env);
}
@Override
diff --git a/java/com/google/gerrit/sshd/commands/ShowConnections.java b/java/com/google/gerrit/sshd/commands/ShowConnections.java
index 231bcf6..decf5d5 100644
--- a/java/com/google/gerrit/sshd/commands/ShowConnections.java
+++ b/java/com/google/gerrit/sshd/commands/ShowConnections.java
@@ -44,6 +44,7 @@
import org.apache.sshd.common.io.nio2.Nio2Acceptor;
import org.apache.sshd.common.session.helpers.AbstractSession;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.kohsuke.args4j.Option;
/** Show the current SSH connections. */
@@ -71,7 +72,7 @@
private int columns = 80;
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
String s = env.getEnv().get(Environment.ENV_COLUMNS);
if (s != null && !s.isEmpty()) {
try {
@@ -80,7 +81,7 @@
columns = 80;
}
}
- super.start(env);
+ super.start(channel, env);
}
@Override
diff --git a/java/com/google/gerrit/sshd/commands/ShowQueue.java b/java/com/google/gerrit/sshd/commands/ShowQueue.java
index a6ed629..2ec9e2d 100644
--- a/java/com/google/gerrit/sshd/commands/ShowQueue.java
+++ b/java/com/google/gerrit/sshd/commands/ShowQueue.java
@@ -40,6 +40,7 @@
import java.util.List;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.kohsuke.args4j.Option;
/** Display the current work queue. */
@@ -70,7 +71,7 @@
private int maxCommandWidth;
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
String s = env.getEnv().get(Environment.ENV_COLUMNS);
if (s != null && !s.isEmpty()) {
try {
@@ -79,7 +80,7 @@
columns = 80;
}
}
- super.start(env);
+ super.start(channel, env);
}
@Override
diff --git a/java/com/google/gerrit/sshd/commands/StreamEvents.java b/java/com/google/gerrit/sshd/commands/StreamEvents.java
index c680d30..45540a0 100644
--- a/java/com/google/gerrit/sshd/commands/StreamEvents.java
+++ b/java/com/google/gerrit/sshd/commands/StreamEvents.java
@@ -41,6 +41,7 @@
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.apache.sshd.server.Environment;
+import org.apache.sshd.server.channel.ChannelSession;
import org.kohsuke.args4j.Option;
@RequiresCapability(GlobalCapability.STREAM_EVENTS)
@@ -105,7 +106,7 @@
private Future<?> task;
@Override
- public void start(Environment env) throws IOException {
+ public void start(ChannelSession channel, Environment env) throws IOException {
try {
parseCommandLine();
} catch (UnloggedFailure e) {
@@ -179,7 +180,7 @@
}
@Override
- public void destroy() {
+ public void destroy(ChannelSession channel) {
removeEventListenerRegistration();
final boolean exit;
diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java
index a22cdaf..5a3c745 100644
--- a/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/java/com/google/gerrit/sshd/commands/Upload.java
@@ -20,6 +20,7 @@
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
+import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.validators.UploadValidationException;
@@ -35,6 +36,7 @@
import com.google.inject.Inject;
import java.io.IOException;
import java.util.List;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.PostUploadHook;
import org.eclipse.jgit.transport.PostUploadHookChain;
import org.eclipse.jgit.transport.PreUploadHook;
@@ -65,7 +67,8 @@
throw new Failure(1, "fatal: unable to check permissions " + e);
}
- final UploadPack up = new UploadPack(repo);
+ Repository permissionAwareRepository = PermissionAwareRepositoryManager.wrap(repo, perm);
+ final UploadPack up = new UploadPack(permissionAwareRepository);
up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
@@ -73,7 +76,8 @@
List<PreUploadHook> allPreUploadHooks = Lists.newArrayList(preUploadHooks);
allPreUploadHooks.add(
- uploadValidatorsFactory.create(project, repo, session.getRemoteAddressAsString()));
+ uploadValidatorsFactory.create(
+ project, permissionAwareRepository, session.getRemoteAddressAsString()));
up.setPreUploadHook(PreUploadHookChain.newChain(allPreUploadHooks));
for (UploadPackInitializer initializer : uploadPackInitializers) {
initializer.init(projectState.getNameKey(), up);
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index 2ab91ae..3722bad 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -10,7 +10,7 @@
visibility = ["//visibility:public"],
exports = [
"//lib:junit",
- "//lib/easymock",
+ "//lib/mockito",
],
deps = [
"//java/com/google/gerrit/acceptance/testsuite/project",
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
index 9ccb74b..13085b9 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
@@ -19,7 +19,9 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.util.stream.Collectors.toSet;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.common.Nullable;
@@ -38,6 +40,7 @@
import com.google.gerrit.server.notedb.Sequences;
import com.google.inject.Inject;
import java.util.Optional;
+import java.util.Set;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -437,6 +440,44 @@
}
@Test
+ public void canFlagExistingExternalIdMailAsPreferred() throws Exception {
+ String email = "foo@example.com";
+
+ // Create an account with a SCHEME_GERRIT external ID
+ String username = "foo";
+ ExternalId.Key gerritExtIdKey = ExternalId.Key.create(ExternalId.SCHEME_GERRIT, username);
+ Account.Id accountId = Account.id(seq.nextAccountId());
+ accountsUpdate.insert(
+ "Create Test Account",
+ accountId,
+ u -> u.addExternalId(ExternalId.create(gerritExtIdKey, accountId)));
+
+ // Add the additional mail external ID with SCHEME_EMAIL
+ accountManager.link(accountId, AuthRequest.forEmail(email));
+
+ // Try to authenticate and update the email for the account.
+ // Expect that this to succeed because even if the email already exist
+ // it is associated to the same account-id and thus is not really
+ // a duplicate but simply a promotion of external id to preferred email.
+ AuthRequest who = AuthRequest.forUser(username);
+ who.setEmailAddress(email);
+ AuthResult authResult = accountManager.authenticate(who);
+
+ // Verify that no new accounts have been created
+ assertThat(authResult.isNew()).isFalse();
+
+ // Verify that the account external ids with scheme 'mailto:' contains the email
+ AccountState account = accounts.get(authResult.getAccountId()).get();
+ ImmutableSet<ExternalId> accountExternalIds = account.externalIds();
+ assertThat(accountExternalIds).isNotEmpty();
+ Set<String> emails = ExternalId.getEmails(accountExternalIds).collect(toSet());
+ assertThat(emails).contains(email);
+
+ // Verify the preferred email
+ assertThat(account.account().preferredEmail()).isEqualTo(email);
+ }
+
+ @Test
public void linkNewExternalId() throws Exception {
// Create an account with a SCHEME_GERRIT external ID and no email
String username = "foo";
@@ -539,12 +580,31 @@
// this fails because the email is already assigned to the first account.
AuthRequest who = AuthRequest.forEmail(email);
AccountException thrown =
- assertThrows(AccountException.class, () -> accountManager.link(accountId, who));
+ assertThrows(AccountException.class, () -> accountManager.link(accountId2, who));
assertThat(thrown)
.hasMessageThat()
.contains("Email 'foo@example.com' in use by another account");
}
+ @Test
+ public void allowLinkingExistingExternalIdEmailAsPreferred() throws Exception {
+ String email = "foo@example.com";
+
+ // Create an account with an SCHEME_EXTERNAL external ID that occupies the email.
+ String username = "foo";
+ Account.Id accountId = Account.id(seq.nextAccountId());
+ ExternalId.Key externalExtIdKey = ExternalId.Key.create(ExternalId.SCHEME_EXTERNAL, username);
+ accountsUpdate.insert(
+ "Create Test Account",
+ accountId,
+ u -> u.addExternalId(ExternalId.createWithEmail(externalExtIdKey, accountId, email)));
+
+ AuthRequest who = AuthRequest.forEmail(email);
+ AuthResult result = accountManager.link(accountId, who);
+ assertThat(result.isNew()).isFalse();
+ assertThat(result.getAccountId().get()).isEqualTo(accountId.get());
+ }
+
private void assertNoSuchExternalIds(ExternalId.Key... extIdKeys) throws Exception {
for (ExternalId.Key extIdKey : extIdKeys) {
assertWithMessage(extIdKey.get())
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
new file mode 100644
index 0000000..76166e1
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.server.restapi.change.QueryChanges;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.List;
+import org.junit.Test;
+
+@NoHttpd
+public class QueryChangeIT extends AbstractDaemonTest {
+
+ @Inject private Provider<QueryChanges> queryChangesProvider;
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void multipleQueriesInOneRequestCanContainSameChange() throws Exception {
+ String cId1 = createChange().getChangeId();
+ String cId2 = createChange().getChangeId();
+ int numericId1 = gApi.changes().id(cId1).get()._number;
+ int numericId2 = gApi.changes().id(cId2).get()._number;
+
+ gApi.changes().id(cId2).setWorkInProgress();
+
+ QueryChanges queryChanges = queryChangesProvider.get();
+
+ queryChanges.addQuery("is:open");
+ queryChanges.addQuery("is:wip");
+
+ List<List<ChangeInfo>> result =
+ (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+ assertThat(result).hasSize(2);
+ assertThat(result.get(0)).hasSize(2);
+ assertThat(result.get(1)).hasSize(1);
+
+ List<Integer> firstResultIds =
+ ImmutableList.of(result.get(0).get(0)._number, result.get(0).get(1)._number);
+ assertThat(firstResultIds).containsExactly(numericId1, numericId2);
+ assertThat(result.get(1).get(0)._number).isEqualTo(numericId2);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index a4157da..894e980 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -30,6 +30,7 @@
import static com.google.gerrit.server.project.testing.TestLabels.value;
import static org.eclipse.jgit.lib.Constants.HEAD;
+import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -47,10 +48,14 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.change.ChangeKindCacheImpl;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.inject.Inject;
+import com.google.inject.name.Named;
import java.util.EnumSet;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
@@ -64,6 +69,10 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject
+ @Named("change_kind")
+ private Cache<ChangeKindCacheImpl.Key, ChangeKind> changeKindCache;
+
@Before
public void setup() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -320,6 +329,42 @@
}
@Test
+ public void stickyAcrossMultiplePatchSetsDoNotRegressPerformance() throws Exception {
+ // The purpose of this test is to make sure that we compute change kind only against the parent
+ // patch set. Change kind is a heavy operation. In prior version of Gerrit, we computed the
+ // change kind against all prior patch sets. This is a regression that made Gerrit do expensive
+ // work in O(num-patch-sets). This test ensures that we aren't regressing.
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
+ u.getConfig().getLabelSections().get("Verified").setCopyAllScoresIfNoCodeChange(true);
+ u.save();
+ }
+
+ String changeId = createChange(REWORK);
+ vote(admin, changeId, 2, 1);
+ updateChange(changeId, NO_CODE_CHANGE);
+ updateChange(changeId, NO_CODE_CHANGE);
+ updateChange(changeId, NO_CODE_CHANGE);
+
+ Map<Integer, ObjectId> revisions = new HashMap<>();
+ gApi.changes()
+ .id(changeId)
+ .get()
+ .revisions
+ .forEach(
+ (revId, revisionInfo) ->
+ revisions.put(revisionInfo._number, ObjectId.fromString(revId)));
+ assertThat(revisions.size()).isEqualTo(4);
+ assertChangeKindCacheContains(revisions.get(3), revisions.get(4));
+ assertChangeKindCacheContains(revisions.get(2), revisions.get(3));
+ assertChangeKindCacheContains(revisions.get(1), revisions.get(2));
+
+ assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(4));
+ assertChangeKindCacheDoesNotContain(revisions.get(2), revisions.get(4));
+ assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(3));
+ }
+
+ @Test
public void copyMinMaxAcrossMultiplePatchSets() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
@@ -379,6 +424,18 @@
assertVotes(detailedChange(changeId), admin, label, 0, REWORK);
}
+ private void assertChangeKindCacheContains(ObjectId prior, ObjectId next) {
+ ChangeKind kind =
+ changeKindCache.getIfPresent(ChangeKindCacheImpl.Key.create(prior, next, "recursive"));
+ assertThat(kind).isNotNull();
+ }
+
+ private void assertChangeKindCacheDoesNotContain(ObjectId prior, ObjectId next) {
+ ChangeKind kind =
+ changeKindCache.getIfPresent(ChangeKindCacheImpl.Key.create(prior, next, "recursive"));
+ assertThat(kind).isNull();
+ }
+
private ChangeInfo detailedChange(String changeId) throws Exception {
return gApi.changes().id(changeId).get(DETAILED_LABELS, CURRENT_REVISION, CURRENT_COMMIT);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index e3796c4..13ac0044 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -41,15 +41,18 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.extensions.api.projects.ConfigInfo;
import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.api.projects.ConfigValue;
import com.google.gerrit.extensions.api.projects.DescriptionInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -61,10 +64,13 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.project.CommentLinkInfoImpl;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
+import com.google.inject.Module;
import java.util.HashMap;
import java.util.Map;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -95,6 +101,18 @@
private ProjectIndexedCounter projectIndexedCounter;
private RegistrationHandle projectIndexedCounterHandle;
+ @Override
+ public Module createModule() {
+ return new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ProjectConfigEntry.class)
+ .annotatedWith(Exports.named("test-plugin-key"))
+ .toInstance(new ProjectConfigEntry("Test Plugin Config Item", true));
+ }
+ };
+ }
+
@Before
public void addProjectIndexedCounter() {
projectIndexedCounter = new ProjectIndexedCounter();
@@ -171,6 +189,17 @@
}
@Test
+ public void createProjectWithPluginConfigs() throws Exception {
+ String name = name("foo");
+ ProjectInput input = new ProjectInput();
+ input.name = name;
+ input.description = "foo description";
+ input.pluginConfigValues = newPluginConfigValues();
+ ProjectInfo info = gApi.projects().create(input).get();
+ assertThat(info.description).isEqualTo(input.description);
+ }
+
+ @Test
public void createProjectWithMismatchedInput() throws Exception {
ProjectInput in = new ProjectInput();
in.name = name("foo");
@@ -729,6 +758,16 @@
return setConfig(name, input);
}
+ private static Map<String, Map<String, ConfigValue>> newPluginConfigValues() {
+ Map<String, Map<String, ConfigValue>> pluginConfigValues = new HashMap<>();
+ Map<String, ConfigValue> configValues = new HashMap<>();
+ ConfigValue value = new ConfigValue();
+ value.value = "true";
+ configValues.put("test-plugin-key", value);
+ pluginConfigValues.put("gerrit", configValues);
+ return pluginConfigValues;
+ }
+
private static class ProjectIndexedCounter implements ProjectIndexedListener {
private final AtomicLongMap<String> countsByProject = AtomicLongMap.create();
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 2bba4e6..34cdcb7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -533,6 +533,7 @@
admin.id(),
"admin.other@example.com",
"secret-password"));
+ insertExtId(ExternalId.createEmail(admin.id(), "admin.other@example.com"));
insertExtId(createExternalIdWithOtherCaseEmail(nextId(scheme, i)));
}
@@ -649,7 +650,7 @@
}
private ExternalId createExternalIdWithDuplicateEmail(String externalId) {
- return ExternalId.createWithEmail(ExternalId.Key.parse(externalId), admin.id(), admin.email());
+ return ExternalId.createWithEmail(ExternalId.Key.parse(externalId), user.id(), admin.email());
}
private ExternalId createExternalIdWithBadPassword(String username) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 42b82c5..3b7a294 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -433,106 +433,26 @@
}
@Test
- @GerritConfig(name = "suggest.maxSuggestedReviewers", value = "10")
- public void reviewerRanking() throws Exception {
- // Assert that user are ranked by the number of times they have applied a
- // a label to a change (highest), added comments (medium) or owned a
- // change (low).
- String fullName = "Primum Finalis";
- TestAccount userWhoOwns = user("customuser1", fullName);
- TestAccount reviewer1 = user("customuser2", fullName);
- TestAccount reviewer2 = user("customuser3", fullName);
- TestAccount userWhoComments = user("customuser4", fullName);
- TestAccount userWhoLooksForSuggestions = user("customuser5", fullName);
-
- // Create a change as userWhoOwns and add some reviews
- requestScopeOperations.setApiUser(userWhoOwns.id());
- String changeId1 = createChangeFromApi();
-
- requestScopeOperations.setApiUser(reviewer1.id());
- reviewChange(changeId1);
-
- requestScopeOperations.setApiUser(user1.id());
- String changeId2 = createChangeFromApi();
-
- requestScopeOperations.setApiUser(reviewer1.id());
- reviewChange(changeId2);
-
- requestScopeOperations.setApiUser(reviewer2.id());
- reviewChange(changeId2);
-
- // Create a comment as a different user
- requestScopeOperations.setApiUser(userWhoComments.id());
- ReviewInput ri = new ReviewInput();
- ri.message = "Test";
- gApi.changes().id(changeId1).revision(1).review(ri);
-
- // Create a change as a new user to assert that we receive the correct
- // ranking
-
- requestScopeOperations.setApiUser(userWhoLooksForSuggestions.id());
- List<SuggestedReviewerInfo> reviewers = suggestReviewers(createChangeFromApi(), "Pri", 4);
- assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList()))
- .containsExactly(
- reviewer1.id().get(),
- reviewer2.id().get(),
- userWhoOwns.id().get(),
- userWhoComments.id().get())
- .inOrder();
- }
-
- @Test
- public void reviewerRankingProjectIsolation() throws Exception {
- // Create new project
- Project.NameKey newProject = projectOperations.newProject().create();
-
- // Create users who review changes in both the default and the new project
- String fullName = "Primum Finalis";
- TestAccount userWhoOwns = user("customuser1", fullName);
- TestAccount reviewer1 = user("customuser2", fullName);
- TestAccount reviewer2 = user("customuser3", fullName);
-
- requestScopeOperations.setApiUser(userWhoOwns.id());
- String changeId1 = createChangeFromApi();
-
- requestScopeOperations.setApiUser(reviewer1.id());
- reviewChange(changeId1);
-
- requestScopeOperations.setApiUser(userWhoOwns.id());
- String changeId2 = createChangeFromApi(newProject);
-
- requestScopeOperations.setApiUser(reviewer2.id());
- reviewChange(changeId2);
-
- requestScopeOperations.setApiUser(userWhoOwns.id());
- String changeId3 = createChangeFromApi(newProject);
-
- requestScopeOperations.setApiUser(reviewer2.id());
- reviewChange(changeId3);
-
- requestScopeOperations.setApiUser(userWhoOwns.id());
- List<SuggestedReviewerInfo> reviewers = suggestReviewers(createChangeFromApi(), "Prim", 4);
-
- // Assert that reviewer1 is on top, even though reviewer2 has more reviews
- // in other projects
- assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList()))
- .containsExactly(reviewer1.id().get(), reviewer2.id().get())
- .inOrder();
- }
-
- @Test
public void suggestNoInactiveAccounts() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ String changeIdReviewed = createChangeFromApi();
+ String changeId = createChangeFromApi();
+
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
+ requestScopeOperations.setApiUser(foo1.id());
+ reviewChange(changeIdReviewed);
assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue();
TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo2.id());
+ reviewChange(changeIdReviewed);
assertThat(gApi.accounts().id(foo2.username()).getActive()).isTrue();
- String changeId = createChange().getChangeId();
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+ requestScopeOperations.setApiUser(user.id());
gApi.accounts().id(foo2.username()).setActive(false);
assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isFalse();
assertReviewers(suggestReviewers(changeId, name), ImmutableList.of(foo1), ImmutableList.of());
@@ -540,11 +460,19 @@
@Test
public void suggestNoExistingReviewers() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ String changeId = createChangeFromApi();
+ String changeIdReviewed = createChangeFromApi();
+
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo1.id());
+ reviewChange(changeIdReviewed);
- String changeId = createChange().getChangeId();
+ TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo2.id());
+ reviewChange(changeIdReviewed);
+
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
@@ -554,11 +482,19 @@
@Test
public void suggestCcAsReviewer() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ String changeId = createChangeFromApi();
+ String changeIdReviewed = createChangeFromApi();
+
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo1.id());
+ reviewChange(changeIdReviewed);
- String changeId = createChange().getChangeId();
+ TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo2.id());
+ reviewChange(changeIdReviewed);
+
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
@@ -572,11 +508,19 @@
@Test
public void suggestReviewerAsCc() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ String changeId = createChangeFromApi();
+ String changeIdReviewed = createChangeFromApi();
+
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo1.id());
+ reviewChange(changeIdReviewed);
- String changeId = createChange().getChangeId();
+ TestAccount foo2 = accountCreator.create(name + "-2");
+ requestScopeOperations.setApiUser(foo2.id());
+ reviewChange(changeIdReviewed);
+
assertReviewers(suggestCcs(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
AddReviewerInput reviewerInput = new AddReviewerInput();
@@ -591,25 +535,17 @@
String secondaryEmail = "foo.secondary@example.com";
TestAccount foo = createAccountWithSecondaryEmail("foo", secondaryEmail);
- List<SuggestedReviewerInfo> reviewers =
- suggestReviewers(createChange().getChangeId(), secondaryEmail, 4);
- assertReviewers(reviewers, ImmutableList.of(foo), ImmutableList.of());
-
- reviewers = suggestReviewers(createChange().getChangeId(), "secondary", 4);
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(createChangeFromApi(), "secondary", 4);
assertReviewers(reviewers, ImmutableList.of(foo), ImmutableList.of());
}
@Test
public void cannotSuggestBySecondaryEmailWithoutModifyAccount() throws Exception {
String secondaryEmail = "foo.secondary@example.com";
- createAccountWithSecondaryEmail("foo", secondaryEmail);
+ TestAccount foo = createAccountWithSecondaryEmail("foo", secondaryEmail);
requestScopeOperations.setApiUser(user.id());
- List<SuggestedReviewerInfo> reviewers =
- suggestReviewers(createChange().getChangeId(), secondaryEmail, 4);
- assertThat(reviewers).isEmpty();
-
- reviewers = suggestReviewers(createChange().getChangeId(), "secondary2", 4);
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(createChangeFromApi(), "secondary", 4);
assertThat(reviewers).isEmpty();
}
@@ -630,6 +566,24 @@
assertThat(Iterables.getOnlyElement(reviewers).account.secondaryEmails).isNull();
}
+ @Test
+ public void suggestsPeopleWithNoReviewsWhenExplicitlyQueried() throws Exception {
+ TestAccount newTeamMember = accountCreator.create("newTeamMember");
+
+ requestScopeOperations.setApiUser(user.id());
+ String changeId = createChangeFromApi();
+ String changeIdReviewed = createChangeFromApi();
+
+ TestAccount reviewer = accountCreator.create("newReviewer");
+ requestScopeOperations.setApiUser(reviewer.id());
+ reviewChange(changeIdReviewed);
+
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, "new", 4);
+ assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList()))
+ .containsExactly(reviewer.id().get(), newTeamMember.id().get())
+ .inOrder();
+ }
+
private TestAccount createAccountWithSecondaryEmail(String name, String secondaryEmail)
throws Exception {
TestAccount foo = accountCreator.create(name(name), "foo.primary@example.com", "Foo");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
index f9011c7..b18db81 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
@@ -43,11 +43,7 @@
@Before
public void setUp() throws Exception {
repo = GitUtil.newTestRepository(repoManager.openRepository(project));
- projectOperations
- .project(project)
- .forUpdate()
- .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
- .update();
+ blockRead();
}
@After
@@ -117,8 +113,17 @@
@Test
public void getOpenChange_NotFound() throws Exception {
+ // Need to unblock read to allow the push operation to succeed if not, when retrieving the
+ // advertised refs during
+ // the push, the client won't be sent the initial commit and will send it again as part of the
+ // change.
+ unblockRead();
+
PushOneCommit.Result r = pushFactory.create(admin.newIdent(), testRepo).to("refs/for/master");
r.assertOkStatus();
+
+ // Re-blocking the read
+ blockRead();
assertNotFound(r.getCommit());
}
@@ -129,6 +134,14 @@
}
}
+ private void blockRead() {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ }
+
private void assertNotFound(ObjectId id) throws Exception {
userRestSession.get("/projects/" + project.get() + "/commits/" + id.name()).assertNotFound();
}
diff --git a/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java b/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
index 1c6559b0..4932248 100644
--- a/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
+++ b/javatests/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
@@ -14,9 +14,12 @@
package com.google.gerrit.httpd;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.capture;
-import static org.easymock.EasyMock.eq;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.ReloadableRegistrationHandle;
@@ -29,11 +32,10 @@
import javax.servlet.FilterConfig;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.easymock.Capture;
-import org.easymock.EasyMockSupport;
-import org.easymock.IMocksControl;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
public class AllRequestFilterFilterProxyTest {
/**
@@ -81,16 +83,11 @@
@Test
public void noFilters() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req = new FakeHttpServletRequest();
HttpServletResponse res = new FakeHttpServletResponse();
- FilterChain chain = ems.createMock(FilterChain.class);
- chain.doFilter(req, res);
-
- ems.replayAll();
+ FilterChain chain = mock(FilterChain.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
@@ -98,25 +95,18 @@
filterProxy.doFilter(req, res, chain);
filterProxy.destroy();
- ems.verifyAll();
+ verify(chain).doFilter(req, res);
}
@Test
public void singleFilterNoBubbling() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock("config", FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req = new FakeHttpServletRequest();
HttpServletResponse res = new FakeHttpServletResponse();
- FilterChain chain = ems.createMock("chain", FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- AllRequestFilter filter = ems.createStrictMock("filter", AllRequestFilter.class);
- filter.init(config);
- filter.doFilter(eq(req), eq(res), anyObject(FilterChain.class));
- filter.destroy();
-
- ems.replayAll();
+ AllRequestFilter filter = mock(AllRequestFilter.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
addFilter(filter);
@@ -125,63 +115,52 @@
filterProxy.doFilter(req, res, chain);
filterProxy.destroy();
- ems.verifyAll();
+ InOrder inorder = inOrder(filter);
+ inorder.verify(filter).init(config);
+ inorder.verify(filter).doFilter(eq(req), eq(res), any(FilterChain.class));
+ inorder.verify(filter).destroy();
}
@Test
public void singleFilterBubbling() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req = new FakeHttpServletRequest();
HttpServletResponse res = new FakeHttpServletResponse();
- IMocksControl mockControl = ems.createStrictControl();
- FilterChain chain = mockControl.createMock(FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- Capture<FilterChain> capturedChain = new Capture<>();
+ ArgumentCaptor<FilterChain> capturedChain = ArgumentCaptor.forClass(FilterChain.class);
- AllRequestFilter filter = mockControl.createMock(AllRequestFilter.class);
- filter.init(config);
- filter.doFilter(eq(req), eq(res), capture(capturedChain));
- chain.doFilter(req, res);
- filter.destroy();
-
- ems.replayAll();
+ AllRequestFilter filter = mock(AllRequestFilter.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
addFilter(filter);
+ InOrder inorder = inOrder(filter, chain);
+
filterProxy.init(config);
filterProxy.doFilter(req, res, chain);
- capturedChain.getValue().doFilter(req, res);
- filterProxy.destroy();
- ems.verifyAll();
+ inorder.verify(filter).init(config);
+ inorder.verify(filter).doFilter(eq(req), eq(res), capturedChain.capture());
+ capturedChain.getValue().doFilter(req, res);
+ inorder.verify(chain).doFilter(req, res);
+
+ filterProxy.destroy();
+ inorder.verify(filter).destroy();
}
@Test
public void twoFiltersNoBubbling() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req = new FakeHttpServletRequest();
HttpServletResponse res = new FakeHttpServletResponse();
- IMocksControl mockControl = ems.createStrictControl();
- FilterChain chain = mockControl.createMock(FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- AllRequestFilter filterA = mockControl.createMock(AllRequestFilter.class);
+ AllRequestFilter filterA = mock(AllRequestFilter.class);
- AllRequestFilter filterB = mockControl.createMock(AllRequestFilter.class);
- filterA.init(config);
- filterB.init(config);
- filterA.doFilter(eq(req), eq(res), anyObject(FilterChain.class));
- filterA.destroy();
- filterB.destroy();
-
- ems.replayAll();
-
+ AllRequestFilter filterB = mock(AllRequestFilter.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
addFilter(filterA);
addFilter(filterB);
@@ -190,35 +169,27 @@
filterProxy.doFilter(req, res, chain);
filterProxy.destroy();
- ems.verifyAll();
+ InOrder inorder = inOrder(filterA, filterB);
+ inorder.verify(filterA).init(config);
+ inorder.verify(filterB).init(config);
+ inorder.verify(filterA).doFilter(eq(req), eq(res), any(FilterChain.class));
+ inorder.verify(filterA).destroy();
+ inorder.verify(filterB).destroy();
}
@Test
public void twoFiltersBubbling() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req = new FakeHttpServletRequest();
HttpServletResponse res = new FakeHttpServletResponse();
- IMocksControl mockControl = ems.createStrictControl();
- FilterChain chain = mockControl.createMock(FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- Capture<FilterChain> capturedChainA = new Capture<>();
- Capture<FilterChain> capturedChainB = new Capture<>();
+ ArgumentCaptor<FilterChain> capturedChainA = ArgumentCaptor.forClass(FilterChain.class);
+ ArgumentCaptor<FilterChain> capturedChainB = ArgumentCaptor.forClass(FilterChain.class);
- AllRequestFilter filterA = mockControl.createMock(AllRequestFilter.class);
- AllRequestFilter filterB = mockControl.createMock(AllRequestFilter.class);
-
- filterA.init(config);
- filterB.init(config);
- filterA.doFilter(eq(req), eq(res), capture(capturedChainA));
- filterB.doFilter(eq(req), eq(res), capture(capturedChainB));
- chain.doFilter(req, res);
- filterA.destroy();
- filterB.destroy();
-
- ems.replayAll();
+ AllRequestFilter filterA = mock(AllRequestFilter.class);
+ AllRequestFilter filterB = mock(AllRequestFilter.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
addFilter(filterA);
@@ -226,70 +197,69 @@
filterProxy.init(config);
filterProxy.doFilter(req, res, chain);
- capturedChainA.getValue().doFilter(req, res);
- capturedChainB.getValue().doFilter(req, res);
- filterProxy.destroy();
- ems.verifyAll();
+ InOrder inorder = inOrder(filterA, filterB, chain);
+
+ inorder.verify(filterA).init(config);
+ inorder.verify(filterB).init(config);
+ inorder.verify(filterA).doFilter(eq(req), eq(res), capturedChainA.capture());
+ capturedChainA.getValue().doFilter(req, res);
+ inorder.verify(filterB).doFilter(eq(req), eq(res), capturedChainB.capture());
+ capturedChainB.getValue().doFilter(req, res);
+ inorder.verify(chain).doFilter(req, res);
+
+ filterProxy.destroy();
+ inorder.verify(filterA).destroy();
+ inorder.verify(filterB).destroy();
}
@Test
public void postponedLoading() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req1 = new FakeHttpServletRequest();
HttpServletRequest req2 = new FakeHttpServletRequest();
HttpServletResponse res1 = new FakeHttpServletResponse();
HttpServletResponse res2 = new FakeHttpServletResponse();
- IMocksControl mockControl = ems.createStrictControl();
- FilterChain chain = mockControl.createMock("chain", FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- Capture<FilterChain> capturedChainA1 = new Capture<>();
- Capture<FilterChain> capturedChainA2 = new Capture<>();
- Capture<FilterChain> capturedChainB = new Capture<>();
+ ArgumentCaptor<FilterChain> capturedChainA1 = ArgumentCaptor.forClass(FilterChain.class);
+ ArgumentCaptor<FilterChain> capturedChainA2 = ArgumentCaptor.forClass(FilterChain.class);
+ ArgumentCaptor<FilterChain> capturedChainB = ArgumentCaptor.forClass(FilterChain.class);
- AllRequestFilter filterA = mockControl.createMock("filterA", AllRequestFilter.class);
- AllRequestFilter filterB = mockControl.createMock("filterB", AllRequestFilter.class);
+ AllRequestFilter filterA = mock(AllRequestFilter.class);
+ AllRequestFilter filterB = mock(AllRequestFilter.class);
- filterA.init(config);
- filterA.doFilter(eq(req1), eq(res1), capture(capturedChainA1));
- chain.doFilter(req1, res1);
-
- filterA.doFilter(eq(req2), eq(res2), capture(capturedChainA2));
- filterB.init(config); // <-- This is crucial part. filterB got loaded
- // after filterProxy's init finished. Nonetheless filterB gets initialized.
- filterB.doFilter(eq(req2), eq(res2), capture(capturedChainB));
- chain.doFilter(req2, res2);
-
- filterA.destroy();
- filterB.destroy();
-
- ems.replayAll();
+ InOrder inorder = inOrder(filterA, filterB, chain);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
addFilter(filterA);
filterProxy.init(config);
filterProxy.doFilter(req1, res1, chain);
+ inorder.verify(filterA).init(config);
+ inorder.verify(filterA).doFilter(eq(req1), eq(res1), capturedChainA1.capture());
capturedChainA1.getValue().doFilter(req1, res1);
+ inorder.verify(chain).doFilter(req1, res1);
addFilter(filterB); // <-- Adds filter after filterProxy's init got called.
filterProxy.doFilter(req2, res2, chain);
+ // after filterProxy's init finished. Nonetheless filterB gets initialized.
+ inorder.verify(filterA).doFilter(eq(req2), eq(res2), capturedChainA2.capture());
capturedChainA2.getValue().doFilter(req2, res2);
+ inorder.verify(filterB).init(config); // <-- This is crucial part. filterB got loaded
+ inorder.verify(filterB).doFilter(eq(req2), eq(res2), capturedChainB.capture());
capturedChainB.getValue().doFilter(req2, res2);
+ inorder.verify(chain).doFilter(req2, res2);
filterProxy.destroy();
-
- ems.verifyAll();
+ inorder.verify(filterA).destroy();
+ inorder.verify(filterB).destroy();
}
@Test
public void dynamicUnloading() throws Exception {
- EasyMockSupport ems = new EasyMockSupport();
-
- FilterConfig config = ems.createMock(FilterConfig.class);
+ FilterConfig config = mock(FilterConfig.class);
HttpServletRequest req1 = new FakeHttpServletRequest();
HttpServletRequest req2 = new FakeHttpServletRequest();
HttpServletRequest req3 = new FakeHttpServletRequest();
@@ -297,64 +267,62 @@
HttpServletResponse res2 = new FakeHttpServletResponse();
HttpServletResponse res3 = new FakeHttpServletResponse();
- Plugin plugin = ems.createMock(Plugin.class);
+ Plugin plugin = mock(Plugin.class);
- IMocksControl mockControl = ems.createStrictControl();
- FilterChain chain = mockControl.createMock("chain", FilterChain.class);
+ FilterChain chain = mock(FilterChain.class);
- Capture<FilterChain> capturedChainA1 = new Capture<>();
- Capture<FilterChain> capturedChainB1 = new Capture<>();
- Capture<FilterChain> capturedChainB2 = new Capture<>();
+ ArgumentCaptor<FilterChain> capturedChainA1 = ArgumentCaptor.forClass(FilterChain.class);
+ ArgumentCaptor<FilterChain> capturedChainB1 = ArgumentCaptor.forClass(FilterChain.class);
+ ArgumentCaptor<FilterChain> capturedChainB2 = ArgumentCaptor.forClass(FilterChain.class);
- AllRequestFilter filterA = mockControl.createMock("filterA", AllRequestFilter.class);
- AllRequestFilter filterB = mockControl.createMock("filterB", AllRequestFilter.class);
-
- filterA.init(config);
- filterB.init(config);
-
- filterA.doFilter(eq(req1), eq(res1), capture(capturedChainA1));
- filterB.doFilter(eq(req1), eq(res1), capture(capturedChainB1));
- chain.doFilter(req1, res1);
-
- filterA.destroy(); // Cleaning up of filterA after it got unloaded
-
- filterB.doFilter(eq(req2), eq(res2), capture(capturedChainB2));
- chain.doFilter(req2, res2);
-
- filterB.destroy(); // Cleaning up of filterA after it got unloaded
-
- chain.doFilter(req3, res3);
-
- ems.replayAll();
+ AllRequestFilter filterA = mock(AllRequestFilter.class);
+ AllRequestFilter filterB = mock(AllRequestFilter.class);
AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
ReloadableRegistrationHandle<AllRequestFilter> handleFilterA = addFilter(filterA);
ReloadableRegistrationHandle<AllRequestFilter> handleFilterB = addFilter(filterB);
+ InOrder inorder = inOrder(filterA, filterB, chain);
+
filterProxy.init(config);
+ inorder.verify(filterA).init(config);
+ inorder.verify(filterB).init(config);
+
// Request #1 with filterA and filterB
filterProxy.doFilter(req1, res1, chain);
+ inorder.verify(filterA).doFilter(eq(req1), eq(res1), capturedChainA1.capture());
capturedChainA1.getValue().doFilter(req1, res1);
+ inorder.verify(filterB).doFilter(eq(req1), eq(res1), capturedChainB1.capture());
capturedChainB1.getValue().doFilter(req1, res1);
+ inorder.verify(chain).doFilter(req1, res1);
// Unloading filterA
handleFilterA.remove();
filterProxy.onStopPlugin(plugin);
- // Request #1 only with filterB
+ inorder.verify(filterA).destroy(); // Cleaning up of filterA after it got unloaded
+
+ // Request #2 only with filterB
filterProxy.doFilter(req2, res2, chain);
- capturedChainA1.getValue().doFilter(req2, res2);
+
+ inorder.verify(filterB).doFilter(eq(req2), eq(res2), capturedChainB2.capture());
+ inorder.verify(filterA, never()).doFilter(eq(req2), eq(res2), any(FilterChain.class));
+ capturedChainB2.getValue().doFilter(req2, res2);
+ inorder.verify(chain).doFilter(req2, res2);
// Unloading filterB
handleFilterB.remove();
filterProxy.onStopPlugin(plugin);
- // Request #1 with no additional filters
+ inorder.verify(filterB).destroy(); // Cleaning up of filterA after it got unloaded
+
+ // Request #3 with no additional filters
filterProxy.doFilter(req3, res3, chain);
+ inorder.verify(chain).doFilter(req3, res3);
+ inorder.verify(filterA, never()).doFilter(eq(req2), eq(res2), any(FilterChain.class));
+ inorder.verify(filterB, never()).doFilter(eq(req2), eq(res2), any(FilterChain.class));
filterProxy.destroy();
-
- ems.verifyAll();
}
}
diff --git a/lib/easymock/BUILD b/lib/easymock/BUILD
deleted file mode 100644
index 90c9673..0000000
--- a/lib/easymock/BUILD
+++ /dev/null
@@ -1,26 +0,0 @@
-load("@rules_java//java:defs.bzl", "java_library")
-
-java_library(
- name = "easymock",
- data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
- visibility = ["//visibility:public"],
- exports = ["@easymock//jar"],
- runtime_deps = [
- ":cglib-3_2",
- ":objenesis",
- ],
-)
-
-java_library(
- name = "cglib-3_2",
- data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
- visibility = ["//visibility:public"],
- exports = ["@cglib-3_2//jar"],
-)
-
-java_library(
- name = "objenesis",
- data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
- visibility = ["//visibility:public"],
- exports = ["@objenesis//jar"],
-)
diff --git a/lib/mina/BUILD b/lib/mina/BUILD
index 5ad47cd..2f98ee3 100644
--- a/lib/mina/BUILD
+++ b/lib/mina/BUILD
@@ -6,6 +6,7 @@
visibility = ["//visibility:public"],
exports = [
":eddsa",
+ "@sshd-common//jar",
"@sshd-mina//jar",
"@sshd//jar",
],
diff --git a/plugins/download-commands b/plugins/download-commands
index 8914550..addee7f 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 891455076417dd097fdfd63f4afc0d28a3e85aff
+Subproject commit addee7fe76fdfe8b1929e3dd5d4c31b57b2f24a6
diff --git a/plugins/replication b/plugins/replication
index 4ca9342..bb42fdd 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4ca93421cb84b80da2c76ac6bba95117aa53543c
+Subproject commit bb42fdd40655b75c2cc7db7eb36c99796bd53528
diff --git a/polygerrit-ui/Polymer2.md b/polygerrit-ui/Polymer2.md
new file mode 100644
index 0000000..78112de
--- /dev/null
+++ b/polygerrit-ui/Polymer2.md
@@ -0,0 +1,15 @@
+## Polymer 2 upgrade
+
+Gerrit is updating to use polymer 2 from polymer 1 by following the [Polymer 2.0 upgrade guide](https://polymer-library.polymer-project.org/2.0/docs/upgrade).
+
+Polymer 2 contains several breaking changes that may affect some of the UI features and plugins. One of the biggest change is to have the shadow DOM enabled. This will affect how you query elements inside of your component, how css style works within and across components, and several other usages.
+
+If you are owner of any plugins, please start following the [Polymer 2.0 upgrade guide](https://polymer-library.polymer-project.org/2.0/docs/upgrade) to migrate your plugins to be polymer 2 ready.
+
+If you notice any issues or need help with anything, don't hesitate to report to us [here](https://bugs.chromium.org/p/gerrit/issues/list).
+
+
+### Related resources
+
+- [Polymer 2.0 upgrade guide](https://polymer-library.polymer-project.org/2.0/docs/upgrade)
+- [Polymner Shadow DOM](https://polymer-library.polymer-project.org/2.0/docs/devguide/shadow-dom)
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
index 6616dab..cbc5f13 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
@@ -145,12 +145,6 @@
return Promise.resolve();
}
- const user = params.user || 'self';
-
- // NOTE: This method may be called before attachment. Fire title-change
- // in an async so that attachment to the DOM can take place first.
- const title = params.title || this._computeTitle(user);
- this.async(() => this.fire('title-change', {title}));
return this._reload();
},
@@ -171,11 +165,19 @@
const checkForNewUser = !project && user === 'self';
return dashboardPromise
- .then(res => this._fetchDashboardChanges(res, checkForNewUser))
+ .then(res => {
+ if (res && res.title) {
+ this.fire('title-change', {title: res.title});
+ }
+ return this._fetchDashboardChanges(res, checkForNewUser);
+ })
.then(() => {
this._maybeShowDraftsBanner();
this.$.reporting.dashboardDisplayed();
}).catch(err => {
+ this.fire('title-change', {
+ title: title || this._computeTitle(user),
+ });
console.warn(err);
}).then(() => { this._loading = false; });
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 54e2edd..9f842c1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -269,6 +269,7 @@
/** @type {?} */
revisionActions: {
type: Object,
+ notify: true,
value() { return {}; },
},
// If property binds directly to [[revisionActions.submit]] it is not
@@ -461,7 +462,7 @@
return this._getRevisionActions().then(revisionActions => {
if (!revisionActions) { return; }
- this.revisionActions = revisionActions;
+ this.revisionActions = this._updateRebaseAction(revisionActions);
this._handleLoadingComplete();
}).catch(err => {
this.fire('show-alert', {message: ERR_REVISION_ACTIONS});
@@ -474,6 +475,18 @@
Gerrit.awaitPluginsLoaded().then(() => this._loading = false);
},
+ _updateRebaseAction(revisionActions) {
+ if (revisionActions && revisionActions.rebase) {
+ revisionActions.rebase.rebaseOnCurrent =
+ !!revisionActions.rebase.enabled;
+ this._parentIsCurrent = !revisionActions.rebase.enabled;
+ revisionActions.rebase.enabled = true;
+ } else {
+ this._parentIsCurrent = true;
+ }
+ return revisionActions;
+ },
+
_changeChanged() {
this.reload();
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index b88e06b..37201ac 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -376,6 +376,7 @@
};
assert.isTrue(fetchChangesStub.called);
element._handleRebaseConfirm({detail: {base: '1234'}});
+ rebaseAction.rebaseOnCurrent = true;
assert.deepEqual(fireActionStub.lastCall.args,
['/rebase', rebaseAction, true, {base: '1234'}]);
done();
@@ -1558,5 +1559,58 @@
assert.strictEqual(element.$.confirmSubmitDialog.action, null);
assert.strictEqual(element.$.confirmRebase.rebaseOnCurrent, null);
});
+
+ test('_updateRebaseAction sets _parentIsCurrent on no rebase', () => {
+ const currentRevisionActions = {
+ cherrypick: {
+ enabled: true,
+ label: 'Cherry Pick',
+ method: 'POST',
+ title: 'cherrypick',
+ },
+ };
+ element._parentIsCurrent = undefined;
+ element._updateRebaseAction(currentRevisionActions);
+ assert.isTrue(element._parentIsCurrent);
+ });
+
+ test('_updateRebaseAction', () => {
+ const currentRevisionActions = {
+ cherrypick: {
+ enabled: true,
+ label: 'Cherry Pick',
+ method: 'POST',
+ title: 'cherrypick',
+ },
+ rebase: {
+ enabled: true,
+ label: 'Rebase',
+ method: 'POST',
+ title: 'Rebase onto tip of branch or parent change',
+ },
+ };
+ element._parentIsCurrent = undefined;
+
+ // Rebase enabled should always end up true.
+ // When rebase is enabled initially, rebaseOnCurrent should be set to
+ // true.
+ assert.equal(element._updateRebaseAction(currentRevisionActions),
+ currentRevisionActions);
+
+ assert.isTrue(currentRevisionActions.rebase.enabled);
+ assert.isTrue(currentRevisionActions.rebase.rebaseOnCurrent);
+ assert.isFalse(element._parentIsCurrent);
+
+ delete currentRevisionActions.rebase.enabled;
+
+ // When rebase is not enabled initially, rebaseOnCurrent should be set to
+ // false.
+ assert.equal(element._updateRebaseAction(currentRevisionActions),
+ currentRevisionActions);
+
+ assert.isTrue(currentRevisionActions.rebase.enabled);
+ assert.isFalse(currentRevisionActions.rebase.rebaseOnCurrent);
+ assert.isTrue(element._parentIsCurrent);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index cb72b32..e8297af 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -408,7 +408,7 @@
disable-edit="[[disableEdit]]"
has-parent="[[hasParent]]"
actions="[[_change.actions]]"
- revision-actions="[[_currentRevisionActions]]"
+ revision-actions="{{_currentRevisionActions}}"
change-num="[[_changeNum]]"
change-status="[[_change.status]]"
commit-num="[[_commitInfo.commit]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 301bdbd..7a0700f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -227,7 +227,8 @@
},
_changeStatuses: {
type: String,
- computed: '_computeChangeStatusChips(_change, _mergeable)',
+ computed:
+ '_computeChangeStatusChips(_change, _mergeable, _submitEnabled)',
},
_commitCollapsed: {
type: Boolean,
@@ -249,7 +250,10 @@
observer: '_updateToggleContainerClass',
},
_parentIsCurrent: Boolean,
- _submitEnabled: Boolean,
+ _submitEnabled: {
+ type: Boolean,
+ computed: '_isSubmitEnabled(_currentRevisionActions)',
+ },
/** @type {?} */
_mergeable: {
@@ -464,7 +468,7 @@
this._editingCommitMessage = false;
},
- _computeChangeStatusChips(change, mergeable) {
+ _computeChangeStatusChips(change, mergeable, submitEnabled) {
// Polymer 2: check for undefined
if ([
change,
@@ -483,7 +487,7 @@
const options = {
includeDerived: true,
mergeable: !!mergeable,
- submitEnabled: this._submitEnabled,
+ submitEnabled: !!submitEnabled,
};
return this.changeStatuses(change, options);
},
@@ -1208,18 +1212,6 @@
return this.$.restAPI.getPreferences();
},
- _updateRebaseAction(revisionActions) {
- if (revisionActions && revisionActions.rebase) {
- revisionActions.rebase.rebaseOnCurrent =
- !!revisionActions.rebase.enabled;
- this._parentIsCurrent = !revisionActions.rebase.enabled;
- revisionActions.rebase.enabled = true;
- } else {
- this._parentIsCurrent = true;
- }
- return revisionActions;
- },
-
_prepareCommitMsgForLinkify(msg) {
// TODO(wyatta) switch linkify sequence, see issue 5526.
// This is a zero-with space. It is added to prevent the linkify library
@@ -1285,8 +1277,6 @@
this._latestCommitMessage = null;
}
- // Update the submit enabled based on current revision.
- this._submitEnabled = this._isSubmitEnabled(currentRevision);
const lineHeight = getComputedStyle(this).lineHeight;
@@ -1303,8 +1293,6 @@
currentRevision.commit.commit = latestRevisionSha;
}
this._commitInfo = currentRevision.commit;
- this._currentRevisionActions =
- this._updateRebaseAction(currentRevision.actions);
this._selectedRevision = currentRevision;
// TODO: Fetch and process files.
} else {
@@ -1316,9 +1304,9 @@
});
},
- _isSubmitEnabled(currentRevision) {
- return !!(currentRevision.actions && currentRevision.actions.submit &&
- currentRevision.actions.submit.enabled);
+ _isSubmitEnabled(revisionActions) {
+ return !!(revisionActions && revisionActions.submit &&
+ revisionActions.submit.enabled);
},
_getEdit() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index eef4dbf..2267d27 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -530,65 +530,11 @@
assert.equal(result, 'CC=\u200Btest@google.com');
}),
- test('_updateRebaseAction', () => {
- const currentRevisionActions = {
- cherrypick: {
- enabled: true,
- label: 'Cherry Pick',
- method: 'POST',
- title: 'cherrypick',
- },
- rebase: {
- enabled: true,
- label: 'Rebase',
- method: 'POST',
- title: 'Rebase onto tip of branch or parent change',
- },
- };
- element._parentIsCurrent = undefined;
-
- // Rebase enabled should always end up true.
- // When rebase is enabled initially, rebaseOnCurrent should be set to
- // true.
- assert.equal(element._updateRebaseAction(currentRevisionActions),
- currentRevisionActions);
-
- assert.isTrue(currentRevisionActions.rebase.enabled);
- assert.isTrue(currentRevisionActions.rebase.rebaseOnCurrent);
- assert.isFalse(element._parentIsCurrent);
-
- delete currentRevisionActions.rebase.enabled;
-
- // When rebase is not enabled initially, rebaseOnCurrent should be set to
- // false.
- assert.equal(element._updateRebaseAction(currentRevisionActions),
- currentRevisionActions);
-
- assert.isTrue(currentRevisionActions.rebase.enabled);
- assert.isFalse(currentRevisionActions.rebase.rebaseOnCurrent);
- assert.isTrue(element._parentIsCurrent);
- });
-
test('_isSubmitEnabled', () => {
assert.isFalse(element._isSubmitEnabled({}));
- assert.isFalse(element._isSubmitEnabled({actions: {}}));
- assert.isFalse(element._isSubmitEnabled({actions: {submit: {}}}));
+ assert.isFalse(element._isSubmitEnabled({submit: {}}));
assert.isTrue(element._isSubmitEnabled(
- {actions: {submit: {enabled: true}}}));
- });
-
- test('_updateRebaseAction sets _parentIsCurrent on no rebase', () => {
- const currentRevisionActions = {
- cherrypick: {
- enabled: true,
- label: 'Cherry Pick',
- method: 'POST',
- title: 'cherrypick',
- },
- };
- element._parentIsCurrent = undefined;
- element._updateRebaseAction(currentRevisionActions);
- assert.isTrue(element._parentIsCurrent);
+ {submit: {enabled: true}}));
});
test('_reload is called when an approved label is removed', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 73b12f7..1114160 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -48,10 +48,6 @@
SEND: 'Send reply',
};
- // TODO(logan): Remove once the fix for issue 6841 is stable on
- // googlesource.com.
- const START_REVIEW_MESSAGE = 'This change is ready for review.';
-
const EMPTY_REPLY_MESSAGE = 'Cannot send an empty reply.';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
@@ -220,10 +216,6 @@
FocusTarget,
- // TODO(logan): Remove once the fix for issue 6841 is stable on
- // googlesource.com.
- START_REVIEW_MESSAGE,
-
behaviors: [
Gerrit.BaseUrlBehavior,
Gerrit.FireBehavior,
@@ -468,13 +460,6 @@
this.disabled = true;
- if (obj.ready && !obj.message) {
- // TODO(logan): The server currently doesn't send email in this case.
- // Insert a dummy message to force an email to be sent. Remove this
- // once the fix for issue 6841 is stable on googlesource.com.
- obj.message = START_REVIEW_MESSAGE;
- }
-
const errFn = this._handle400Error.bind(this);
return this._saveReview(obj, errFn).then(response => {
if (!response) {
@@ -487,18 +472,10 @@
return {};
}
- // TODO(logan): Remove once the required API changes are live and stable
- // on googlesource.com.
- return this._maybeSetReady(startReview, response).catch(err => {
- // We catch error here because we still want to treat this as a
- // successful review.
- console.error('error setting ready:', err);
- }).then(() => {
- this.draft = '';
- this._includeComments = true;
- this.fire('send', null, {bubbles: false});
- return accountAdditions;
- });
+ this.draft = '';
+ this._includeComments = true;
+ this.fire('send', null, {bubbles: false});
+ return accountAdditions;
}).then(result => {
this.disabled = false;
return result;
@@ -508,32 +485,6 @@
});
},
- /**
- * Returns a promise resolving to true if review was successfully posted,
- * false otherwise.
- *
- * TODO(logan): Remove this once the required API changes are live and
- * stable on googlesource.com.
- */
- _maybeSetReady(startReview, response) {
- return this.$.restAPI.getResponseObject(response).then(result => {
- if (!startReview || result.ready) {
- return Promise.resolve();
- }
- // We don't have confirmation that review was started, so attempt to
- // start review explicitly.
- return this.$.restAPI.startReview(
- this.change._number, null, response => {
- // If we see a 409 response code, then that means the server
- // *does* support moving from WIP->ready when posting a
- // review. Only alert user for non-409 failures.
- if (response.status !== 409) {
- this.fire('server-error', {response});
- }
- });
- });
- },
-
_focusOn(section) {
// Safeguard- always want to focus on something.
if (!section || section === FocusTarget.ANY) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 6b738d8..99b91b7 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -989,21 +989,6 @@
assert.isFalse(startReviewStub.called);
});
});
-
- test('fall back to start review against old backend', () => {
- stubSaveReview(review => {
- return {}; // old backend won't set ready: true
- });
-
- return element.send(true, true).then(() => {
- assert.isTrue(startReviewStub.called);
- }).then(() => {
- startReviewStub.reset();
- return element.send(true, false);
- }).then(() => {
- assert.isFalse(startReviewStub.called);
- });
- });
});
suite('start review and save buttons', () => {
@@ -1029,28 +1014,9 @@
});
});
- test('dummy message to force email on start review', () => {
- stubSaveReview(review => {
- assert.equal(review.message, element.START_REVIEW_MESSAGE);
- return {ready: true};
- });
- return element.send(true, true);
- });
-
test('buttons disabled until all API calls are resolved', () => {
stubSaveReview(review => {
- return {}; // old backend won't set ready: true
- });
- // Check that element is disabled asynchronously after the setReady
- // promise is returned. The element should not be reenabled until
- // that promise is resolved.
- sandbox.stub(element, '_maybeSetReady', (startReview, response) => {
- return new Promise(resolve => {
- Polymer.Base.async(() => {
- assert.isTrue(element.disabled);
- resolve();
- });
- });
+ return {ready: true};
});
return element.send(true, true).then(() => {
assert.isFalse(element.disabled);
@@ -1070,11 +1036,6 @@
assert.isFalse(element.disabled);
}
- function assertDialogClosed() {
- assert.strictEqual('', element.draft);
- assert.isFalse(element.disabled);
- }
-
test('error occurs in _saveReview', () => {
stubSaveReview(review => {
throw expectedError;
@@ -1085,46 +1046,6 @@
});
});
- test('error occurs during startReview', () => {
- stubSaveReview(review => {
- return {}; // old backend won't set ready: true
- });
- const errorStub = sandbox.stub(
- console, 'error', (msg, err) => undefined);
- sandbox.stub(element.$.restAPI, 'startReview', () => {
- throw expectedError;
- });
- return element.send(true, true).then(() => {
- assertDialogClosed();
- assert.isTrue(
- errorStub.calledWith('error setting ready:', expectedError));
- });
- });
-
- test('non-ok response received by startReview', () => {
- stubSaveReview(review => {
- return {}; // old backend won't set ready: true
- });
- sandbox.stub(element.$.restAPI, 'startReview', (c, b, f) => {
- f({status: 500});
- });
- return element.send(true, true).then(() => {
- assertDialogClosed();
- });
- });
-
- test('409 response received by startReview', () => {
- stubSaveReview(review => {
- return {}; // old backend won't set ready: true
- });
- sandbox.stub(element.$.restAPI, 'startReview', (c, b, f) => {
- f({status: 409});
- });
- return element.send(true, true).then(() => {
- assertDialogClosed();
- });
- });
-
suite('pending diff drafts?', () => {
test('yes', () => {
const promise = mockPromise();
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 8c57748..fc84ca8 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -601,16 +601,6 @@
params.patchNum = params.basePatchNum;
params.basePatchNum = null;
}
- // In GWTUI, edits are represented in URLs with either 0 or 'edit'.
- // TODO(kaspern): Remove this normalization when GWT UI is gone.
- if (this.patchNumEquals(params.basePatchNum, 0)) {
- params.basePatchNum = this.EDIT_NAME;
- needsRedirect = true;
- }
- if (this.patchNumEquals(params.patchNum, 0)) {
- params.patchNum = this.EDIT_NAME;
- needsRedirect = true;
- }
return needsRedirect;
},
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index ecb4152..4bfc35b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -604,30 +604,6 @@
assert.isNotOk(params.basePatchNum);
assert.equal(params.patchNum, 4);
});
-
- test('range 0..n normalizes to edit..n', () => {
- const params = {basePatchNum: 0, patchNum: 4};
- const needsRedirect = element._normalizePatchRangeParams(params);
- assert.isTrue(needsRedirect);
- assert.equal(params.basePatchNum, 'edit');
- assert.equal(params.patchNum, 4);
- });
-
- test('range n..0 normalizes to n..edit', () => {
- const params = {basePatchNum: 4, patchNum: 0};
- const needsRedirect = element._normalizePatchRangeParams(params);
- assert.isTrue(needsRedirect);
- assert.equal(params.basePatchNum, 4);
- assert.equal(params.patchNum, 'edit');
- });
-
- test('range 0..0 normalizes to edit', () => {
- const params = {basePatchNum: 0, patchNum: 0};
- const needsRedirect = element._normalizePatchRangeParams(params);
- assert.isTrue(needsRedirect);
- assert.isNotOk(params.basePatchNum);
- assert.equal(params.patchNum, 'edit');
- });
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 968db93..3344e1d 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -73,6 +73,10 @@
match: 'test (.+)',
html: '<a href="/r/awesomesauce">$1</a>',
},
+ anotatstartwithbaseurl: {
+ match: 'a test (.+)',
+ html: '[Lookup: <a href="/r/awesomesauce">$1</a>]',
+ },
disabledconfig: {
match: 'foo:(.+)',
link: 'https://google.com/search?q=$1',
@@ -216,6 +220,15 @@
assert.equal(linkEl.textContent, 'foo');
});
+ test('a is not at start', () => {
+ window.CANONICAL_PATH = '/r';
+
+ element.content = 'a test foo';
+ const linkEl = element.$.output.childNodes[1];
+ assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
+ assert.equal(linkEl.textContent, 'foo');
+ });
+
test('hash html with base url', () => {
window.CANONICAL_PATH = '/r';
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
index 897512b..cff345d 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
@@ -195,7 +195,7 @@
function(html, position, length, outputArray) {
if (this.hasOverlap(position, length, outputArray)) { return; }
if (!!this.baseUrl && html.match(/<a href=\"\//g) &&
- !html.match(`/<a href=\"${this.baseUrl}/g`)) {
+ !new RegExp(`<a href="${this.baseUrl}`, 'g').test(html)) {
html = html.replace(/<a href=\"\//g, `<a href=\"${this.baseUrl}\/`);
}
this.addItem(null, null, html, position, length, outputArray);
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index b63acda..87e4975 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -91,7 +91,8 @@
};
const JSON_PREFIX = ')]}\'';
const MAX_PROJECT_RESULTS = 25;
- const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 900;
+ // This value is somewhat arbitrary and not based on research or calculations.
+ const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
const PARENT_PATCH_NUM = 'PARENT';
const Requests = {
@@ -942,6 +943,8 @@
const req = {url: '/accounts/self/preferences', reportUrlAsIs: true};
return this._fetchSharedCacheURL(req).then(res => {
if (this._isNarrowScreen()) {
+ // Note that this can be problematic, because the diff will stay
+ // unified even after increasing the window width.
res.default_diff_view = DiffViewMode.UNIFIED;
} else {
res.default_diff_view = res.diff_view;
@@ -1092,7 +1095,6 @@
this.ListChangesOption.ALL_COMMITS,
this.ListChangesOption.ALL_REVISIONS,
this.ListChangesOption.CHANGE_ACTIONS,
- this.ListChangesOption.CURRENT_ACTIONS,
this.ListChangesOption.DETAILED_LABELS,
this.ListChangesOption.DOWNLOAD_COMMANDS,
this.ListChangesOption.MESSAGES,
