Merge changes from topic "gr-confirm-*-dialog-to-ts"
* changes:
Convert gr-confirm-*-dialog to typescript
Rename files to preserve history
diff --git a/.bazelversion b/.bazelversion
index 47b322c..1545d96 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-3.4.1
+3.5.0
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index d96ae46..6b89d67 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2828,7 +2828,8 @@
+
Enable (or disable) the `'$site_path'/logs/httpd_log` request log.
If enabled, an NCSA combined log format request log file is written
-out by the internal HTTP daemon.
+out by the internal HTTP daemon. The httpd log format is documented
+link:logs.html#_httpd_log[here].
+
`log4j.appender` with the name `httpd_log` can be configured to overwrite
programmatic configuration.
@@ -4881,6 +4882,21 @@
+
By default, 30s.
+[[sshd.gracefulStopTimeout]]sshd.gracefulStopTimeout::
++
+Set a graceful stop time. If set, Gerrit ensures that all open SSH
+sessions are preserved for a maximum period of time, before forcing the
+shutdown of the SSH daemon. During this period, no new requests
+will be accepted. This option is meant to be used in setups performing
+rolling restarts.
++
+Values should use common unit suffixes to express their setting:
++
+* s, sec, second, seconds
+* m, min, minute, minutes
++
+By default, 0 seconds (immediate shutdown).
+
[[sshd.maxConnectionsPerUser]]sshd.maxConnectionsPerUser::
+
Maximum number of concurrent SSH sessions that a user account
@@ -5006,6 +5022,7 @@
+
Enable (or disable) the `'$site_path'/logs/sshd_log` request log.
If enabled, a request log file is written out by the SSH daemon.
+The sshd log format is documented link:logs.html#_sshd_log[here].
+
`log4j.appender` with the name `sshd_log` can be configured to overwrite
programmatic configuration.
diff --git a/Documentation/config-reverseproxy.txt b/Documentation/config-reverseproxy.txt
index eff777b..3a9bcc7 100644
--- a/Documentation/config-reverseproxy.txt
+++ b/Documentation/config-reverseproxy.txt
@@ -21,6 +21,13 @@
listenUrl = proxy-http://127.0.0.1:8081/r/
----
+== Reverse proxy and client IPs
+
+When behind a reverse proxy the http_log will log the IP of the reverse proxy
+as client.ip. To log the correct client IP you must provide the
+'X-Forwarded-For' header from the reverse proxy.
+See the nginx configuration example below.
+
== Apache 2 Configuration
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 01cd494..ee2f4a1 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -90,12 +90,12 @@
```
$ cat << EOF > ~/.bazelrc
-> build --define=ABSOLUTE_JAVABASE=<path-to-java-13>
-> build --javabase=@bazel_tools//tools/jdk:absolute_javabase
-> build --host_javabase=@bazel_tools//tools/jdk:absolute_javabase
-> build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
-> build --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
-> EOF
+build --define=ABSOLUTE_JAVABASE=<path-to-java-13>
+build --javabase=@bazel_tools//tools/jdk:absolute_javabase
+build --host_javabase=@bazel_tools//tools/jdk:absolute_javabase
+build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+build --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+EOF
```
Now, invoking Bazel with just `bazel build :release` would include
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 78b2c15..c5b3bfc 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -27,14 +27,33 @@
leveraged to run tests at the Git protocol level.
Gatling is written in Scala, but the abstraction provided by the Gatling DSL makes the scenarios
-implementation easy even without any Scala knowledge. The
-link:https://gitenterprise.me/2019/12/20/stress-your-gerrit-with-gatling/[Stress your Gerrit with Gatling,role=external,window=_blank]
-blog post has more introductory information.
+implementation easy even without any Scala knowledge. The online `End-to-end tests`
+link:https://www.gerritcodereview.com/presentations.html#list-of-presentations[presentation,role=external,window=_blank]
+links posted on the homepage have more introductory information.
+
+== IDE: IntelliJ
Examples of scenarios can be found in the `e2e-tests` directory. The files in that directory should
be formatted using the mainstream
link:https://plugins.jetbrains.com/plugin/1347-scala[Scala plugin for IntelliJ,role=external,window=_blank].
The latter is not mandatory but preferred for `sbt` and Scala IDE purposes in this project.
+So, Eclipse can also be used alongside as a development IDE; this is described below.
+
+=== Eclipse
+
+1. Install the link:http://scala-ide.org/docs/user/gettingstarted.html[Scala plugin for Eclipse,role=external,window=_blank].
+1. Run `sbt eclipse` from the `e2e-tests` root directory.
+1. Import the resulting `e2e-tests` eclipse file inside the Gerrit project, in Eclipse.
+1. You should see errors in Eclipse telling you there are missing packages.
+1. This is due to the sbt-eclipse plugin not properly linking the Gerrit Gatling e2e tests with
+ Gatling Git plugin.
+1. You then have to right-click on the root directory and choose the build path->link source option.
+1. Then you have to browse to `.sbt/1.0/staging`, find the folder where gatling-git is contained,
+ and choose that.
+1. That last step should link the gatling-git plugin to the project; e2e tests should not show
+ errors anymore.
+1. You may get errors in the gatling-git directory; these should not affect Gerrit Gatling
+ development and can be ignored.
== How to build the tests
@@ -163,10 +182,11 @@
Plugin or otherwise non-core scenarios may do so just as well. The core java package
`com.google.gerrit.scenarios` from the example above has to be replaced with the one under which
those scenario classes are. Such extending scenarios can also add extension-specific properties.
-Early examples of this can be found in the Gerrit
-`link:https://gerrit.googlesource.com/plugins/high-availability[high-availability,role=external,window=_blank]`
-and `link:https://gerrit.googlesource.com/plugins/multi-site[multi-site,role=external,window=_blank]`
-plugins test code.
+Examples of this can be found in these Gerrit plugins test code:
+
+* `link:https://gerrit.googlesource.com/plugins/high-availability[high-availability,role=external,window=_blank]`
+* `link:https://gerrit.googlesource.com/plugins/multi-site[multi-site,role=external,window=_blank]`
+* `link:https://gerrit.googlesource.com/plugins/rename-project[rename-project,role=external,window=_blank]`
Further above, the `_PROJECT` keyword is prefixed with an underscore, which means that its value
gets automatically generated by the scenario. Any property setting for it is therefore not
diff --git a/Documentation/dev-eclipse.txt b/Documentation/dev-eclipse.txt
index 9596a55..742cf42 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -41,6 +41,37 @@
Filters on a folder, they will be overwritten the next time you run
`tools/eclipse/project.py`.
+=== Eclipse project on MacOS
+
+By default, bazel uses `/private/var/tmp` as the
+link:https://docs.bazel.build/versions/master/output_directories.html[outputRoot on MacOS].
+This means that the eclipse project will reference libraries stored under that directory.
+However, MacOS runs periodic cleanup task which deletes the content under `/private/var/tmp`
+which wasn't accessed or modified for some days, by default 3 days. This can lead to a broken
+Eclipse project as referenced libraries get deleted.
+
+There are two possibilities to mitigate this issue.
+
+==== Change the location of the bazel output directory
+On Linux, the output directory defaults to `$HOME/.cache/bazel` and the same can be configured
+on Mac too. Edit, or create, the `$HOME/.bazelrc` file and add the following line:
+----
+startup --output_user_root=/Users/johndoe/.cache/bazel
+----
+
+==== Increase the treshold for the cleanup of temporary files
+The default treshold for the cleanup can be overriden by creating a configuration file under
+`/etc/periodic.conf` and setting a larger value for the `daily_clean_tmps_days`.
+
+An example `/etc/periodic.conf` file:
+
+----
+# This file overrides the settings from /etc/defaults/periodic.conf
+daily_clean_tmps_days="45" # If not accessed for
+----
+
+For more details about the proposed workaround see link:https://superuser.com/a/187105[this post]
+
=== Eclipse project with custom plugins ===
To add custom plugins to the eclipse project add them to `tools/bzl/plugins.bzl`
diff --git a/Documentation/images/inline-edit-create-change-project-screen-dialog.png b/Documentation/images/inline-edit-create-change-project-screen-dialog.png
deleted file mode 100644
index ea5daa9..0000000
--- a/Documentation/images/inline-edit-create-change-project-screen-dialog.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change-project-screen.png b/Documentation/images/inline-edit-create-change-project-screen.png
deleted file mode 100644
index e9c7033..0000000
--- a/Documentation/images/inline-edit-create-change-project-screen.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-create-follow-up-change.png b/Documentation/images/inline-edit-create-follow-up-change.png
deleted file mode 100644
index 3e81eee..0000000
--- a/Documentation/images/inline-edit-create-follow-up-change.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png b/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png
deleted file mode 100644
index bdbc59d..0000000
--- a/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-edit-in-patch-list.png b/Documentation/images/inline-edit-edit-in-patch-list.png
deleted file mode 100644
index 9a31e02..0000000
--- a/Documentation/images/inline-edit-edit-in-patch-list.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 4fb977a..8f36ecc 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -69,6 +69,7 @@
. link:cmd-index.html[Command Line Tools]
. link:config-plugins.html#replication[Replication]
. link:config-plugins.html[Plugins]
+. link:logs.html[Log Files]
. link:metrics.html[Metrics]
. link:config-reverseproxy.html[Reverse Proxy]
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
diff --git a/Documentation/logs.txt b/Documentation/logs.txt
new file mode 100644
index 0000000..43a8e62
--- /dev/null
+++ b/Documentation/logs.txt
@@ -0,0 +1,174 @@
+= Gerrit Code Review - Logs
+
+Gerrit writes log files in the `$site_path/logs/` folder tracking requests,
+background and plugin activity and errors. By default logs are written in
+link:config-gerrit.html#log.textLogging[text format], optionally in
+link:config-gerrit.html#log.jsonLogging[JSON format].
+By default log files are link:config-gerrit.html#log.compress[compressed]
+at server startup and then daily at 11pm and
+link:config-gerrit.html#log.rotate[rotated] every midnight.
+
+== Logs
+
+The following logs can be written.
+
+=== HTTPD Log
+
+The httpd log tracks HTTP requests processed by Gerrit's http daemon
+and is written to `$site_path/logs/httpd_log`. Enabled or disabled via the
+link:config-gerrit.html#httpd.requestLog[httpd.requestLog] option.
+
+Format is an enhanced
+link:https://httpd.apache.org/docs/2.4/logs.html#combined[NCSA combined log],
+if a log field is not present, a "-" is substituted:
+
+* `host`: The IP address of the HTTP client that made the HTTP resource request.
+ If you are using a reverse proxy it depends on the proxy configuration if the
+ proxy IP address or the client IP address is logged.
+* `[thread name]`: name of the Java thread executing the request.
+* `remote logname`: the identifier used to
+ link: https://tools.ietf.org/html/rfc1413[identify the client making the HTTP request],
+ Gerrit always logs a dash `-`.
+* `username`: the username used by the client for authentication. "-" for
+ anonymous requests.
+* `[date:time]`: The date and time stamp of the HTTP request.
+ The time that the request was received. The format is until Gerrit 3.1
+ `[dd/MMM/yyyy:HH:mm:ss.SSS ZZZZ]`. For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `request`: The request line from the client is given in double quotes.
+** the HTTP method used by the client.
+** the resource the client requested.
+** the protocol/version used by the client.
+* `statuscode`: the link:https://tools.ietf.org/html/rfc2616#section-10[HTTP status code]
+ that the server sent back to the client.
+* `response size`: the number of bytes of data transferred as part of the HTTP
+ response, not including the HTTP header.
+* `latency`: response time in milliseconds.
+* `referer`: the `Referer` HTTP request header. This gives the site that
+ the client reports having been referred from.
+* `client agent`: the client agent which sent the request.
+
+Example:
+```
+12.34.56.78 [HTTP-4136374] - johndoe [28/Aug/2020:10:02:20 +0200] "GET /a/plugins/metrics-reporter-prometheus/metrics HTTP/1.1" 200 1247498 1900 - "Prometheus/2.13.1"
+```
+
+=== SSHD Log
+
+The sshd log tracks ssh requests processed by Gerrit's ssh daemon
+and is written to `$site_path/logs/sshd_log`. Enabled or disabled
+via option link:config-gerrit.html#sshd.requestLog[sshd.requestLog].
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `sessionid`: hexadecimal session identifier, all requests of the
+ same connection share the same sessionid. Gerrit does not support multiplexing multiple
+ sessions on the same connection. Grep the log file using the sessionid as filter to
+ get all requests from that session.
+* `[thread name]`: name of the Java thread executing the request.
+* `username`: the username used by the client for authentication.
+* `a/accountid`: identifier of the Gerrit account which is logged on.
+* `operation`: the operation being executed via ssh.
+** `LOGIN FROM <host>`: login and start new SSH session from the given host.
+** `AUTH FAILURE FROM <host> <message>`: failed authentication from given host and cause of failure.
+** `LOGOUT`: logout and terminate SSH session.
+** `git-upload-pack.<projectname>`: git fetch or clone command for given project.
+** `git-receive-pack.<projectname>`: git push command for given project.
+** Gerrit ssh commands which may be logged in this field are documented
+ link:cmd-index.html#_server[here].
+* `wait`: command wait time, time in milliseconds the command waited for an execution thread.
+* `exec`: command execution time, time in milliseconds to execute the command.
+* `status`: status code. 0 means success, any other value is an error.
+
+The `git-upload-pack` command provides the following additional fields after the `exec`
+and before the `status` field. All times are in milliseconds. Fields are -1 if not available
+when the upload-pack request returns an empty result since the client's repository was up to date:
+
+* `time negotiating`: time for negotiating which objects need to be transferred.
+* `time searching for reuse`: time jgit searched for deltas which can be reused.
+ That is the time spent matching existing representations against objects that
+ will be transmitted, or that the client can be assumed to already have.
+* `time searching for sizes`: time jgit was searching for sizes of all objects that
+ will enter the delta compression search window. The sizes need to
+ be known to better match similar objects together and improve
+ delta compression ratios.
+* `time counting`: time jgit spent enumerating the objects that need to
+ be included in the output. This time includes any restarts that
+ occur when a cached pack is selected for reuse.
+* `time compressing`: time jgit was compressing objects. This is observed
+ wall-clock time and does not accurately track CPU time used when
+ multiple threads were used to perform the delta compression.
+* `time writing`: time jgit needed to write packfile, from start of
+ header until end of trailer. The transfer speed can be
+ approximated by dividing `total bytes` by this value.
+* `total time in UploadPack`: total time jgit spent in upload-pack.
+* `bitmap index misses`: number of misses when trying to use bitmap index,
+ -1 means no bitmap index available. This is the count of objects that
+ needed to be discovered through an object walk because they were not found
+ in bitmap indices.
+* `total deltas`: total number of deltas transferred. This may be lower than the actual
+ number of deltas if a cached pack was reused.
+* `total objects`: total number of objects transferred. This total includes
+ the value of `total deltas`.
+* `total bytes`: total number of bytes transferred. This size includes the pack
+ header, trailer, thin pack, and reused cached packs.
+* `client agent`: the client agent and version which sent the request.
+
+Example: a CI system established a SSH connection, sent an upload-pack command (git fetch) and closed the connection:
+```
+[2020-08-28 11:00:01,391 +0200] 8a154cae [sshd-SshServer[570fc452]-nio2-thread-299] voter a/1000023 LOGIN FROM 12.34.56.78
+[2020-08-28 11:00:01,556 +0200] 8a154cae [SSH git-upload-pack /AP/ajs/jpaas-msg-svc.git (voter)] voter a/1000056 git-upload-pack./demo/project.git 0ms 115ms 92ms 1ms 0ms 6ms 0ms 0ms 7ms 3 10 26 2615 0 git/2.26.2
+[2020-08-28 11:00:01,583 +0200] 8a154cae [sshd-SshServer[570fc452]-nio2-thread-168] voter a/1000023 LOGOUT
+```
+
+=== Error Log
+
+The error log tracks errors and stack traces and is written to
+`$site_path/logs/error_log`.
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `[thread name]`: : name of the Java thread executing the request.
+* `level`: log level (ERROR, WARN, INFO, DEBUG).
+* `logger`: name of the logger.
+* `message`: log message.
+* `stacktrace`: Java stacktrace when an execption was caught, usually spans multiple lines.
+
+=== GC Log
+
+The gc log tracks git garbage collection running in a background thread
+if enabled and is written to `$site_path/logs/gc_log`.
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `level`: log level (ERROR, WARN, INFO, DEBUG).
+* `message`: log message.
+
+=== Plugin Logs
+
+Some plugins write their own log file.
+E.g. the replication plugin writes its log to `$site_path/logs/replication_log`.
+Refer to each plugin's documentation for more details on their logs.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/WORKSPACE b/WORKSPACE
index 6be35cf..4c2fe35 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -219,15 +219,15 @@
sha1 = GUAVA_BIN_SHA1,
)
-CAFFEINE_VERS = "2.8.0"
+CAFFEINE_VERS = "2.8.5"
maven_jar(
name = "caffeine",
artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
- sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
+ sha1 = "f0eafef6e1529a44e36549cd9d1fc06d3a57f384",
)
-CAFFEINE_GUAVA_SHA256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1"
+CAFFEINE_GUAVA_SHA256 = "a7ce6d29c40bccd688815a6734070c55b20cd326351a06886a6144005aa32299"
# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
# naming collision between caffeine guava adapter and guava library itself.
@@ -758,8 +758,8 @@
# Keep this version of Soy synchronized with the version used in Gitiles.
maven_jar(
name = "soy",
- artifact = "com.google.template:soy:2019-10-08",
- sha1 = "4518bf8bac2dbbed684849bc209c39c4cb546237",
+ artifact = "com.google.template:soy:2020-08-24",
+ sha1 = "e774bf5cc95923d2685292883fe219e231346e50",
)
maven_jar(
diff --git a/e2e-tests/build.sbt b/e2e-tests/build.sbt
index a322970..294212c 100644
--- a/e2e-tests/build.sbt
+++ b/e2e-tests/build.sbt
@@ -2,7 +2,6 @@
enablePlugins(GatlingPlugin)
-lazy val gatlingGitExtension = RootProject(uri("git://github.com/GerritForge/gatling-git.git"))
lazy val root = (project in file("."))
.settings(
inThisBuild(List(
@@ -12,8 +11,8 @@
)),
name := "gerrit",
libraryDependencies ++=
- gatling ++
+ gatling ++ gatlingGit ++
Seq("io.gatling" % "gatling-core" % GatlingVersion) ++
Seq("io.gatling" % "gatling-app" % GatlingVersion),
scalacOptions += "-language:postfixOps"
- ) dependsOn gatlingGitExtension
+ )
diff --git a/e2e-tests/project/Dependencies.scala b/e2e-tests/project/Dependencies.scala
index 63328f9..56ef740 100644
--- a/e2e-tests/project/Dependencies.scala
+++ b/e2e-tests/project/Dependencies.scala
@@ -2,9 +2,17 @@
object Dependencies {
val GatlingVersion = "3.2.0"
+ val GatlingGitVersion = "1.0.12"
lazy val gatling = Seq(
"io.gatling.highcharts" % "gatling-charts-highcharts",
"io.gatling" % "gatling-test-framework",
).map(_ % GatlingVersion % Test)
+
+ lazy val gatlingGit = Seq(
+ "com.gerritforge" %% "gatling-git" % GatlingGitVersion excludeAll(
+ ExclusionRule(organization = "io.gatling"),
+ ExclusionRule(organization = "io.gatling.highcharts")
+ )
+ )
}
diff --git a/e2e-tests/project/plugins.sbt b/e2e-tests/project/plugins.sbt
index 36cd201..9ed0f89 100644
--- a/e2e-tests/project/plugins.sbt
+++ b/e2e-tests/project/plugins.sbt
@@ -1 +1,2 @@
addSbtPlugin("io.gatling" % "gatling-sbt" % "3.0.0")
+addSbtPlugin("com.typesafe.sbteclipse" % "sbteclipse-plugin" % "5.2.4")
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
index bcf4708..282ac99 100644
--- a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
@@ -1,3 +1,4 @@
{
- "create_empty_commit": "true"
+ "create_empty_commit": "true",
+ "parent": "${parent}"
}
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
index cd90739..c141bb8 100644
--- a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
@@ -1,5 +1,6 @@
[
{
- "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT"
+ "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT",
+ "parent": "PARENT"
}
]
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
index d631292..f2b3d12 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
@@ -28,7 +28,7 @@
val test: ScenarioBuilder = scenario(unique)
.feed(data)
- .exec(httpRequest.body(RawFileBody(body)).asJson)
+ .exec(httpRequest.body(ElFileBody(body)).asJson)
setUp(
test.inject(
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
index 4832392..679320d 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
@@ -67,6 +67,8 @@
case ("number", number) =>
val precedes = replaceKeyWith("_number", 0, number.toString)
replaceProperty("number", 1, precedes)
+ case ("parent", parent) =>
+ replaceProperty("parent", "All-Projects", parent.toString)
case ("project", project) =>
var precedes = replaceKeyWith("_project", name, project.toString)
precedes = replaceOverride(precedes)
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 357ea0c..78a621c 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -429,13 +429,16 @@
baseConfig.setInt("receive", null, "changeUpdateThreads", 4);
Module module = createModule();
+ Module auditModule = createAuditModule();
if (classDesc.equals(methodDesc) && !classDesc.sandboxed() && !methodDesc.sandboxed()) {
if (commonServer == null) {
- commonServer = GerritServer.initAndStart(temporaryFolder, classDesc, baseConfig, module);
+ commonServer =
+ GerritServer.initAndStart(temporaryFolder, classDesc, baseConfig, module, auditModule);
}
server = commonServer;
} else {
- server = GerritServer.initAndStart(temporaryFolder, methodDesc, baseConfig, module);
+ server =
+ GerritServer.initAndStart(temporaryFolder, methodDesc, baseConfig, module, auditModule);
}
server.getTestInjector().injectMembers(this);
@@ -528,6 +531,11 @@
return null;
}
+ /** Override to bind an alternative audit Guice module */
+ public Module createAuditModule() {
+ return null;
+ }
+
protected void initSsh() throws Exception {
if (testRequiresSsh
&& SshMode.useSsh()
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 5206957..5942c0f 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -35,6 +35,7 @@
import com.google.gerrit.acceptance.testsuite.change.PerCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.change.PerDraftCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.change.PerPatchsetOperationsImpl;
+import com.google.gerrit.acceptance.testsuite.change.PerRobotCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.group.GroupOperationsImpl;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -329,14 +330,15 @@
TemporaryFolder temporaryFolder,
Description desc,
Config baseConfig,
- @Nullable Module testSysModule)
+ @Nullable Module testSysModule,
+ @Nullable Module testAuditModule)
throws Exception {
Path site = temporaryFolder.newFolder().toPath();
try {
if (!desc.memory()) {
init(desc, baseConfig, site);
}
- return start(desc, baseConfig, site, testSysModule, null);
+ return start(desc, baseConfig, site, testSysModule, testAuditModule, null);
} catch (Exception e) {
throw e;
}
@@ -364,6 +366,7 @@
Config baseConfig,
Path site,
@Nullable Module testSysModule,
+ @Nullable Module testAuditModule,
@Nullable InMemoryRepositoryManager inMemoryRepoManager,
String... additionalArgs)
throws Exception {
@@ -382,7 +385,8 @@
},
site);
daemon.setEmailModuleForTesting(new FakeEmailSender.Module());
- daemon.setAuditEventModuleForTesting(new FakeGroupAuditService.Module());
+ daemon.setAuditEventModuleForTesting(
+ MoreObjects.firstNonNull(testAuditModule, new FakeGroupAuditService.Module()));
if (testSysModule != null) {
daemon.addAdditionalSysModuleForTesting(testSysModule);
}
@@ -515,6 +519,7 @@
factory(PerPatchsetOperationsImpl.Factory.class);
factory(PerCommentOperationsImpl.Factory.class);
factory(PerDraftCommentOperationsImpl.Factory.class);
+ factory(PerRobotCommentOperationsImpl.Factory.class);
factory(PushOneCommit.Factory.class);
install(InProcessProtocol.module());
install(new NoSshModule());
@@ -609,7 +614,7 @@
server.close();
server.daemon.stop();
- return start(server.desc, cfg, site, null, inMemoryRepoManager);
+ return start(server.desc, cfg, site, null, null, inMemoryRepoManager);
}
private static boolean hasBinding(Injector injector, Class<?> clazz) {
diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
index c38f5fa..43fe4eb 100644
--- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
+++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -187,7 +187,7 @@
private GerritServer startImpl(@Nullable Module testSysModule, String... additionalArgs)
throws Exception {
return GerritServer.start(
- serverDesc, baseConfig, sitePaths.site_path, testSysModule, null, additionalArgs);
+ serverDesc, baseConfig, sitePaths.site_path, testSysModule, null, null, additionalArgs);
}
protected static void runGerrit(String... args) throws Exception {
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
index aa0391d..c4e4192 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
@@ -127,5 +127,13 @@
* @return an aggregation of operations on a specific draft comment
*/
PerDraftCommentOperations draftComment(String commentUuid);
+
+ /**
+ * Starts the fluent chain for querying or modifying a robot comment. Please see the methods of
+ * {@link PerRobotCommentOperations} for details on possible operations.
+ *
+ * @return an aggregation of operations on a specific robot comment
+ */
+ PerRobotCommentOperations robotComment(String commentUuid);
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index f04de17..3b15b57 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -87,6 +87,7 @@
private final PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory;
private final PerCommentOperationsImpl.Factory perCommentOperationsFactory;
private final PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory;
+ private final PerRobotCommentOperationsImpl.Factory perRobotCommentOperationsFactory;
@Inject
public ChangeOperationsImpl(
@@ -102,7 +103,8 @@
ChangeFinder changeFinder,
PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory,
PerCommentOperationsImpl.Factory perCommentOperationsFactory,
- PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory) {
+ PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory,
+ PerRobotCommentOperationsImpl.Factory perRobotCommentOperationsFactory) {
this.seq = seq;
this.changeInserterFactory = changeInserterFactory;
this.patchsetInserterFactory = patchsetInserterFactory;
@@ -116,6 +118,7 @@
this.perPatchsetOperationsFactory = perPatchsetOperationsFactory;
this.perCommentOperationsFactory = perCommentOperationsFactory;
this.perDraftCommentOperationsFactory = perDraftCommentOperationsFactory;
+ this.perRobotCommentOperationsFactory = perRobotCommentOperationsFactory;
}
@Override
@@ -555,5 +558,11 @@
ChangeNotes changeNotes = getChangeNotes();
return perDraftCommentOperationsFactory.create(changeNotes, commentUuid);
}
+
+ @Override
+ public PerRobotCommentOperations robotComment(String commentUuid) {
+ ChangeNotes changeNotes = getChangeNotes();
+ return perRobotCommentOperationsFactory.create(changeNotes, commentUuid);
+ }
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java b/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java
new file mode 100644
index 0000000..b7e720b
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java
@@ -0,0 +1,36 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+/**
+ * Marks the commit that contains the comment (also known as side). Used by {@link
+ * TestCommentCreation} and {@link TestRobotCommentCreation}.
+ */
+enum CommentSide {
+ PATCHSET_COMMIT(1),
+ AUTO_MERGE_COMMIT(0),
+ PARENT_COMMIT(-1),
+ SECOND_PARENT_COMMIT(-2);
+
+ private final short numericSide;
+
+ CommentSide(int numericSide) {
+ this.numericSide = (short) numericSide;
+ }
+
+ public short getNumericSide() {
+ return numericSide;
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/FileBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/FileBuilder.java
new file mode 100644
index 0000000..c8514a7
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/FileBuilder.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import java.util.function.Function;
+
+/**
+ * Builder for the file specification of line/range comments. Used by {@link TestCommentCreation}
+ * and {@link TestRobotCommentCreation}.
+ */
+public class FileBuilder<T> {
+ private final Function<String, T> nextStepProvider;
+
+ public FileBuilder(Function<String, T> nextStepProvider) {
+ this.nextStepProvider = nextStepProvider;
+ }
+ /** File on which the comment should be added. */
+ public T ofFile(String file) {
+ return nextStepProvider.apply(file);
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
index 5f046ca..0218731 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
@@ -56,6 +56,11 @@
}
static TestHumanComment toTestComment(HumanComment comment) {
- return TestHumanComment.builder().uuid(comment.key.uuid).parentUuid(comment.parentUuid).build();
+ return TestHumanComment.builder()
+ .uuid(comment.key.uuid)
+ .parentUuid(comment.parentUuid)
+ .tag(comment.tag)
+ .unresolved(comment.unresolved)
+ .build();
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
index 33d2d43..f4c70bd 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
@@ -67,4 +67,25 @@
* @return builder to create a new comment
*/
TestCommentCreation.Builder newDraftComment();
+
+ /**
+ * Starts the fluent chain to create a new robot comment. The returned builder can be used to
+ * specify the attributes of the robot comment. To create the robot comment for real, {@link
+ * TestRobotCommentCreation.Builder#create()} must be called.
+ *
+ * <p>Example:
+ *
+ * <pre>
+ * String createdRobotCommentUuid = changeOperations
+ * .change(changeId)
+ * .currentPatchset()
+ * .newRobotComment()
+ * .onLine(2)
+ * .ofFile("file1")
+ * .create();
+ * </pre>
+ *
+ * @return builder to create a new comment
+ */
+ TestRobotCommentCreation.Builder newRobotComment();
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
index d39f1e1..20f9c49 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
@@ -16,13 +16,13 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
-import com.google.gerrit.acceptance.testsuite.change.TestCommentCreation.CommentSide;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment.Status;
import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -101,6 +101,11 @@
return TestCommentCreation.builder(this::createComment, Status.DRAFT);
}
+ @Override
+ public TestRobotCommentCreation.Builder newRobotComment() {
+ return TestRobotCommentCreation.builder(this::createRobotComment);
+ }
+
private String createComment(TestCommentCreation commentCreation)
throws IOException, RestApiException, UpdateException {
Project.NameKey project = changeNotes.getProjectName();
@@ -126,6 +131,20 @@
return userFactory.create(authorId);
}
+ private IdentifiedUser getAuthor(TestRobotCommentCreation robotCommentCreation) {
+ Account.Id authorId = robotCommentCreation.author().orElse(changeNotes.getChange().getOwner());
+ return userFactory.create(authorId);
+ }
+
+ private static Comment.Range toCommentRange(TestRange range) {
+ Comment.Range commentRange = new Range();
+ commentRange.startLine = range.start().line();
+ commentRange.startCharacter = range.start().charOffset();
+ commentRange.endLine = range.end().line();
+ commentRange.endCharacter = range.end().charOffset();
+ return commentRange;
+ }
+
private class CommentAdditionOp implements BatchUpdateOp {
private String createdCommentUuid;
private final TestCommentCreation commentCreation;
@@ -173,7 +192,7 @@
// Specification of range trumps explicit line specification.
commentCreation
.range()
- .map(this::toCommentRange)
+ .map(PerPatchsetOperationsImpl::toCommentRange)
.ifPresent(range -> newComment.setLineNbrAndRange(null, range));
setCommentCommitId(
@@ -183,14 +202,89 @@
changeNotes.getPatchSets().get(patchsetId));
return newComment;
}
+ }
- private Comment.Range toCommentRange(TestRange range) {
- Comment.Range commentRange = new Range();
- commentRange.startLine = range.start().line();
- commentRange.startCharacter = range.start().charOffset();
- commentRange.endLine = range.end().line();
- commentRange.endCharacter = range.end().charOffset();
- return commentRange;
+ private String createRobotComment(TestRobotCommentCreation robotCommentCreation)
+ throws IOException, RestApiException, UpdateException {
+ Project.NameKey project = changeNotes.getProjectName();
+
+ try (Repository repository = repositoryManager.openRepository(project);
+ ObjectInserter objectInserter = repository.newObjectInserter();
+ RevWalk revWalk = new RevWalk(objectInserter.newReader())) {
+ Timestamp now = TimeUtil.nowTs();
+
+ IdentifiedUser author = getAuthor(robotCommentCreation);
+ RobotCommentAdditionOp robotCommentAdditionOp =
+ new RobotCommentAdditionOp(robotCommentCreation);
+ try (BatchUpdate batchUpdate = batchUpdateFactory.create(project, author, now)) {
+ batchUpdate.setRepository(repository, revWalk, objectInserter);
+ batchUpdate.addOp(changeNotes.getChangeId(), robotCommentAdditionOp);
+ batchUpdate.execute();
+ }
+ return robotCommentAdditionOp.createdRobotCommentUuid;
+ }
+ }
+
+ private class RobotCommentAdditionOp implements BatchUpdateOp {
+ private String createdRobotCommentUuid;
+ private final TestRobotCommentCreation robotCommentCreation;
+
+ public RobotCommentAdditionOp(TestRobotCommentCreation robotCommentCreation) {
+ this.robotCommentCreation = robotCommentCreation;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext context) throws Exception {
+ RobotComment robotComment = toNewRobotComment(context, robotCommentCreation);
+ ChangeUpdate changeUpdate = context.getUpdate(patchsetId);
+ changeUpdate.putRobotComment(robotComment);
+ // For robot comments, only the tag set on the ChangeUpdate (and not on the RobotComment)
+ // matters.
+ robotCommentCreation.tag().ifPresent(changeUpdate::setTag);
+ createdRobotCommentUuid = robotComment.key.uuid;
+ return true;
+ }
+
+ private RobotComment toNewRobotComment(
+ ChangeContext context, TestRobotCommentCreation robotCommentCreation)
+ throws PatchListNotAvailableException {
+ String message = robotCommentCreation.message().orElse("The text of a test robot comment.");
+
+ String filePath = robotCommentCreation.file().orElse(Patch.PATCHSET_LEVEL);
+ short side = robotCommentCreation.side().orElse(CommentSide.PATCHSET_COMMIT).getNumericSide();
+ String robotId = robotCommentCreation.robotId().orElse("robot");
+ String robotRunId = robotCommentCreation.robotId().orElse("1");
+ RobotComment newRobotComment =
+ commentsUtil.newRobotComment(
+ context, filePath, patchsetId, side, message, robotId, robotRunId);
+
+ // TODO(paiking): This should not be needed, as the tag only matters in ChangeUpdate.
+ robotCommentCreation.tag().ifPresent(tag -> newRobotComment.tag = tag);
+
+ robotCommentCreation.line().ifPresent(line -> newRobotComment.setLineNbrAndRange(line, null));
+ // Specification of range trumps explicit line specification.
+ robotCommentCreation
+ .range()
+ .map(PerPatchsetOperationsImpl::toCommentRange)
+ .ifPresent(range -> newRobotComment.setLineNbrAndRange(null, range));
+
+ robotCommentCreation
+ .unresolved()
+ .ifPresent(unresolved -> newRobotComment.unresolved = unresolved);
+ robotCommentCreation
+ .parentUuid()
+ .ifPresent(parentUuid -> newRobotComment.parentUuid = parentUuid);
+ robotCommentCreation.url().ifPresent(url -> newRobotComment.url = url);
+ if (!robotCommentCreation.properties().isEmpty()) {
+ newRobotComment.properties = robotCommentCreation.properties();
+ }
+
+ setCommentCommitId(
+ newRobotComment,
+ patchListCache,
+ context.getChange(),
+ changeNotes.getPatchSets().get(patchsetId));
+ return newRobotComment;
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java
new file mode 100644
index 0000000..c9718aa
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+/** An aggregation of methods on a specific, robot comment. */
+public interface PerRobotCommentOperations {
+
+ /**
+ * Retrieves the robot comment.
+ *
+ * <p><strong>Note:</strong> This call will fail with an exception if the requested comment
+ * doesn't exist or if it is a comment of another type.
+ *
+ * @return the corresponding {@code TestRobotComment}
+ */
+ TestRobotComment get();
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java
new file mode 100644
index 0000000..075c451
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import static com.google.common.collect.MoreCollectors.onlyElement;
+
+import com.google.gerrit.entities.RobotComment;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+/**
+ * The implementation of {@link PerRobotCommentOperations}.
+ *
+ * <p>There is only one implementation of {@link PerRobotCommentOperations}. Nevertheless, we keep
+ * the separation between interface and implementation to enhance clarity.
+ */
+public class PerRobotCommentOperationsImpl implements PerRobotCommentOperations {
+ private final CommentsUtil commentsUtil;
+
+ private final ChangeNotes changeNotes;
+ private final String commentUuid;
+
+ public interface Factory {
+ PerRobotCommentOperationsImpl create(ChangeNotes changeNotes, String commentUuid);
+ }
+
+ @Inject
+ public PerRobotCommentOperationsImpl(
+ CommentsUtil commentsUtil, @Assisted ChangeNotes changeNotes, @Assisted String commentUuid) {
+ this.commentsUtil = commentsUtil;
+ this.changeNotes = changeNotes;
+ this.commentUuid = commentUuid;
+ }
+
+ @Override
+ public TestRobotComment get() {
+ RobotComment comment =
+ commentsUtil.robotCommentsByChange(changeNotes).stream()
+ .filter(foundComment -> foundComment.key.uuid.equals(commentUuid))
+ .collect(onlyElement());
+ return toTestRobotComment(comment);
+ }
+
+ static TestRobotComment toTestRobotComment(RobotComment robotComment) {
+ return TestRobotComment.builder()
+ .uuid(robotComment.key.uuid)
+ .parentUuid(robotComment.parentUuid)
+ .build();
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java
new file mode 100644
index 0000000..b061c81
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import java.util.function.IntFunction;
+
+/**
+ * Builder to simplify a position specification. Used by {@link TestCommentCreation} and {@link
+ * TestRobotCommentCreation}.
+ */
+public class PositionBuilder<T> {
+ private final IntFunction<T> nextStepProvider;
+
+ public PositionBuilder(IntFunction<T> nextStepProvider) {
+ this.nextStepProvider = nextStepProvider;
+ }
+
+ /** Character offset within the line. A value of 0 indicates the beginning of the line. */
+ public T charOffset(int characterOffset) {
+ return nextStepProvider.apply(characterOffset);
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java
new file mode 100644
index 0000000..c9a0eff
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+/**
+ * Builder for the end position of a range. Used by {@link TestCommentCreation} and {@link
+ * TestRobotCommentCreation}.
+ */
+public class StartAwarePositionBuilder<T> {
+ private final TestRange.Builder testRangeBuilder;
+ private final Consumer<TestRange> rangeConsumer;
+ private final Function<String, T> fileFunction;
+
+ public StartAwarePositionBuilder(
+ TestRange.Builder testRangeBuilder,
+ Consumer<TestRange> rangeConsumer,
+ Function<String, T> fileFunction) {
+ this.testRangeBuilder = testRangeBuilder;
+ this.rangeConsumer = rangeConsumer;
+ this.fileFunction = fileFunction;
+ }
+
+ /** Line of the end position of the range. */
+ public PositionBuilder<FileBuilder<T>> toLine(int endLine) {
+ return new PositionBuilder<>(
+ endCharOffset -> {
+ TestRange.Position end =
+ TestRange.Position.builder().line(endLine).charOffset(endCharOffset).build();
+ TestRange range = testRangeBuilder.setEnd(end).build();
+ rangeConsumer.accept(range);
+ return new FileBuilder<T>(fileFunction);
+ });
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
index d1d1567..2f4ddd0 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
@@ -22,10 +22,12 @@
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
import java.util.Optional;
-import java.util.function.Function;
-import java.util.function.IntFunction;
-/** Attributes of the comment. If not provided, arbitrary values will be used. */
+/**
+ * Attributes of the human comment. If not provided, arbitrary values will be used. This class is
+ * very similar to {@link TestRobotCommentCreation} to allow separation between robot and human
+ * comments.
+ */
@AutoValue
public abstract class TestCommentCreation {
@@ -51,7 +53,7 @@
abstract ThrowingFunction<TestCommentCreation, String> commentCreator();
- public static TestCommentCreation.Builder builder(
+ public static Builder builder(
ThrowingFunction<TestCommentCreation, String> commentCreator, Comment.Status commentStatus) {
return new AutoValue_TestCommentCreation.Builder()
.commentCreator(commentCreator)
@@ -82,8 +84,8 @@
* Starts the fluent change to create a line comment. The line comment will be at the indicated
* line. Lines start with 1.
*/
- public FileBuilder onLine(int line) {
- return new FileBuilder(file -> file(file).line(line).range(null));
+ public FileBuilder<Builder> onLine(int line) {
+ return new FileBuilder<>(file -> file(file).line(line).range(null));
}
/**
@@ -91,12 +93,12 @@
* Lines start at 1. The start position (line, charOffset) is inclusive, the end position (line,
* charOffset) is exclusive.
*/
- public PositionBuilder<StartAwarePositionBuilder> fromLine(int startLine) {
+ public PositionBuilder<StartAwarePositionBuilder<Builder>> fromLine(int startLine) {
return new PositionBuilder<>(
startCharOffset -> {
Position start = Position.builder().line(startLine).charOffset(startCharOffset).build();
TestRange.Builder testRangeBuilder = TestRange.builder().setStart(start);
- return new StartAwarePositionBuilder(this, testRangeBuilder);
+ return new StartAwarePositionBuilder<>(testRangeBuilder, this::range, this::file);
});
}
@@ -179,8 +181,7 @@
*/
abstract Builder status(Comment.Status value);
- abstract TestCommentCreation.Builder commentCreator(
- ThrowingFunction<TestCommentCreation, String> commentCreator);
+ abstract Builder commentCreator(ThrowingFunction<TestCommentCreation, String> commentCreator);
abstract TestCommentCreation autoBuild();
@@ -194,72 +195,4 @@
return commentCreation.commentCreator().applyAndThrowSilently(commentCreation);
}
}
-
- /** Builder for the file specification of line/range comments. */
- public static class FileBuilder {
- private final Function<String, Builder> nextStepProvider;
-
- private FileBuilder(Function<String, Builder> nextStepProvider) {
- this.nextStepProvider = nextStepProvider;
- }
-
- /** File on which the comment should be added. */
- public Builder ofFile(String file) {
- return nextStepProvider.apply(file);
- }
- }
-
- /** Builder to simplify a position specification. */
- public static class PositionBuilder<T> {
- private final IntFunction<T> nextStepProvider;
-
- private PositionBuilder(IntFunction<T> nextStepProvider) {
- this.nextStepProvider = nextStepProvider;
- }
-
- /** Character offset within the line. A value of 0 indicates the beginning of the line. */
- public T charOffset(int characterOffset) {
- return nextStepProvider.apply(characterOffset);
- }
- }
-
- /** Builder for the end position of a range. */
- public static class StartAwarePositionBuilder {
- private final TestCommentCreation.Builder testCommentCreationBuilder;
- private final TestRange.Builder testRangeBuilder;
-
- private StartAwarePositionBuilder(
- Builder testCommentCreationBuilder, TestRange.Builder testRangeBuilder) {
- this.testCommentCreationBuilder = testCommentCreationBuilder;
- this.testRangeBuilder = testRangeBuilder;
- }
-
- /** Line of the end position of the range. */
- public PositionBuilder<FileBuilder> toLine(int endLine) {
- return new PositionBuilder<>(
- endCharOffset -> {
- Position end = Position.builder().line(endLine).charOffset(endCharOffset).build();
- TestRange range = testRangeBuilder.setEnd(end).build();
- testCommentCreationBuilder.range(range);
- return new FileBuilder(testCommentCreationBuilder::file);
- });
- }
- }
-
- enum CommentSide {
- PATCHSET_COMMIT(1),
- AUTO_MERGE_COMMIT(0),
- PARENT_COMMIT(-1),
- SECOND_PARENT_COMMIT(-2);
-
- private final short numericSide;
-
- CommentSide(int numericSide) {
- this.numericSide = (short) numericSide;
- }
-
- public short getNumericSide() {
- return numericSide;
- }
- }
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java b/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
index 9bb026f..3a7f2ae 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
@@ -28,6 +28,12 @@
/** UUID of another comment to which this comment is a reply. */
public abstract Optional<String> parentUuid();
+ /** Tag of a comment. */
+ public abstract Optional<String> tag();
+
+ /** Unresolved state of a comment. */
+ public abstract boolean unresolved();
+
static Builder builder() {
return new AutoValue_TestHumanComment.Builder();
}
@@ -38,6 +44,10 @@
abstract Builder parentUuid(@Nullable String parentUuid);
+ abstract Builder tag(@Nullable String tag);
+
+ abstract Builder unresolved(boolean unresolved);
+
abstract TestHumanComment build();
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java
new file mode 100644
index 0000000..76fb52f
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import java.util.Optional;
+
+/** Representation of a robot comment used for testing purposes. */
+@AutoValue
+public abstract class TestRobotComment {
+
+ /** The UUID of the comment. Should be unique. */
+ public abstract String uuid();
+
+ /** UUID of another comment to which this comment is a reply. */
+ public abstract Optional<String> parentUuid();
+
+ static Builder builder() {
+ return new AutoValue_TestRobotComment.Builder();
+ }
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder uuid(String uuid);
+
+ abstract Builder parentUuid(@Nullable String parentUuid);
+
+ abstract TestRobotComment build();
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java
new file mode 100644
index 0000000..809190d
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java
@@ -0,0 +1,214 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.testsuite.change;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import com.google.gerrit.acceptance.testsuite.change.TestRange.Position;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Patch;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * Attributes of the robot comment. If not provided, arbitrary values will be used. This class is
+ * very similar to {@link TestCommentCreation} to allow separation between robot and human comments.
+ */
+@AutoValue
+public abstract class TestRobotCommentCreation {
+
+ public abstract Optional<String> message();
+
+ public abstract Optional<String> file();
+
+ public abstract Optional<Integer> line();
+
+ public abstract Optional<TestRange> range();
+
+ public abstract Optional<CommentSide> side();
+
+ public abstract Optional<Boolean> unresolved();
+
+ public abstract Optional<String> parentUuid();
+
+ public abstract Optional<String> tag();
+
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<String> robotId();
+
+ public abstract Optional<String> robotRunId();
+
+ public abstract Optional<String> url();
+
+ public abstract ImmutableMap<String, String> properties();
+
+ abstract ThrowingFunction<TestRobotCommentCreation, String> commentCreator();
+
+ public static Builder builder(ThrowingFunction<TestRobotCommentCreation, String> commentCreator) {
+ return new AutoValue_TestRobotCommentCreation.Builder().commentCreator(commentCreator);
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public Builder noMessage() {
+ return message("");
+ }
+
+ /** Message text of the comment. */
+ public abstract Builder message(String message);
+
+ /** Indicates a patchset-level comment. */
+ public Builder onPatchsetLevel() {
+ return file(Patch.PATCHSET_LEVEL);
+ }
+
+ /** Indicates a file comment. The comment will be on the specified file. */
+ public Builder onFileLevelOf(String filePath) {
+ return file(filePath).line(null).range(null);
+ }
+
+ /**
+ * Starts the fluent change to create a line comment. The line comment will be at the indicated
+ * line. Lines start with 1.
+ */
+ public FileBuilder<Builder> onLine(int line) {
+ return new FileBuilder<>(file -> file(file).line(line).range(null));
+ }
+
+ /**
+ * Starts the fluent chain to create a range comment. The range begins at the specified line.
+ * Lines start at 1. The start position (line, charOffset) is inclusive, the end position (line,
+ * charOffset) is exclusive.
+ */
+ public PositionBuilder<StartAwarePositionBuilder<Builder>> fromLine(int startLine) {
+ return new PositionBuilder<>(
+ startCharOffset -> {
+ Position start = Position.builder().line(startLine).charOffset(startCharOffset).build();
+ TestRange.Builder testRangeBuilder = TestRange.builder().setStart(start);
+ return new StartAwarePositionBuilder<>(testRangeBuilder, this::range, this::file);
+ });
+ }
+
+ /** File on which the comment should be added. */
+ abstract Builder file(String filePath);
+
+ /** Line on which the comment should be added. */
+ abstract Builder line(@Nullable Integer line);
+
+ /** Range on which the comment should be added. */
+ abstract Builder range(@Nullable TestRange range);
+
+ /**
+ * Indicates that the comment refers to a file, line, range, ... in the commit of the patchset.
+ *
+ * <p>On the UI, such comments are shown on the right side of a diff view when a diff against
+ * base is selected. See {@link #onParentCommit()} for comments shown on the left side.
+ */
+ public Builder onPatchsetCommit() {
+ return side(CommentSide.PATCHSET_COMMIT);
+ }
+
+ /**
+ * Indicates that the comment refers to a file, line, range, ... in the parent commit of the
+ * patchset.
+ *
+ * <p>On the UI, such comments are shown on the left side of a diff view when a diff against
+ * base is selected. See {@link #onPatchsetCommit()} for comments shown on the right side.
+ *
+ * <p>For merge commits, this indicates the first parent commit.
+ */
+ public Builder onParentCommit() {
+ return side(CommentSide.PARENT_COMMIT);
+ }
+
+ /** Like {@link #onParentCommit()} but for the second parent of a merge commit. */
+ public Builder onSecondParentCommit() {
+ return side(CommentSide.SECOND_PARENT_COMMIT);
+ }
+
+ /**
+ * Like {@link #onParentCommit()} but for the AutoMerge commit created from the parents of a
+ * merge commit.
+ */
+ public Builder onAutoMergeCommit() {
+ return side(CommentSide.AUTO_MERGE_COMMIT);
+ }
+
+ abstract Builder side(CommentSide side);
+
+ /** Indicates a resolved comment. */
+ public Builder resolved() {
+ return unresolved(false);
+ }
+
+ /** Indicates an unresolved comment. */
+ public Builder unresolved() {
+ return unresolved(true);
+ }
+
+ abstract Builder unresolved(boolean unresolved);
+
+ /**
+ * UUID of another comment to which this comment is a reply. This comment must have similar
+ * attributes (e.g. file, line, side) as the parent comment. The parent comment must be a
+ * published comment.
+ */
+ public abstract Builder parentUuid(String parentUuid);
+
+ /** Tag to attach to the comment. */
+ public abstract Builder tag(String value);
+
+ /** Author of the comment. Must be an existing user account. */
+ public abstract Builder author(Account.Id accountId);
+
+ /** Id of the robot that created the comment. */
+ public abstract Builder robotId(String robotId);
+
+ /** An ID of the run of the robot that created the comment. */
+ public abstract Builder robotRunId(String robotRunId);
+
+ /** Url for more information for the robot comment. */
+ public abstract Builder url(String url);
+
+ /** Robot specific properties as map that maps arbitrary keys to values. */
+ public abstract Builder properties(Map<String, String> properties);
+
+ abstract ImmutableMap.Builder<String, String> propertiesBuilder();
+
+ public Builder addProperty(String key, String value) {
+ propertiesBuilder().put(key, value);
+ return this;
+ }
+
+ abstract Builder commentCreator(
+ ThrowingFunction<TestRobotCommentCreation, String> commentCreator);
+
+ abstract TestRobotCommentCreation autoBuild();
+
+ /**
+ * Creates the robot comment.
+ *
+ * @return the UUID of the created robot comment
+ */
+ public String create() {
+ TestRobotCommentCreation commentCreation = autoBuild();
+ return commentCreation.commentCreator().applyAndThrowSilently(commentCreation);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
index dd226ed..5176145 100644
--- a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.truth.ListSubject.elements;
import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.MapSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
@@ -59,6 +60,26 @@
return check("path").that(robotCommentInfo.path);
}
+ public StringSubject robotId() {
+ isNotNull();
+ return check("robotId").that(robotCommentInfo.robotId);
+ }
+
+ public StringSubject robotRunId() {
+ isNotNull();
+ return check("robotRunId").that(robotCommentInfo.robotRunId);
+ }
+
+ public StringSubject url() {
+ isNotNull();
+ return check("url").that(robotCommentInfo.url);
+ }
+
+ public MapSubject properties() {
+ isNotNull();
+ return check("property").that(robotCommentInfo.properties);
+ }
+
public FixSuggestionInfoSubject onlyFixSuggestion() {
return fixSuggestions().onlyElement();
}
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 64a6aa3..28407a7 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -491,12 +491,13 @@
modules.add(new AccountDeactivator.Module());
modules.add(new ChangeCleanupRunner.Module());
}
- modules.addAll(testSysModules);
modules.add(new LocalMergeSuperSetComputation.Module());
modules.add(new DefaultProjectNameLockManager.Module());
- return cfgInjector.createChildInjector(
- ModuleOverloader.override(
- modules, LibModuleLoader.loadModules(cfgInjector, LibModuleType.SYS_MODULE)));
+
+ List<Module> libModules = LibModuleLoader.loadModules(cfgInjector, LibModuleType.SYS_MODULE);
+ libModules.addAll(testSysModules);
+
+ return cfgInjector.createChildInjector(ModuleOverloader.override(modules, libModules));
}
private Module createIndexModule() {
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index 30913f7..b83eae8 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -189,6 +189,12 @@
.findFirst();
}
+ public Optional<HumanComment> getPublishedHumanComment(ChangeNotes notes, String uuid) {
+ return publishedHumanCommentsByChange(notes).stream()
+ .filter(c -> c.key.uuid.equals(uuid))
+ .findFirst();
+ }
+
public Optional<HumanComment> getDraft(ChangeNotes notes, IdentifiedUser user, Comment.Key key) {
return draftByChangeAuthor(notes, user.getAccountId()).stream()
.filter(c -> key.equals(c.key))
@@ -205,6 +211,10 @@
return sort(Lists.newArrayList(notes.getRobotComments().values()));
}
+ public Optional<RobotComment> getRobotComment(ChangeNotes notes, String uuid) {
+ return robotCommentsByChange(notes).stream().filter(c -> c.key.uuid.equals(uuid)).findFirst();
+ }
+
public List<HumanComment> draftByChange(ChangeNotes notes) {
List<HumanComment> comments = new ArrayList<>();
for (Ref ref : getDraftRefs(notes.getChangeId())) {
diff --git a/java/com/google/gerrit/server/change/DraftCommentResource.java b/java/com/google/gerrit/server/change/DraftCommentResource.java
index e0648cf..19a495d 100644
--- a/java/com/google/gerrit/server/change/DraftCommentResource.java
+++ b/java/com/google/gerrit/server/change/DraftCommentResource.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.inject.TypeLiteral;
public class DraftCommentResource implements RestResource {
@@ -50,6 +51,10 @@
return comment;
}
+ public ChangeNotes getNotes() {
+ return rev.getNotes();
+ }
+
public String getId() {
return comment.key.uuid;
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
index b749e6a..399da8f 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
@@ -81,6 +81,11 @@
throw new BadRequestException("line must be >= 0");
} else if (in.line != null && in.range != null && in.line != in.range.endLine) {
throw new BadRequestException("range endLine must be on the same line as the comment");
+ } else if (in.inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(rsrc.getNotes(), in.inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(rsrc.getNotes(), in.inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", in.inReplyTo));
}
try (BatchUpdate bu =
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 9064dc8..32c1656 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -616,6 +616,7 @@
ensureCommentNotOnMagicFilesOfAutoMerge(path, comment);
ensureRangeIsValid(path, comment.range);
ensureValidPatchsetLevelComment(path, comment);
+ ensureValidInReplyTo(revision.getNotes(), comment.inReplyTo);
}
}
}
@@ -662,6 +663,16 @@
}
}
+ private void ensureValidInReplyTo(ChangeNotes changeNotes, String inReplyTo)
+ throws BadRequestException {
+ if (inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(changeNotes, inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(changeNotes, inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", inReplyTo));
+ }
+ }
+
private void checkRobotComments(
RevisionResource revision, Map<String, List<RobotCommentInput>> in)
throws BadRequestException, PatchListNotAvailableException {
diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
index f327f16..39de43d 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
@@ -86,8 +86,12 @@
throw new BadRequestException("patchset-level comments can't have side, range, or line");
} else if (in.line != null && in.range != null && in.line != in.range.endLine) {
throw new BadRequestException("range endLine must be on the same line as the comment");
+ } else if (in.inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(rsrc.getNotes(), in.inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(rsrc.getNotes(), in.inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", in.inReplyTo));
}
-
try (BatchUpdate bu =
updateFactory.create(rsrc.getChange().getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
Op op = new Op(rsrc.getComment().key, in);
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index 5145c13..c43bf91 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -62,7 +62,9 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.mina.transport.socket.SocketSessionConfig;
import org.apache.sshd.common.BaseBuilder;
@@ -72,6 +74,8 @@
import org.apache.sshd.common.compression.BuiltinCompressions;
import org.apache.sshd.common.compression.Compression;
import org.apache.sshd.common.forward.DefaultForwarderFactory;
+import org.apache.sshd.common.future.CloseFuture;
+import org.apache.sshd.common.future.SshFutureListener;
import org.apache.sshd.common.io.AbstractIoServiceFactory;
import org.apache.sshd.common.io.IoAcceptor;
import org.apache.sshd.common.io.IoServiceFactory;
@@ -142,6 +146,7 @@
private final List<HostKey> hostKeys;
private volatile IoAcceptor daemonAcceptor;
private final Config cfg;
+ private final long gracefulStopTimeout;
@Inject
SshDaemon(
@@ -212,6 +217,8 @@
SshSessionBackend backend = cfg.getEnum("sshd", null, "backend", SshSessionBackend.NIO2);
boolean channelIdTracking = cfg.getBoolean("sshd", "enableChannelIdTracking", true);
+ gracefulStopTimeout = cfg.getTimeUnit("sshd", null, "gracefulStopTimeout", 0, TimeUnit.SECONDS);
+
System.setProperty(
IoServiceFactoryFactory.class.getName(),
backend == SshSessionBackend.MINA
@@ -341,6 +348,12 @@
public synchronized void stop() {
if (daemonAcceptor != null) {
try {
+ if (gracefulStopTimeout > 0) {
+ logger.atInfo().log(
+ "Stopping SSHD sessions gracefully with %d seconds timeout.", gracefulStopTimeout);
+ daemonAcceptor.unbind(daemonAcceptor.getBoundAddresses());
+ waitForSessionClose();
+ }
daemonAcceptor.close(true).await();
shutdownExecutors();
logger.atInfo().log("Stopped Gerrit SSHD");
@@ -352,6 +365,30 @@
}
}
+ private void waitForSessionClose() {
+ Collection<IoSession> ioSessions = daemonAcceptor.getManagedSessions().values();
+ CountDownLatch allSessionsClosed = new CountDownLatch(ioSessions.size());
+ for (IoSession io : ioSessions) {
+ logger.atFine().log("Waiting for session %s to stop.", io.getId());
+ io.addCloseFutureListener(
+ new SshFutureListener<CloseFuture>() {
+ @Override
+ public void operationComplete(CloseFuture future) {
+ allSessionsClosed.countDown();
+ }
+ });
+ }
+ try {
+ if (!allSessionsClosed.await(gracefulStopTimeout, TimeUnit.SECONDS)) {
+ logger.atWarning().log(
+ "Timeout waiting for SSH session to close. SSHD will be shut down immediately.");
+ }
+ } catch (InterruptedException e) {
+ logger.atWarning().withCause(e).log(
+ "Interrupted waiting for SSH-sessions to close. SSHD will be shut down immediately.");
+ }
+ }
+
private void shutdownExecutors() {
if (executor != null) {
executor.shutdownNow();
diff --git a/java/com/google/gerrit/testing/TestCommentHelper.java b/java/com/google/gerrit/testing/TestCommentHelper.java
index 889bc18..76a5521 100644
--- a/java/com/google/gerrit/testing/TestCommentHelper.java
+++ b/java/com/google/gerrit/testing/TestCommentHelper.java
@@ -17,6 +17,7 @@
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -148,13 +149,30 @@
addRobotComment(targetChangeId, robotCommentInput, "robot comment test");
}
+ public void addRobotComment(Change.Id targetChangeId, RobotCommentInput robotCommentInput)
+ throws Exception {
+ addRobotComment(targetChangeId, robotCommentInput, "robot comment test");
+ }
+
public void addRobotComment(
String targetChangeId, RobotCommentInput robotCommentInput, String message) throws Exception {
+ ReviewInput reviewInput = createReviewInput(robotCommentInput, message);
+ gApi.changes().id(targetChangeId).current().review(reviewInput);
+ }
+
+ public void addRobotComment(
+ Change.Id targetChangeId, RobotCommentInput robotCommentInput, String message)
+ throws Exception {
+ ReviewInput reviewInput = createReviewInput(robotCommentInput, message);
+ gApi.changes().id(targetChangeId.get()).current().review(reviewInput);
+ }
+
+ private ReviewInput createReviewInput(RobotCommentInput robotCommentInput, String message) {
ReviewInput reviewInput = new ReviewInput();
reviewInput.robotComments =
Collections.singletonMap(robotCommentInput.path, ImmutableList.of(robotCommentInput));
reviewInput.message = message;
reviewInput.tag = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX;
- gApi.changes().id(targetChangeId).current().review(reviewInput);
+ return reviewInput;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/DaemonOverridesTestLibModulesIT.java b/javatests/com/google/gerrit/acceptance/DaemonOverridesTestLibModulesIT.java
new file mode 100644
index 0000000..f3a2324
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/DaemonOverridesTestLibModulesIT.java
@@ -0,0 +1,62 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.server.ModuleImpl;
+import com.google.gerrit.server.audit.AuditModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.name.Named;
+import com.google.inject.name.Names;
+import org.junit.Test;
+
+public class DaemonOverridesTestLibModulesIT extends AbstractDaemonTest {
+ private static final String TEST_MODULE = "test-module";
+
+ @Inject
+ @Named(value = TEST_MODULE)
+ private String testModuleClassName;
+
+ public abstract static class TestModule extends AuditModule {
+ @Override
+ protected void configure() {
+ super.configure();
+ bind(String.class).annotatedWith(Names.named(TEST_MODULE)).toInstance(getClass().getName());
+ }
+ }
+
+ @ModuleImpl(name = TEST_MODULE)
+ public static class DefaultModule extends TestModule {}
+
+ @ModuleImpl(name = TEST_MODULE)
+ public static class OverriddenModule extends TestModule {}
+
+ @Override
+ public Module createAuditModule() {
+ return new DefaultModule();
+ }
+
+ @Override
+ public Module createModule() {
+ return new OverriddenModule();
+ }
+
+ @Test
+ public void testSysModuleShouldOverrideTheDefaultOneWithSameModuleAnnotation() {
+ assertThat(testModuleClassName).isEqualTo(OverriddenModule.class.getName());
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 27b866b..1ad952c 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -14,9 +14,11 @@
package com.google.gerrit.acceptance.api.revision;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.MoreCollectors.onlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
@@ -32,8 +34,11 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
@@ -67,6 +72,7 @@
public class RobotCommentsIT extends AbstractDaemonTest {
@Inject private TestCommentHelper testCommentHelper;
+ @Inject private ChangeOperations changeOperations;
private static final String PLAIN_TEXT_CONTENT_TYPE = "text/plain";
private static final String GERRIT_COMMIT_MESSAGE_TYPE = "text/x-gerrit-commit-message";
@@ -320,6 +326,58 @@
}
@Test
+ public void robotCommentInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ RobotCommentInput input = TestCommentHelper.createRobotCommentInput(PATCHSET_LEVEL);
+ input.inReplyTo = "invalid";
+ BadRequestException ex =
+ assertThrows(
+ BadRequestException.class, () -> testCommentHelper.addRobotComment(changeId, input));
+ assertThat(ex.getMessage()).contains("inReplyTo");
+ }
+
+ @Test
+ public void canCreateRobotCommentWithRobotCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ ReviewInput.RobotCommentInput robotCommentInput =
+ TestCommentHelper.createRobotCommentInputWithMandatoryFields(COMMIT_MSG);
+ robotCommentInput.message = "comment reply";
+ robotCommentInput.inReplyTo = parentRobotCommentUuid;
+ testCommentHelper.addRobotComment(changeId, robotCommentInput);
+
+ RobotCommentInfo resultComment =
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeId.get()).current().robotCommentsAsList().stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void canCreateRobotCommentWithHumanCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String changeIdString = changeOperations.change(changeId).get().changeId();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ ReviewInput.RobotCommentInput robotCommentInput =
+ TestCommentHelper.createRobotCommentInputWithMandatoryFields(COMMIT_MSG);
+ robotCommentInput.message = "comment reply";
+ robotCommentInput.inReplyTo = parentCommentUuid;
+ testCommentHelper.addRobotComment(changeIdString, robotCommentInput);
+
+ RobotCommentInfo resultComment =
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeIdString).current().robotCommentsAsList().stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultComment.inReplyTo).isEqualTo(parentCommentUuid);
+ }
+
+ @Test
public void hugeRobotCommentIsRejected() {
int defaultSizeLimit = 1 << 20;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit + 1);
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index 9638658..1a2ae7c 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -82,14 +82,17 @@
.update();
}
+ @SuppressWarnings("TruthIncompatibleType")
@Test
public void mixingMagicAndRegularPush() throws Exception {
testRepo.branch("HEAD").commit().create();
PushResult r = push("HEAD:refs/heads/master", "HEAD:refs/for/master");
String msg = "cannot combine normal pushes and magic pushes";
- assertThat(r.getRemoteUpdate("refs/heads/master")).isNotEqualTo(Status.OK);
- assertThat(r.getRemoteUpdate("refs/for/master")).isNotEqualTo(Status.OK);
+ assertThat(r.getRemoteUpdate("refs/heads/master"))
+ .isNotEqualTo(/* expected: RemoteRefUpdate, actual: Status */ Status.OK);
+ assertThat(r.getRemoteUpdate("refs/for/master"))
+ .isNotEqualTo(/* expected: RemoteRefUpdate, actual: Status */ Status.OK);
assertThat(r.getRemoteUpdate("refs/for/master").getMessage()).isEqualTo(msg);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 88705e1..4f61d79 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -14,10 +14,12 @@
package com.google.gerrit.acceptance.server.change;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
@@ -51,13 +53,11 @@
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
-import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
@@ -66,7 +66,6 @@
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.sql.Timestamp;
@@ -98,8 +97,6 @@
@Inject private ChangeOperations changeOperations;
@Inject private AccountOperations accountOperations;
@Inject private RequestScopeOperations requestScopeOperations;
- @Inject private CommentsUtil commentsUtil;
- @Inject private TestCommentHelper testCommentHelper;
private final Integer[] lines = {0, 1};
@@ -658,6 +655,34 @@
}
@Test
+ public void putDraft_humanInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = parentCommentUuid;
+ String createdDraftUuid = addDraft(changeId, draft).id;
+ TestHumanComment actual =
+ changeOperations.change(changeId).draftComment(createdDraftUuid).get();
+ assertThat(actual.parentUuid()).hasValue(parentCommentUuid);
+ }
+
+ @Test
+ public void putDraft_robotInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = parentRobotCommentUuid;
+ String createdDraftUuid = addDraft(changeId, draft).id;
+ TestHumanComment actual =
+ changeOperations.change(changeId).draftComment(createdDraftUuid).get();
+ assertThat(actual.parentUuid()).hasValue(parentRobotCommentUuid);
+ }
+
+ @Test
public void putDraft_idMismatch() throws Exception {
String file = "file";
PushOneCommit.Result r = createChange();
@@ -702,6 +727,16 @@
}
@Test
+ public void putDraft_invalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = "invalid";
+ BadRequestException exception =
+ assertThrows(BadRequestException.class, () -> addDraft(changeId, draft));
+ assertThat(exception.getMessage()).contains(String.format("%s not found", draft.inReplyTo));
+ }
+
+ @Test
public void putDraft_updatePath() throws Exception {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
@@ -715,22 +750,62 @@
}
@Test
- public void putDraft_updateInReplyToAndTag() throws Exception {
- PushOneCommit.Result r = createChange();
- String changeId = r.getChangeId();
- String revId = r.getCommit().getName();
- DraftInput draftInput1 = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
- CommentInfo commentInfo = addDraft(changeId, revId, draftInput1);
- DraftInput draftInput2 = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
- String inReplyTo = "in_reply_to";
+ public void putDraft_updateInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updatedDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updatedDraftInput.inReplyTo = "invalid";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () -> updateDraft(changeId, updatedDraftInput, originalDraft.id));
+ assertThat(exception.getMessage()).contains(String.format("Invalid inReplyTo"));
+ }
+
+ @Test
+ public void putDraft_updateHumanInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updateDraftInput.inReplyTo = parentCommentUuid;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().parentUuid())
+ .hasValue(parentCommentUuid);
+ }
+
+ @Test
+ public void putDraft_updateRobotInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updateDraftInput.inReplyTo = parentRobotCommentUuid;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().parentUuid())
+ .hasValue(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void putDraft_updateTag() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
String tag = "täg";
- draftInput2.inReplyTo = inReplyTo;
- draftInput2.tag = tag;
- updateDraft(changeId, revId, draftInput2, commentInfo.id);
- com.google.gerrit.entities.Comment comment =
- Iterables.getOnlyElement(commentsUtil.draftByChange(r.getChange().notes()));
- assertThat(comment.parentUuid).isEqualTo(inReplyTo);
- assertThat(comment.tag).isEqualTo(tag);
+ updateDraftInput.tag = tag;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().tag())
+ .hasValue(tag);
}
@Test
@@ -1474,26 +1549,73 @@
@Test
public void canCreateHumanCommentWithRobotCommentAsParentAndUnsetUnresolved() throws Exception {
- PushOneCommit.Result result = createChange();
- String changeId = result.getChangeId();
- String ps1 = result.getCommit().name();
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
- testCommentHelper.addRobotComment(
- result.getChangeId(),
- TestCommentHelper.createRobotCommentInputWithMandatoryFields(FILE_NAME));
- RobotCommentInfo robotCommentInfo =
- Iterables.getOnlyElement(gApi.changes().id(changeId).current().robotCommentsAsList());
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentRobotCommentUuid;
+ createdCommentInput.unresolved = null;
+ addComments(changeId, createdCommentInput);
- CommentInput comment = newComment(FILE_NAME, "comment 1 reply");
- comment.inReplyTo = robotCommentInfo.id;
- comment.unresolved = null;
- addComments(changeId, ps1, comment);
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
- CommentInfo resultComment = Iterables.getOnlyElement(getPublishedCommentsAsList(changeId));
- assertThat(resultComment.inReplyTo).isEqualTo(robotCommentInfo.id);
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
// Default unresolved is false.
- assertThat(resultComment.unresolved).isFalse();
+ assertThat(resultNewComment.unresolved).isFalse();
+ }
+
+ @Test
+ public void canCreateHumanCommentWithHumanCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentCommentUuid;
+ addComments(changeId, createdCommentInput);
+
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentCommentUuid);
+ }
+
+ @Test
+ public void canCreateHumanCommentWithRobotCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentRobotCommentUuid;
+ addComments(changeId, createdCommentInput);
+
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void cannotCreateCommentWithInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ CommentInput comment = newComment(COMMIT_MSG, "comment 1 reply");
+ comment.inReplyTo = "invalid";
+
+ BadRequestException exception =
+ assertThrows(BadRequestException.class, () -> addComments(changeId, comment));
+ assertThat(exception.getMessage()).contains(String.format("%s not found", comment.inReplyTo));
}
private List<CommentInfo> getRevisionComments(String changeId, String revId) throws Exception {
@@ -1510,6 +1632,12 @@
return comment;
}
+ private void addComments(Change.Id changeId, CommentInput... commentInputs) throws Exception {
+ ReviewInput input = new ReviewInput();
+ input.comments = Arrays.stream(commentInputs).collect(groupingBy(c -> c.path));
+ gApi.changes().id(changeId.get()).current().review(input);
+ }
+
private void addComments(String changeId, String revision, CommentInput... commentInputs)
throws Exception {
ReviewInput input = new ReviewInput();
@@ -1610,11 +1738,19 @@
return gApi.changes().id(changeId).revision(revId).createDraft(in).get();
}
+ private CommentInfo addDraft(Change.Id changeId, DraftInput in) throws Exception {
+ return gApi.changes().id(changeId.get()).current().createDraft(in).get();
+ }
+
private void updateDraft(String changeId, String revId, DraftInput in, String uuid)
throws Exception {
gApi.changes().id(changeId).revision(revId).draft(uuid).update(in);
}
+ private void updateDraft(Change.Id changeId, DraftInput in, String uuid) throws Exception {
+ gApi.changes().id(changeId.get()).current().draft(uuid).update(in);
+ }
+
private void deleteDraft(String changeId, String revId, String uuid) throws Exception {
gApi.changes().id(changeId).revision(revId).draft(uuid).delete();
}
@@ -1633,6 +1769,10 @@
return gApi.changes().id(changeId).commentsAsList();
}
+ private List<CommentInfo> getPublishedCommentsAsList(Change.Id changeId) throws Exception {
+ return gApi.changes().id(changeId.get()).commentsAsList();
+ }
+
private Map<String, List<CommentInfo>> getDraftComments(String changeId, String revId)
throws Exception {
return gApi.changes().id(changeId).revision(revId).drafts();
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 975d7ec..0bd6554 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -1175,6 +1175,39 @@
}
@Test
+ public void tagOfPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().tag("tag").create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.tag()).value().isEqualTo("tag");
+ }
+
+ @Test
+ public void unresolvedOfUnresolvedPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().unresolved().create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.unresolved()).isTrue();
+ }
+
+ @Test
+ public void unresolvedOfResolvedPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().resolved().create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.unresolved()).isFalse();
+ }
+
+ @Test
public void draftCommentCanBeRetrieved() {
Change.Id changeId = changeOperations.newChange().create();
String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create();
@@ -1212,6 +1245,36 @@
assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
+ @Test
+ public void robotCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ TestRobotComment comment = changeOperations.change(changeId).robotComment(commentUuid).get();
+
+ assertThat(comment.uuid()).isEqualTo(commentUuid);
+ }
+
+ @Test
+ public void parentUuidOfRobotCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ String childCommentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .parentUuid(parentCommentUuid)
+ .create();
+
+ TestRobotComment comment =
+ changeOperations.change(changeId).robotComment(childCommentUuid).get();
+
+ assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ }
+
private ChangeInfo getChangeFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).get();
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
index cf687aa..fa28396 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
@@ -16,6 +16,8 @@
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThatList;
+import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThat;
+import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThatList;
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -27,6 +29,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
@@ -44,7 +47,6 @@
Change.Id changeId = changeOperations.newChange().create();
String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create();
-
List<CommentInfo> comments = getCommentsFromServer(changeId);
assertThatList(comments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid);
}
@@ -643,10 +645,393 @@
assertThatList(comments).isEmpty();
}
+ @Test
+ public void robotCommentCanBeCreatedWithoutSpecifyingAnyParameters() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ List<RobotCommentInfo> robotComments = getRobotCommentsFromServerFromCurrentPatchset(changeId);
+ assertThatList(robotComments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnOlderPatchset() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ PatchSet.Id previousPatchsetId =
+ changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ changeOperations.change(changeId).newPatchset().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).patchset(previousPatchsetId).newRobotComment().create();
+
+ CommentInfo comment = getRobotCommentFromServer(previousPatchsetId, commentUuid);
+ assertThat(comment).uuid().isEqualTo(commentUuid);
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithSpecifiedMessage() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .message("Test comment message")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).message().isEqualTo("Test comment message");
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedWithEmptyMessage() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().noMessage().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).message().isNull();
+ }
+
+ @Test
+ public void patchsetLevelRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onPatchsetLevel()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).path().isEqualTo(Patch.PATCHSET_LEVEL);
+ }
+
+ @Test
+ public void fileRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().file("file1").content("Line 1").create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onFileLevelOf("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).path().isEqualTo("file1");
+ assertThat(comment).line().isNull();
+ assertThat(comment).range().isNull();
+ }
+
+ @Test
+ public void lineRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .file("file1")
+ .content("Line 1\nLine 2\nLine 3\nLine 4\n")
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onLine(3)
+ .ofFile("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).line().isEqualTo(3);
+ assertThat(comment).range().isNull();
+ }
+
+ @Test
+ public void rangeRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .file("file1")
+ .content("Line 1\nLine 2\nLine 3\nLine 4\n")
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .fromLine(2)
+ .charOffset(4)
+ .toLine(3)
+ .charOffset(5)
+ .ofFile("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).range().startLine().isEqualTo(2);
+ assertThat(comment).range().startCharacter().isEqualTo(4);
+ assertThat(comment).range().endLine().isEqualTo(3);
+ assertThat(comment).range().endCharacter().isEqualTo(5);
+ // Line is automatically filled from specified range. It's the end line.
+ assertThat(comment).line().isEqualTo(3);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnPatchsetCommit() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onPatchsetCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ // Null is often used instead of Side.REVISION as Side.REVISION is the default.
+ assertThat(comment).side().isAnyOf(Side.REVISION, null);
+ assertThat(comment).parent().isNull();
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnParentCommit() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onParentCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(1);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnSecondParentCommit() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onSecondParentCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(2);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnNonExistingSecondParentCommit() throws Exception {
+ Change.Id parentChangeId = changeOperations.newChange().create();
+ Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onSecondParentCommit()
+ .create();
+
+ // We want to be able to create such invalid robot comments for testing purposes (e.g. testing
+ // error handling or resilience of an endpoint) and hence we need to allow such invalid robot
+ // comments in the test API.
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(2);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnAutoMergeCommit() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onAutoMergeCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isNull();
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedAsResolved() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().resolved().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).unresolved().isFalse();
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedAsUnresolved() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().unresolved().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).unresolved().isTrue();
+ }
+
+ @Test
+ public void replyToRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .parentUuid(parentCommentUuid)
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).inReplyTo().isEqualTo(parentCommentUuid);
+ }
+
+ @Test
+ public void tagCanBeAttachedToARobotComment() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .tag("my special tag")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).tag().isEqualTo("my special tag");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithSpecifiedAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ Account.Id accountId = accountOperations.newAccount().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .author(accountId)
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).author().id().isEqualTo(accountId.get());
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithRobotId() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .robotId("robot-id")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).robotId().isEqualTo("robot-id");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithRobotRunId() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .robotId("robot-run-id")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).robotId().isEqualTo("robot-run-id");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithUrl() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().url("url").create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).url().isEqualTo("url");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithProperty() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .addProperty("key", "value")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).properties().containsExactly("key", "value");
+ }
+
private List<CommentInfo> getCommentsFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).commentsAsList();
}
+ private List<RobotCommentInfo> getRobotCommentsFromServerFromCurrentPatchset(Change.Id changeId)
+ throws RestApiException {
+ return gApi.changes().id(changeId.get()).current().robotCommentsAsList();
+ }
+
private List<CommentInfo> getDraftCommentsFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).draftsAsList();
}
@@ -662,6 +1047,33 @@
String.format("Comment %s not found on change %d", uuid, changeId.get())));
}
+ private RobotCommentInfo getRobotCommentFromServerInCurrentPatchset(
+ Change.Id changeId, String uuid) throws RestApiException {
+ return gApi.changes().id(changeId.get()).current().robotCommentsAsList().stream()
+ .filter(comment -> comment.id.equals(uuid))
+ .findAny()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Robot Comment %s not found on change %d on the latest patchset",
+ uuid, changeId.get())));
+ }
+
+ private RobotCommentInfo getRobotCommentFromServer(PatchSet.Id patchsetId, String uuid)
+ throws RestApiException {
+ return gApi.changes().id(patchsetId.changeId().toString())
+ .revision(patchsetId.getId().toString()).robotCommentsAsList().stream()
+ .filter(comment -> comment.id.equals(uuid))
+ .findAny()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Robot Comment %s not found on change %d on patchset %d",
+ uuid, patchsetId.changeId().get(), patchsetId.get())));
+ }
+
private CommentInfo getDraftCommentFromServer(Change.Id changeId, String uuid)
throws RestApiException {
return gApi.changes().id(changeId.get()).draftsAsList().stream()
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 60bf64c..1a43269 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -41,7 +41,7 @@
case V6_7:
return "blacktop/elasticsearch:6.7.2";
case V6_8:
- return "blacktop/elasticsearch:6.8.11";
+ return "blacktop/elasticsearch:6.8.12";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
index 9296a6b..b9d5313 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -72,15 +72,15 @@
public void atLeastMinorVersion() throws Exception {
assertThat(ElasticVersion.V6_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isTrue();
assertThat(ElasticVersion.V6_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isTrue();
- assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_1.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_8.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
+ assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_1.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
}
@Test
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
index d19073d..5d420d3 100644
--- a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
@@ -21,12 +21,16 @@
import org.junit.Test;
public class PerThreadCacheTest {
+
+ @SuppressWarnings("TruthIncompatibleType")
@Test
public void key_respectsClass() {
assertThat(PerThreadCache.Key.create(String.class))
.isEqualTo(PerThreadCache.Key.create(String.class));
assertThat(PerThreadCache.Key.create(String.class))
- .isNotEqualTo(PerThreadCache.Key.create(Integer.class));
+ .isNotEqualTo(
+ /* expected: Key<String>, actual: Key<Integer> */ PerThreadCache.Key.create(
+ Integer.class));
}
@Test
diff --git a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
index daefd7c..5cefe74 100644
--- a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
+++ b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
@@ -32,6 +32,8 @@
@RunWith(JUnit4.class)
public class ListChangeCommentsTest {
+
+ @SuppressWarnings("TruthIncompatibleType")
@Test
public void commentsLinkedToChangeMessagesIgnoreGerritAutoGenTaggedMessages() {
/* Comments should not be linked to Gerrit's autogenerated messages */
@@ -54,7 +56,9 @@
// Make sure no comment is linked to the auto-gen message
assertThat(comments.stream().map(c -> c.changeMessageId).collect(Collectors.toSet()))
- .doesNotContain(getChangeMessage(changeMessages, "cmAutoGenByGerrit"));
+ .doesNotContain(
+ /* expected: String, actual: ChangeMessage */ getChangeMessage(
+ changeMessages, "cmAutoGenByGerrit"));
}
@Test
diff --git a/plugins/delete-project b/plugins/delete-project
index 516fbd8..2dc456a 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 516fbd8aebfcc49b278b0eb985add293d753bb3f
+Subproject commit 2dc456a6891f1bc55a9d637cf2553f27ceae6c49
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.js b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.js
deleted file mode 100644
index 7901c53..0000000
--- a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.js
+++ /dev/null
@@ -1,116 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../styles/shared-styles.js';
-import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.js';
-import '../../plugins/gr-endpoint-param/gr-endpoint-param.js';
-import '../../shared/gr-avatar/gr-avatar.js';
-import '../../shared/gr-date-formatter/gr-date-formatter.js';
-import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
-import '../../../styles/dashboard-header-styles.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-user-header_html.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-/**
- * @extends PolymerElement
- */
-class GrUserHeader extends GestureEventListeners(
- LegacyElementMixin(
- PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-user-header'; }
-
- static get properties() {
- return {
- /** @type {?string} */
- userId: {
- type: String,
- observer: '_accountChanged',
- },
-
- showDashboardLink: {
- type: Boolean,
- value: false,
- },
-
- loggedIn: {
- type: Boolean,
- value: false,
- },
-
- /**
- * @type {?{name: ?, email: ?, registered_on: ?}}
- */
- _accountDetails: {
- type: Object,
- value: null,
- },
-
- /** @type {?string} */
- _status: {
- type: String,
- value: null,
- },
- };
- }
-
- _accountChanged(userId) {
- if (!userId) {
- this._accountDetails = null;
- this._status = null;
- return;
- }
-
- this.$.restAPI.getAccountDetails(userId).then(details => {
- this._accountDetails = details;
- });
- this.$.restAPI.getAccountStatus(userId).then(status => {
- this._status = status;
- });
- }
-
- _computeDisplayClass(status) {
- return status ? ' ' : 'hide';
- }
-
- _computeDetail(accountDetails, name) {
- return accountDetails ? accountDetails[name] : '';
- }
-
- _computeStatusClass(accountDetails) {
- return this._computeDetail(accountDetails, 'status') ? '' : 'hide';
- }
-
- _computeDashboardUrl(accountDetails) {
- if (!accountDetails) { return null; }
- const id = accountDetails._account_id;
- const email = accountDetails.email;
- if (!id && !email ) { return null; }
- return GerritNav.getUrlForUserDashboard(id ? id : email);
- }
-
- _computeDashboardLinkClass(showDashboardLink, loggedIn) {
- return showDashboardLink && loggedIn ?
- 'dashboardLink' : 'dashboardLink hide';
- }
-}
-
-customElements.define(GrUserHeader.is, GrUserHeader);
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.ts b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.ts
new file mode 100644
index 0000000..25369b8
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.ts
@@ -0,0 +1,113 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../../styles/shared-styles';
+import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
+import '../../plugins/gr-endpoint-param/gr-endpoint-param';
+import '../../shared/gr-avatar/gr-avatar';
+import '../../shared/gr-date-formatter/gr-date-formatter';
+import '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import '../../../styles/dashboard-header-styles';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-user-header_html';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {customElement, property} from '@polymer/decorators';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+import {AccountDetailInfo, AccountId} from '../../../types/common';
+
+export interface GrUserHeader {
+ $: {
+ restAPI: RestApiService & Element;
+ };
+}
+
+@customElement('gr-user-header')
+export class GrUserHeader extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ @property({type: String, observer: '_accountChanged'})
+ userId?: AccountId;
+
+ @property({type: Boolean})
+ showDashboardLink = false;
+
+ @property({type: Boolean})
+ loggedIn = false;
+
+ @property({type: Object})
+ _accountDetails: AccountDetailInfo | null = null;
+
+ @property({type: String})
+ _status = '';
+
+ _accountChanged(userId?: AccountId) {
+ if (!userId) {
+ this._accountDetails = null;
+ this._status = '';
+ return;
+ }
+
+ this.$.restAPI.getAccountDetails(userId).then(details => {
+ this._accountDetails = details ?? null;
+ this._status = details?.status ?? '';
+ });
+ }
+
+ _computeDetail(
+ accountDetails: AccountDetailInfo | null,
+ name: keyof AccountDetailInfo
+ ) {
+ return accountDetails ? accountDetails[name] : '';
+ }
+
+ _computeStatusClass(status: string) {
+ return status ? '' : 'hide';
+ }
+
+ _computeDashboardUrl(accountDetails: AccountDetailInfo | null) {
+ if (!accountDetails) {
+ return null;
+ }
+ const id = accountDetails._account_id;
+ if (id) {
+ return GerritNav.getUrlForUserDashboard(String(id));
+ }
+ const email = accountDetails.email;
+ if (email) {
+ return GerritNav.getUrlForUserDashboard(email);
+ }
+ return null;
+ }
+
+ _computeDashboardLinkClass(showDashboardLink: boolean, loggedIn: boolean) {
+ return showDashboardLink && loggedIn
+ ? 'dashboardLink'
+ : 'dashboardLink hide';
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-user-header': GrUserHeader;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
index 72bdca6..136835d 100644
--- a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
@@ -37,7 +37,7 @@
[[_computeDetail(_accountDetails, 'name')]]
</h1>
<hr />
- <div class$="status [[_computeStatusClass(_accountDetails)]]">
+ <div class$="status [[_computeStatusClass(_status)]]">
<span>Status:</span> [[_status]]
</div>
<div>
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_test.js b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_test.js
index 6baacef..15fbf8b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_test.js
@@ -32,10 +32,9 @@
.returns(Promise.resolve({
name: 'foo',
email: 'bar',
+ status: 'OOO',
registered_on: '2015-03-12 18:32:08.000000000',
}));
- sinon.stub(element.$.restAPI, 'getAccountStatus')
- .returns(Promise.resolve('baz'));
element.userId = 'foo.bar@baz';
flush(() => {
@@ -46,7 +45,7 @@
flush(() => {
flushAsynchronousOperations();
assert.isNull(element._accountDetails);
- assert.isNull(element._status);
+ assert.equal(element._status, '');
done();
});
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 6ecdabb..630681c 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
@@ -540,7 +540,7 @@
if (reviewer.account) {
reviewerId = reviewer.account._account_id || reviewer.account.email;
} else if (reviewer.group) {
- reviewerId = reviewer.group.id;
+ reviewerId = decodeURIComponent(reviewer.group.id);
confirmed = reviewer.group.confirmed;
}
return {reviewer: reviewerId, confirmed};
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
deleted file mode 100644
index 174bbbb..0000000
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
+++ /dev/null
@@ -1,303 +0,0 @@
-/**
- * @license
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import '../../shared/gr-account-chip/gr-account-chip.js';
-import '../../shared/gr-button/gr-button.js';
-import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
-import '../../../styles/shared-styles.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-reviewer-list_html.js';
-import {
- hasAttention,
- isServiceUser,
-} from '../../../utils/account-util.js';
-
-/**
- * @extends PolymerElement
- */
-class GrReviewerList extends GestureEventListeners(
- LegacyElementMixin(PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-reviewer-list'; }
- /**
- * Fired when the "Add reviewer..." button is tapped.
- *
- * @event show-reply-dialog
- */
-
- static get properties() {
- return {
- change: Object,
- serverConfig: Object,
- disabled: {
- type: Boolean,
- value: false,
- reflectToAttribute: true,
- },
- mutable: {
- type: Boolean,
- value: false,
- },
- reviewersOnly: {
- type: Boolean,
- value: false,
- },
- ccsOnly: {
- type: Boolean,
- value: false,
- },
-
- _displayedReviewers: {
- type: Array,
- value() { return []; },
- },
- _reviewers: {
- type: Array,
- value() { return []; },
- },
- _showInput: {
- type: Boolean,
- value: false,
- },
- _addLabel: {
- type: String,
- computed: '_computeAddLabel(ccsOnly)',
- },
- _hiddenReviewerCount: {
- type: Number,
- computed: '_computeHiddenCount(_reviewers, _displayedReviewers)',
- },
-
- // Used for testing.
- _lastAutocompleteRequest: Object,
- _xhrPromise: Object,
- };
- }
-
- static get observers() {
- return [
- '_reviewersChanged(change.reviewers.*, change.owner, serverConfig)',
- ];
- }
-
- /**
- * Converts change.permitted_labels to an array of hashes of label keys to
- * numeric scores.
- * Example:
- * [{
- * 'Code-Review': ['-1', ' 0', '+1']
- * }]
- * will be converted to
- * [{
- * label: 'Code-Review',
- * scores: [-1, 0, 1]
- * }]
- */
- _permittedLabelsToNumericScores(labels) {
- if (!labels) return [];
- return Object.keys(labels).map(label => {
- return {
- label,
- scores: labels[label].map(v => parseInt(v, 10)),
- };
- });
- }
-
- /**
- * Returns hash of labels to max permitted score.
- *
- * @param {!Object} change
- * @returns {!Object} labels to max permitted scores hash
- */
- _getMaxPermittedScores(change) {
- return this._permittedLabelsToNumericScores(change.permitted_labels)
- .map(({label, scores}) => {
- return {
- [label]: scores
- .map(v => parseInt(v, 10))
- .reduce((a, b) => Math.max(a, b))};
- })
- .reduce((acc, i) => Object.assign(acc, i), {});
- }
-
- /**
- * Returns max permitted score for reviewer.
- *
- * @param {!Object} reviewer
- * @param {!Object} change
- * @param {string} label
- * @return {number}
- */
- _getReviewerPermittedScore(reviewer, change, label) {
- // Note (issue 7874): sometimes the "all" list is not included in change
- // detail responses, even when DETAILED_LABELS is included in options.
- if (!change.labels[label].all) { return NaN; }
- const detailed = change.labels[label].all.filter(
- ({_account_id}) => reviewer._account_id === _account_id).pop();
- if (!detailed) {
- return NaN;
- }
- if (detailed.hasOwnProperty('permitted_voting_range')) {
- return detailed.permitted_voting_range.max;
- } else if (detailed.hasOwnProperty('value')) {
- // If preset, user can vote on the label.
- return 0;
- }
- return NaN;
- }
-
- _computeVoteableText(reviewer, change) {
- if (!change || !change.labels) { return ''; }
- const maxScores = [];
- const maxPermitted = this._getMaxPermittedScores(change);
- for (const label of Object.keys(change.labels)) {
- const maxScore =
- this._getReviewerPermittedScore(reviewer, change, label);
- if (isNaN(maxScore) || maxScore < 0) { continue; }
- if (maxScore > 0 && maxScore === maxPermitted[label]) {
- maxScores.push(`${label}: +${maxScore}`);
- } else {
- maxScores.push(`${label}`);
- }
- }
- return maxScores.join(', ');
- }
-
- _reviewersChanged(changeRecord, owner, serverConfig) {
- // Polymer 2: check for undefined
- if ([changeRecord, owner, serverConfig].includes(undefined)) {
- return;
- }
-
- let result = [];
- const reviewers = changeRecord.base;
- for (const key in reviewers) {
- if (this.reviewersOnly && key !== 'REVIEWER') {
- continue;
- }
- if (this.ccsOnly && key !== 'CC') {
- continue;
- }
- if (key === 'REVIEWER' || key === 'CC') {
- result = result.concat(reviewers[key]);
- }
- }
- this._reviewers = result
- .filter(reviewer => reviewer._account_id != owner._account_id)
- // Sort order:
- // 1. Human users in the attention set.
- // 2. Other human users.
- // 3. Service users.
- .sort((r1, r2) => {
- const a1 = hasAttention(serverConfig, r1, this.change) ? 1 : 0;
- const a2 = hasAttention(serverConfig, r2, this.change) ? 1 : 0;
- const s1 = isServiceUser(r1) ? -2 : 0;
- const s2 = isServiceUser(r2) ? -2 : 0;
- return a2 - a1 + s2 - s1;
- });
-
- if (this._reviewers.length > 8) {
- this._displayedReviewers = this._reviewers.slice(0, 6);
- } else {
- this._displayedReviewers = this._reviewers;
- }
- }
-
- _computeHiddenCount(reviewers, displayedReviewers) {
- // Polymer 2: check for undefined
- if ([reviewers, displayedReviewers].includes(undefined)) {
- return undefined;
- }
-
- return reviewers.length - displayedReviewers.length;
- }
-
- _computeCanRemoveReviewer(reviewer, mutable) {
- if (!mutable) { return false; }
-
- let current;
- for (let i = 0; i < this.change.removable_reviewers.length; i++) {
- current = this.change.removable_reviewers[i];
- if (current._account_id === reviewer._account_id ||
- (!reviewer._account_id && current.email === reviewer.email)) {
- return true;
- }
- }
- return false;
- }
-
- _handleRemove(e) {
- e.preventDefault();
- const target = dom(e).rootTarget;
- if (!target.account) { return; }
- const accountID = target.account._account_id || target.account.email;
- this.disabled = true;
- this._xhrPromise = this._removeReviewer(accountID).then(response => {
- this.disabled = false;
- if (!response.ok) { return response; }
-
- const reviewers = this.change.reviewers;
-
- for (const type of ['REVIEWER', 'CC']) {
- reviewers[type] = reviewers[type] || [];
- for (let i = 0; i < reviewers[type].length; i++) {
- if (reviewers[type][i]._account_id == accountID ||
- reviewers[type][i].email == accountID) {
- this.splice('change.reviewers.' + type, i, 1);
- break;
- }
- }
- }
- })
- .catch(err => {
- this.disabled = false;
- throw err;
- });
- }
-
- _handleAddTap(e) {
- e.preventDefault();
- const value = {};
- if (this.reviewersOnly) {
- value.reviewersOnly = true;
- }
- if (this.ccsOnly) {
- value.ccsOnly = true;
- }
- this.dispatchEvent(new CustomEvent('show-reply-dialog', {
- detail: {value},
- composed: true, bubbles: true,
- }));
- }
-
- _handleViewAll(e) {
- this._displayedReviewers = this._reviewers;
- }
-
- _removeReviewer(id) {
- return this.$.restAPI.removeChangeReviewer(this.change._number, id);
- }
-
- _computeAddLabel(ccsOnly) {
- return ccsOnly ? 'Add CC' : 'Add reviewer';
- }
-}
-
-customElements.define(GrReviewerList.is, GrReviewerList);
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
new file mode 100644
index 0000000..70e7ba7
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -0,0 +1,339 @@
+/**
+ * @license
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import '../../shared/gr-account-chip/gr-account-chip';
+import '../../shared/gr-button/gr-button';
+import '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import '../../../styles/shared-styles';
+import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-reviewer-list_html';
+import {hasAttention, isServiceUser} from '../../../utils/account-util';
+import {customElement, property, computed, observe} from '@polymer/decorators';
+import {
+ ChangeInfo,
+ ServerInfo,
+ LabelNameToValueMap,
+ AccountInfo,
+ ApprovalInfo,
+ Reviewers,
+ AccountId,
+ DetailedLabelInfo,
+} from '../../../types/common';
+import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
+import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+import {hasOwnProperty} from '../../../utils/common-util';
+
+export interface GrReviewerList {
+ $: {
+ restAPI: RestApiService & Element;
+ };
+}
+
+@customElement('gr-reviewer-list')
+export class GrReviewerList extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ /**
+ * Fired when the "Add reviewer..." button is tapped.
+ *
+ * @event show-reply-dialog
+ */
+
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ @property({type: Object})
+ serverConfig?: ServerInfo;
+
+ @property({type: Boolean, reflectToAttribute: true})
+ disabled = false;
+
+ @property({type: Boolean})
+ mutable = false;
+
+ @property({type: Boolean})
+ reviewersOnly = false;
+
+ @property({type: Boolean})
+ ccsOnly = false;
+
+ @property({type: Array})
+ _displayedReviewers: AccountInfo[] = [];
+
+ @property({type: Array})
+ _reviewers: AccountInfo[] = [];
+
+ @property({type: Boolean})
+ _showInput = false;
+
+ @property({type: Object})
+ _xhrPromise?: Promise<Response | undefined>;
+
+ @computed('ccsOnly')
+ get _addLabel() {
+ return this.ccsOnly ? 'Add CC' : 'Add reviewer';
+ }
+
+ @computed('_reviewers', '_displayedReviewers')
+ get _hiddenReviewerCount() {
+ // Polymer 2: check for undefined
+ if (
+ this._reviewers === undefined ||
+ this._displayedReviewers === undefined
+ ) {
+ return undefined;
+ }
+ return this._reviewers.length - this._displayedReviewers.length;
+ }
+
+ /**
+ * Converts change.permitted_labels to an array of hashes of label keys to
+ * numeric scores.
+ * Example:
+ * [{
+ * 'Code-Review': ['-1', ' 0', '+1']
+ * }]
+ * will be converted to
+ * [{
+ * label: 'Code-Review',
+ * scores: [-1, 0, 1]
+ * }]
+ */
+ _permittedLabelsToNumericScores(labels: LabelNameToValueMap | undefined) {
+ if (!labels) return [];
+ return Object.keys(labels).map(label => {
+ return {
+ label,
+ scores: labels[label].map(v => parseInt(v, 10)),
+ };
+ });
+ }
+
+ /**
+ * Returns hash of labels to max permitted score.
+ *
+ * @returns labels to max permitted scores hash
+ */
+ _getMaxPermittedScores(change: ChangeInfo) {
+ return this._permittedLabelsToNumericScores(change.permitted_labels)
+ .map(({label, scores}) => {
+ return {
+ [label]: scores.reduce((a, b) => Math.max(a, b)),
+ };
+ })
+ .reduce((acc, i) => Object.assign(acc, i), {});
+ }
+
+ /**
+ * Returns max permitted score for reviewer.
+ */
+ _getReviewerPermittedScore(
+ reviewer: AccountInfo,
+ change: ChangeInfo,
+ label: string
+ ) {
+ // Note (issue 7874): sometimes the "all" list is not included in change
+ // detail responses, even when DETAILED_LABELS is included in options.
+ if (!change.labels) {
+ return NaN;
+ }
+ const detailedLabel = change.labels[label] as DetailedLabelInfo;
+ if (!detailedLabel.all) {
+ return NaN;
+ }
+ const detailed = detailedLabel.all
+ .filter(
+ (approval: ApprovalInfo) =>
+ reviewer._account_id === approval._account_id
+ )
+ .pop();
+ if (!detailed) {
+ return NaN;
+ }
+ if (hasOwnProperty(detailed, 'permitted_voting_range')) {
+ if (!detailed.permitted_voting_range) return NaN;
+ return detailed.permitted_voting_range.max;
+ } else if (hasOwnProperty(detailed, 'value')) {
+ // If preset, user can vote on the label.
+ return 0;
+ }
+ return NaN;
+ }
+
+ _computeVoteableText(reviewer: AccountInfo, change: ChangeInfo) {
+ if (!change || !change.labels) {
+ return '';
+ }
+ const maxScores = [];
+ const maxPermitted = this._getMaxPermittedScores(change);
+ for (const label of Object.keys(change.labels)) {
+ const maxScore = this._getReviewerPermittedScore(reviewer, change, label);
+ if (isNaN(maxScore) || maxScore < 0) {
+ continue;
+ }
+ if (maxScore > 0 && maxScore === maxPermitted[label]) {
+ maxScores.push(`${label}: +${maxScore}`);
+ } else {
+ maxScores.push(`${label}`);
+ }
+ }
+ return maxScores.join(', ');
+ }
+
+ @observe('change.reviewers.*', 'change.owner', 'serverConfig')
+ _reviewersChanged(
+ changeRecord: PolymerDeepPropertyChange<Reviewers, Reviewers>,
+ owner: AccountInfo,
+ serverConfig: ServerInfo
+ ) {
+ // Polymer 2: check for undefined
+ if (
+ changeRecord === undefined ||
+ owner === undefined ||
+ serverConfig === undefined ||
+ this.change === undefined
+ ) {
+ return;
+ }
+ let result: AccountInfo[] = [];
+ const reviewers = changeRecord.base;
+ for (const key in reviewers) {
+ if (this.reviewersOnly && key !== 'REVIEWER') {
+ continue;
+ }
+ if (this.ccsOnly && key !== 'CC') {
+ continue;
+ }
+ if (key === 'REVIEWER' || key === 'CC') {
+ result = result.concat(reviewers[key]!);
+ }
+ }
+ this._reviewers = result
+ .filter(reviewer => reviewer._account_id !== owner._account_id)
+ // Sort order:
+ // 1. Human users in the attention set.
+ // 2. Other human users.
+ // 3. Service users.
+ .sort((r1, r2) => {
+ const a1 = hasAttention(serverConfig, r1, this.change!) ? 1 : 0;
+ const a2 = hasAttention(serverConfig, r2, this.change!) ? 1 : 0;
+ const s1 = isServiceUser(r1) ? -2 : 0;
+ const s2 = isServiceUser(r2) ? -2 : 0;
+ return a2 - a1 + s2 - s1;
+ });
+
+ if (this._reviewers.length > 8) {
+ this._displayedReviewers = this._reviewers.slice(0, 6);
+ } else {
+ this._displayedReviewers = this._reviewers;
+ }
+ }
+
+ _computeCanRemoveReviewer(reviewer: AccountInfo, mutable: boolean) {
+ if (
+ !mutable ||
+ this.change === undefined ||
+ this.change.removable_reviewers === undefined
+ ) {
+ return false;
+ }
+
+ let current;
+ for (let i = 0; i < this.change.removable_reviewers.length; i++) {
+ current = this.change.removable_reviewers[i];
+ if (
+ current._account_id === reviewer._account_id ||
+ (!reviewer._account_id && current.email === reviewer.email)
+ ) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ _handleRemove(e: Event) {
+ e.preventDefault();
+ const target = (dom(e) as EventApi).rootTarget as GrAccountChip;
+ if (!target.account || !this.change) {
+ return;
+ }
+ const accountID = target.account._account_id;
+ this.disabled = true;
+ if (!accountID) return;
+ this._xhrPromise = this._removeReviewer(accountID)
+ .then((response: Response | undefined) => {
+ this.disabled = false;
+ if (!response || !response.ok) {
+ return response;
+ }
+ if (!this.change || !this.change.reviewers) return;
+ const reviewers: {[type: string]: AccountInfo[] | undefined} = this
+ .change!.reviewers;
+ for (const type of ['REVIEWER', 'CC']) {
+ reviewers[type] = reviewers[type] || [];
+ for (let i = 0; i < reviewers[type]!.length; i++) {
+ if (reviewers[type]![i]._account_id === accountID) {
+ this.splice('change.reviewers.' + type, i, 1);
+ break;
+ }
+ }
+ }
+ return;
+ })
+ .catch((err: Error) => {
+ this.disabled = false;
+ throw err;
+ });
+ }
+
+ _handleAddTap(e: Event) {
+ e.preventDefault();
+ const value = {
+ reviewersOnly: false,
+ ccsOnly: false,
+ };
+ if (this.reviewersOnly) {
+ value.reviewersOnly = true;
+ }
+ if (this.ccsOnly) {
+ value.ccsOnly = true;
+ }
+ this.dispatchEvent(
+ new CustomEvent('show-reply-dialog', {
+ detail: {value},
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _handleViewAll() {
+ this._displayedReviewers = this._reviewers;
+ }
+
+ _removeReviewer(id: AccountId): Promise<Response | undefined> {
+ if (!this.change) return Promise.resolve(undefined);
+ return this.$.restAPI.removeChangeReviewer(this.change._number, id);
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
index 521945f..834cc43 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
@@ -163,21 +163,24 @@
element.reviewersOnly = false;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
- assert.deepEqual(fireStub.lastCall.args[0].detail, {value: {}});
+ assert.deepEqual(fireStub.lastCall.args[0].detail, {value: {
+ reviewersOnly: false,
+ ccsOnly: false,
+ }});
element.reviewersOnly = true;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
assert.deepEqual(
fireStub.lastCall.args[0].detail,
- {value: {reviewersOnly: true}});
+ {value: {reviewersOnly: true, ccsOnly: false}});
element.ccsOnly = true;
element.reviewersOnly = false;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
assert.deepEqual(fireStub.lastCall.args[0].detail,
- {value: {ccsOnly: true}});
+ {value: {ccsOnly: true, reviewersOnly: false}});
});
test('dont show all reviewers button with 4 reviewers', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 6f1bc60..aa2e4ce 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -72,6 +72,7 @@
DASHBOARD: /^\/dashboard\/(.+)$/,
CUSTOM_DASHBOARD: /^\/dashboard\/?$/,
PROJECT_DASHBOARD: /^\/p\/(.+)\/\+\/dashboard\/(.+)/,
+ LEGACY_PROJECT_DASHBOARD: /^\/projects\/(.+),dashboards\/(.+)/,
AGREEMENTS: /^\/settings\/agreements\/?/,
NEW_AGREEMENTS: /^\/settings\/new-agreement\/?/,
@@ -880,6 +881,11 @@
'_handleProjectDashboardRoute'
);
+ this._mapRoute(
+ RoutePattern.LEGACY_PROJECT_DASHBOARD,
+ '_handleLegacyProjectDashboardRoute'
+ );
+
this._mapRoute(RoutePattern.GROUP_INFO, '_handleGroupInfoRoute', true);
this._mapRoute(
@@ -1255,6 +1261,10 @@
this.reporting.setRepoName(project);
}
+ _handleLegacyProjectDashboardRoute(data: PageContextWithQueryMap) {
+ this._redirect('/p/' + data.params[0] + '/+/dashboard/' + data.params[1]);
+ }
+
_handleGroupInfoRoute(data: PageContextWithQueryMap) {
this._redirect('/admin/groups/' + encodeURIComponent(data.params[0]));
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index 5ee58c9..927434b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -197,6 +197,7 @@
'_handleImproperlyEncodedPlusRoute',
'_handlePassThroughRoute',
'_handleProjectDashboardRoute',
+ '_handleLegacyProjectDashboardRoute',
'_handleProjectsOldRoute',
'_handleRepoAccessRoute',
'_handleRepoDashboardsRoute',
@@ -617,6 +618,14 @@
handlePassThroughRoute = sinon.stub(element, '_handlePassThroughRoute');
});
+ test('_handleLegacyProjectDashboardRoute', () => {
+ const params = {0: 'gerrit/project', 1: 'dashboard:main'};
+ element._handleLegacyProjectDashboardRoute({params});
+ assert.isTrue(redirectStub.calledOnce);
+ assert.equal(redirectStub.lastCall.args[0],
+ '/p/gerrit/project/+/dashboard/dashboard:main');
+ });
+
test('_handleAgreementsRoute', () => {
const data = {params: {}};
element._handleAgreementsRoute(data);
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index a2036b7..b49f522 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -541,6 +541,9 @@
comments: T[]
): T[] {
return comments.slice(0).sort((c1, c2) => {
+ const d1 = !!(c1 as HumanCommentInfoWithPath).__draft;
+ const d2 = !!(c2 as HumanCommentInfoWithPath).__draft;
+ if (d1 !== d2) return d1 ? 1 : -1;
const dateDiff =
parseDate(c1.updated).valueOf() - parseDate(c2.updated).valueOf();
if (dateDiff) {
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index 0a7c3b5..df5e450 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -227,10 +227,12 @@
},
{
id: 12,
- in_reply_to: 2,
+ in_reply_to: 4,
patch_set: 2,
line: 1,
- updated: makeTime(3),
+ // Draft gets lower timestamp than published comment, because we
+ // want to test that the draft still gets sorted to the end.
+ updated: makeTime(2),
},
],
'file/two': [
@@ -262,7 +264,7 @@
patch_set: 2,
unresolved: true,
line: 1,
- updated: makeTime(2),
+ updated: makeTime(3),
},
],
};
@@ -549,16 +551,16 @@
unresolved: true,
line: 1,
__path: 'file/one',
- updated: '2013-02-26 15:02:43.986000000',
+ updated: '2013-02-26 15:03:43.986000000',
},
{
id: 12,
- in_reply_to: 2,
+ in_reply_to: 4,
patch_set: 2,
line: 1,
__path: 'file/one',
__draft: true,
- updated: '2013-02-26 15:03:43.986000000',
+ updated: '2013-02-26 15:02:43.986000000',
},
],
patchNum: 2,
@@ -708,16 +710,16 @@
patch_set: 2,
unresolved: true,
line: 1,
- updated: '2013-02-26 15:02:43.986000000',
+ updated: '2013-02-26 15:03:43.986000000',
},
{
__path: 'file/one',
__draft: true,
id: 12,
- in_reply_to: 2,
+ in_reply_to: 4,
patch_set: 2,
line: 1,
- updated: '2013-02-26 15:03:43.986000000',
+ updated: '2013-02-26 15:02:43.986000000',
},
];
assert.deepEqual(element._changeComments.getCommentsForThread(4),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 5a36917..7693d56 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -331,7 +331,7 @@
const layers = [this.$.syntaxLayer];
// Get layers from plugins (if any).
for (const pluginLayer of this.$.jsAPI.getDiffLayers(
- this.path, this.changeNum, this.patchNum)) {
+ this.path, this.changeNum)) {
layers.push(pluginLayer);
}
this._layers = layers;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 2502e45..8ac47a2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -347,7 +347,7 @@
// document.getSelection() cannot reference the actual DOM elements making
// up the diff, because they are in the shadow DOM of the gr-diff element.
// This takes the shadow DOM selection if one exists.
- return this.root instanceof ShadowRoot
+ return this.root instanceof ShadowRoot && this.root.getSelection
? this.root.getSelection()
: document.getSelection();
}
@@ -561,7 +561,13 @@
);
return;
}
- this._createComment(el, lineNum);
+
+ // TODO(TS): existing logic always pass undefined lineNum
+ // for file level comment, the drafts API will reject the
+ // request if file level draft contains the `line: 'FILE'` field
+ // probably should do this inside of the _createComment, this
+ // is just to keep existing behavior.
+ this._createComment(el, lineNum === FILE ? undefined : lineNum);
}
createRangeComment() {
@@ -644,7 +650,7 @@
_createComment(
lineEl: Element,
- lineNum: LineNumber,
+ lineNum?: LineNumber,
side?: Side,
range?: CommentRange
) {
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
index e4cf3a1..d970b0a 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
@@ -164,25 +164,30 @@
}
_maybeSetName() {
+ // Note that we are intentionally not acting on this._account.name being the
+ // empty string (which is falsy).
return this._hasNameChange && this.nameMutable && this._account?.name
? this.$.restAPI.setAccountName(this._account.name)
: Promise.resolve();
}
_maybeSetUsername() {
+ // Note that we are intentionally not acting on this._username being the
+ // empty string (which is falsy).
return this._hasUsernameChange && this.usernameMutable && this._username
? this.$.restAPI.setAccountUsername(this._username)
: Promise.resolve();
}
_maybeSetDisplayName() {
- return this._hasDisplayNameChange && this._account?.display_name
+ return this._hasDisplayNameChange &&
+ this._account?.display_name !== undefined
? this.$.restAPI.setAccountDisplayName(this._account.display_name)
: Promise.resolve();
}
_maybeSetStatus() {
- return this._hasStatusChange && this._account?.status
+ return this._hasStatusChange && this._account?.status !== undefined
? this.$.restAPI.setAccountStatus(this._account.status)
: Promise.resolve();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.ts
index 4a47a13..0cb628c 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.ts
@@ -17,7 +17,6 @@
import {GrAnnotation} from '../../diff/gr-diff-highlight/gr-annotation';
import {GrStyleObject} from '../../plugins/gr-styles-api/gr-styles-api';
-import {PatchSetNum} from '../../../types/common';
import {GrDiffLine} from '../../diff/gr-diff/gr-diff-line';
/**
@@ -43,15 +42,12 @@
changeNum: number;
- patchNum: number;
-
constructor(
contentEl: HTMLElement,
lineNumberEl: HTMLElement,
line: GrDiffLine,
path: string,
- changeNum: string | number,
- patchNum: PatchSetNum
+ changeNum: string | number
) {
this._contentEl = contentEl;
this._lineNumberEl = lineNumberEl;
@@ -59,9 +55,10 @@
this.line = line;
this.path = path;
this.changeNum = Number(changeNum);
- this.patchNum = Number(patchNum);
- if (isNaN(this.changeNum) || isNaN(this.patchNum)) {
- console.error('invalid parameters');
+ if (isNaN(this.changeNum)) {
+ console.error(
+ `GrAnnotationActionsContext: Invalid changeNum: ${changeNum}`
+ );
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
index 80e09d4..331fb42 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
@@ -18,7 +18,6 @@
import {GrDiffLine, LineNumber} from '../../diff/gr-diff/gr-diff-line';
import {CoverageRange} from '../../../types/types';
import {Side} from '../../../constants/constants';
-import {PatchSetNum} from '../../../types/common';
import {PluginApi} from '../../plugins/gr-plugin-types';
type AddLayerFunc = (ctx: GrAnnotationActionsContext) => void;
@@ -188,13 +187,11 @@
*
* @param path The file path (eg: /COMMIT_MSG').
* @param changeNum The Gerrit change number.
- * @param patchNum The Gerrit patch number.
*/
- getLayer(path: string, changeNum: number, patchNum: number) {
+ getLayer(path: string, changeNum: number) {
const annotationLayer = new AnnotationLayer(
path,
changeNum,
- patchNum,
this.addLayerFunc
);
this.annotationLayers.push(annotationLayer);
@@ -216,14 +213,12 @@
*
* @param path The file path (eg: /COMMIT_MSG').
* @param changeNum The Gerrit change number.
- * @param patchNum The Gerrit patch number.
* @param addLayerFunc The function
* that will be called when the AnnotationLayer is ready to annotate.
*/
constructor(
readonly path: string,
private readonly changeNum: number,
- private readonly patchNum: number,
private readonly addLayerFunc: AddLayerFunc
) {
this.listeners = [];
@@ -264,8 +259,7 @@
lineNumberEl,
line,
this.path,
- this.changeNum,
- this.patchNum as PatchSetNum
+ this.changeNum
);
this.addLayerFunc(annotationActionsContext);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.js
index e819529..8b3f501 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.js
@@ -52,19 +52,17 @@
const el = document.createElement('div');
el.textContent = str;
const changeNum = 1234;
- const patchNum = 2;
let testLayerFuncCalled = false;
const testLayerFunc = context => {
testLayerFuncCalled = true;
assert.equal(context.line, line);
assert.equal(context.changeNum, changeNum);
- assert.equal(context.patchNum, 2);
};
annotationActions.addLayer(testLayerFunc);
const annotationLayer = annotationActions.getLayer(
- '/dummy/path', changeNum, patchNum);
+ '/dummy/path', changeNum);
const lineNumberEl = document.createElement('td');
annotationLayer.annotate(el, lineNumberEl, line);
@@ -74,8 +72,8 @@
test('add notifier', () => {
const path1 = '/dummy/path1';
const path2 = '/dummy/path2';
- const annotationLayer1 = annotationActions.getLayer(path1, 1, 2);
- const annotationLayer2 = annotationActions.getLayer(path2, 1, 2);
+ const annotationLayer1 = annotationActions.getLayer(path1, 1);
+ const annotationLayer2 = annotationActions.getLayer(path2, 1);
const layer1Spy = sinon.spy(annotationLayer1, 'notifyListeners');
const layer2Spy = sinon.spy(annotationLayer2, 'notifyListeners');
@@ -148,8 +146,7 @@
});
test('layer notify listeners', () => {
- const annotationLayer = annotationActions.getLayer(
- '/dummy/path', 1, 2);
+ const annotationLayer = annotationActions.getLayer('/dummy/path', 1);
let listenerCalledTimes = 0;
const startRange = 10;
const endRange = 20;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index 33ad3ca..ff9446c 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -247,12 +247,12 @@
return revertSubmissionMsg;
}
- getDiffLayers(path: string, changeNum: number, patchNum: number) {
+ getDiffLayers(path: string, changeNum: number) {
const layers = [];
for (const cb of this._getEventCallbacks(EventType.ANNOTATE_DIFF)) {
const annotationApi = (cb as unknown) as GrAnnotationActionsInterface;
try {
- const layer = annotationApi.getLayer(path, changeNum, patchNum);
+ const layer = annotationApi.getLayer(path, changeNum);
layers.push(layer);
} catch (err) {
console.error(err);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
index ddf4c21..0bf6676 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
@@ -37,4 +37,11 @@
details
);
}
+
+ reportLifeCycle(eventName: string, details?: EventDetails) {
+ return this.reporting.reportLifeCycle(
+ `${this.plugin.getPluginName()}-${eventName}`,
+ details
+ );
+ }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
index e05dff3..1229641 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
@@ -57,5 +57,19 @@
{}
);
});
+
+ test('redirect reportLifeCycle call to reportingService', () => {
+ sinon.spy(appContext.reportingService, 'reportLifeCycle');
+ reporting.reportLifeCycle('test', {});
+ assert.isTrue(appContext.reportingService.reportLifeCycle.called);
+ assert.equal(
+ appContext.reportingService.reportLifeCycle.lastCall.args[0],
+ 'testplugin-test'
+ );
+ assert.deepEqual(
+ appContext.reportingService.reportLifeCycle.lastCall.args[1],
+ {}
+ );
+ });
});
});
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index a140f50..f4111e7 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -1125,7 +1125,7 @@
return this._restApiHelper.fetchJSON({
url: `/accounts/${encodeURIComponent(userId)}/status`,
anonymizedUrl: '/accounts/*/status',
- });
+ }) as Promise<string | undefined>;
}
// https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#list-groups
diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index 18c24d2..e3c2ce8 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -534,6 +534,10 @@
getAccountGroups(): Promise<GroupInfo[] | undefined>;
+ getAccountDetails(userId: AccountId): Promise<AccountDetailInfo | undefined>;
+
+ getAccountStatus(userId: AccountId): Promise<string | undefined>;
+
saveAccountAgreement(name: ContributorAgreementInput): Promise<Response>;
generateAccountHttpPassword(): Promise<Password>;
@@ -601,4 +605,8 @@
changeNum: ChangeNum,
messageId: ChangeMessageId
): Promise<Response>;
+ removeChangeReviewer(
+ changeNum: ChangeNum,
+ reviewerID: AccountId | GroupId
+ ): Promise<Response | undefined>;
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index e131b4f..0c4bc8b 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -142,6 +142,10 @@
*/
export type LabelInfo = QuickLabelInfo | DetailedLabelInfo;
+export type Reviewers = {
+ REVIEWER?: AccountInfo[];
+ CC?: AccountInfo[];
+};
interface LabelCommonInfo {
optional?: boolean; // not set if false
}
@@ -211,10 +215,7 @@
labels?: LabelNameToInfoMap;
permitted_labels?: LabelNameToValueMap;
removable_reviewers?: AccountInfo[];
- reviewers?: {
- REVIEWER?: AccountInfo[];
- CC?: AccountInfo[];
- };
+ reviewers?: Reviewers;
pending_reviewers?: AccountInfo[];
reviewer_updates?: ReviewerUpdateInfo[];
messages?: ChangeMessageInfo[];
