Merge "Small redesign of diff expansion row"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 193a96f..a3b9d0b 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -344,6 +344,13 @@
Defaults to true.
+[[label_copyValue]]
+=== `label.Label-Name.copyValue`
+
+Value that should be copied forward when a new patch set is uploaded.
+This can be used to enable sticky votes. Can be specified multiple
+times. By default not set.
+
[[label_canOverride]]
=== `label.Label-Name.canOverride`
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 946dcc1..628a0ec 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -1,64 +1,105 @@
:linkattrs:
= Gerrit Code Review - End to end load tests
-This document provides a description of a Gerrit load test scenario implemented using the link:http://gatling.io[`Gatling`] framework.
+This document provides a description of a Gerrit load test scenario implemented using the
+link:https://gatling.io/[Gatling,role=external,window=_blank] framework.
-Similar scenarios have been successfully used to compare performance of different Gerrit versions or study the Gerrit response
-under different load profiles.
+Similar scenarios have been successfully used to compare performance of different Gerrit versions
+or study the Gerrit response under different load profiles.
== What is Gatling?
-Gatling is a load testing tool which provides out of the box support for the HTTP protocol. Documentation on how to write an
-HTTP load test can be found link:https://gatling.io/docs/current/http/http_protocol/[`here`,role=external,window=_blank].
+Gatling is a load testing tool which provides out of the box support for the HTTP protocol.
+Documentation on how to write an HTTP load test can be found
+link:https://gatling.io/docs/current/http/http_protocol/[here,role=external,window=_blank].
-However, in the scenario we are proposing, we are leveraging the link:https://github.com/GerritForge/gatling-git[`Gatling Git extension`,role=external,window=_blank]
+However, in the scenario we are proposing, we are leveraging the
+link:https://github.com/GerritForge/gatling-git[Gatling Git extension,role=external,window=_blank]
to run tests at 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.
+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.
-Examples of scenarios can be found in the `e2e-tests` directory.
+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.
-=== How to run the load tests
+== How to build the tests
-==== How to build
+An link:https://www.scala-sbt.org/download.html[sbt-based installation,role=external,window=_blank]
+of link:https://www.scala-lang.org/download/[Scala,role=external,window=_blank] is required.
-An link:https://www.scala-sbt.org/download.html[sbt-based installation] of
-link:https://www.scala-lang.org/download/[Scala] is required.
-
-The `scalaVersion` used by `sbt` once installed is defined in the `build.sbt` file.
-That specific version of Scala is automatically used by `sbt` while building:
+The `scalaVersion` used by `sbt` once installed is defined in the `build.sbt` file. That specific
+version of Scala is automatically used by `sbt` while building:
----
sbt compile
----
-==== Setup
+The following warning, if present when executing `sbt` commands, can be removed by creating the
+link:https://www.scala-sbt.org/1.x/docs/Using-Sonatype.html#step+3%3A+Credentials[related credentials file,role=external,window=_blank]
+locally. Dummy values for `user` and `password` in that file can be used initially.
+
+----
+[warn] Credentials file ~/.sbt/sonatype_credentials does not exist
+----
+
+Every `sbt` command can include an optional log level
+link:https://www.scala-sbt.org/1.x/docs/Howto-Logging.html#Change+the+logging+level+globally[argument,role=external,window=_blank].
+Below, `[info]` logs are no longer shown:
+
+----
+sbt --warn compile
+----
+
+=== How to build using Docker
+
+----
+docker build . -t e2e-tests
+----
+
+== How to set-up
+
+=== SSH keys
If you are running SSH commands, the private keys of the users used for testing need to go in
`/tmp/ssh-keys`. The keys need to be generated this way (JSch won't validate them
link:https://stackoverflow.com/questions/53134212/invalid-privatekey-when-using-jsch[otherwise,role=external,window=_blank]):
----
+mkdir /tmp/ssh-keys
ssh-keygen -m PEM -t rsa -C "test@mail.com" -f /tmp/ssh-keys/id_rsa
----
-*NOTE*: Don't forget to add the public keys for the testing user(s) to your git server.
+The public key in `/tmp/ssh-keys/id_rsa.pub` has to be added to the test user(s) `SSH Keys` in
+Gerrit. Now, the host from which the latter runs may need public key scanning to become known.
+This applies to the local user that runs the forthcoming `sbt` testing commands. An example
+assuming `localhost` follows:
-==== Input file
+----
+ssh-keyscan -t rsa -p 29418 localhost > ~/.ssh/known_hosts
+----
-The `ReplayRecordsFromFeederScenario` is fed with the data coming from the
-`src/test/resources/data/requests.json` file. Such a file contains the commands and repo used
-during the load test. Example below:
+=== Input file
+
+The `CloneUsingBothProtocols` scenario is fed with the data coming from the
+`src/test/resources/data/CloneUsingBothProtocols.json` file. Such a file contains the commands and
+repository used during the load test. That file currently looks like below. This scenario serves
+as a simple example with no actual load in it. It can be used to test or validate the local setup.
+More complex scenarios can be further developed, under the `com.google.gerrit.scenarios` package.
----
[
{
- "url": "ssh://admin@localhost:29418/loadtest-repo.git",
+ "url": "ssh://admin@localhost:29418/loadtest-repo",
"cmd": "clone"
},
{
- "url": "http://localhost:8080/loadtest-repo.git",
- "cmd": "fetch"
+ "url": "http://localhost:8080/loadtest-repo",
+ "cmd": "clone"
}
]
----
@@ -70,7 +111,9 @@
* `push`
* `clone`
-==== How to use the framework
+The example above assumes that the `loadtest-repo` project exists in the Gerrit under test.
+
+== How to run tests
Run all tests:
----
@@ -79,7 +122,7 @@
Run a single test:
----
-sbt "gatling:testOnly com.google.gerrit.scenarios.ReplayRecordsFromFeederScenario"
+sbt "gatling:testOnly com.google.gerrit.scenarios.CloneUsingBothProtocols"
----
Generate the last report:
@@ -87,6 +130,16 @@
sbt "gatling:lastReport"
----
+The `src/test/resources/logback.xml` file
+link:http://logback.qos.ch/manual/configuration.html[configures,role=external,window=_blank]
+Gatling's logging level.
+
+=== How to run using Docker
+
+----
+docker run -it e2e-tests -s com.google.gerrit.scenarios.CloneUsingBothProtocols
+----
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 9714e18..3266cb1 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -476,9 +476,9 @@
----
Plugins which define new Events should register them via the
-`com.google.gerrit.server.events.EventTypes.registerClass()`
-method. This will make the EventType known to the system.
-Deserializing events with the
+`com.google.gerrit.server.events.EventTypes.register()` method.
+This will make the EventType known to the system. Deserializing
+events with the
`com.google.gerrit.server.events.EventDeserializer` class requires
that the event be registered in EventTypes.
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 00e5b97..41c241f 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -65,6 +65,18 @@
=== HTTP
+==== Jetty
+
+* `http/server/jetty/threadpool/active_threads`: Active threads
+* `http/server/jetty/threadpool/idle_threads`: Idle threads
+* `http/server/jetty/threadpool/reserved_threads`: Reserved threads
+* `http/server/jetty/threadpool/max_pool_size`: Maximum thread pool size
+* `http/server/jetty/threadpool/min_pool_size`: Minimum thread pool size
+* `http/server/jetty/threadpool/pool_size`: Current thread pool size
+* `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a thread
+
+==== REST API
+
* `http/server/error_count`: Rate of REST API error responses.
* `http/server/success_count`: Rate of REST API success responses.
* `http/server/rest_api/count`: Rate of REST API calls by view.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ca0e188..284eb27 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -5502,8 +5502,8 @@
}
----
-As response a link:#cherry-pick-change-info[CherryPickChangeInfo]
-entity is returned that describes the resulting cherry-pick change.
+As response a link:#change-info[ChangeInfo] entity is returned that
+describes the resulting cherry-pick change.
.Response
----
@@ -6042,10 +6042,21 @@
If the change that triggered the submission also has a topic, it will be
"<id>-<topic>" of the change that triggered the submission.
The callers must not rely on the format of the submission ID.
-|`cherry_pick_of_change` |optional|
+|`cherry_pick_of_change` |optional|
The numeric Change-Id of the change that this change was cherry-picked from.
-|`cherry_pick_of_patch_set` |optional|
+|`cherry_pick_of_patch_set`|optional|
The patchset number of the change that this change was cherry-picked from.
+|`contains_git_conflicts` |optional, not set if `false`|
+Whether the change contains conflicts. +
+If `true`, some of the file contents of the change contain git conflict
+markers to indicate the conflicts. +
+Only set if this change info is returned in response to a request that
+creates a new change or patch set and conflicts are allowed. In
+particular this field is only populated if the change info is returned
+by one of the following REST endpoints: link:#create-change[Create
+Change], link:#create-merge-patch-set-for-change[Create Merge Patch Set
+For Change], link:#cherry-pick[Cherry Pick Revision],
+link:rest-api-project.html#cherry-pick-commit[Cherry Pick Commit]
|==================================
[[change-input]]
@@ -6124,23 +6135,6 @@
Which patchset (if any) generated this message.
|==================================
-[[cherry-pick-change-info]]
-=== CherryPickChangeInfo
-The `CherryPickChangeInfo` entity contains information about a
-cherry-pick change.
-
-`CherryPickChangeInfo` has the same fields as link:#change-info[
-ChangeInfo]. In addition `CherryPickChangeInfo` has the following
-fields:
-
-[options="header",cols="1,^1,5"]
-|======================================
-|Field Name ||Description
-|`contains_git_conflicts` |optional, not set if `false`|
-Whether any file in the change contains Git conflict markers.
-|======================================
-
-
[[cherrypick-input]]
=== CherryPickInput
The `CherryPickInput` entity contains information for cherry-picking a change to a new branch.
@@ -6172,9 +6166,8 @@
there are conflicts. If there are conflicts the file contents of the
created change contain git conflict markers to indicate the conflicts.
Callers can find out if there were conflicts by checking the
-`contains_git_conflicts` field in the link:#cherry-pick-change-info[
-CherryPickChangeInfo] that is returned by the cherry-pick REST
-endpoints. If there are conflicts the cherry-pick change is marked as
+`contains_git_conflicts` field in the link:#change-info[ChangeInfo]. If
+there are conflicts the cherry-pick change is marked as
work-in-progress.
|===========================
@@ -6804,21 +6797,31 @@
The `MergeInput` entity contains information about the merge
[options="header",cols="1,^1,5"]
-|============================
-|Field Name ||Description
-|`source` ||
+|==============================
+|Field Name ||Description
+|`source` ||
The source to merge from, e.g. a complete or abbreviated commit SHA-1,
a complete reference name, a short reference name under `refs/heads`, `refs/tags`,
or `refs/remotes` namespace, etc.
-|`source_branch` |optional|
+|`source_branch` |optional|
A branch from which `source` is reachable. If specified,
`source` is checked for visibility and reachability against only this
branch. This speeds up the operation, especially for large repos with
many branches.
-|`strategy` |optional|
+|`strategy` |optional|
The strategy of the merge, can be `recursive`, `resolve`,
`simple-two-way-in-core`, `ours` or `theirs`, default will use project settings.
-|============================
+|`allow_conflicts`|optional, defaults to false|
+If `true`, creating the merge succeeds also if there are conflicts. +
+If there are conflicts the file contents of the created change contain
+git conflict markers to indicate the conflicts. +
+Callers can find out whether there were conflicts by checking the
+`contains_git_conflicts` field in the link:#change-info[ChangeInfo]. +
+If there are conflicts the change is marked as work-in-progress. +
+This option is not supported for all merge strategies (e.g. it's
+supported for `recursive` and `resolve`, but not for
+`simple-two-way-in-core`).
+|==============================
[[merge-patch-set-input]]
=== MergePatchSetInput
@@ -7218,9 +7221,12 @@
|`reviewer` ||
The link:rest-api-accounts.html#account-id[ID] of one account that
should be added as reviewer or the link:rest-api-groups.html#group-id[
-ID] of one group for which all members should be added as reviewers. +
+ID] of one internal group for which all members should be added as reviewers. +
If an ID identifies both an account and a group, only the account is
added as reviewer to the change.
+External groups, such as LDAP groups, will be silently omitted from a
+link:#set-review[set-review] or
+link:rest-api-changes.html#add-reviewer[add-reviewer] call.
|`state` |optional|
Add reviewer in this state. Possible reviewer states are `REVIEWER`
and `CC`. If not given, defaults to `REVIEWER`.
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 6fbb338..a4e27b3 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -2576,9 +2576,8 @@
}
----
-As response a link:rest-api-changes.html#cherry-pick-change-info[
-CherryPickChangeInfo] entity is returned that describes the resulting
-cherry-picked change.
+As response a link:rest-api-changes.html#change-info[ChangeInfo] entity
+is returned that describes the resulting cherry-picked change.
.Response
----
@@ -3917,6 +3916,8 @@
|`copy_all_scores_on_merge_first_parent_update`|`false` if not set|
Whether link:config-labels.html#label_copyAllScoresOnMergeFirstParentUpdate[
copyAllScoresOnMergeFirstParentUpdate] is set on the label.
+|`copy_values` |optional|
+List of values that should be copied forward when a new patch set is uploaded.
|`allow_post_submit`|`false` if not set|
Whether link:config-labels.html#label_allowPostSubmit[allowPostSubmit] is set
on the label.
@@ -3982,6 +3983,8 @@
|`copy_all_scores_on_merge_first_parent_update`|optional|
Whether link:config-labels.html#label_copyAllScoresOnMergeFirstParentUpdate[
copyAllScoresOnMergeFirstParentUpdate] is set on the label.
+|`copy_values` |optional|
+List of values that should be copied forward when a new patch set is uploaded.
|`allow_post_submit`|optional|
Whether link:config-labels.html#label_allowPostSubmit[allowPostSubmit] is set
on the label.
diff --git a/WORKSPACE b/WORKSPACE
index 1619c45..7c4ee1b 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -57,10 +57,10 @@
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "f04d2373bcaf8aa09bccb08a98a57e721306c8f6043a2a0ee610fd6853dcde3d",
+ sha256 = "b34cbe1a7514f5f5487c3bfee7340a4496713ddf4f119f7a225583d6cafd793a",
urls = [
- "https://storage.googleapis.com/bazel-mirror/github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
- "https://github.com/bazelbuild/rules_go/releases/download/0.18.6/rules_go-0.18.6.tar.gz",
+ "https://storage.googleapis.com/bazel-mirror/github.com/bazelbuild/rules_go/releases/download/v0.21.1/rules_go-v0.21.1.tar.gz",
+ "https://github.com/bazelbuild/rules_go/releases/download/v0.21.1/rules_go-v0.21.1.tar.gz",
],
)
diff --git a/e2e-tests/load-tests/.gitignore b/e2e-tests/load-tests/.gitignore
index 052f424..097d9fa 100644
--- a/e2e-tests/load-tests/.gitignore
+++ b/e2e-tests/load-tests/.gitignore
@@ -1,16 +1,13 @@
-.idea/
+/.idea/
+
+# mpeltonen/sbt-idea plugin
+/.idea_modules/
# File-based project format
*.iws
# IntelliJ
-out/
+/out/
-# mpeltonen/sbt-idea plugin
-.idea_modules/
-
-### Scala ###
-*.class
-*.log
-target
-project/target
+# Scala sbt
+target/
diff --git a/e2e-tests/load-tests/build.sbt b/e2e-tests/load-tests/build.sbt
index 46a3202..685db37 100644
--- a/e2e-tests/load-tests/build.sbt
+++ b/e2e-tests/load-tests/build.sbt
@@ -4,15 +4,15 @@
lazy val gatlingGitExtension = RootProject(uri("git://github.com/GerritForge/gatling-git.git"))
lazy val root = (project in file("."))
- .settings(
- inThisBuild(List(
- organization := "com.google.gerrit",
- scalaVersion := "2.12.8",
- version := "0.1.0-SNAPSHOT"
- )),
- name := "gerrit",
- libraryDependencies ++=
- gatling ++
- Seq("io.gatling" % "gatling-core" % "3.1.1" ) ++
- Seq("io.gatling" % "gatling-app" % "3.1.1" )
- ) dependsOn(gatlingGitExtension)
+ .settings(
+ inThisBuild(List(
+ organization := "com.google.gerrit",
+ scalaVersion := "2.12.8",
+ version := "0.1.0-SNAPSHOT"
+ )),
+ name := "gerrit",
+ libraryDependencies ++=
+ gatling ++
+ Seq("io.gatling" % "gatling-core" % "3.1.1") ++
+ Seq("io.gatling" % "gatling-app" % "3.1.1")
+ ) dependsOn gatlingGitExtension
diff --git a/e2e-tests/load-tests/project/build.properties b/e2e-tests/load-tests/project/build.properties
index 0cd8b07..a82bb05 100644
--- a/e2e-tests/load-tests/project/build.properties
+++ b/e2e-tests/load-tests/project/build.properties
@@ -1 +1 @@
-sbt.version=1.2.3
+sbt.version=1.3.7
diff --git a/e2e-tests/load-tests/src/test/resources/data/CloneUsingBothProtocols.json b/e2e-tests/load-tests/src/test/resources/data/CloneUsingBothProtocols.json
new file mode 100644
index 0000000..0335b2f
--- /dev/null
+++ b/e2e-tests/load-tests/src/test/resources/data/CloneUsingBothProtocols.json
@@ -0,0 +1,10 @@
+[
+ {
+ "url": "ssh://admin@localhost:29418/loadtest-repo",
+ "cmd": "clone"
+ },
+ {
+ "url": "http://localhost:8080/loadtest-repo",
+ "cmd": "clone"
+ }
+]
diff --git a/e2e-tests/load-tests/src/test/resources/data/requests.json b/e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/data/requests.json
rename to e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
diff --git a/e2e-tests/load-tests/src/test/resources/logback.xml b/e2e-tests/load-tests/src/test/resources/logback.xml
new file mode 100644
index 0000000..a139e69
--- /dev/null
+++ b/e2e-tests/load-tests/src/test/resources/logback.xml
@@ -0,0 +1,12 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ <root level="warn">
+ <appender-ref ref="STDOUT"/>
+ </root>
+</configuration>
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
new file mode 100644
index 0000000..c5a7cba
--- /dev/null
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
@@ -0,0 +1,32 @@
+// 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.scenarios
+
+import io.gatling.core.Predef._
+import io.gatling.core.structure.ScenarioBuilder
+
+import scala.concurrent.duration._
+
+class CloneUsingBothProtocols extends GitSimulation {
+
+ private val test: ScenarioBuilder = scenario(name)
+ .feed(data)
+ .exec(request)
+
+ setUp(
+ test.inject(
+ constantUsersPerSec(1) during (2 seconds)
+ )).protocols(protocol)
+}
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala
new file mode 100644
index 0000000..4d5130f
--- /dev/null
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala
@@ -0,0 +1,48 @@
+// 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.scenarios
+
+import java.io.{File, IOException}
+
+import com.github.barbasa.gatling.git.protocol.GitProtocol
+import com.github.barbasa.gatling.git.request.builder.GitRequestBuilder
+import com.github.barbasa.gatling.git.{GatlingGitConfiguration, GitRequestSession}
+import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
+import org.apache.commons.io.FileUtils
+import org.eclipse.jgit.hooks.CommitMsgHook
+
+class GitSimulation extends Simulation {
+
+ implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+ implicit val postMessageHook: Option[String] = Some(s"hooks/${CommitMsgHook.NAME}")
+
+ protected val name: String = this.getClass.getSimpleName
+ protected val data: FileBasedFeederBuilder[Any]#F = jsonFile(s"data/$name.json").circular
+ protected val request = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
+ protected val protocol: GitProtocol = GitProtocol()
+
+ after {
+ Thread.sleep(5000)
+ val path = conf.tmpBasePath
+ try {
+ FileUtils.deleteDirectory(new File(path))
+ } catch {
+ case e: IOException =>
+ System.err.println("Unable to delete temporary directory " + path)
+ e.printStackTrace()
+ }
+ }
+}
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
index c0eab39..82342be 100644
--- a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
@@ -14,59 +14,26 @@
package com.google.gerrit.scenarios
-import com.github.barbasa.gatling.git.protocol.GitProtocol
-import com.github.barbasa.gatling.git.request.builder.GitRequestBuilder
import io.gatling.core.Predef._
import io.gatling.core.structure.ScenarioBuilder
-import java.io._
-
-import com.github.barbasa.gatling.git.{
- GatlingGitConfiguration,
- GitRequestSession
-}
-import org.apache.commons.io.FileUtils
import scala.concurrent.duration._
-import org.eclipse.jgit.hooks._
-class ReplayRecordsFromFeederScenario extends Simulation {
+class ReplayRecordsFromFeeder extends GitSimulation {
- val gitProtocol = GitProtocol()
- implicit val conf = GatlingGitConfiguration()
- implicit val postMessageHook: Option[String] = Some(
- s"hooks/${CommitMsgHook.NAME}")
-
- val feeder = jsonFile("data/requests.json").circular
-
- val replayCallsScenario: ScenarioBuilder =
- scenario("Git commands")
+ private val test: ScenarioBuilder = scenario(name)
.repeat(10000) {
- feed(feeder)
- .exec(new GitRequestBuilder(GitRequestSession("${cmd}", "${url}")))
+ feed(data)
+ .exec(request)
}
setUp(
- replayCallsScenario.inject(
+ test.inject(
nothingFor(4 seconds),
atOnceUsers(10),
rampUsers(10) during (5 seconds),
constantUsersPerSec(20) during (15 seconds),
constantUsersPerSec(20) during (15 seconds) randomized
- ))
- .protocols(gitProtocol)
- .maxDuration(60 seconds)
-
- after {
- try {
- //After is often called too early. Some retries should be implemented.
- Thread.sleep(5000)
- FileUtils.deleteDirectory(new File(conf.tmpBasePath))
- } catch {
- case e: IOException => {
- System.err.println(
- "Unable to delete temporary directory: " + conf.tmpBasePath)
- e.printStackTrace
- }
- }
- }
+ )).protocols(protocol)
+ .maxDuration(60 seconds)
}
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index f9116a1..2371bd0 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.events.GroupIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
+import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl;
@@ -69,6 +70,7 @@
private final DynamicSet<AccountActivationValidationListener>
accountActivationValidationListeners;
private final DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
+ private final DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners;
@Inject
ExtensionRegistry(
@@ -93,7 +95,8 @@
DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
DynamicSet<GroupBackend> groupBackends,
DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
- DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners) {
+ DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners,
+ DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -116,6 +119,7 @@
this.groupBackends = groupBackends;
this.accountActivationValidationListeners = accountActivationValidationListeners;
this.onSubmitValidationListeners = onSubmitValidationListeners;
+ this.workInProgressStateChangedListeners = workInProgressStateChangedListeners;
}
public Registration newRegistration() {
@@ -219,6 +223,10 @@
return add(onSubmitValidationListeners, onSubmitValidationListener);
}
+ public Registration add(WorkInProgressStateChangedListener workInProgressStateChangedListener) {
+ return add(workInProgressStateChangedListeners, workInProgressStateChangedListener);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 14b8310..964cf67 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -14,14 +14,17 @@
package com.google.gerrit.common.data;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toList;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSetApproval;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -37,6 +40,7 @@
public static final boolean DEF_COPY_ANY_SCORE = false;
public static final boolean DEF_COPY_MAX_SCORE = false;
public static final boolean DEF_COPY_MIN_SCORE = false;
+ public static final ImmutableList<Short> DEF_COPY_VALUES = ImmutableList.of();
public static final boolean DEF_IGNORE_SELF_APPROVAL = false;
public static LabelType withDefaultValues(String name) {
@@ -104,6 +108,7 @@
protected boolean copyAllScoresOnTrivialRebase;
protected boolean copyAllScoresIfNoCodeChange;
protected boolean copyAllScoresIfNoChange;
+ protected ImmutableList<Short> copyValues;
protected boolean allowPostSubmit;
protected boolean ignoreSelfApproval;
protected short defaultValue;
@@ -144,6 +149,7 @@
setCopyAnyScore(DEF_COPY_ANY_SCORE);
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
+ setCopyValues(DEF_COPY_VALUES);
setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
setIgnoreSelfApproval(DEF_IGNORE_SELF_APPROVAL);
@@ -298,6 +304,14 @@
this.copyAllScoresIfNoChange = copyAllScoresIfNoChange;
}
+ public ImmutableList<Short> getCopyValues() {
+ return copyValues;
+ }
+
+ public void setCopyValues(Collection<Short> copyValues) {
+ this.copyValues = copyValues.stream().sorted().collect(toImmutableList());
+ }
+
public boolean isMaxNegative(PatchSetApproval ca) {
return maxNegative == ca.value();
}
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index c768094..b36b5f9 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -16,20 +16,14 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.entities.RefNames.REFS_CHANGES;
-import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.ChangeStatus;
-import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
import java.sql.Timestamp;
import java.util.Arrays;
import java.util.Optional;
-import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
/**
* A change proposed to be merged into a branch.
@@ -101,15 +95,6 @@
* notice of a replacement patch set is sent, or when notice of the change submission occurs.
*/
public final class Change {
- private static final SecureRandom rng;
-
- static {
- try {
- rng = SecureRandom.getInstance("SHA1PRNG");
- } catch (NoSuchAlgorithmException e) {
- throw new RuntimeException("Cannot create RNG for Change-Id generator", e);
- }
- }
public static Id id(int id) {
return new AutoValue_Change_Id(id);
@@ -282,20 +267,6 @@
}
}
- public static ObjectId generateChangeId() {
- byte[] rand = new byte[Constants.OBJECT_ID_STRING_LENGTH];
- rng.nextBytes(rand);
- String randomString = new String(rand, UTF_8);
-
- try (ObjectInserter f = new ObjectInserter.Formatter()) {
- return f.idFor(Constants.OBJ_COMMIT, Constants.encode(randomString));
- }
- }
-
- public static Key generateKey() {
- return key("I" + generateChangeId().name());
- }
-
public static Key key(String key) {
return new AutoValue_Change_Key(key);
}
diff --git a/java/com/google/gerrit/exceptions/BUILD b/java/com/google/gerrit/exceptions/BUILD
index ef59be1..873b659 100644
--- a/java/com/google/gerrit/exceptions/BUILD
+++ b/java/com/google/gerrit/exceptions/BUILD
@@ -4,5 +4,8 @@
name = "exceptions",
srcs = glob(["*.java"]),
visibility = ["//visibility:public"],
- deps = ["//java/com/google/gerrit/entities"],
+ deps = [
+ "//java/com/google/gerrit/entities",
+ "//lib:jgit",
+ ],
)
diff --git a/java/com/google/gerrit/exceptions/MergeWithConflictsNotSupportedException.java b/java/com/google/gerrit/exceptions/MergeWithConflictsNotSupportedException.java
new file mode 100644
index 0000000..cab1d22
--- /dev/null
+++ b/java/com/google/gerrit/exceptions/MergeWithConflictsNotSupportedException.java
@@ -0,0 +1,25 @@
+// 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.exceptions;
+
+import org.eclipse.jgit.merge.MergeStrategy;
+
+public class MergeWithConflictsNotSupportedException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ public MergeWithConflictsNotSupportedException(MergeStrategy strategy) {
+ super("merge with conflicts is not supported with merge strategy: " + strategy.getName());
+ }
+}
diff --git a/java/com/google/gerrit/extensions/api/changes/Changes.java b/java/com/google/gerrit/extensions/api/changes/Changes.java
index bcb49de1..8609207 100644
--- a/java/com/google/gerrit/extensions/api/changes/Changes.java
+++ b/java/com/google/gerrit/extensions/api/changes/Changes.java
@@ -69,6 +69,8 @@
ChangeApi create(ChangeInput in) throws RestApiException;
+ ChangeInfo createAsInfo(ChangeInput in) throws RestApiException;
+
QueryRequest query();
QueryRequest query(String query);
@@ -208,6 +210,11 @@
}
@Override
+ public ChangeInfo createAsInfo(ChangeInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public QueryRequest query() {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 7d4f555..6c6389e5 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -19,7 +19,7 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
-import com.google.gerrit.extensions.common.CherryPickChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.DiffInfo;
@@ -68,7 +68,7 @@
ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
- CherryPickChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException;
+ ChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException;
default ChangeApi rebase() throws RestApiException {
RebaseInput in = new RebaseInput();
@@ -204,7 +204,7 @@
}
@Override
- public CherryPickChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException {
+ public ChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/client/ListOption.java b/java/com/google/gerrit/extensions/client/ListOption.java
index e694c0e..4dea42f 100644
--- a/java/com/google/gerrit/extensions/client/ListOption.java
+++ b/java/com/google/gerrit/extensions/client/ListOption.java
@@ -16,6 +16,7 @@
import java.lang.reflect.InvocationTargetException;
import java.util.EnumSet;
+import java.util.Set;
/** Enum that can be expressed as a bitset in query parameters. */
public interface ListOption {
@@ -46,4 +47,13 @@
}
return r;
}
+
+ static String toHex(Set<ListChangesOption> options) {
+ int v = 0;
+ for (ListChangesOption option : options) {
+ v |= 1 << option.getValue();
+ }
+
+ return Integer.toHexString(v);
+ }
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 3b3f2ad..d4a8477 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -56,6 +56,22 @@
public Integer cherryPickOfChange;
public Integer cherryPickOfPatchSet;
+ /**
+ * Whether the change contains conflicts.
+ *
+ * <p>If {@code true}, some of the file contents of the change contain git conflict markers to
+ * indicate the conflicts.
+ *
+ * <p>Only set if this change info is returned in response to a request that creates a new change
+ * or patch set and conflicts are allowed. In particular this field is only populated if the
+ * change info is returned by one of the following REST endpoints: {@link
+ * com.google.gerrit.server.restapi.change.CreateChange}, {@link
+ * com.google.gerrit.server.restapi.change.CreateMergePatchSet}, {@link
+ * com.google.gerrit.server.restapi.change.CherryPick}, {@link
+ * com.google.gerrit.server.restapi.change.CherryPickCommit}
+ */
+ public Boolean containsGitConflicts;
+
public int _number;
public AccountInfo owner;
diff --git a/java/com/google/gerrit/extensions/common/CherryPickChangeInfo.java b/java/com/google/gerrit/extensions/common/CherryPickChangeInfo.java
deleted file mode 100644
index 5e2b902..0000000
--- a/java/com/google/gerrit/extensions/common/CherryPickChangeInfo.java
+++ /dev/null
@@ -1,19 +0,0 @@
-// Copyright (C) 2018 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.extensions.common;
-
-public class CherryPickChangeInfo extends ChangeInfo {
- public Boolean containsGitConflicts;
-}
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
index 64c3997..f552566 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInfo.java
@@ -32,6 +32,7 @@
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
+ public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}
diff --git a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
index 0523f61..23d5df1 100644
--- a/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
+++ b/java/com/google/gerrit/extensions/common/LabelDefinitionInput.java
@@ -31,6 +31,7 @@
public Boolean copyAllScoresIfNoCodeChange;
public Boolean copyAllScoresOnTrivialRebase;
public Boolean copyAllScoresOnMergeFirstParentUpdate;
+ public List<Short> copyValues;
public Boolean allowPostSubmit;
public Boolean ignoreSelfApproval;
}
diff --git a/java/com/google/gerrit/extensions/common/MergeInput.java b/java/com/google/gerrit/extensions/common/MergeInput.java
index c3cfcee..7de5b38 100644
--- a/java/com/google/gerrit/extensions/common/MergeInput.java
+++ b/java/com/google/gerrit/extensions/common/MergeInput.java
@@ -35,4 +35,12 @@
* @see org.eclipse.jgit.merge.MergeStrategy
*/
public String strategy;
+
+ /**
+ * Whether the creation of the merge should succeed if there are conflicts.
+ *
+ * <p>If there are conflicts the file contents of the created change contain git conflict markers
+ * to indicate the conflicts.
+ */
+ public boolean allowConflicts;
}
diff --git a/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java b/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java
deleted file mode 100644
index 180a0e6..0000000
--- a/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.extensions.systemstatus;
-
-import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import java.util.Calendar;
-import java.util.Date;
-import java.util.TimeZone;
-
-/**
- * Supplies a message of the day when the page is first loaded.
- *
- * <pre>
- * DynamicSet.bind(binder(), MessageOfTheDay.class).to(MyMessage.class);
- * </pre>
- */
-@ExtensionPoint
-public abstract class MessageOfTheDay {
- /**
- * Retrieve the message of the day as an HTML fragment.
- *
- * @return message as an HTML fragment; null if no message is available.
- */
- public abstract String getHtmlMessage();
-
- /**
- * Unique identifier for this message.
- *
- * <p>Messages with the same identifier will be hidden from the user until redisplay has occurred.
- *
- * @return unique message identifier. This identifier should be unique within the server.
- */
- public abstract String getMessageId();
-
- /**
- * When should the message be displayed?
- *
- * <p>Default implementation returns {@code tomorrow at 00:00:00 GMT}.
- *
- * @return a future date after which the message should be redisplayed.
- */
- public Date getRedisplay() {
- Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT"));
- cal.set(Calendar.HOUR_OF_DAY, 0);
- cal.set(Calendar.MINUTE, 0);
- cal.set(Calendar.SECOND, 0);
- cal.set(Calendar.MILLISECOND, 0);
- cal.add(Calendar.DAY_OF_MONTH, 1);
- return cal.getTime();
- }
-}
diff --git a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
index c7b65d0..ea0c148 100644
--- a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
+++ b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
@@ -237,7 +237,7 @@
try {
return SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
- throw new IllegalArgumentException("No SecureRandom available for GitHub authentication", e);
+ throw new IllegalStateException("No SecureRandom available for GitHub authentication", e);
}
}
diff --git a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
index f9e6286..b987c68 100644
--- a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
+++ b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
@@ -222,7 +222,7 @@
try {
return SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
- throw new IllegalArgumentException("No SecureRandom available for GitHub authentication", e);
+ throw new IllegalStateException("No SecureRandom available for GitHub authentication", e);
}
}
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index b1d4ac6..6a66ba3 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -18,15 +18,20 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.config.Server;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.json.OutputFormat;
import com.google.gson.Gson;
import com.google.template.soy.data.SanitizedContent;
@@ -34,11 +39,46 @@
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
import java.util.function.Function;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/** Helper for generating parts of {@code index.html}. */
public class IndexHtmlUtil {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ static final String changeCanonicalUrl = ".*/c/(?<project>.+)/\\+/(?<changeNum>\\d+)";
+ static final String basePatchNumUrlPart = "(/(-?\\d+|edit)(\\.\\.(\\d+|edit))?)";
+ static final Pattern changeUrlPattern =
+ Pattern.compile(changeCanonicalUrl + basePatchNumUrlPart + "?" + "/?$");
+ static final Pattern diffUrlPattern =
+ Pattern.compile(changeCanonicalUrl + basePatchNumUrlPart + "(/(.+))" + "/?$");
+
+ public static String getDefaultChangeDetailHex() {
+ Set<ListChangesOption> options =
+ ImmutableSet.of(
+ ListChangesOption.ALL_COMMITS,
+ ListChangesOption.ALL_REVISIONS,
+ ListChangesOption.CHANGE_ACTIONS,
+ ListChangesOption.DETAILED_LABELS,
+ ListChangesOption.DOWNLOAD_COMMANDS,
+ ListChangesOption.MESSAGES,
+ ListChangesOption.SUBMITTABLE,
+ ListChangesOption.WEB_LINKS,
+ ListChangesOption.SKIP_DIFFSTAT);
+
+ return ListOption.toHex(options);
+ }
+
+ public static String getDefaultDiffDetailHex() {
+ Set<ListChangesOption> options =
+ ImmutableSet.of(
+ ListChangesOption.ALL_COMMITS,
+ ListChangesOption.ALL_REVISIONS,
+ ListChangesOption.SKIP_DIFFSTAT);
+
+ return ListOption.toHex(options);
+ }
/**
* Returns both static and dynamic parameters of {@code index.html}. The result is to be used when
@@ -50,12 +90,18 @@
String cdnPath,
String faviconPath,
Map<String, String[]> urlParameterMap,
- Function<String, SanitizedContent> urlInScriptTagOrdainer)
+ Function<String, SanitizedContent> urlInScriptTagOrdainer,
+ String requestedURL)
throws URISyntaxException, RestApiException {
return ImmutableMap.<String, Object>builder()
.putAll(
staticTemplateData(
- canonicalURL, cdnPath, faviconPath, urlParameterMap, urlInScriptTagOrdainer))
+ canonicalURL,
+ cdnPath,
+ faviconPath,
+ urlParameterMap,
+ urlInScriptTagOrdainer,
+ requestedURL))
.putAll(dynamicTemplateData(gerritApi))
.build();
}
@@ -98,7 +144,8 @@
String cdnPath,
String faviconPath,
Map<String, String[]> urlParameterMap,
- Function<String, SanitizedContent> urlInScriptTagOrdainer)
+ Function<String, SanitizedContent> urlInScriptTagOrdainer,
+ String requestedURL)
throws URISyntaxException {
String canonicalPath = computeCanonicalPath(canonicalURL);
@@ -133,9 +180,39 @@
if (urlParameterMap.containsKey("gf")) {
data.put("useGoogleFonts", "true");
}
+
+ if (urlParameterMap.containsKey("pl") && requestedURL != null) {
+ data.put("defaultChangeDetailHex", getDefaultChangeDetailHex());
+ data.put("defaultDiffDetailHex", getDefaultDiffDetailHex());
+
+ String changeRequestsPath = computeChangeRequestsPath(requestedURL, changeUrlPattern);
+ if (changeRequestsPath != null) {
+ data.put("preloadChangePage", "true");
+ } else {
+ changeRequestsPath = computeChangeRequestsPath(requestedURL, diffUrlPattern);
+ data.put("preloadDiffPage", "true");
+ }
+
+ if (changeRequestsPath != null) {
+ data.put("changeRequestsPath", changeRequestsPath);
+ }
+ }
+
return data.build();
}
+ static String computeChangeRequestsPath(String requestedURL, Pattern pattern) {
+ Matcher matcher = pattern.matcher(requestedURL);
+ if (matcher.matches()) {
+ Integer changeId = Ints.tryParse(matcher.group("changeNum"));
+ if (changeId != null) {
+ return "changes/" + Url.encode(matcher.group("project")) + "~" + changeId;
+ }
+ }
+
+ return null;
+ }
+
private static String computeCanonicalPath(@Nullable String canonicalURL)
throws URISyntaxException {
if (Strings.isNullOrEmpty(canonicalURL)) {
diff --git a/java/com/google/gerrit/httpd/raw/IndexServlet.java b/java/com/google/gerrit/httpd/raw/IndexServlet.java
index a0b41b21..97d2270 100644
--- a/java/com/google/gerrit/httpd/raw/IndexServlet.java
+++ b/java/com/google/gerrit/httpd/raw/IndexServlet.java
@@ -70,10 +70,11 @@
SoySauce.Renderer renderer;
try {
Map<String, String[]> parameterMap = req.getParameterMap();
+ String requestUrl = req.getRequestURL() == null ? null : req.getRequestURL().toString();
// TODO(hiesel): Remove URL ordainer as parameter once Soy is consistent
ImmutableMap<String, Object> templateData =
IndexHtmlUtil.templateData(
- gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer);
+ gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl);
renderer = soySauce.renderTemplate("com.google.gerrit.httpd.raw.Index").setData(templateData);
} catch (URISyntaxException | RestApiException e) {
throw new IOException(e);
diff --git a/java/com/google/gerrit/metrics/dropwizard/BUILD b/java/com/google/gerrit/metrics/dropwizard/BUILD
index 4b3859f..3079809 100644
--- a/java/com/google/gerrit/metrics/dropwizard/BUILD
+++ b/java/com/google/gerrit/metrics/dropwizard/BUILD
@@ -8,6 +8,7 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/metrics",
+ "//java/com/google/gerrit/pgm/http/jetty",
"//java/com/google/gerrit/server",
"//lib:args4j",
"//lib:guava",
diff --git a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
index 438f70e..3819786 100644
--- a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
+++ b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
@@ -28,12 +28,12 @@
new Description("Bytes of memory retained in JGit block cache.")
.setGauge()
.setUnit(Units.BYTES),
- WindowCacheStats::getOpenBytes);
+ () -> WindowCacheStats.getStats().getOpenByteCount());
metrics.newCallbackMetric(
"jgit/block_cache/open_files",
- Integer.class,
+ Long.class,
new Description("File handles held open by JGit block cache.").setGauge().setUnit("fds"),
- WindowCacheStats::getOpenFiles);
+ () -> WindowCacheStats.getStats().getOpenFileCount());
}
}
diff --git a/java/com/google/gerrit/pgm/http/jetty/BUILD b/java/com/google/gerrit/pgm/http/jetty/BUILD
index 7b1c3eb..32247fb 100644
--- a/java/com/google/gerrit/pgm/http/jetty/BUILD
+++ b/java/com/google/gerrit/pgm/http/jetty/BUILD
@@ -8,6 +8,7 @@
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/httpd",
"//java/com/google/gerrit/lifecycle",
+ "//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java b/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java
new file mode 100644
index 0000000..b6a2d38
--- /dev/null
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyMetrics.java
@@ -0,0 +1,91 @@
+// 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.pgm.http.jetty;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.metrics.CallbackMetric;
+import com.google.gerrit.metrics.CallbackMetric0;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+@Singleton
+public class JettyMetrics {
+
+ @Inject
+ JettyMetrics(JettyServer jetty, MetricMaker metrics) {
+ CallbackMetric0<Integer> minPoolSize =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/min_pool_size",
+ Integer.class,
+ new Description("Minimum thread pool size").setGauge());
+ CallbackMetric0<Integer> maxPoolSize =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/max_pool_size",
+ Integer.class,
+ new Description("Maximum thread pool size").setGauge());
+ CallbackMetric0<Integer> poolSize =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/pool_size",
+ Integer.class,
+ new Description("Current thread pool size").setGauge());
+ CallbackMetric0<Integer> idleThreads =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/idle_threads",
+ Integer.class,
+ new Description("Idle threads").setGauge().setUnit("threads"));
+ CallbackMetric0<Integer> busyThreads =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/active_threads",
+ Integer.class,
+ new Description("Active threads").setGauge().setUnit("threads"));
+ CallbackMetric0<Integer> reservedThreads =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/reserved_threads",
+ Integer.class,
+ new Description("Reserved threads").setGauge().setUnit("threads"));
+ CallbackMetric0<Integer> queueSize =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/queue_size",
+ Integer.class,
+ new Description("Queued requests waiting for a thread").setGauge().setUnit("requests"));
+ CallbackMetric0<Boolean> lowOnThreads =
+ metrics.newCallbackMetric(
+ "http/server/jetty/threadpool/is_low_on_threads",
+ Boolean.class,
+ new Description("Whether thread pool is low on threads").setGauge());
+ JettyServer.Metrics jettyMetrics = jetty.getMetrics();
+ metrics.newTrigger(
+ ImmutableSet.<CallbackMetric<?>>of(
+ idleThreads,
+ busyThreads,
+ reservedThreads,
+ minPoolSize,
+ maxPoolSize,
+ poolSize,
+ queueSize,
+ lowOnThreads),
+ () -> {
+ minPoolSize.set(jettyMetrics.getMinThreads());
+ maxPoolSize.set(jettyMetrics.getMaxThreads());
+ poolSize.set(jettyMetrics.getThreads());
+ idleThreads.set(jettyMetrics.getIdleThreads());
+ busyThreads.set(jettyMetrics.getBusyThreads());
+ reservedThreads.set(jettyMetrics.getReservedThreads());
+ queueSize.set(jettyMetrics.getQueueSize());
+ lowOnThreads.set(jettyMetrics.isLowOnThreads());
+ });
+ }
+}
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyModule.java b/java/com/google/gerrit/pgm/http/jetty/JettyModule.java
index c818276..32a8b6d 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyModule.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyModule.java
@@ -31,5 +31,6 @@
bind(JettyServer.class);
listener().to(JettyServer.Lifecycle.class);
install(new FactoryModuleBuilder().build(HttpLogFactory.class));
+ bind(JettyMetrics.class);
}
}
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
index 096e4a1..5851fd0 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -68,7 +68,6 @@
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
-import org.eclipse.jetty.util.thread.ThreadPool;
import org.eclipse.jgit.lib.Config;
@Singleton
@@ -117,9 +116,49 @@
}
}
+ static class Metrics {
+ private final QueuedThreadPool threadPool;
+
+ Metrics(QueuedThreadPool threadPool) {
+ this.threadPool = threadPool;
+ }
+
+ public int getIdleThreads() {
+ return threadPool.getIdleThreads();
+ }
+
+ public int getBusyThreads() {
+ return threadPool.getBusyThreads();
+ }
+
+ public int getReservedThreads() {
+ return threadPool.getReservedThreads();
+ }
+
+ public int getMinThreads() {
+ return threadPool.getMinThreads();
+ }
+
+ public int getMaxThreads() {
+ return threadPool.getMaxThreads();
+ }
+
+ public int getThreads() {
+ return threadPool.getThreads();
+ }
+
+ public int getQueueSize() {
+ return threadPool.getQueueSize();
+ }
+
+ public boolean isLowOnThreads() {
+ return threadPool.isLowOnThreads();
+ }
+ }
+
private final SitePaths site;
private final Server httpd;
-
+ private final Metrics metrics;
private boolean reverseProxy;
@Inject
@@ -131,8 +170,10 @@
HttpLogFactory httpLogFactory) {
this.site = site;
- httpd = new Server(threadPool(cfg, threadSettingsConfig));
+ QueuedThreadPool pool = threadPool(cfg, threadSettingsConfig);
+ httpd = new Server(pool);
httpd.setConnectors(listen(httpd, cfg));
+ metrics = new Metrics(pool);
Handler app = makeContext(env, cfg);
if (cfg.getBoolean("httpd", "requestLog", !reverseProxy)) {
@@ -161,6 +202,10 @@
httpd.setStopAtShutdown(false);
}
+ Metrics getMetrics() {
+ return metrics;
+ }
+
private Connector[] listen(Server server, Config cfg) {
// OpenID and certain web-based single-sign-on products can cause
// some very long headers, especially in the Referer header. We
@@ -336,7 +381,7 @@
return site.resolve(path);
}
- private ThreadPool threadPool(Config cfg, ThreadSettingsConfig threadSettingsConfig) {
+ private QueuedThreadPool threadPool(Config cfg, ThreadSettingsConfig threadSettingsConfig) {
int maxThreads = threadSettingsConfig.getHttpdMaxThreads();
int minThreads = cfg.getInt("httpd", null, "minthreads", 5);
int maxQueued = cfg.getInt("httpd", null, "maxqueued", 200);
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 359321a..d9211a9 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -57,6 +57,7 @@
import com.google.gerrit.server.extensions.events.EventUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.extensions.events.RevisionCreated;
+import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.PureRevertCache;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
@@ -177,6 +178,7 @@
bind(EventUtil.class).toProvider(Providers.of(null));
bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
bind(RevisionCreated.class).toInstance(RevisionCreated.DISABLED);
+ bind(WorkInProgressStateChanged.class).toInstance(WorkInProgressStateChanged.DISABLED);
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
}
}
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/ApprovalInference.java
index 44b0529..a4c9f2a 100644
--- a/java/com/google/gerrit/server/ApprovalInference.java
+++ b/java/com/google/gerrit/server/ApprovalInference.java
@@ -101,6 +101,8 @@
return true;
} else if (type.isCopyAnyScore()) {
return true;
+ } else if (type.getCopyValues().contains(psa.value())) {
+ return true;
}
switch (kind) {
case MERGE_FIRST_PARENT_UPDATE:
diff --git a/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/java/com/google/gerrit/server/api/changes/ChangesImpl.java
index b9635fb..d6ef61c 100644
--- a/java/com/google/gerrit/server/api/changes/ChangesImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangesImpl.java
@@ -99,6 +99,15 @@
}
@Override
+ public ChangeInfo createAsInfo(ChangeInput in) throws RestApiException {
+ try {
+ return createChange.apply(TopLevelResource.INSTANCE, in).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot create change", e);
+ }
+ }
+
+ @Override
public QueryRequest query() {
return new QueryRequest() {
@Override
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 1a4cbb8..48a8689 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -40,7 +40,7 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
-import com.google.gerrit.extensions.common.CherryPickChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.DescriptionInput;
@@ -294,7 +294,7 @@
}
@Override
- public CherryPickChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException {
+ public ChangeInfo cherryPickAsInfo(CherryPickInput in) throws RestApiException {
try {
return cherryPick.apply(revision, in).value();
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
index d07ef0c..c53ba83 100644
--- a/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
+++ b/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
@@ -337,7 +337,7 @@
@Override
public boolean accountBelongsToRealm(Collection<ExternalId> externalIds) {
for (ExternalId id : externalIds) {
- if (id.toString().contains(SCHEME_GERRIT)) {
+ if (id.isScheme(SCHEME_GERRIT)) {
return true;
}
}
diff --git a/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java b/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
index 1a50014..944bd44 100644
--- a/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
+++ b/java/com/google/gerrit/server/auth/oauth/OAuthRealm.java
@@ -122,7 +122,7 @@
@Override
public boolean accountBelongsToRealm(Collection<ExternalId> externalIds) {
for (ExternalId id : externalIds) {
- if (id.toString().contains(SCHEME_EXTERNAL)) {
+ if (id.isScheme(SCHEME_EXTERNAL)) {
return true;
}
}
diff --git a/java/com/google/gerrit/server/change/AccountPatchReviewStore.java b/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
index 8da2a90..1b9008d 100644
--- a/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/change/AccountPatchReviewStore.java
@@ -96,8 +96,8 @@
*
* @param psId patch set ID
* @param accountId account ID of the user
- * @return optionally, all files the have been reviewed by the given user that belong to the patch
- * set that is smaller or equals to the given patch set
+ * @return optionally, all files that have been reviewed by the given user that belong to the
+ * patch set that is smaller or equals to the given patch set
*/
Optional<PatchSetWithReviewedFiles> findReviewed(PatchSet.Id psId, Account.Id accountId);
}
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 770f36c..f0a3024 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -68,6 +68,7 @@
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.InsertChangeOp;
import com.google.gerrit.server.update.RepoContext;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -207,7 +208,7 @@
}
// A Change-Id is generated for the review, but not appended to the commit message.
// This can happen if requireChangeId is false.
- return Change.generateKey();
+ return CommitMessageUtil.generateKey();
}
public PatchSet.Id getPatchSetId() {
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 70e7967..65ca741 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -110,7 +110,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.function.Supplier;
import org.eclipse.jgit.lib.Config;
/**
@@ -276,17 +275,13 @@
return format(changeDataFactory.create(change));
}
- public ChangeInfo format(Project.NameKey project, Change.Id id) {
- return format(project, id, ChangeInfo::new);
- }
-
public ChangeInfo format(ChangeData cd) {
- return format(cd, Optional.empty(), true, ChangeInfo::new);
+ return format(cd, Optional.empty(), true);
}
public ChangeInfo format(RevisionResource rsrc) {
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- return format(cd, Optional.of(rsrc.getPatchSet().id()), true, ChangeInfo::new);
+ return format(cd, Optional.of(rsrc.getPatchSet().id()), true);
}
public List<List<ChangeInfo>> format(List<QueryResult<ChangeData>> in)
@@ -312,14 +307,13 @@
ensureLoaded(in);
List<ChangeInfo> out = new ArrayList<>(in.size());
for (ChangeData cd : in) {
- out.add(format(cd, Optional.empty(), false, ChangeInfo::new));
+ out.add(format(cd, Optional.empty(), false));
}
accountLoader.fill();
return out;
}
- public <I extends ChangeInfo> I format(
- Project.NameKey project, Change.Id id, Supplier<I> changeInfoSupplier) {
+ public ChangeInfo format(Project.NameKey project, Change.Id id) {
ChangeNotes notes;
try {
notes = notesFactory.createChecked(project, id);
@@ -327,9 +321,9 @@
if (!has(CHECK)) {
throw e;
}
- return checkOnly(changeDataFactory.create(project, id), changeInfoSupplier);
+ return checkOnly(changeDataFactory.create(project, id));
}
- return format(changeDataFactory.create(notes), Optional.empty(), true, changeInfoSupplier);
+ return format(changeDataFactory.create(notes), Optional.empty(), true);
}
private static Collection<SubmitRequirementInfo> requirementsFor(ChangeData cd) {
@@ -360,19 +354,16 @@
return !Sets.intersection(toFind, set).isEmpty();
}
- private <I extends ChangeInfo> I format(
- ChangeData cd,
- Optional<PatchSet.Id> limitToPsId,
- boolean fillAccountLoader,
- Supplier<I> changeInfoSupplier) {
+ private ChangeInfo format(
+ ChangeData cd, Optional<PatchSet.Id> limitToPsId, boolean fillAccountLoader) {
try {
if (fillAccountLoader) {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- I res = toChangeInfo(cd, limitToPsId, changeInfoSupplier);
+ ChangeInfo res = toChangeInfo(cd, limitToPsId);
accountLoader.fill();
return res;
}
- return toChangeInfo(cd, limitToPsId, changeInfoSupplier);
+ return toChangeInfo(cd, limitToPsId);
} catch (PatchListNotAvailableException
| GpgException
| IOException
@@ -382,7 +373,7 @@
Throwables.throwIfInstanceOf(e, StorageException.class);
throw new StorageException(e);
}
- return checkOnly(cd, changeInfoSupplier);
+ return checkOnly(cd);
}
}
@@ -431,7 +422,7 @@
// Compute and cache if possible
try {
ensureLoaded(Collections.singleton(cd));
- info = format(cd, Optional.empty(), false, ChangeInfo::new);
+ info = format(cd, Optional.empty(), false);
changeInfos.add(info);
if (isCacheable) {
cache.put(Change.id(info._number), info);
@@ -445,14 +436,14 @@
}
}
- private <I extends ChangeInfo> I checkOnly(ChangeData cd, Supplier<I> changeInfoSupplier) {
+ private ChangeInfo checkOnly(ChangeData cd) {
ChangeNotes notes;
try {
notes = cd.notes();
} catch (StorageException e) {
String msg = "Error loading change";
logger.atWarning().withCause(e).log(msg + " %s", cd.getId());
- I info = changeInfoSupplier.get();
+ ChangeInfo info = new ChangeInfo();
info._number = cd.getId().get();
ProblemInfo p = new ProblemInfo();
p.message = msg;
@@ -461,7 +452,7 @@
}
ConsistencyChecker.Result result = checkerProvider.get().check(notes, fix);
- I info = changeInfoSupplier.get();
+ ChangeInfo info = new ChangeInfo();
Change c = result.change();
if (c != null) {
info.project = c.getProject().get();
@@ -486,18 +477,16 @@
return info;
}
- private <I extends ChangeInfo> I toChangeInfo(
- ChangeData cd, Optional<PatchSet.Id> limitToPsId, Supplier<I> changeInfoSupplier)
+ private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
throws PatchListNotAvailableException, GpgException, PermissionBackendException, IOException {
try (Timer0.Context ignored = metrics.toChangeInfoLatency.start()) {
- return toChangeInfoImpl(cd, limitToPsId, changeInfoSupplier);
+ return toChangeInfoImpl(cd, limitToPsId);
}
}
- private <I extends ChangeInfo> I toChangeInfoImpl(
- ChangeData cd, Optional<PatchSet.Id> limitToPsId, Supplier<I> changeInfoSupplier)
+ private ChangeInfo toChangeInfoImpl(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
throws PatchListNotAvailableException, GpgException, PermissionBackendException, IOException {
- I out = changeInfoSupplier.get();
+ ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
if (has(CHECK)) {
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 71c54b1..d64854c 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.RevisionCreated;
+import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
@@ -74,6 +75,7 @@
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final PatchSetUtil psUtil;
+ private final WorkInProgressStateChanged wipStateChanged;
// Assisted-injected fields.
private final PatchSet.Id psId;
@@ -86,6 +88,7 @@
// Fields exposed as setters.
private String message;
private String description;
+ private Boolean workInProgress;
private boolean validate = true;
private boolean checkAddPatchSetPermission = true;
private List<String> groups = Collections.emptyList();
@@ -99,6 +102,7 @@
private PatchSetInfo patchSetInfo;
private ChangeMessage changeMessage;
private ReviewerSet oldReviewers;
+ private boolean oldWorkInProgressState;
@Inject
public PatchSetInserter(
@@ -111,6 +115,7 @@
PatchSetUtil psUtil,
RevisionCreated revisionCreated,
ProjectCache projectCache,
+ WorkInProgressStateChanged wipStateChanged,
@Assisted ChangeNotes notes,
@Assisted PatchSet.Id psId,
@Assisted ObjectId commitId) {
@@ -123,6 +128,7 @@
this.psUtil = psUtil;
this.revisionCreated = revisionCreated;
this.projectCache = projectCache;
+ this.wipStateChanged = wipStateChanged;
this.origNotes = notes;
this.psId = psId;
@@ -143,6 +149,11 @@
return this;
}
+ public PatchSetInserter setWorkInProgress(boolean workInProgress) {
+ this.workInProgress = workInProgress;
+ return this;
+ }
+
public PatchSetInserter setValidate(boolean validate) {
this.validate = validate;
return this;
@@ -230,6 +241,13 @@
changeMessage.setMessage(message);
}
+ oldWorkInProgressState = change.isWorkInProgress();
+ if (workInProgress != null) {
+ change.setWorkInProgress(workInProgress);
+ change.setReviewStarted(!workInProgress);
+ update.setWorkInProgress(workInProgress);
+ }
+
patchSetInfo =
patchSetInfoFactory.get(ctx.getRevWalk(), ctx.getRevWalk().parseCommit(commitId), psId);
if (!allowClosed) {
@@ -265,6 +283,10 @@
if (fireRevisionCreated) {
revisionCreated.fire(change, patchSet, ctx.getAccount(), ctx.getWhen(), notify);
}
+
+ if (workInProgress != null && oldWorkInProgressState != workInProgress) {
+ wipStateChanged.fire(change, patchSet, ctx.getAccount(), ctx.getWhen());
+ }
}
private void validate(RepoContext ctx)
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 012e4cd..3914205 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -61,7 +61,6 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.systemstatus.MessageOfTheDay;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.extensions.webui.BranchWebLink;
import com.google.gerrit.extensions.webui.DiffWebLink;
@@ -365,7 +364,6 @@
DynamicItem.itemOf(binder(), AvatarProvider.class);
DynamicSet.setOf(binder(), LifecycleListener.class);
DynamicSet.setOf(binder(), TopMenu.class);
- DynamicSet.setOf(binder(), MessageOfTheDay.class);
DynamicMap.mapOf(binder(), DownloadScheme.class);
DynamicMap.mapOf(binder(), DownloadCommand.class);
DynamicMap.mapOf(binder(), CloneCommand.class);
diff --git a/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java b/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
index 359a3a8..06b244b 100644
--- a/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
+++ b/java/com/google/gerrit/server/extensions/events/WorkInProgressStateChanged.java
@@ -38,6 +38,12 @@
public class WorkInProgressStateChanged {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ public static final WorkInProgressStateChanged DISABLED =
+ new WorkInProgressStateChanged() {
+ @Override
+ public void fire(Change change, PatchSet patchSet, AccountState account, Timestamp when) {}
+ };
+
private final PluginSetContext<WorkInProgressStateChangedListener> listeners;
private final EventUtil util;
@@ -48,6 +54,11 @@
this.util = util;
}
+ private WorkInProgressStateChanged() {
+ this.listeners = null;
+ this.util = null;
+ }
+
public void fire(Change change, PatchSet patchSet, AccountState account, Timestamp when) {
if (listeners.isEmpty()) {
return;
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 818212c..df53133 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -49,6 +49,7 @@
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -151,7 +152,7 @@
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
RevWalk revWalk = new RevWalk(reader)) {
- ObjectId generatedChangeId = Change.generateChangeId();
+ ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
ObjectId revCommit =
createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId);
return createRevertChangeFromCommit(
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 18394b5..39fd103 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -41,6 +41,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.exceptions.InvalidMergeStrategyException;
+import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -424,15 +425,16 @@
return dc.writeTree(ins);
}
- public static RevCommit createMergeCommit(
+ public static CodeReviewCommit createMergeCommit(
ObjectInserter inserter,
Config repoConfig,
RevCommit mergeTip,
RevCommit originalCommit,
String mergeStrategy,
+ boolean allowConflicts,
PersonIdent committerIndent,
String commitMsg,
- RevWalk rw)
+ CodeReviewRevWalk rw)
throws IOException, MergeIdenticalTreeException, MergeConflictException,
InvalidMergeStrategyException {
@@ -443,22 +445,63 @@
}
Merger m = newMerger(inserter, repoConfig, mergeStrategy);
- if (m.merge(false, mergeTip, originalCommit)) {
- ObjectId tree = m.getResultTreeId();
- CommitBuilder mergeCommit = new CommitBuilder();
- mergeCommit.setTreeId(tree);
- mergeCommit.setParentIds(mergeTip, originalCommit);
- mergeCommit.setAuthor(committerIndent);
- mergeCommit.setCommitter(committerIndent);
- mergeCommit.setMessage(commitMsg);
- return rw.parseCommit(inserter.insert(mergeCommit));
+ DirCache dc = DirCache.newInCore();
+ if (allowConflicts && m instanceof ResolveMerger) {
+ // The DirCache must be set on ResolveMerger before calling
+ // ResolveMerger#merge(AnyObjectId...) otherwise the entries in DirCache don't get populated.
+ ((ResolveMerger) m).setDirCache(dc);
}
- List<String> conflicts = ImmutableList.of();
- if (m instanceof ResolveMerger) {
- conflicts = ((ResolveMerger) m).getUnmergedPaths();
+
+ ObjectId tree;
+ ImmutableSet<String> filesWithGitConflicts;
+ if (m.merge(false, mergeTip, originalCommit)) {
+ filesWithGitConflicts = null;
+ tree = m.getResultTreeId();
+ } else {
+ List<String> conflicts = ImmutableList.of();
+ if (m instanceof ResolveMerger) {
+ conflicts = ((ResolveMerger) m).getUnmergedPaths();
+ }
+
+ if (!allowConflicts) {
+ throw new MergeConflictException(createConflictMessage(conflicts));
+ }
+
+ // For merging with conflict markers we need a ResolveMerger, double-check that we have one.
+ if (!(m instanceof ResolveMerger)) {
+ throw new MergeWithConflictsNotSupportedException(MergeStrategy.get(mergeStrategy));
+ }
+ Map<String, MergeResult<? extends Sequence>> mergeResults =
+ ((ResolveMerger) m).getMergeResults();
+
+ filesWithGitConflicts =
+ mergeResults.entrySet().stream()
+ .filter(e -> e.getValue().containsConflicts())
+ .map(Map.Entry::getKey)
+ .collect(toImmutableSet());
+
+ tree =
+ mergeWithConflicts(
+ rw,
+ inserter,
+ dc,
+ "TARGET BRANCH",
+ mergeTip,
+ "SOURCE BRANCH",
+ originalCommit,
+ mergeResults);
}
- throw new MergeConflictException(createConflictMessage(conflicts));
+
+ CommitBuilder mergeCommit = new CommitBuilder();
+ mergeCommit.setTreeId(tree);
+ mergeCommit.setParentIds(mergeTip, originalCommit);
+ mergeCommit.setAuthor(committerIndent);
+ mergeCommit.setCommitter(committerIndent);
+ mergeCommit.setMessage(commitMsg);
+ CodeReviewCommit commit = rw.parseCommit(inserter.insert(mergeCommit));
+ commit.setFilesWithGitConflicts(filesWithGitConflicts);
+ return commit;
}
public static String createConflictMessage(List<String> conflicts) {
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index c52bbbc..2ed755a 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -18,7 +18,6 @@
import com.google.common.base.MoreObjects;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.git.GitUpdateFailureException;
import com.google.gerrit.git.LockFailureException;
@@ -26,6 +25,7 @@
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
+import com.google.gerrit.server.util.CommitMessageUtil;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
@@ -330,7 +330,8 @@
}
if (update.insertChangeId()) {
- commit.setMessage(ChangeIdUtil.insertId(commit.getMessage(), Change.generateChangeId()));
+ commit.setMessage(
+ ChangeIdUtil.insertId(commit.getMessage(), CommitMessageUtil.generateChangeId()));
}
src = rw.parseCommit(inserter.insert(commit));
diff --git a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
index 6a34786..55f2352 100644
--- a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
+++ b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
@@ -234,7 +234,7 @@
byte[] bytes = hash.digest(data.getBytes(UTF_8));
return BaseEncoding.base64Url().encode(bytes);
} catch (NoSuchAlgorithmException e) {
- throw new RuntimeException("No MD5 available", e);
+ throw new IllegalStateException("No MD5 available", e);
}
}
}
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
index cf16073..98c9873 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
@@ -17,8 +17,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Comment;
import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
@@ -31,44 +29,44 @@
* @param <T> the RevisionNote for the comment type.
*/
class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
- // CommitID => blob ID
+ /** CommitID => blob ID */
final NoteMap noteMap;
- // CommitID => parsed data, immutable map.
+ /** CommitID => parsed data */
final ImmutableMap<ObjectId, T> revisionNotes;
+ private RevisionNoteMap(NoteMap noteMap, ImmutableMap<ObjectId, T> revisionNotes) {
+ this.noteMap = noteMap;
+ this.revisionNotes = revisionNotes;
+ }
+
static RevisionNoteMap<ChangeRevisionNote> parse(
ChangeNoteJson noteJson, ObjectReader reader, NoteMap noteMap, Comment.Status status)
throws ConfigInvalidException, IOException {
- Map<ObjectId, ChangeRevisionNote> result = new HashMap<>();
+ ImmutableMap.Builder<ObjectId, ChangeRevisionNote> result = ImmutableMap.builder();
for (Note note : noteMap) {
ChangeRevisionNote rn = new ChangeRevisionNote(noteJson, reader, note.getData(), status);
rn.parse();
result.put(note.copy(), rn);
}
- return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result));
+ return new RevisionNoteMap<>(noteMap, result.build());
}
static RevisionNoteMap<RobotCommentsRevisionNote> parseRobotComments(
ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws ConfigInvalidException, IOException {
- Map<ObjectId, RobotCommentsRevisionNote> result = new HashMap<>();
+ ImmutableMap.Builder<ObjectId, RobotCommentsRevisionNote> result = ImmutableMap.builder();
for (Note note : noteMap) {
RobotCommentsRevisionNote rn =
new RobotCommentsRevisionNote(changeNoteJson, reader, note.getData());
rn.parse();
result.put(note.copy(), rn);
}
- return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result));
+ return new RevisionNoteMap<>(noteMap, result.build());
}
static <T extends RevisionNote<? extends Comment>> RevisionNoteMap<T> emptyMap() {
return new RevisionNoteMap<>(NoteMap.newEmptyMap(), ImmutableMap.of());
}
-
- private RevisionNoteMap(NoteMap noteMap, ImmutableMap<ObjectId, T> revisionNotes) {
- this.noteMap = noteMap;
- this.revisionNotes = revisionNotes;
- }
}
diff --git a/java/com/google/gerrit/server/project/LabelDefinitionJson.java b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
index 2ecd8c2..0452d0b 100644
--- a/java/com/google/gerrit/server/project/LabelDefinitionJson.java
+++ b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
@@ -40,6 +40,7 @@
label.copyAllScoresOnTrivialRebase = toBoolean(labelType.isCopyAllScoresOnTrivialRebase());
label.copyAllScoresOnMergeFirstParentUpdate =
toBoolean(labelType.isCopyAllScoresOnMergeFirstParentUpdate());
+ label.copyValues = labelType.getCopyValues().isEmpty() ? null : labelType.getCopyValues();
label.allowPostSubmit = toBoolean(labelType.allowPostSubmit());
label.ignoreSelfApproval = toBoolean(labelType.ignoreSelfApproval());
return label;
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index fa877af..4ab583d 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -109,6 +109,7 @@
public static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
public static final String KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = "copyAllScoresIfNoCodeChange";
public static final String KEY_COPY_ALL_SCORES_IF_NO_CHANGE = "copyAllScoresIfNoChange";
+ public static final String KEY_COPY_VALUE = "copyValue";
public static final String KEY_VALUE = "value";
public static final String KEY_CAN_OVERRIDE = "canOverride";
public static final String KEY_BRANCH = "branch";
@@ -1014,6 +1015,27 @@
name,
KEY_COPY_ALL_SCORES_IF_NO_CHANGE,
LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE));
+ Set<Short> copyValues = new HashSet<>();
+ for (String value : rc.getStringList(LABEL, name, KEY_COPY_VALUE)) {
+ try {
+ short copyValue = Shorts.checkedCast(PermissionRule.parseInt(value));
+ if (!copyValues.add(copyValue)) {
+ error(
+ new ValidationError(
+ PROJECT_CONFIG,
+ String.format(
+ "Duplicate %s \"%s\" for label \"%s\"", KEY_COPY_VALUE, value, name)));
+ }
+ } catch (IllegalArgumentException notValue) {
+ error(
+ new ValidationError(
+ PROJECT_CONFIG,
+ String.format(
+ "Invalid %s \"%s\" for label \"%s\": %s",
+ KEY_COPY_VALUE, value, name, notValue.getMessage())));
+ }
+ }
+ label.setCopyValues(copyValues);
label.setCanOverride(
rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, LabelType.DEF_CAN_OVERRIDE));
label.setRefPatterns(getStringListOrNull(rc, LABEL, name, KEY_BRANCH));
@@ -1492,6 +1514,11 @@
KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE,
label.isCopyAllScoresOnMergeFirstParentUpdate(),
LabelType.DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
+ rc.setStringList(
+ LABEL,
+ name,
+ KEY_COPY_VALUE,
+ label.getCopyValues().stream().map(LabelValue::formatValue).collect(toList()));
setBooleanConfigKey(
rc, LABEL, name, KEY_CAN_OVERRIDE, label.canOverride(), LabelType.DEF_CAN_OVERRIDE);
List<String> values = new ArrayList<>(label.getValues().size());
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteEmail.java b/java/com/google/gerrit/server/restapi/account/DeleteEmail.java
index 7a03005..f690acb 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteEmail.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteEmail.java
@@ -16,6 +16,7 @@
import static java.util.stream.Collectors.toSet;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -79,12 +80,14 @@
public Response<?> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, ResourceConflictException, MethodNotAllowedException,
IOException, ConfigInvalidException {
- if (!realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) {
+ Account.Id accountId = user.getAccountId();
+ if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+ && !realm.allowsEdit(AccountFieldName.REGISTER_NEW_EMAIL)) {
throw new MethodNotAllowedException("realm does not allow deleting emails");
}
Set<ExternalId> extIds =
- externalIds.byAccount(user.getAccountId()).stream()
+ externalIds.byAccount(accountId).stream()
.filter(e -> email.equals(e.email()))
.collect(toSet());
if (extIds.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
index 11bcf74..2427def 100644
--- a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
+++ b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
@@ -68,7 +68,7 @@
try {
rng = SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
- throw new RuntimeException("Cannot create RNG for password generator", e);
+ throw new IllegalStateException("Cannot create RNG for password generator", e);
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/PutName.java b/java/com/google/gerrit/server/restapi/account/PutName.java
index c7496b9..78430c3 100644
--- a/java/com/google/gerrit/server/restapi/account/PutName.java
+++ b/java/com/google/gerrit/server/restapi/account/PutName.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.account;
import com.google.common.base.Strings;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.extensions.common.NameInput;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -29,6 +30,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -50,6 +52,7 @@
private final Provider<CurrentUser> self;
private final Realm realm;
private final PermissionBackend permissionBackend;
+ private final ExternalIds externalIds;
private final Provider<AccountsUpdate> accountsUpdateProvider;
@Inject
@@ -57,10 +60,12 @@
Provider<CurrentUser> self,
Realm realm,
PermissionBackend permissionBackend,
+ ExternalIds externalIds,
@ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
+ this.externalIds = externalIds;
this.accountsUpdateProvider = accountsUpdateProvider;
}
@@ -81,7 +86,9 @@
input = new NameInput();
}
- if (!realm.allowsEdit(AccountFieldName.FULL_NAME)) {
+ Account.Id accountId = user.getAccountId();
+ if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+ && !realm.allowsEdit(AccountFieldName.FULL_NAME)) {
throw new MethodNotAllowedException("realm does not allow editing name");
}
@@ -89,7 +96,7 @@
AccountState accountState =
accountsUpdateProvider
.get()
- .update("Set Full Name via API", user.getAccountId(), u -> u.setFullName(newName))
+ .update("Set Full Name via API", accountId, u -> u.setFullName(newName))
.orElseThrow(() -> new ResourceNotFoundException("account not found"));
return Strings.isNullOrEmpty(accountState.account().fullName())
? Response.none()
diff --git a/java/com/google/gerrit/server/restapi/account/PutUsername.java b/java/com/google/gerrit/server/restapi/account/PutUsername.java
index dc841b8..05bf1fd 100644
--- a/java/com/google/gerrit/server/restapi/account/PutUsername.java
+++ b/java/com/google/gerrit/server/restapi/account/PutUsername.java
@@ -89,15 +89,16 @@
permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
}
- if (!realm.allowsEdit(AccountFieldName.USER_NAME)) {
- throw new MethodNotAllowedException("realm does not allow editing username");
- }
-
Account.Id accountId = rsrc.getUser().getAccountId();
if (!externalIds.byAccount(accountId, SCHEME_USERNAME).isEmpty()) {
throw new MethodNotAllowedException("Username cannot be changed.");
}
+ if (realm.accountBelongsToRealm(externalIds.byAccount(accountId))
+ && !realm.allowsEdit(AccountFieldName.USER_NAME)) {
+ throw new MethodNotAllowedException("realm does not allow editing username");
+ }
+
if (input == null || Strings.isNullOrEmpty(input.username)) {
throw new BadRequestException("input required");
}
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPick.java b/java/com/google/gerrit/server/restapi/change/CherryPick.java
index 2902eca..4886a4f 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPick.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPick.java
@@ -20,7 +20,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
-import com.google.gerrit.extensions.common.CherryPickChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -71,7 +71,7 @@
}
@Override
- public Response<CherryPickChangeInfo> apply(RevisionResource rsrc, CherryPickInput input)
+ public Response<ChangeInfo> apply(RevisionResource rsrc, CherryPickInput input)
throws IOException, UpdateException, RestApiException, PermissionBackendException,
ConfigInvalidException, NoSuchProjectException {
input.parent = input.parent == null ? 1 : input.parent;
@@ -96,9 +96,8 @@
rsrc.getPatchSet(),
input,
BranchNameKey.create(rsrc.getProject(), refName));
- CherryPickChangeInfo changeInfo =
- json.noOptions()
- .format(rsrc.getProject(), cherryPickResult.changeId(), CherryPickChangeInfo::new);
+ ChangeInfo changeInfo =
+ json.noOptions().format(rsrc.getProject(), cherryPickResult.changeId());
changeInfo.containsGitConflicts =
!cherryPickResult.filesWithGitConflicts().isEmpty() ? true : null;
return Response.ok(changeInfo);
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index ac81b45..9354727 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -60,6 +60,7 @@
import com.google.gerrit.server.submit.MergeIdenticalTreeException;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -304,7 +305,9 @@
PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverTimeZone);
final ObjectId generatedChangeId =
- changeIdForNewChange != null ? changeIdForNewChange : Change.generateChangeId();
+ changeIdForNewChange != null
+ ? changeIdForNewChange
+ : CommitMessageUtil.generateChangeId();
String commitMessage = ChangeIdUtil.insertId(message, generatedChangeId).trim() + '\n';
CodeReviewCommit cherryPickCommit;
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
index 8b5b07c..2773e29 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickCommit.java
@@ -19,7 +19,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
-import com.google.gerrit.extensions.common.CherryPickChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -65,7 +65,7 @@
}
@Override
- public Response<CherryPickChangeInfo> apply(CommitResource rsrc, CherryPickInput input)
+ public Response<ChangeInfo> apply(CommitResource rsrc, CherryPickInput input)
throws IOException, UpdateException, RestApiException, PermissionBackendException,
ConfigInvalidException, NoSuchProjectException {
String destination = Strings.nullToEmpty(input.destination).trim();
@@ -93,9 +93,7 @@
rsrc.getCommit(),
input,
BranchNameKey.create(rsrc.getProjectState().getNameKey(), refName));
- CherryPickChangeInfo changeInfo =
- json.noOptions()
- .format(projectName, cherryPickResult.changeId(), CherryPickChangeInfo::new);
+ ChangeInfo changeInfo = json.noOptions().format(projectName, cherryPickResult.changeId());
changeInfo.containsGitConflicts =
!cherryPickResult.filesWithGitConflicts().isEmpty() ? true : null;
return Response.ok(changeInfo);
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 537993a..60631d2 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -29,6 +29,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.InvalidMergeStrategyException;
+import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.SubmitType;
@@ -56,6 +57,8 @@
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -73,6 +76,7 @@
import com.google.gerrit.server.restapi.project.ProjectsCollection;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -186,9 +190,8 @@
checkRequiredPermissions(project, input.branch);
- Change newChange = createNewChange(input, me, projectState, updateFactory);
- ChangeJson json = jsonFactory.noOptions();
- return Response.created(json.format(newChange));
+ ChangeInfo newChange = createNewChange(input, me, projectState, updateFactory);
+ return Response.created(newChange);
}
/**
@@ -289,7 +292,7 @@
.check(RefPermission.CREATE_CHANGE);
}
- private Change createNewChange(
+ private ChangeInfo createNewChange(
ChangeInput input,
IdentifiedUser me,
ProjectState projectState,
@@ -304,7 +307,7 @@
try (Repository git = gitManager.openRepository(projectState.getNameKey());
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
- RevWalk rw = new RevWalk(reader)) {
+ CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(reader)) {
PatchSet basePatchSet = null;
List<String> groups = Collections.emptyList();
@@ -327,10 +330,15 @@
PersonIdent author = me.newCommitterIdent(now, serverTimeZone);
String commitMessage = getCommitMessage(input.subject, me);
- RevCommit c;
+ CodeReviewCommit c;
if (input.merge != null) {
// create a merge commit
c = newMergeCommit(git, oi, rw, projectState, mergeTip, input.merge, author, commitMessage);
+ if (!c.getFilesWithGitConflicts().isEmpty()) {
+ logger.atFine().log(
+ "merge commit has conflicts in the following files: %s",
+ c.getFilesWithGitConflicts());
+ }
} else {
// create an empty commit
c = newCommit(oi, rw, author, mergeTip, commitMessage);
@@ -338,10 +346,10 @@
Change.Id changeId = Change.id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory.create(changeId, c, input.branch);
- ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));
+ ins.setMessage(messageForNewChange(ins.getPatchSetId(), c));
ins.setTopic(input.topic);
ins.setPrivate(input.isPrivate);
- ins.setWorkInProgress(input.workInProgress);
+ ins.setWorkInProgress(input.workInProgress || !c.getFilesWithGitConflicts().isEmpty());
ins.setGroups(groups);
try (BatchUpdate bu = updateFactory.create(projectState.getNameKey(), me, now)) {
bu.setRepository(git, rw, oi);
@@ -351,8 +359,10 @@
bu.insertChange(ins);
bu.execute();
}
- return ins.getChange();
- } catch (InvalidMergeStrategyException e) {
+ ChangeInfo changeInfo = jsonFactory.noOptions().format(ins.getChange());
+ changeInfo.containsGitConflicts = !c.getFilesWithGitConflicts().isEmpty() ? true : null;
+ return changeInfo;
+ } catch (InvalidMergeStrategyException | MergeWithConflictsNotSupportedException e) {
throw new BadRequestException(e.getMessage());
}
}
@@ -452,7 +462,7 @@
// Add a Change-Id line if there isn't already one
String commitMessage = subject;
if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) {
- ObjectId id = Change.generateChangeId();
+ ObjectId id = CommitMessageUtil.generateChangeId();
commitMessage = ChangeIdUtil.insertId(commitMessage, id);
}
@@ -469,9 +479,9 @@
return commitMessage;
}
- private static RevCommit newCommit(
+ private static CodeReviewCommit newCommit(
ObjectInserter oi,
- RevWalk rw,
+ CodeReviewRevWalk rw,
PersonIdent authorIdent,
RevCommit mergeTip,
String commitMessage)
@@ -490,10 +500,10 @@
return rw.parseCommit(insert(oi, commit));
}
- private RevCommit newMergeCommit(
+ private CodeReviewCommit newMergeCommit(
Repository repo,
ObjectInserter oi,
- RevWalk rw,
+ CodeReviewRevWalk rw,
ProjectState projectState,
RevCommit mergeTip,
MergeInput merge,
@@ -501,7 +511,8 @@
String commitMessage)
throws RestApiException, IOException {
logger.atFine().log(
- "Creating merge commit: source = %s, strategy = %s", merge.source, merge.strategy);
+ "Creating merge commit: source = %s, strategy = %s, allowConflicts = %s",
+ merge.source, merge.strategy, merge.allowConflicts);
if (Strings.isNullOrEmpty(merge.source)) {
throw new BadRequestException("merge.source must be non-empty");
@@ -531,6 +542,7 @@
mergeTip,
sourceCommit,
mergeStrategy,
+ merge.allowConflicts,
authorIdent,
commitMessage,
rw);
@@ -540,6 +552,20 @@
}
}
+ private static String messageForNewChange(PatchSet.Id patchSetId, CodeReviewCommit commit) {
+ StringBuilder stringBuilder =
+ new StringBuilder(String.format("Uploaded patch set %s.", patchSetId.get()));
+
+ if (!commit.getFilesWithGitConflicts().isEmpty()) {
+ stringBuilder.append("\n\nThe following files contain Git conflicts:\n");
+ commit.getFilesWithGitConflicts().stream()
+ .sorted()
+ .forEach(filePath -> stringBuilder.append("* ").append(filePath).append("\n"));
+ }
+
+ return stringBuilder.toString();
+ }
+
private static ObjectId insert(ObjectInserter inserter, CommitBuilder commit) throws IOException {
ObjectId id = inserter.insert(commit);
inserter.flush();
diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
index cfb4c77..cfe6bdf 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
@@ -22,6 +22,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.InvalidMergeStrategyException;
+import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.MergeInput;
@@ -44,6 +45,8 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -71,7 +74,6 @@
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.ChangeIdUtil;
@Singleton
@@ -141,7 +143,7 @@
try (Repository git = gitManager.openRepository(project);
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
- RevWalk rw = new RevWalk(reader)) {
+ CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(reader)) {
RevCommit sourceCommit = MergeUtil.resolveCommit(git, rw, merge.source);
if (!commits.canRead(projectState, git, sourceCommit)) {
@@ -162,7 +164,7 @@
Timestamp now = TimeUtil.nowTs();
IdentifiedUser me = user.get().asIdentifiedUser();
PersonIdent author = me.newCommitterIdent(now, serverTimeZone);
- RevCommit newCommit =
+ CodeReviewCommit newCommit =
createMergeCommit(
in,
projectState,
@@ -182,7 +184,8 @@
bu.setRepository(git, rw, oi);
bu.setNotify(NotifyResolver.Result.none());
psInserter
- .setMessage("Uploaded patch set " + nextPsId.get() + ".")
+ .setMessage(messageForChange(nextPsId, newCommit))
+ .setWorkInProgress(!newCommit.getFilesWithGitConflicts().isEmpty())
.setCheckAddPatchSetPermission(false);
if (groups != null) {
psInserter.setGroups(groups);
@@ -192,8 +195,11 @@
}
ChangeJson json = jsonFactory.create(ListChangesOption.CURRENT_REVISION);
- return Response.ok(json.format(psInserter.getChange()));
- } catch (InvalidMergeStrategyException e) {
+ ChangeInfo changeInfo = json.format(psInserter.getChange());
+ changeInfo.containsGitConflicts =
+ !newCommit.getFilesWithGitConflicts().isEmpty() ? true : null;
+ return Response.ok(changeInfo);
+ } catch (InvalidMergeStrategyException | MergeWithConflictsNotSupportedException e) {
throw new BadRequestException(e.getMessage());
}
}
@@ -213,13 +219,13 @@
return psUtil.current(change);
}
- private RevCommit createMergeCommit(
+ private CodeReviewCommit createMergeCommit(
MergePatchSetInput in,
ProjectState projectState,
BranchNameKey dest,
Repository git,
ObjectInserter oi,
- RevWalk rw,
+ CodeReviewRevWalk rw,
RevCommit currentPsCommit,
RevCommit sourceCommit,
PersonIdent author,
@@ -258,6 +264,28 @@
mergeUtilFactory.create(projectState).mergeStrategyName());
return MergeUtil.createMergeCommit(
- oi, git.getConfig(), mergeTip, sourceCommit, mergeStrategy, author, commitMsg, rw);
+ oi,
+ git.getConfig(),
+ mergeTip,
+ sourceCommit,
+ mergeStrategy,
+ in.merge.allowConflicts,
+ author,
+ commitMsg,
+ rw);
+ }
+
+ private static String messageForChange(PatchSet.Id patchSetId, CodeReviewCommit commit) {
+ StringBuilder stringBuilder =
+ new StringBuilder(String.format("Uploaded patch set %s.", patchSetId.get()));
+
+ if (!commit.getFilesWithGitConflicts().isEmpty()) {
+ stringBuilder.append("\n\nThe following files contain Git conflicts:\n");
+ commit.getFilesWithGitConflicts().stream()
+ .sorted()
+ .forEach(filePath -> stringBuilder.append("* ").append(filePath).append("\n"));
+ }
+
+ return stringBuilder.toString();
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index d67fc6a..032f08e 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -68,6 +68,7 @@
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -302,7 +303,7 @@
// TODO (paiking): As a future change, the revert should just be done directly on the
// target rather than just creating a commit and then cherry-picking it.
cherryPickInput.message = revertInput.message;
- ObjectId generatedChangeId = Change.generateChangeId();
+ ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
// TODO (paiking): In the the future, the timestamp should be the same for all the revert
// changes.
@@ -526,14 +527,6 @@
potentialCommitToReturn.getName(), changeNotes.getChange().getChangeId()));
}
- /**
- * @param submissionId the submission id of the change.
- * @return True if the submission has more than one change, false otherwise.
- */
- private Boolean isChangePartOfSubmission(String submissionId) {
- return (queryProvider.get().setLimit(2).bySubmissionId(submissionId).size() > 1);
- }
-
private class CreateCherryPickOp implements BatchUpdateOp {
private final ObjectId revCommitId;
private final String topic;
@@ -582,9 +575,7 @@
.getCurrentPatchSet()
.commitId()
.getName();
- results.add(
- json.noOptions()
- .format(change.getProject(), cherryPickResult.changeId(), ChangeInfo::new));
+ results.add(json.noOptions().format(change.getProject(), cherryPickResult.changeId()));
return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetSummary.java b/java/com/google/gerrit/server/restapi/config/GetSummary.java
index 1df485f..d0a1498 100644
--- a/java/com/google/gerrit/server/restapi/config/GetSummary.java
+++ b/java/com/google/gerrit/server/restapi/config/GetSummary.java
@@ -126,8 +126,8 @@
long mTotal = r.totalMemory();
long mInuse = mTotal - mFree;
- int jgitOpen = WindowCacheStats.getOpenFiles();
- long jgitBytes = WindowCacheStats.getOpenBytes();
+ long jgitOpen = WindowCacheStats.getStats().getOpenFileCount();
+ long jgitBytes = WindowCacheStats.getStats().getOpenByteCount();
MemSummaryInfo memSummaryInfo = new MemSummaryInfo();
memSummaryInfo.total = bytes(mTotal);
@@ -135,7 +135,7 @@
memSummaryInfo.free = bytes(mFree);
memSummaryInfo.buffers = bytes(jgitBytes);
memSummaryInfo.max = bytes(mMax);
- memSummaryInfo.openFiles = toInteger(jgitOpen);
+ memSummaryInfo.openFiles = Long.valueOf(jgitOpen);
return memSummaryInfo;
}
@@ -258,7 +258,7 @@
public String free;
public String buffers;
public String max;
- public Integer openFiles;
+ public Long openFiles;
}
public static class ThreadSummaryInfo {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index 5d51527..a85ad39 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -191,6 +191,10 @@
input.copyAllScoresOnMergeFirstParentUpdate);
}
+ if (input.copyValues != null) {
+ labelType.setCopyValues(input.copyValues);
+ }
+
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index 824b4ed..0a35865 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -205,6 +205,11 @@
dirty = true;
}
+ if (input.copyValues != null) {
+ labelType.setCopyValues(input.copyValues);
+ dirty = true;
+ }
+
if (input.allowPostSubmit != null) {
labelType.setAllowPostSubmit(input.allowPostSubmit);
dirty = true;
diff --git a/java/com/google/gerrit/server/util/CommitMessageUtil.java b/java/com/google/gerrit/server/util/CommitMessageUtil.java
index e984f46..1c8ce0c 100644
--- a/java/com/google/gerrit/server/util/CommitMessageUtil.java
+++ b/java/com/google/gerrit/server/util/CommitMessageUtil.java
@@ -14,12 +14,29 @@
package com.google.gerrit.server.util;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
/** Utility functions to manipulate commit messages. */
public class CommitMessageUtil {
+ private static final SecureRandom rng;
+
+ static {
+ try {
+ rng = SecureRandom.getInstance("SHA1PRNG");
+ } catch (NoSuchAlgorithmException e) {
+ throw new IllegalStateException("Cannot create RNG for Change-Id generator", e);
+ }
+ }
private CommitMessageUtil() {}
@@ -42,4 +59,18 @@
trimmed = trimmed + "\n";
return trimmed;
}
+
+ public static ObjectId generateChangeId() {
+ byte[] rand = new byte[Constants.OBJECT_ID_STRING_LENGTH];
+ rng.nextBytes(rand);
+ String randomString = new String(rand, UTF_8);
+
+ try (ObjectInserter f = new ObjectInserter.Formatter()) {
+ return f.idFor(Constants.OBJ_COMMIT, Constants.encode(randomString));
+ }
+ }
+
+ public static Change.Key generateKey() {
+ return Change.key("I" + generateChangeId().name());
+ }
}
diff --git a/java/com/google/gerrit/sshd/commands/ShowCaches.java b/java/com/google/gerrit/sshd/commands/ShowCaches.java
index 7e0439f..1d756de 100644
--- a/java/com/google/gerrit/sshd/commands/ShowCaches.java
+++ b/java/com/google/gerrit/sshd/commands/ShowCaches.java
@@ -297,6 +297,10 @@
return i != null ? i : 0;
}
+ private static long nullToZero(Long i) {
+ return i != null ? i : 0;
+ }
+
private void sshSummary() {
IoAcceptor acceptor = daemon.getIoAcceptor();
if (acceptor == null) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c889984..0eefe02 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -47,6 +47,7 @@
import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REMOVED;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
@@ -144,8 +145,10 @@
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.TrackingIdInfo;
+import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -180,6 +183,7 @@
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.name.Named;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
@@ -3207,13 +3211,29 @@
MergePatchSetInput in = new MergePatchSetInput();
in.merge = mergeInput;
in.subject = "update change by merge ps2";
- gApi.changes().id(changeId).createMergePatchSet(in);
+
+ TestWorkInProgressStateChangedListener wipStateChangedListener =
+ new TestWorkInProgressStateChangedListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(wipStateChangedListener)) {
+ ChangeInfo changeInfo = gApi.changes().id(changeId).createMergePatchSet(in);
+ assertThat(changeInfo.subject).isEqualTo(in.subject);
+ assertThat(changeInfo.containsGitConflicts).isNull();
+ assertThat(changeInfo.workInProgress).isNull();
+ }
+ assertThat(wipStateChangedListener.invoked).isFalse();
+
+ // To get the revisions, we must retrieve the change with more change options.
ChangeInfo changeInfo =
gApi.changes().id(changeId).get(ALL_REVISIONS, CURRENT_COMMIT, CURRENT_REVISION);
assertThat(changeInfo.revisions).hasSize(2);
- assertThat(changeInfo.subject).isEqualTo(in.subject);
assertThat(changeInfo.revisions.get(changeInfo.currentRevision).commit.parents.get(0).commit)
.isEqualTo(parent);
+
+ // Verify the message that has been posted on the change.
+ List<ChangeMessageInfo> messages = gApi.changes().id(changeId).messages();
+ assertThat(messages).hasSize(2);
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Uploaded patch set 2.");
}
@Test
@@ -3252,6 +3272,142 @@
}
@Test
+ public void createMergePatchSet_ConflictAllowed() throws Exception {
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ createBranch("dev");
+
+ // create a change for master
+ String changeId = createChange().getChangeId();
+
+ String fileName = "shared.txt";
+ String sourceSubject = "source change";
+ String sourceContent = "source content";
+ String targetSubject = "target change";
+ String targetContent = "target content";
+ testRepo.reset(initialHead);
+ PushOneCommit.Result currentMaster =
+ pushFactory
+ .create(admin.newIdent(), testRepo, targetSubject, fileName, targetContent)
+ .to("refs/heads/master");
+ currentMaster.assertOkStatus();
+ String parent = currentMaster.getCommit().getName();
+
+ // push a commit into dev branch
+ testRepo.reset(initialHead);
+ PushOneCommit.Result changeA =
+ pushFactory
+ .create(user.newIdent(), testRepo, sourceSubject, fileName, sourceContent)
+ .to("refs/heads/dev");
+ changeA.assertOkStatus();
+ MergeInput mergeInput = new MergeInput();
+ mergeInput.source = "dev";
+ mergeInput.allowConflicts = true;
+ MergePatchSetInput in = new MergePatchSetInput();
+ in.merge = mergeInput;
+ in.subject = "update change by merge ps2";
+
+ TestWorkInProgressStateChangedListener wipStateChangedListener =
+ new TestWorkInProgressStateChangedListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(wipStateChangedListener)) {
+ ChangeInfo changeInfo = gApi.changes().id(changeId).createMergePatchSet(in);
+ assertThat(changeInfo.subject).isEqualTo(in.subject);
+ assertThat(changeInfo.containsGitConflicts).isTrue();
+ assertThat(changeInfo.workInProgress).isTrue();
+ }
+ assertThat(wipStateChangedListener.invoked).isTrue();
+ assertThat(wipStateChangedListener.wip).isTrue();
+
+ // To get the revisions, we must retrieve the change with more change options.
+ ChangeInfo changeInfo =
+ gApi.changes().id(changeId).get(ALL_REVISIONS, CURRENT_COMMIT, CURRENT_REVISION);
+ assertThat(changeInfo.revisions).hasSize(2);
+ assertThat(changeInfo.revisions.get(changeInfo.currentRevision).commit.parents.get(0).commit)
+ .isEqualTo(parent);
+
+ // Verify that the file content in the created patch set is correct.
+ // We expect that it has conflict markers to indicate the conflict.
+ BinaryResult bin = gApi.changes().id(changeId).current().file(fileName).content();
+ ByteArrayOutputStream os = new ByteArrayOutputStream();
+ bin.writeTo(os);
+ String fileContent = new String(os.toByteArray(), UTF_8);
+ String sourceSha1 = abbreviateName(changeA.getCommit(), 6);
+ String targetSha1 = abbreviateName(currentMaster.getCommit(), 6);
+ assertThat(fileContent)
+ .isEqualTo(
+ "<<<<<<< TARGET BRANCH ("
+ + targetSha1
+ + " "
+ + targetSubject
+ + ")\n"
+ + targetContent
+ + "\n"
+ + "=======\n"
+ + sourceContent
+ + "\n"
+ + ">>>>>>> SOURCE BRANCH ("
+ + sourceSha1
+ + " "
+ + sourceSubject
+ + ")\n");
+
+ // Verify the message that has been posted on the change.
+ List<ChangeMessageInfo> messages = gApi.changes().id(changeId).messages();
+ assertThat(messages).hasSize(2);
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ "Uploaded patch set 2.\n\n"
+ + "The following files contain Git conflicts:\n"
+ + "* "
+ + fileName
+ + "\n");
+ }
+
+ @Test
+ public void createMergePatchSet_ConflictAllowedNotSupportedByMergeStrategy() throws Exception {
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ createBranch("dev");
+
+ // create a change for master
+ String changeId = createChange().getChangeId();
+
+ String fileName = "shared.txt";
+ String sourceSubject = "source change";
+ String sourceContent = "source content";
+ String targetSubject = "target change";
+ String targetContent = "target content";
+ testRepo.reset(initialHead);
+ PushOneCommit.Result currentMaster =
+ pushFactory
+ .create(admin.newIdent(), testRepo, targetSubject, fileName, targetContent)
+ .to("refs/heads/master");
+ currentMaster.assertOkStatus();
+
+ // push a commit into dev branch
+ testRepo.reset(initialHead);
+ PushOneCommit.Result changeA =
+ pushFactory
+ .create(user.newIdent(), testRepo, sourceSubject, fileName, sourceContent)
+ .to("refs/heads/dev");
+ changeA.assertOkStatus();
+ MergeInput mergeInput = new MergeInput();
+ mergeInput.source = "dev";
+ mergeInput.allowConflicts = true;
+ mergeInput.strategy = "simple-two-way-in-core";
+ MergePatchSetInput in = new MergePatchSetInput();
+ in.merge = mergeInput;
+ in.subject = "update change by merge ps2";
+
+ BadRequestException ex =
+ assertThrows(
+ BadRequestException.class, () -> gApi.changes().id(changeId).createMergePatchSet(in));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ "merge with conflicts is not supported with merge strategy: " + mergeInput.strategy);
+ }
+
+ @Test
public void createMergePatchSetInheritParent() throws Exception {
RevCommit initialHead = projectOperations.project(project).getHead("master");
createBranch("dev");
@@ -4481,7 +4637,6 @@
ListChangesOption.ALL_COMMITS,
ListChangesOption.ALL_REVISIONS,
ListChangesOption.CHANGE_ACTIONS,
- ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.DETAILED_LABELS,
ListChangesOption.DOWNLOAD_COMMANDS,
ListChangesOption.MESSAGES,
@@ -4535,4 +4690,17 @@
private interface AddReviewerCaller {
void call(String changeId, String reviewer) throws RestApiException;
}
+
+ private static class TestWorkInProgressStateChangedListener
+ implements WorkInProgressStateChangedListener {
+ boolean invoked;
+ Boolean wip;
+
+ @Override
+ public void onWorkInProgressStateChanged(Event event) {
+ this.invoked = true;
+ this.wip =
+ event.getChange().workInProgress != null ? event.getChange().workInProgress : false;
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 7e69251..923b66f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -185,6 +185,35 @@
}
@Test
+ public void stickyOnCopyValues() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .getLabelSections()
+ .get("Code-Review")
+ .setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ u.save();
+ }
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = createChange(changeKind);
+ vote(admin, changeId, -1, 1);
+ vote(user, changeId, -2, -1);
+ vote(user2, changeId, 1, -1);
+
+ updateChange(changeId, changeKind);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, -1, 0, changeKind);
+ assertVotes(c, user, 0, 0, changeKind);
+ assertVotes(c, user2, 1, 0, changeKind);
+ }
+ }
+
+ @Test
public void stickyOnTrivialRebase() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresOnTrivialRebase(true);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 36b7265..70fcfc4 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -73,7 +73,6 @@
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
-import com.google.gerrit.extensions.common.CherryPickChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.FileInfo;
@@ -324,7 +323,7 @@
ChangeApi orig = gApi.changes().id(project.get() + "~master~" + r.getChangeId());
assertThat(orig.get().messages).hasSize(1);
- CherryPickChangeInfo changeInfo = orig.revision(r.getCommit().name()).cherryPickAsInfo(in);
+ ChangeInfo changeInfo = orig.revision(r.getCommit().name()).cherryPickAsInfo(in);
assertThat(changeInfo.containsGitConflicts).isNull();
assertThat(changeInfo.workInProgress).isNull();
ChangeApi cherry = gApi.changes().id(changeInfo._number);
@@ -576,8 +575,7 @@
// Cherry-pick with auto merge should succeed.
in.allowConflicts = true;
- CherryPickChangeInfo cherryPickChange =
- changeApi.revision(r.getCommit().name()).cherryPickAsInfo(in);
+ ChangeInfo cherryPickChange = changeApi.revision(r.getCommit().name()).cherryPickAsInfo(in);
assertThat(cherryPickChange.containsGitConflicts).isTrue();
assertThat(cherryPickChange.workInProgress).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index cdffa3c..9624ec2 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -57,7 +57,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -645,14 +644,7 @@
// |
// C0 -- Master
//
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push1 =
pushFactory.create(admin.newIdent(), testRepo, PushOneCommit.SUBJECT, "a.txt", "content");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 3f80dd1..0b6ecdc 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -18,12 +18,15 @@
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.common.data.Permission.READ;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
+import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.PushOneCommit.Result;
@@ -44,9 +47,11 @@
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -54,6 +59,7 @@
import com.google.gerrit.server.submit.ChangeAlreadyMergedException;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.inject.Inject;
+import java.io.ByteArrayOutputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -141,6 +147,11 @@
ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
assertThat(info.revisions.get(info.currentRevision).commit.message)
.contains("Change-Id: " + info.changeId);
+
+ // Verify the message that has been posted on the change.
+ List<ChangeMessageInfo> messages = gApi.changes().id(info._number).messages();
+ assertThat(messages).hasSize(1);
+ assertThat(Iterables.getOnlyElement(messages).message).isEqualTo("Uploaded patch set 1.");
}
@Test
@@ -364,7 +375,12 @@
public void createMergeChange() throws Exception {
changeInTwoBranches("branchA", "a.txt", "branchB", "b.txt");
ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
- assertCreateSucceeds(in);
+ ChangeInfo change = assertCreateSucceeds(in);
+
+ // Verify the message that has been posted on the change.
+ List<ChangeMessageInfo> messages = gApi.changes().id(change._number).messages();
+ assertThat(messages).hasSize(1);
+ assertThat(Iterables.getOnlyElement(messages).message).isEqualTo("Uploaded patch set 1.");
}
@Test
@@ -382,6 +398,87 @@
}
@Test
+ public void createMergeChange_ConflictsAllowed() throws Exception {
+ String fileName = "shared.txt";
+ String sourceBranch = "sourceBranch";
+ String sourceSubject = "source change";
+ String sourceContent = "source content";
+ String targetBranch = "targetBranch";
+ String targetSubject = "target change";
+ String targetContent = "target content";
+ changeInTwoBranches(
+ sourceBranch,
+ sourceSubject,
+ fileName,
+ sourceContent,
+ targetBranch,
+ targetSubject,
+ fileName,
+ targetContent);
+ ChangeInput in = newMergeChangeInput(targetBranch, sourceBranch, "", true);
+ ChangeInfo change = assertCreateSucceedsWithConflicts(in);
+
+ // Verify that the file content in the created change is correct.
+ // We expect that it has conflict markers to indicate the conflict.
+ BinaryResult bin = gApi.changes().id(change._number).current().file(fileName).content();
+ ByteArrayOutputStream os = new ByteArrayOutputStream();
+ bin.writeTo(os);
+ String fileContent = new String(os.toByteArray(), UTF_8);
+ String sourceSha1 = abbreviateName(projectOperations.project(project).getHead(sourceBranch), 6);
+ String targetSha1 = abbreviateName(projectOperations.project(project).getHead(targetBranch), 6);
+ assertThat(fileContent)
+ .isEqualTo(
+ "<<<<<<< TARGET BRANCH ("
+ + targetSha1
+ + " "
+ + targetSubject
+ + ")\n"
+ + targetContent
+ + "\n"
+ + "=======\n"
+ + sourceContent
+ + "\n"
+ + ">>>>>>> SOURCE BRANCH ("
+ + sourceSha1
+ + " "
+ + sourceSubject
+ + ")\n");
+
+ // Verify the message that has been posted on the change.
+ List<ChangeMessageInfo> messages = gApi.changes().id(change._number).messages();
+ assertThat(messages).hasSize(1);
+ assertThat(Iterables.getOnlyElement(messages).message)
+ .isEqualTo(
+ "Uploaded patch set 1.\n\n"
+ + "The following files contain Git conflicts:\n"
+ + "* "
+ + fileName
+ + "\n");
+ }
+
+ @Test
+ public void createMergeChange_ConflictAllowedNotSupportedByMergeStrategy() throws Exception {
+ String fileName = "shared.txt";
+ String sourceBranch = "sourceBranch";
+ String targetBranch = "targetBranch";
+ changeInTwoBranches(
+ sourceBranch,
+ "source change",
+ fileName,
+ "source content",
+ targetBranch,
+ "target change",
+ fileName,
+ "target content");
+ String mergeStrategy = "simple-two-way-in-core";
+ ChangeInput in = newMergeChangeInput(targetBranch, sourceBranch, mergeStrategy, true);
+ assertCreateFails(
+ in,
+ BadRequestException.class,
+ "merge with conflicts is not supported with merge strategy: " + mergeStrategy);
+ }
+
+ @Test
public void createMergeChangeFailsWithConflictIfThereAreTooManyCommonPredecessors()
throws Exception {
// Create an initial commit in master.
@@ -732,6 +829,26 @@
}
assertThat(out.revisions).hasSize(1);
assertThat(out.submitted).isNull();
+ assertThat(out.containsGitConflicts).isNull();
+ assertThat(in.status).isEqualTo(ChangeStatus.NEW);
+ return out;
+ }
+
+ private ChangeInfo assertCreateSucceedsWithConflicts(ChangeInput in) throws Exception {
+ ChangeInfo out = gApi.changes().createAsInfo(in);
+ assertThat(out.project).isEqualTo(in.project);
+ assertThat(RefNames.fullName(out.branch)).isEqualTo(RefNames.fullName(in.branch));
+ assertThat(out.subject).isEqualTo(in.subject.split("\n")[0]);
+ assertThat(out.topic).isEqualTo(in.topic);
+ assertThat(out.status).isEqualTo(in.status);
+ if (in.isPrivate) {
+ assertThat(out.isPrivate).isTrue();
+ } else {
+ assertThat(out.isPrivate).isNull();
+ }
+ assertThat(out.submitted).isNull();
+ assertThat(out.containsGitConflicts).isTrue();
+ assertThat(out.workInProgress).isTrue();
assertThat(in.status).isEqualTo(ChangeStatus.NEW);
return out;
}
@@ -764,6 +881,11 @@
}
private ChangeInput newMergeChangeInput(String targetBranch, String sourceRef, String strategy) {
+ return newMergeChangeInput(targetBranch, sourceRef, strategy, false);
+ }
+
+ private ChangeInput newMergeChangeInput(
+ String targetBranch, String sourceRef, String strategy, boolean allowConflicts) {
// create a merge change from branchA to master in gerrit
ChangeInput in = new ChangeInput();
in.project = project.get();
@@ -776,6 +898,7 @@
if (!Strings.isNullOrEmpty(strategy)) {
in.merge.strategy = strategy;
}
+ in.merge.allowConflicts = allowConflicts;
return in;
}
@@ -791,6 +914,34 @@
*/
private Map<String, Result> changeInTwoBranches(
String branchA, String fileA, String branchB, String fileB) throws Exception {
+ return changeInTwoBranches(
+ branchA, "change A", fileA, "A content", branchB, "change B", fileB, "B content");
+ }
+
+ /**
+ * Create an empty commit in master, two new branches with one commit each.
+ *
+ * @param branchA name of first branch to create
+ * @param subjectA commit message subject for the change on branchA
+ * @param fileA name of file to commit to branchA
+ * @param contentA file content to commit to branchA
+ * @param branchB name of second branch to create
+ * @param subjectB commit message subject for the change on branchB
+ * @param fileB name of file to commit to branchB
+ * @param contentB file content to commit to branchB
+ * @return A {@code Map} of branchName => commit result.
+ * @throws Exception
+ */
+ private Map<String, Result> changeInTwoBranches(
+ String branchA,
+ String subjectA,
+ String fileA,
+ String contentA,
+ String branchB,
+ String subjectB,
+ String fileB,
+ String contentB)
+ throws Exception {
// create a initial commit in master
Result initialCommit =
pushFactory
@@ -805,13 +956,13 @@
// create a commit in branchA
Result changeA =
pushFactory
- .create(user.newIdent(), testRepo, "change A", fileA, "A content")
+ .create(user.newIdent(), testRepo, subjectA, fileA, contentA)
.to("refs/heads/" + branchA);
changeA.assertOkStatus();
// create a commit in branchB
PushOneCommit commitB =
- pushFactory.create(user.newIdent(), testRepo, "change B", fileB, "B content");
+ pushFactory.create(user.newIdent(), testRepo, subjectB, fileB, contentB);
commitB.setParent(initialCommit.getCommit());
Result changeB = commitB.to("refs/heads/" + branchB);
changeB.assertOkStatus();
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
index 57a1e56..e5587a9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
@@ -244,6 +244,7 @@
assertThat(createdLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(createdLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(createdLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(createdLabel.copyValues).isNull();
assertThat(createdLabel.allowPostSubmit).isTrue();
assertThat(createdLabel.ignoreSelfApproval).isNull();
}
@@ -537,6 +538,17 @@
}
@Test
+ public void createWithCopyValues() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.copyValues = ImmutableList.of((short) -1, (short) 1);
+
+ LabelDefinitionInfo createdLabel =
+ gApi.projects().name(project.get()).label("foo").create(input).get();
+ assertThat(createdLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
+ }
+
+ @Test
public void createWithAllowPostSubmit() throws Exception {
LabelDefinitionInput input = new LabelDefinitionInput();
input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
index 9f98490..940fae5 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
@@ -117,6 +117,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(fooLabel.copyValues).isNull();
assertThat(fooLabel.allowPostSubmit).isNull();
assertThat(fooLabel.ignoreSelfApproval).isNull();
}
@@ -134,6 +135,7 @@
labelType.setCopyAllScoresIfNoCodeChange(true);
labelType.setCopyAllScoresOnTrivialRebase(true);
labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
labelType.setIgnoreSelfApproval(true);
u.getConfig().getLabelSections().put(labelType.getName(), labelType);
u.save();
@@ -148,6 +150,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isTrue();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isTrue();
+ assertThat(fooLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
assertThat(fooLabel.allowPostSubmit).isTrue();
assertThat(fooLabel.ignoreSelfApproval).isTrue();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java b/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
index 7998ecb..65e352b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/LabelAssert.java
@@ -47,6 +47,7 @@
assertThat(codeReviewLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(codeReviewLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(codeReviewLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(codeReviewLabel.copyValues).isNull();
assertThat(codeReviewLabel.allowPostSubmit).isTrue();
assertThat(codeReviewLabel.ignoreSelfApproval).isNull();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
index d2539e5..ef08079 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
@@ -139,6 +139,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isNull();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isNull();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isNull();
+ assertThat(fooLabel.copyValues).isNull();
assertThat(fooLabel.allowPostSubmit).isNull();
assertThat(fooLabel.ignoreSelfApproval).isNull();
}
@@ -156,6 +157,7 @@
labelType.setCopyAllScoresIfNoCodeChange(true);
labelType.setCopyAllScoresOnTrivialRebase(true);
labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
labelType.setIgnoreSelfApproval(true);
u.getConfig().getLabelSections().put(labelType.getName(), labelType);
u.save();
@@ -173,6 +175,7 @@
assertThat(fooLabel.copyAllScoresIfNoCodeChange).isTrue();
assertThat(fooLabel.copyAllScoresOnTrivialRebase).isTrue();
assertThat(fooLabel.copyAllScoresOnMergeFirstParentUpdate).isTrue();
+ assertThat(fooLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
assertThat(fooLabel.allowPostSubmit).isTrue();
assertThat(fooLabel.ignoreSelfApproval).isTrue();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
index 97b795f..b08c72b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
@@ -771,6 +771,44 @@
}
@Test
+ public void setCopyValues() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNull();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyValues = ImmutableList.of((short) -1, (short) 1);
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyValues).containsExactly((short) -1, (short) 1).inOrder();
+
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues)
+ .containsExactly((short) -1, (short) 1)
+ .inOrder();
+ }
+
+ @Test
+ public void unsetCopyValues() throws Exception {
+ configLabel("foo", LabelFunction.NO_OP);
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ LabelType labelType = u.getConfig().getLabelSections().get("foo");
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.save();
+ }
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNotEmpty();
+
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.copyValues = ImmutableList.of();
+
+ LabelDefinitionInfo updatedLabel =
+ gApi.projects().name(project.get()).label("foo").update(input);
+ assertThat(updatedLabel.copyValues).isNull();
+
+ assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNull();
+ }
+
+ @Test
public void setAllowPostSubmit() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
diff --git a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
index d9438f0..cad4796 100644
--- a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
@@ -15,6 +15,9 @@
package com.google.gerrit.httpd.raw;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.raw.IndexHtmlUtil.changeUrlPattern;
+import static com.google.gerrit.httpd.raw.IndexHtmlUtil.computeChangeRequestsPath;
+import static com.google.gerrit.httpd.raw.IndexHtmlUtil.diffUrlPattern;
import static com.google.gerrit.httpd.raw.IndexHtmlUtil.staticTemplateData;
import com.google.template.soy.data.SanitizedContent;
@@ -29,7 +32,12 @@
public void noPathAndNoCDN() throws Exception {
assertThat(
staticTemplateData(
- "http://example.com/", null, null, new HashMap<>(), IndexHtmlUtilTest::ordain))
+ "http://example.com/",
+ null,
+ null,
+ new HashMap<>(),
+ IndexHtmlUtilTest::ordain,
+ null))
.containsExactly("canonicalPath", "", "staticResourcePath", ordain(""));
}
@@ -41,7 +49,8 @@
null,
null,
new HashMap<>(),
- IndexHtmlUtilTest::ordain))
+ IndexHtmlUtilTest::ordain,
+ null))
.containsExactly("canonicalPath", "/gerrit", "staticResourcePath", ordain("/gerrit"));
}
@@ -53,7 +62,8 @@
"http://my-cdn.com/foo/bar/",
null,
new HashMap<>(),
- IndexHtmlUtilTest::ordain))
+ IndexHtmlUtilTest::ordain,
+ null))
.containsExactly(
"canonicalPath", "", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/"));
}
@@ -66,7 +76,8 @@
"http://my-cdn.com/foo/bar/",
null,
new HashMap<>(),
- IndexHtmlUtilTest::ordain))
+ IndexHtmlUtilTest::ordain,
+ null))
.containsExactly(
"canonicalPath", "/gerrit", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/"));
}
@@ -77,11 +88,51 @@
urlParms.put("gf", new String[0]);
assertThat(
staticTemplateData(
- "http://example.com/", null, null, urlParms, IndexHtmlUtilTest::ordain))
+ "http://example.com/", null, null, urlParms, IndexHtmlUtilTest::ordain, null))
.containsExactly(
"canonicalPath", "", "staticResourcePath", ordain(""), "useGoogleFonts", "true");
}
+ @Test
+ public void usePreloadRest() throws Exception {
+ Map<String, String[]> urlParms = new HashMap<>();
+ urlParms.put("pl", new String[0]);
+ assertThat(
+ staticTemplateData(
+ "http://example.com/",
+ null,
+ null,
+ urlParms,
+ IndexHtmlUtilTest::ordain,
+ "/c/project/+/123"))
+ .containsExactly(
+ "canonicalPath", "",
+ "staticResourcePath", ordain(""),
+ "defaultChangeDetailHex", "916314",
+ "defaultDiffDetailHex", "800014",
+ "preloadChangePage", "true",
+ "changeRequestsPath", "changes/project~123");
+ }
+
+ @Test
+ public void computeChangePath() throws Exception {
+ assertThat(computeChangeRequestsPath("/c/project/+/123", changeUrlPattern))
+ .isEqualTo("changes/project~123");
+
+ assertThat(computeChangeRequestsPath("/c/project/+/124/2", changeUrlPattern))
+ .isEqualTo("changes/project~124");
+
+ assertThat(computeChangeRequestsPath("/c/project/src/+/23", changeUrlPattern))
+ .isEqualTo("changes/project%2Fsrc~23");
+
+ assertThat(computeChangeRequestsPath("/q/project/src/+/23", changeUrlPattern)).isEqualTo(null);
+
+ assertThat(computeChangeRequestsPath("/c/Scripts/+/232/1//COMMIT_MSG", changeUrlPattern))
+ .isEqualTo(null);
+ assertThat(computeChangeRequestsPath("/c/Scripts/+/232/1//COMMIT_MSG", diffUrlPattern))
+ .isEqualTo("changes/Scripts~232");
+ }
+
private static SanitizedContent ordain(String s) {
return UnsafeSanitizedContentOrdainer.ordainAsSafe(
s, SanitizedContent.ContentKind.TRUSTED_RESOURCE_URI);
diff --git a/javatests/com/google/gerrit/server/auth/ldap/LdapRealmTest.java b/javatests/com/google/gerrit/server/auth/ldap/LdapRealmTest.java
new file mode 100644
index 0000000..ba40d8c
--- /dev/null
+++ b/javatests/com/google/gerrit/server/auth/ldap/LdapRealmTest.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.auth.ldap;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GERRIT;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+import static com.google.gerrit.server.auth.ldap.LdapModule.GROUP_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.GROUP_EXIST_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.PARENT_GROUPS_CACHE;
+import static com.google.gerrit.server.auth.ldap.LdapModule.USERNAME_CACHE;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.testing.InMemoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.TypeLiteral;
+import java.util.Arrays;
+import java.util.Optional;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class LdapRealmTest {
+ @Inject private LdapRealm ldapRealm = null;
+
+ @Before
+ public void setUpInjector() throws Exception {
+ Injector injector =
+ Guice.createInjector(
+ new InMemoryModule(),
+ new CacheModule() {
+ @Override
+ protected void configure() {
+ cache(GROUP_CACHE, String.class, new TypeLiteral<Set<AccountGroup.UUID>>() {})
+ .loader(LdapRealm.MemberLoader.class);
+ cache(USERNAME_CACHE, String.class, new TypeLiteral<Optional<Account.Id>>() {})
+ .loader(LdapRealm.UserLoader.class);
+ cache(GROUP_EXIST_CACHE, String.class, new TypeLiteral<Boolean>() {})
+ .loader(LdapRealm.ExistenceLoader.class);
+ cache(
+ PARENT_GROUPS_CACHE, String.class, new TypeLiteral<ImmutableSet<String>>() {});
+ }
+ });
+ injector.injectMembers(this);
+ }
+
+ private ExternalId id(String scheme, String id) {
+ return ExternalId.create(scheme, id, Account.id(1000));
+ }
+
+ private boolean accountBelongsToRealm(ExternalId... ids) {
+ return ldapRealm.accountBelongsToRealm(Arrays.asList(ids));
+ }
+
+ private boolean accountBelongsToRealm(String scheme, String id) {
+ return accountBelongsToRealm(id(scheme, id));
+ }
+
+ @Test
+ public void accountBelongsToRealm() throws Exception {
+ assertThat(accountBelongsToRealm(SCHEME_GERRIT, "test")).isTrue();
+ assertThat(accountBelongsToRealm(id(SCHEME_USERNAME, "test"), id(SCHEME_GERRIT, "test")))
+ .isTrue();
+ assertThat(accountBelongsToRealm(id(SCHEME_GERRIT, "test"), id(SCHEME_USERNAME, "test")))
+ .isTrue();
+
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "test")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "foo@bar.com")).isFalse();
+
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "gerrit")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "xxgerritxx")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "gerrit.foo@bar.com")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "bar.gerrit@bar.com")).isFalse();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/auth/oauth/OAuthRealmTest.java b/javatests/com/google/gerrit/server/auth/oauth/OAuthRealmTest.java
new file mode 100644
index 0000000..dc62a61
--- /dev/null
+++ b/javatests/com/google/gerrit/server/auth/oauth/OAuthRealmTest.java
@@ -0,0 +1,69 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.auth.oauth;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_EXTERNAL;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.testing.InMemoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import java.util.Arrays;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class OAuthRealmTest {
+ @Inject private OAuthRealm oauthRealm = null;
+
+ @Before
+ public void setUpInjector() throws Exception {
+ Injector injector = Guice.createInjector(new InMemoryModule());
+ injector.injectMembers(this);
+ }
+
+ private ExternalId id(String scheme, String id) {
+ return ExternalId.create(scheme, id, Account.id(1000));
+ }
+
+ private boolean accountBelongsToRealm(ExternalId... ids) {
+ return oauthRealm.accountBelongsToRealm(Arrays.asList(ids));
+ }
+
+ private boolean accountBelongsToRealm(String scheme, String id) {
+ return accountBelongsToRealm(id(scheme, id));
+ }
+
+ @Test
+ public void accountBelongsToRealm() throws Exception {
+ assertThat(accountBelongsToRealm(SCHEME_EXTERNAL, "test")).isTrue();
+ assertThat(accountBelongsToRealm(id(SCHEME_USERNAME, "test"), id(SCHEME_EXTERNAL, "test")))
+ .isTrue();
+ assertThat(accountBelongsToRealm(id(SCHEME_EXTERNAL, "test"), id(SCHEME_USERNAME, "test")))
+ .isTrue();
+
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "test")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "foo@bar.com")).isFalse();
+
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "external")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_USERNAME, "xxexternalxx")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "external.foo@bar.com")).isFalse();
+ assertThat(accountBelongsToRealm(SCHEME_MAILTO, "bar.external@bar.com")).isFalse();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
index bfcb9e4..3a8d7e4 100644
--- a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
+++ b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java
@@ -176,13 +176,14 @@
private static Ref createRefWithNonEmptyTreeCommit(Repository usersRepo, int changeId, int userId)
throws IOException {
- RevWalk rw = new RevWalk(usersRepo);
- ObjectId fileObj = createBlob(usersRepo, String.format("file %d content", changeId));
- ObjectId treeObj =
- createTree(usersRepo, rw.lookupBlob(fileObj), String.format("file%d.txt", changeId));
- ObjectId commitObj = createCommit(usersRepo, treeObj, null);
- Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId));
- return refObj;
+ try (RevWalk rw = new RevWalk(usersRepo)) {
+ ObjectId fileObj = createBlob(usersRepo, String.format("file %d content", changeId));
+ ObjectId treeObj =
+ createTree(usersRepo, rw.lookupBlob(fileObj), String.format("file%d.txt", changeId));
+ ObjectId commitObj = createCommit(usersRepo, treeObj, null);
+ Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId));
+ return refObj;
+ }
}
private static Ref createRefWithEmptyTreeCommit(Repository usersRepo, int changeId, int userId)
diff --git a/modules/jgit b/modules/jgit
index a7e454b..730b7a5 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit a7e454bc51d359c2d46b19fd559f770cad8fd7d4
+Subproject commit 730b7a5ebf149e8df085a19ce4d4eddcea5958dd
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
index 8a1ed87..495f8ab 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
@@ -148,6 +148,7 @@
NEXT_LINE: 'NEXT_LINE',
PREV_LINE: 'PREV_LINE',
+ VISIBLE_LINE: 'VISIBLE_LINE',
NEXT_CHUNK: 'NEXT_CHUNK',
PREV_CHUNK: 'PREV_CHUNK',
EXPAND_ALL_DIFF_CONTEXT: 'EXPAND_ALL_DIFF_CONTEXT',
@@ -237,6 +238,8 @@
_describe(Shortcut.NEXT_LINE, ShortcutSection.DIFFS, 'Go to next line');
_describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');
+ _describe(Shortcut.VISIBLE_LINE, ShortcutSection.DIFFS,
+ 'Move cursor to currently visible code');
_describe(Shortcut.NEXT_CHUNK, ShortcutSection.DIFFS,
'Go to next diff chunk');
_describe(Shortcut.PREV_CHUNK, ShortcutSection.DIFFS,
diff --git a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
index 5a9213b..d1980a5 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
@@ -38,7 +38,10 @@
input {
width: 20em;
}
- .hideItem {
+ /* Add css selector with #id to increase priority
+ (otherwise ".gr-form-styles section" rule wins) */
+ .hideItem,
+ #itemAnnotationSection.hideItem {
display: none;
}
</style>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
index e3a3b31..5035c92 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
@@ -18,7 +18,7 @@
'use strict';
const LookupQueryPatterns = {
- CHANGE_ID: /^\s*i?[0-9a-f]{8,40}\s*$/i,
+ CHANGE_ID: /^\s*i?[0-9a-f]{7,40}\s*$/i,
CHANGE_NUM: /^\s*[1-9][0-9]*\s*$/g,
};
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 9073342..0ab165a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -207,22 +207,35 @@
max-reviewers-displayed="3"></gr-reviewer-list>
</span>
</section>
- <section>
- <span class="title">Repo</span>
- <span class="value">
- <a href$="[[_computeProjectURL(change.project)]]">
- <gr-limited-text limit="40" text="[[change.project]]"></gr-limited-text>
- </a>
- </span>
- </section>
- <section>
- <span class="title">Branch</span>
- <span class="value">
- <a href$="[[_computeBranchURL(change.project, change.branch)]]">
- <gr-limited-text limit="40" text="[[change.branch]]"></gr-limited-text>
- </a>
- </span>
- </section>
+ <template is="dom-if"
+ if="[[_computeShowRepoBranchTogether(change.project, change.branch)]]">
+ <section>
+ <span class="title">Repo Branch</span>
+ <span class="value">
+ <a href$="[[_computeProjectUrl(change.project)]]">[[change.project]]</a>
+ <a href$="[[_computeBranchUrl(change.project, change.branch)]]">[[change.branch]]</a>
+ </span>
+ </section>
+ </template>
+ <template is="dom-if"
+ if="[[!_computeShowRepoBranchTogether(change.project, change.branch)]]">
+ <section>
+ <span class="title">Repo</span>
+ <span class="value">
+ <a href$="[[_computeProjectUrl(change.project)]]">
+ <gr-limited-text limit="40" text="[[change.project]]"></gr-limited-text>
+ </a>
+ </span>
+ </section>
+ <section>
+ <span class="title">Branch</span>
+ <span class="value">
+ <a href$="[[_computeBranchUrl(change.project, change.branch)]]">
+ <gr-limited-text limit="40" text="[[change.branch]]"></gr-limited-text>
+ </a>
+ </span>
+ </section>
+ </template>
<section>
<span class="title">[[_computeParentsLabel(_currentParents)]]</span>
<span class="value">
@@ -252,7 +265,7 @@
<gr-linked-chip
text="[[change.topic]]"
limit="40"
- href="[[_computeTopicURL(change.topic)]]"
+ href="[[_computeTopicUrl(change.topic)]]"
removable="[[!_topicReadOnly]]"
on-remove="_handleTopicRemoved"></gr-linked-chip>
</template>
@@ -274,7 +287,7 @@
<section>
<span class="title">Cherry pick of</span>
<span class="value">
- <a href$="[[_computeCherryPickOfURL(change.cherry_pick_of_change, change.cherry_pick_of_patch_set, change.project)]]">
+ <a href$="[[_computeCherryPickOfUrl(change.cherry_pick_of_change, change.cherry_pick_of_patch_set, change.project)]]">
<gr-limited-text
text="[[change.cherry_pick_of_change]],[[change.cherry_pick_of_patch_set]]"
limit="40">
@@ -294,7 +307,7 @@
<gr-linked-chip
class="hashtagChip"
text="[[item]]"
- href="[[_computeHashtagURL(item)]]"
+ href="[[_computeHashtagUrl(item)]]"
removable="[[!_hashtagReadOnly]]"
on-remove="_handleHashtagRemoved">
</gr-linked-chip>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 3237d72..0f31f91 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -351,26 +351,30 @@
return [msg + ':'].concat(key.problems).join('\n');
}
- _computeProjectURL(project) {
+ _computeShowRepoBranchTogether(repo, branch) {
+ return !!repo && !!branch && repo.length + branch.length < 40;
+ }
+
+ _computeProjectUrl(project) {
return Gerrit.Nav.getUrlForProjectChanges(project);
}
- _computeBranchURL(project, branch) {
+ _computeBranchUrl(project, branch) {
if (!this.change || !this.change.status) return '';
return Gerrit.Nav.getUrlForBranch(branch, project,
this.change.status == this.ChangeStatus.NEW ? 'open' :
this.change.status.toLowerCase());
}
- _computeCherryPickOfURL(change, patchset, project) {
+ _computeCherryPickOfUrl(change, patchset, project) {
return Gerrit.Nav.getUrlForChangeById(change, project, patchset);
}
- _computeTopicURL(topic) {
+ _computeTopicUrl(topic) {
return Gerrit.Nav.getUrlForTopic(topic);
}
- _computeHashtagURL(hashtag) {
+ _computeHashtagUrl(hashtag) {
return Gerrit.Nav.getUrlForHashtag(hashtag);
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 623e8d1..11e2217 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -353,6 +353,9 @@
z-index: var(--reply-overlay-z-index);
}
}
+ .patch-set-dropdown {
+ margin: var(--spacing-m) 0 0 var(--spacing-m);
+ }
</style>
<div class="container loading" hidden$="[[!_loading]]">Loading...</div>
<div
@@ -612,6 +615,7 @@
title$="[[_computeTotalCommentCounts(_change.unresolved_comment_count, _changeComments)]]">
<span>Comment Threads</span></gr-tooltip-content>
</paper-tab>
+ <paper-tab class="robotComments">Findings</paper-tab>
</paper-tabs>
<template is="dom-if" if="[[_isSelectedView(_currentView,
_commentTabs.CHANGE_LOG)]]">
@@ -634,6 +638,23 @@
change="[[_change]]"
change-num="[[_changeNum]]"
logged-in="[[_loggedIn]]"
+ only-show-robot-comments-with-human-reply
+ on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
+ </template>
+ <template is="dom-if" if="[[_isSelectedView(_currentView,
+ _commentTabs.ROBOT_COMMENTS)]]">
+ <gr-dropdown-list
+ class="patch-set-dropdown"
+ items="[[_robotCommentsPatchSetDropdownItems]]"
+ on-value-change="_handleRobotCommentPatchSetChanged"
+ value="[[_currentRobotCommentsPatchSet]]">
+ </gr-dropdown-list>
+ <gr-thread-list
+ threads="[[_robotCommentThreads]]"
+ change="[[_change]]"
+ change-num="[[_changeNum]]"
+ logged-in="[[_loggedIn]]"
+ hide-toggle-buttons
on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 393cbc3..d2f8c87 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -62,6 +62,7 @@
const CommentTabs = {
CHANGE_LOG: 0,
COMMENT_THREADS: 1,
+ ROBOT_COMMENTS: 2,
};
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
@@ -137,6 +138,11 @@
computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)',
},
_commentThreads: Array,
+ _robotCommentThreads: {
+ type: Array,
+ computed: '_computeRobotCommentThreads(_commentThreads,'
+ + ' _currentRobotCommentsPatchSet)',
+ },
/** @type {?} */
_serverConfig: {
type: Object,
@@ -175,6 +181,7 @@
type: Object,
computed: '_computeCurrentRevision(_change.current_revision, ' +
'_change.revisions)',
+ observer: '_handleCurrentRevisionUpdate',
},
_files: Object,
_changeNum: String,
@@ -312,6 +319,14 @@
_selectedFilesTabPluginEndpoint: {
type: String,
},
+ _robotCommentsPatchSetDropdownItems: {
+ type: Array,
+ value() { return []; },
+ computed: '_computeRobotCommentsPatchSetDropdownItems(_change)',
+ },
+ _currentRobotCommentsPatchSet: {
+ type: Number,
+ },
};
}
@@ -470,6 +485,10 @@
_handleCommentTabChange() {
this._currentView = this.$.commentTabs.selected;
+ const type = Object.keys(CommentTabs).find(key => CommentTabs[key] ===
+ this._currentView);
+ this.$.reporting.reportInteraction('comment-tab-changed', {tabName:
+ type});
}
_isSelectedView(currentView, view) {
@@ -574,6 +593,38 @@
return false;
}
+ _computeRobotCommentsPatchSetDropdownItems(change) {
+ if (!change.revisions) return [];
+ return Object.values(change.revisions)
+ .filter(patch => patch._number !== 'edit')
+ .map(patch => {
+ return {
+ text: 'Patchset ' + patch._number,
+ value: patch._number,
+ };
+ })
+ .sort((a, b) => b.value - a.value);
+ }
+
+ _handleCurrentRevisionUpdate(currentRevision) {
+ this._currentRobotCommentsPatchSet = currentRevision._number;
+ }
+
+ _handleRobotCommentPatchSetChanged(e) {
+ const patchSet = parseInt(e.detail.value);
+ if (patchSet === this._currentRobotCommentsPatchSet) return;
+ this._currentRobotCommentsPatchSet = patchSet;
+ }
+
+ _computeRobotCommentThreads(commentThreads, currentRobotCommentsPatchSet) {
+ if (!commentThreads || !currentRobotCommentsPatchSet) return [];
+ return commentThreads.filter(thread => {
+ const comments = thread.comments || [];
+ return comments.length && comments[0].robot_id && (comments[0].patch_set
+ === currentRobotCommentsPatchSet);
+ });
+ }
+
_handleReloadCommentThreads() {
// Get any new drafts that have been saved in the diff view and show
// in the comment thread view.
@@ -780,12 +831,9 @@
}
_paramsChanged(value) {
- // Change the content of the comment tabs back to messages list, but
- // do not yet change the tab itself. The animation of tab switching will
- // get messed up if changed here, because it requires the tabs to be on
- // the streen, and they are hidden shortly after this. The tab switching
- // animation will happen in post render tasks.
+ // TODO(dhruvsri): Fix underlining of comment tab when page loads
this._currentView = CommentTabs.CHANGE_LOG;
+ this._setPrimaryTab();
if (value.view !== Gerrit.Nav.View.CHANGE) {
this._initialLoadComplete = false;
return;
@@ -820,7 +868,6 @@
}
this._reloadPatchNumDependentResources().then(() => {
this._sendShowChangeEvent();
- this._setPrimaryTab();
});
return;
}
@@ -855,8 +902,6 @@
this._sendShowChangeEvent();
- this._setPrimaryTab();
-
this.async(() => {
if (this.viewState.scrollTop) {
document.documentElement.scrollTop =
@@ -1247,7 +1292,8 @@
* @param {string=} opt_section
*/
_openReplyDialog(opt_section) {
- this.$.replyOverlay.open().then(() => {
+ this.$.replyOverlay.open().finally(() => {
+ // the following code should be executed no matter open succeed or not
this._resetReplyOverlayFocusStops();
this.$.replyDialog.open(opt_section);
Polymer.dom.flush();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 46f6292..f0ab218 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -68,6 +68,197 @@
COMMENT_THREADS: 1,
};
+ const THREADS = [
+ {
+ comments: [
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 4,
+ id: 'ecf0b9fa_fe1a5f62',
+ line: 5,
+ updated: '2018-02-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ },
+ {
+ id: '503008e2_0ab203ee',
+ path: '/COMMIT_MSG',
+ line: 5,
+ in_reply_to: 'ecf0b9fa_fe1a5f62',
+ updated: '2018-02-13 22:48:48.018000000',
+ message: 'draft',
+ unresolved: false,
+ __draft: true,
+ __draftID: '0.m683trwff68',
+ __editing: false,
+ patch_set: '2',
+ },
+ ],
+ patchNum: 4,
+ path: '/COMMIT_MSG',
+ line: 5,
+ rootId: 'ecf0b9fa_fe1a5f62',
+ start_datetime: '2018-02-08 18:49:18.000000000',
+ },
+ {
+ comments: [
+ {
+ __path: 'test.txt',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 3,
+ id: '09a9fb0a_1484e6cf',
+ side: 'PARENT',
+ updated: '2018-02-13 22:47:19.000000000',
+ message: 'Some comment on another patchset.',
+ unresolved: false,
+ },
+ ],
+ patchNum: 3,
+ path: 'test.txt',
+ rootId: '09a9fb0a_1484e6cf',
+ start_datetime: '2018-02-13 22:47:19.000000000',
+ commentSide: 'PARENT',
+ },
+ {
+ comments: [
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 2,
+ id: '8caddf38_44770ec1',
+ line: 4,
+ updated: '2018-02-13 22:48:40.000000000',
+ message: 'Another unresolved comment',
+ unresolved: true,
+ },
+ ],
+ patchNum: 2,
+ path: '/COMMIT_MSG',
+ line: 4,
+ rootId: '8caddf38_44770ec1',
+ start_datetime: '2018-02-13 22:48:40.000000000',
+ },
+ {
+ comments: [
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 2,
+ id: 'scaddf38_44770ec1',
+ line: 4,
+ updated: '2018-02-14 22:48:40.000000000',
+ message: 'Yet another unresolved comment',
+ unresolved: true,
+ },
+ ],
+ patchNum: 2,
+ path: '/COMMIT_MSG',
+ line: 4,
+ rootId: 'scaddf38_44770ec1',
+ start_datetime: '2018-02-14 22:48:40.000000000',
+ },
+ {
+ comments: [
+ {
+ id: 'zcf0b9fa_fe1a5f62',
+ path: '/COMMIT_MSG',
+ line: 6,
+ updated: '2018-02-15 22:48:48.018000000',
+ message: 'resolved draft',
+ unresolved: false,
+ __draft: true,
+ __draftID: '0.m683trwff68',
+ __editing: false,
+ patch_set: '2',
+ },
+ ],
+ patchNum: 4,
+ path: '/COMMIT_MSG',
+ line: 6,
+ rootId: 'zcf0b9fa_fe1a5f62',
+ start_datetime: '2018-02-09 18:49:18.000000000',
+ },
+ {
+ comments: [
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 4,
+ id: 'rc1',
+ line: 5,
+ updated: '2019-02-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ robot_id: 'rc1',
+ },
+ ],
+ patchNum: 4,
+ path: '/COMMIT_MSG',
+ line: 5,
+ rootId: 'rc1',
+ start_datetime: '2019-02-08 18:49:18.000000000',
+ },
+ {
+ comments: [
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 4,
+ id: 'rc2',
+ line: 5,
+ updated: '2019-03-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ robot_id: 'rc2',
+ },
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 4,
+ id: 'c2_1',
+ line: 5,
+ updated: '2019-03-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ },
+ ],
+ patchNum: 4,
+ path: '/COMMIT_MSG',
+ line: 5,
+ rootId: 'rc2',
+ start_datetime: '2019-03-08 18:49:18.000000000',
+ },
+ ];
+
setup(() => {
sandbox = sinon.sandbox.create();
stub('gr-endpoint-decorator', {
@@ -454,12 +645,9 @@
assert.equal(element.$.commentTabs.selected, 1);
assert.equal(element._currentView, CommentTabs.COMMENT_THREADS);
- // When the change is partially reloaded (ex: Shift+R), the content
- // is swapped out before the tab, so messages list will display even
- // though the tab for comment threads is still temporarily selected.
element._paramsChanged(element.params);
assert.equal(element.$.commentTabs.selected,
- CommentTabs.COMMENT_THREADS);
+ CommentTabs.CHANGE_LOG);
assert.equal(element._currentView, CommentTabs.CHANGE_LOG);
flush(() => {
// Correct tab is selected after the patchset is changed
@@ -472,6 +660,40 @@
});
});
+ suite('Findings comment tab', () => {
+ setup(done => {
+ element._change = {
+ change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+ revisions: {
+ rev2: {_number: 2, commit: {parents: []}},
+ rev1: {_number: 1, commit: {parents: []}},
+ rev13: {_number: 13, commit: {parents: []}},
+ rev3: {_number: 3, commit: {parents: []}},
+ rev4: {_number: 4, commit: {parents: []}},
+ },
+ current_revision: 'rev4',
+ };
+ element._commentThreads = THREADS;
+ flush(() => {
+ done();
+ });
+ });
+ test('only robot comments are rendered', () => {
+ assert.equal(element._robotCommentThreads.length, 2);
+ assert.equal(element._robotCommentThreads[0].comments[0].robot_id,
+ 'rc1');
+ assert.equal(element._robotCommentThreads[1].comments[0].robot_id,
+ 'rc2');
+ });
+ test('changing patchsets resets robot comments', done => {
+ element.set('_change.current_revision', 'rev3');
+ flush(() => {
+ assert.equal(element._robotCommentThreads.length, 0);
+ done();
+ });
+ });
+ });
+
test('reply button is not visible when logged out', () => {
assert.equal(getComputedStyle(element.$.replyBtn).display, 'none');
element._loggedIn = true;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
index 1a1276d..a845ed4 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
@@ -16,12 +16,15 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-icon/iron-icon.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
<link rel="import" href="../../plugins/gr-endpoint-param/gr-endpoint-param.html">
+
<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-confirm-submit-dialog">
@@ -33,6 +36,11 @@
p {
margin-bottom: var(--spacing-l);
}
+ .warningBeforeSubmit {
+ color: var(--error-text-color);
+ vertical-align: top;
+ margin-right: var(--spacing-s);
+ }
@media screen and (max-width: 50em) {
#dialog {
min-width: inherit;
@@ -53,7 +61,17 @@
<gr-endpoint-decorator name="confirm-submit-change">
<p>Ready to submit “<strong>[[change.subject]]</strong>”?</p>
<template is="dom-if" if="[[change.is_private]]">
- <p><strong>Heads Up!</strong> Submitting this private change will also make it public.</p>
+ <p>
+ <iron-icon icon="gr-icons:error" class="warningBeforeSubmit"></iron-icon>
+ <strong>Heads Up!</strong>
+ Submitting this private change will also make it public.
+ </p>
+ </template>
+ <template is="dom-if" if="[[change.unresolved_comment_count]]">
+ <p>
+ <iron-icon icon="gr-icons:error" class="warningBeforeSubmit"></iron-icon>
+ [[_computeUnresolvedCommentsWarning(change)]]
+ </p>
</template>
<gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param>
<gr-endpoint-param name="action" value="[[action]]"></gr-endpoint-param>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
index ea8bdb5..aa26681 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
@@ -57,6 +57,12 @@
this.$.dialog.resetFocus();
}
+ _computeUnresolvedCommentsWarning(change) {
+ const unresolvedCount = change.unresolved_comment_count;
+ const plural = unresolvedCount > 1 ? 's' : '';
+ return `Heads Up! ${unresolvedCount} unresolved comment${plural}.`;
+ }
+
_handleConfirmTap(e) {
e.preventDefault();
e.stopPropagation();
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
index 515147f..2ae58af 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
@@ -61,5 +61,15 @@
assert.notEqual(message.textContent.length, 0);
assert.notEqual(message.textContent.indexOf('my-subject'), -1);
});
+
+ test('_computeUnresolvedCommentsWarning', () => {
+ const change = {unresolved_comment_count: 1};
+ assert.equal(element._computeUnresolvedCommentsWarning(change),
+ 'Heads Up! 1 unresolved comment.');
+
+ const change2 = {unresolved_comment_count: 2};
+ assert.equal(element._computeUnresolvedCommentsWarning(change2),
+ 'Heads Up! 2 unresolved comments.');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
index 46fd227..134dd5c 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
@@ -27,42 +27,37 @@
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
</style>
<style include="shared-styles">
- .labelContainer {
- align-items: center;
- display: flex;
- margin-bottom: var(--spacing-m);
+ .labelNameCell,
+ .buttonsCell,
+ .selectedValueCell {
+ padding: var(--spacing-s) var(--spacing-m);
+ display: table-cell;
}
- .labelName {
- display: inline-block;
- flex: 0 0 auto;
- margin-right: var(--spacing-m);
- min-width: 7em;
- text-align: left;
- width: 20%;
+ /* We want the :hover highlight to extend to the border of the dialog. */
+ .labelNameCell {
+ padding-left: var(--spacing-xl);
+ }
+ .selectedValueCell {
+ padding-right: var(--spacing-xl);
+ }
+ /* This is a trick to let the selectedValueCell take the remaining width. */
+ .labelNameCell,
+ .buttonsCell {
+ white-space: nowrap;
+ }
+ .selectedValueCell {
+ width: 75%;
}
.labelMessage {
color: var(--deemphasized-text-color);
}
- .placeholder::before {
- content: ' ';
- }
- .selectedValueText {
- color: var(--deemphasized-text-color);
- font-style: italic;
- margin: 0 var(--spacing-m);
- }
- .selectedValueText.hidden {
- display: none;
- }
- .buttonWrapper {
- flex: none;
- }
gr-button {
- min-width: 40px;
+ min-width: 42px;
+ box-sizing: border-box;
--gr-button: {
background-color: var(--button-background-color, var(--table-header-background-color));
color: var(--primary-text-color);
- padding: var(--spacing-xs) var(--spacing-m);
+ padding: 0 var(--spacing-m);
@apply --vote-chip-styles;
}
}
@@ -83,63 +78,62 @@
}
.placeholder {
display: inline-block;
- width: 40px;
+ width: 42px;
+ height: 1px;
+ }
+ .placeholder::before {
+ content: ' ';
+ }
+ .selectedValueCell {
+ color: var(--deemphasized-text-color);
+ font-style: italic;
+ }
+ .selectedValueCell.hidden {
+ display: none;
}
@media only screen and (max-width: 50em) {
- .selectedValueText {
+ .selectedValueCell {
display: none;
}
}
- @media only screen and (max-width: 25em) {
- .labelName {
- margin: 0;
- text-align: center;
- width: 100%;
- }
- .labelContainer {
- display: block;
- }
- }
</style>
- <div class="labelContainer">
- <span class="labelName">[[label.name]]</span>
- <div class="buttonWrapper">
+ <span class="labelNameCell">[[label.name]]</span>
+ <div class="buttonsCell">
+ <template is="dom-repeat"
+ items="[[_computeBlankItems(permittedLabels, label.name, 'start')]]"
+ as="value">
+ <span class="placeholder" data-label$="[[label.name]]"></span>
+ </template>
+ <iron-selector
+ id="labelSelector"
+ attr-for-selected="data-value"
+ selected="[[_computeLabelValue(labels, permittedLabels, label)]]"
+ hidden$="[[!_computeAnyPermittedLabelValues(permittedLabels, label.name)]]"
+ on-selected-item-changed="_setSelectedValueText">
<template is="dom-repeat"
- items="[[_computeBlankItems(permittedLabels, label.name, 'start')]]"
+ items="[[_items]]"
as="value">
- <span class="placeholder" data-label$="[[label.name]]"></span>
+ <gr-button
+ class$="[[_computeButtonClass(value, index, _items.length)]]"
+ has-tooltip
+ data-name$="[[label.name]]"
+ data-value$="[[value]]"
+ title$="[[_computeLabelValueTitle(labels, label.name, value)]]">
+ [[value]]</gr-button>
</template>
- <iron-selector
- id="labelSelector"
- attr-for-selected="data-value"
- selected="[[_computeLabelValue(labels, permittedLabels, label)]]"
- hidden$="[[!_computeAnyPermittedLabelValues(permittedLabels, label.name)]]"
- on-selected-item-changed="_setSelectedValueText">
- <template is="dom-repeat"
- items="[[_items]]"
- as="value">
- <gr-button
- class$="[[_computeButtonClass(value, index, _items.length)]]"
- has-tooltip
- data-name$="[[label.name]]"
- data-value$="[[value]]"
- title$="[[_computeLabelValueTitle(labels, label.name, value)]]">
- [[value]]</gr-button>
- </template>
- </iron-selector>
- <template is="dom-repeat"
- items="[[_computeBlankItems(permittedLabels, label.name, 'end')]]"
- as="value">
- <span class="placeholder" data-label$="[[label.name]]"></span>
- </template>
- <span class="labelMessage"
- hidden$="[[_computeAnyPermittedLabelValues(permittedLabels, label.name)]]">
- You don't have permission to edit this label.
- </span>
- </div>
- <div class$="selectedValueText [[_computeHiddenClass(permittedLabels, label.name)]]">
- <span id="selectedValueLabel">[[_selectedValueText]]</span>
- </div>
+ </iron-selector>
+ <template is="dom-repeat"
+ items="[[_computeBlankItems(permittedLabels, label.name, 'end')]]"
+ as="value">
+ <span class="placeholder" data-label$="[[label.name]]"></span>
+ </template>
+ <span class="labelMessage"
+ hidden$="[[_computeAnyPermittedLabelValues(permittedLabels, label.name)]]">
+ You don't have permission to edit this label.
+ </span>
+ </div>
+ <div class$="selectedValueCell [[_computeHiddenClass(permittedLabels, label.name)]]">
+ <span id="selectedValueLabel">[[_selectedValueText]]</span>
</div>
</template>
<script src="gr-label-score-row.js"></script>
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
index c607a9f..29b83a3 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
@@ -23,18 +23,23 @@
<dom-module id="gr-label-scores">
<template>
<style include="shared-styles">
+ :host {
+ display: table;
+ width: 100%;
+ }
.mergedMessage {
font-style: italic;
text-align: center;
width: 100%;
}
- gr-label-score-row.no-access {
- display: var(--label-no-access-display, initial);
+ gr-label-score-row:hover {
+ background-color: var(--hover-background-color);
}
- @media only screen and (max-width: 25em) {
- :host {
- text-align: center;
- }
+ gr-label-score-row {
+ display: table-row;
+ }
+ gr-label-score-row.no-access {
+ display: var(--label-no-access-display, table-row);
}
</style>
<template is="dom-repeat" items="[[_labels]]" as="label">
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index e77bf57..2ea1134 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -156,6 +156,9 @@
.commentsIcon {
vertical-align: top;
}
+ .score.removed {
+ background-color: var(--vote-color-neutral);
+ }
.score.negative {
background-color: var(--vote-color-disliked);
}
@@ -195,7 +198,7 @@
on behalf of
</span>
<gr-account-label account="[[author]]" class="authorLabel"></gr-account-label>
- <template is="dom-repeat" items="[[_getScores(message)]]" as="score">
+ <template is="dom-repeat" items="[[_getScores(message, labelExtremes)]]" as="score">
<span class$="score [[_computeScoreClass(score, labelExtremes)]]">
[[score.label]] [[score.value]]
</span>
@@ -215,7 +218,7 @@
class="message hideOnCollapsed"
content="[[_messageContentExpanded]]"
config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
- <template is="dom-if" if="[[!_isMessageContentEmpty(message.message)]]">
+ <template is="dom-if" if="[[!_isMessageContentEmpty()]]">
<div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
<gr-button link small on-click="_handleReplyTap">Reply</gr-button>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 95109bb..d5505a4 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -17,8 +17,8 @@
(function() {
'use strict';
- const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+: /;
- const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
+ const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+:\s*(.*)/;
+ const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?$/;
/**
* @appliesMixin Gerrit.FireMixin
@@ -99,11 +99,13 @@
},
_messageContentExpanded: {
type: String,
- computed: '_computeMessageContentExpanded(message.message)',
+ computed:
+ '_computeMessageContentExpanded(message.message, message.tag)',
},
_messageContentCollapsed: {
type: String,
- computed: '_computeMessageContentCollapsed(message.message)',
+ computed:
+ '_computeMessageContentCollapsed(message.message, message.tag)',
},
_commentCountText: {
type: Number,
@@ -166,29 +168,54 @@
}
}
- _computeMessageContentExpanded(content) {
- return this._computeMessageContent(content, true);
+ _computeMessageContentExpanded(content, tag) {
+ return this._computeMessageContent(content, tag, true);
}
- _computeMessageContentCollapsed(content) {
- return this._computeMessageContent(content, false);
+ _computeMessageContentCollapsed(content, tag) {
+ return this._computeMessageContent(content, tag, false);
}
- _computeMessageContent(content, isExpanded) {
- if (!content) return '';
+ _computeMessageContent(content, tag, isExpanded) {
+ content = content || '';
+ tag = tag || '';
+ const isNewPatchSet = tag.endsWith(':newPatchSet') ||
+ tag.endsWith(':newWipPatchSet');
const lines = content.split('\n');
const filteredLines = lines.filter(line => {
- if (!isExpanded && line.startsWith('>')) return false;
- if (line.startsWith('Patch Set ')) return false;
- if (line.startsWith('(') && line.endsWith(' comment)')) return false;
- if (line.startsWith('(') && line.endsWith(' comments)')) return false;
+ if (!isExpanded && line.startsWith('>')) {
+ return false;
+ }
+ if (line.startsWith('(') && line.endsWith(' comment)')) {
+ return false;
+ }
+ if (line.startsWith('(') && line.endsWith(' comments)')) {
+ return false;
+ }
+ if (!isNewPatchSet && line.match(PATCH_SET_PREFIX_PATTERN)) {
+ return false;
+ }
return true;
});
- return filteredLines.join('\n').trim();
+ const mappedLines = filteredLines.map(line => {
+ // The change message formatting is not very consistent, so
+ // unfortunately we have to do a bit of tweaking here:
+ // Labels should be stripped from lines like this:
+ // Patch Set 29: Verified+1
+ // Rebase messages (which have a ':newPatchSet' tag) should be kept on
+ // lines like this:
+ // Patch Set 27: Patch Set 26 was rebased
+ if (isNewPatchSet) {
+ line = line.replace(PATCH_SET_PREFIX_PATTERN, '$1');
+ }
+ return line;
+ });
+ return mappedLines.join('\n').trim();
}
- _isMessageContentEmpty(content) {
- return this._computeMessageContent(content).trim().length === 0;
+ _isMessageContentEmpty() {
+ return !this._messageContentExpanded
+ || this._messageContentExpanded.length === 0;
}
_computeAuthor(message) {
@@ -247,17 +274,28 @@
return event.type === 'REVIEWER_UPDATE';
}
- _getScores(message) {
- if (!message.message) { return []; }
+ _getScores(message, labelExtremes) {
+ if (!message || !message.message || !labelExtremes) {
+ return [];
+ }
const line = message.message.split('\n', 1)[0];
const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
- if (!line.match(patchSetPrefix)) { return []; }
+ if (!line.match(patchSetPrefix)) {
+ return [];
+ }
const scoresRaw = line.split(patchSetPrefix)[1];
- if (!scoresRaw) { return []; }
+ if (!scoresRaw) {
+ return [];
+ }
return scoresRaw.split(' ')
.map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
- .filter(ms => ms && ms.length === 3)
- .map(ms => { return {label: ms[1], value: ms[2]}; });
+ .filter(ms =>
+ ms && ms.length === 4 && labelExtremes.hasOwnProperty(ms[2]))
+ .map(ms => {
+ const label = ms[2];
+ const value = ms[1] === '-' ? 'removed' : ms[3];
+ return {label, value};
+ });
}
_computeScoreClass(score, labelExtremes) {
@@ -265,6 +303,9 @@
if ([score, labelExtremes].some(arg => arg === undefined)) {
return '';
}
+ if (score.value === 'removed') {
+ return 'removed';
+ }
const classes = [];
if (score.value > 0) {
classes.push('positive');
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index 01bc691..20d87d2 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -184,16 +184,72 @@
assert.deepEqual(stub.lastCall.args[0].detail, {id: element.message.id});
});
+ suite('compute messages', () => {
+ test('empty', () => {
+ assert.equal(element._computeMessageContent('', '', true), '');
+ assert.equal(element._computeMessageContent('', '', false), '');
+ });
+
+ test('new patchset', () => {
+ const original = 'Uploaded patch set 1.';
+ const tag = 'autogenerated:gerrit:newPatchSet';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, original);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, original);
+ });
+
+ test('new patchset rebased', () => {
+ const original = 'Patch Set 27: Patch Set 26 was rebased';
+ const tag = 'autogenerated:gerrit:newPatchSet';
+ const expected = 'Patch Set 26 was rebased';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('ready for review', () => {
+ const original = 'Patch Set 1:\n\nThis change is ready for review.';
+ const tag = undefined;
+ const expected = 'This change is ready for review.';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('vote', () => {
+ const original = 'Patch Set 1: Code-Style+1';
+ const tag = undefined;
+ const expected = '';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('comments', () => {
+ const original = 'Patch Set 1:\n\n(3 comments)';
+ const tag = undefined;
+ const expected = '';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+ });
+
test('votes', () => {
element.message = {
author: {},
expanded: false,
- message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
+ message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Label3+1 Blub+1',
};
element.labelExtremes = {
'Verified': {max: 1, min: -1},
'Code-Review': {max: 2, min: -2},
- 'Trybot-Ready': {max: 3, min: 0},
+ 'Trybot-Label3': {max: 3, min: 0},
};
flushAsynchronousOperations();
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
@@ -209,6 +265,25 @@
assert.isFalse(scoreChips[2].classList.contains('min'));
});
+ test('removed votes', () => {
+ element.message = {
+ author: {},
+ expanded: false,
+ message: 'Patch Set 1: Verified+1 -Code-Review -Commit-Queue',
+ };
+ element.labelExtremes = {
+ 'Verified': {max: 1, min: -1},
+ 'Code-Review': {max: 2, min: -2},
+ 'Commit-Queue': {max: 3, min: 0},
+ };
+ flushAsynchronousOperations();
+ const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
+ assert.equal(scoreChips.length, 3);
+
+ assert.isTrue(scoreChips[1].classList.contains('removed'));
+ assert.isTrue(scoreChips[2].classList.contains('removed'));
+ });
+
test('false negative vote', () => {
element.message = {
author: {},
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index cd759d3..20680b5 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -367,8 +367,7 @@
acc[val] = (acc[val] || 0) + 1;
return acc;
}, {all: messages.length});
- this.$.reporting.reportInteraction('messages-count',
- JSON.stringify(tagsCounted));
+ this.$.reporting.reportInteraction('messages-count', tagsCounted);
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index b0747f4..9c3f53b 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -553,6 +553,23 @@
assert.isFalse(element._hasAutomatedMessages(messages));
});
+ test('initially show only 20 messages', () => {
+ sandbox.stub(element.$.reporting, 'reportInteraction',
+ (eventName, details) => {
+ assert.equal(typeof(eventName), 'string');
+ if (details) {
+ assert.equal(typeof(details), 'object');
+ }
+ });
+ const messages = Array.from(Array(23).keys())
+ .map(() => {
+ return {};
+ });
+ element._processedMessagesChanged(messages);
+
+ assert.equal(element._visibleMessages.length, 20);
+ });
+
test('_computeLabelExtremes', () => {
const computeSpy = sandbox.spy(element, '_computeLabelExtremes');
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index d54669f..7674099 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -64,6 +64,10 @@
padding: var(--spacing-m) var(--spacing-xl);
width: 100%;
}
+ section.labelsContainer {
+ /* We want the :hover highlight to extend to the border of the dialog. */
+ padding: var(--spacing-m) 0;
+ }
.actions {
background-color: var(--dialog-background-color);
bottom: 0;
@@ -295,11 +299,10 @@
class="action save"
has-tooltip
title="[[_saveTooltip]]"
- on-click="_saveClickHandler">Send</gr-button>
+ on-click="_saveClickHandler">Save</gr-button>
</template>
<gr-button
id="sendButton"
- link
primary
disabled="[[_sendDisabled]]"
class="action send"
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 90f8540..fac631b 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
@@ -43,7 +43,7 @@
};
const ButtonTooltips = {
- SAVE: 'Send but do not send notification or change review state',
+ SAVE: 'Save but do not send notification or change review state',
START_REVIEW: 'Mark as ready for review and send reply',
SEND: 'Send reply',
};
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
index 4c82d2a..a096aec 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
@@ -60,18 +60,20 @@
display: block
}
</style>
- <div class="header">
- <div class="toggleItem">
- <paper-toggle-button
- id="unresolvedToggle"
- checked="{{_unresolvedOnly}}"></paper-toggle-button>
- Only unresolved threads</div>
- <div class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]">
- <paper-toggle-button
- id="draftToggle"
- checked="{{_draftsOnly}}"></paper-toggle-button>
- Only threads with drafts</div>
- </div>
+ <template is="dom-if" if="[[!hideToggleButtons]]">
+ <div class="header">
+ <div class="toggleItem">
+ <paper-toggle-button
+ id="unresolvedToggle"
+ checked="{{_unresolvedOnly}}"></paper-toggle-button>
+ Only unresolved threads</div>
+ <div class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]">
+ <paper-toggle-button
+ id="draftToggle"
+ checked="{{_draftsOnly}}"></paper-toggle-button>
+ Only threads with drafts</div>
+ </div>
+ </template>
<div id="threads">
<template is="dom-if" if="[[!threads.length]]">
There are no inline comment threads on any diff for this change.
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
index 9e42de1..e449fad 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
@@ -41,7 +41,8 @@
_filteredThreads: {
type: Array,
computed: '_computeFilteredThreads(_sortedThreads, ' +
- '_unresolvedOnly, _draftsOnly)',
+ '_unresolvedOnly, _draftsOnly,' +
+ 'onlyShowRobotCommentsWithHumanReply)',
},
_unresolvedOnly: {
type: Boolean,
@@ -51,6 +52,16 @@
type: Boolean,
value: false,
},
+ /* Boolean properties used must default to false if passed as attribute
+ by the parent */
+ onlyShowRobotCommentsWithHumanReply: {
+ type: Boolean,
+ value: false,
+ },
+ hideToggleButtons: {
+ type: Boolean,
+ value: false,
+ },
};
}
@@ -97,12 +108,14 @@
});
}
- _computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly) {
+ _computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly,
+ onlyShowRobotCommentsWithHumanReply) {
// Polymer 2: check for undefined
if ([
sortedThreads,
unresolvedOnly,
draftsOnly,
+ onlyShowRobotCommentsWithHumanReply,
].some(arg => arg === undefined)) {
return undefined;
}
@@ -124,8 +137,8 @@
humanReplyToRobotComment = true;
}
});
- if (robotComment) {
- return humanReplyToRobotComment ? c : false;
+ if (robotComment && onlyShowRobotCommentsWithHumanReply) {
+ return humanReplyToRobotComment;
}
return c;
}
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
index b29246d..3bc41ce 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
@@ -43,6 +43,7 @@
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
+ element.onlyShowRobotCommentsWithHumanReply = true;
element.threads = [
{
comments: [
@@ -314,14 +315,15 @@
});
test('toggle unresolved only shows unresolved comments', () => {
- MockInteractions.tap(element.$.unresolvedToggle);
+ MockInteractions.tap(element.shadowRoot.querySelector(
+ '#unresolvedToggle'));
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-comment-thread').length, 5);
});
test('toggle drafts only shows threads with draft comments', () => {
- MockInteractions.tap(element.$.draftToggle);
+ MockInteractions.tap(element.shadowRoot.querySelector('#draftToggle'));
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-comment-thread').length, 2);
@@ -329,8 +331,9 @@
test('toggle drafts and unresolved only shows threads with drafts and ' +
'publicly unresolved ', () => {
- MockInteractions.tap(element.$.draftToggle);
- MockInteractions.tap(element.$.unresolvedToggle);
+ MockInteractions.tap(element.shadowRoot.querySelector('#draftToggle'));
+ MockInteractions.tap(element.shadowRoot.querySelector(
+ '#unresolvedToggle'));
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-comment-thread').length, 2);
@@ -348,5 +351,18 @@
'ecf0b9fa_fe1a5f62');
assert.equal(dispatchSpy.lastCall.args[0].detail.path, '/COMMIT_MSG');
});
+
+ suite('findings tab', () => {
+ setup(done => {
+ element.hideToggleButtons = true;
+ flush(() => {
+ done();
+ });
+ });
+ test('toggle buttons are hidden', () => {
+ assert.equal(element.shadowRoot.querySelector('.header').style.display,
+ 'none');
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
index 1569094..06edb9b 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
@@ -67,10 +67,10 @@
</div>
<div class="diffContainer">
<gr-diff
- prefs="[[overridePartialPrefs(prefs)]]"
- change-num="[[changeNum]]"
- path="[[item.filepath]]"
- diff="[[item.preview]]"></gr-diff>
+ prefs="[[overridePartialPrefs(prefs)]]"
+ change-num="[[changeNum]]"
+ path="[[item.filepath]]"
+ diff="[[item.preview]]"></gr-diff>
</div>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
index 7561f1b..45e8c07 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
@@ -75,6 +75,18 @@
});
},
+ attached() {
+ this.refitOverlay = () => {
+ // re-center the dialog as content changed
+ this.$.applyFixOverlay.fire('iron-resize');
+ };
+ this.addEventListener('diff-context-expanded', this.refitOverlay);
+ },
+
+ detached() {
+ this.removeEventListener('diff-context-expanded', this.refitOverlay);
+ },
+
_showSelectedFixSuggestion(fixSuggestion) {
this._currentFix = fixSuggestion;
return this._fetchFixPreview(fixSuggestion.fix_id);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 40fbe3c..021d962 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -18,6 +18,7 @@
<link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html">
<link rel="import" href="../gr-coverage-layer/gr-coverage-layer.html">
<link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
+<link rel="import" href="../../../elements/shared/gr-hovercard/gr-hovercard.html">
<link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
<dom-module id="gr-diff-builder">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 6fee68e..7a20dbb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -576,14 +576,30 @@
const date = (new Date(commit.time * 1000)).toLocaleDateString();
const blameNode = this._createElement('span',
isStartOfRange ? 'startOfRange' : '');
- const shaNode = this._createElement('span', 'sha');
- shaNode.innerText = commit.id.substr(0, 7);
- shaNode.onclick = function() {
- location.href = '/q/' + shaNode.innerText;
- };
+ const shaNode = this._createElement('span', 'blameDate');
+ shaNode.innerText = `${date}`;
+ shaNode.onclick = function() {
+ location.href = '/q/' + commit.id;
+ };
blameNode.appendChild(shaNode);
- blameNode.append(` on ${date} by ${commit.author}`);
+
+ const shortName = commit.author.split(' ')[0];
+ const authorNode = this._createElement('span', 'blameAuthor');
+ authorNode.innerText = ` ${shortName}`;
+ blameNode.appendChild(authorNode);
+
+ const hoverCardFragment = this._createElement('span', 'blameHoverCard');
+ hoverCardFragment.innerText =
+ `Commit ${commit.id}
+Author: ${commit.author}
+Date: ${date}
+
+${commit.commit_msg}`;
+ const hovercard = this._createElement('gr-hovercard');
+ hovercard.appendChild(hoverCardFragment);
+ blameNode.appendChild(hovercard);
+
return blameNode;
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index debb919..320909c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -1146,6 +1146,30 @@
assert.equal(result.getAttribute('data-line-number'), '3');
assert.equal(result.firstChild, mocbBlameCell);
});
+
+ test('_getBlameForBaseLine', () => {
+ const mockCommit = {
+ time: 1576105200,
+ id: 1234567890,
+ author: 'Clark Kent',
+ commit_msg: 'Testing Commit',
+ ranges: [1],
+ };
+ const blameNode = builder._getBlameForBaseLine(1, mockCommit);
+
+ const authors = blameNode.getElementsByClassName('blameAuthor');
+ assert.equal(authors.length, 1);
+ assert.equal(authors[0].innerText, ' Clark');
+
+ const date = (new Date(mockCommit.time * 1000)).toLocaleDateString();
+ Polymer.dom.flush();
+ const cards = blameNode.getElementsByClassName('blameHoverCard');
+ assert.equal(cards.length, 1);
+ assert.equal(cards[0].innerHTML,
+ `Commit 1234567890<br>Author: Clark Kent<br>Date: ${date}`
+ + '<br><br>Testing Commit'
+ );
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
index 99d0498..1e2d963 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
@@ -25,7 +25,9 @@
scroll-behavior="[[_scrollBehavior]]"
cursor-target-class="target-row"
focus-on-move="[[_focusOnMove]]"
- target="{{diffRow}}"></gr-cursor-manager>
+ target="{{diffRow}}"
+ scroll-top-margin="[[scrollTopMargin]]"
+ ></gr-cursor-manager>
</template>
<script src="gr-diff-cursor.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 726ac10..87152d8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -96,6 +96,19 @@
},
_listeningForScroll: Boolean,
+
+ /**
+ * gr-diff-view has gr-fixed-panel on top. The panel can
+ * intersect a main element and partially hides a content of
+ * the main element. To correctly calculates visibility of an
+ * element, the cursor must know how much height occuped by a fixed
+ * panel.
+ * The scrollTopMargin defines margin occuped by fixed panel.
+ */
+ scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
@@ -167,6 +180,15 @@
}
}
+ moveToVisibleArea() {
+ if (this._getViewMode() === DiffViewMode.SIDE_BY_SIDE) {
+ this.$.cursorManager.moveToVisibleArea(
+ this._rowHasSide.bind(this));
+ } else {
+ this.$.cursorManager.moveToVisibleArea();
+ }
+ }
+
moveToNextChunk(opt_clipToTop) {
this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this),
target => target.parentNode.scrollHeight, opt_clipToTop);
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 a15ca37..da22f6b 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
@@ -640,17 +640,20 @@
}
}
+ _sortComments(comments) {
+ return comments.slice(0).sort((a, b) => {
+ if (b.__draft && !a.__draft ) { return -1; }
+ if (a.__draft && !b.__draft ) { return 1; }
+ return util.parseDate(a.updated) - util.parseDate(b.updated);
+ });
+ }
+
/**
* @param {!Array<!Object>} comments
* @return {!Array<!Object>} Threads for the given comments.
*/
_createThreads(comments) {
- const sortedComments = comments.slice(0).sort((a, b) => {
- if (b.__draft && !a.__draft ) { return 0; }
- if (a.__draft && !b.__draft ) { return 1; }
- return util.parseDate(a.updated) - util.parseDate(b.updated);
- });
-
+ const sortedComments = this._sortComments(comments);
const threads = [];
for (const comment of sortedComments) {
// If the comment is in reply to another comment, find that comment's
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index d28f2a3..5aee282 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -1102,6 +1102,36 @@
});
});
+ test('comments sorting', () => {
+ const comments = [
+ {
+ id: 'new_draft',
+ message: 'i do not like either of you',
+ __commentSide: 'left',
+ __draft: true,
+ updated: '2015-12-20 15:01:20.396000000',
+ },
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ line: 1,
+ __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ __commentSide: 'left',
+ line: 1,
+ in_reply_to: 'sallys_confession',
+ },
+ ];
+ const sortedComments = element._sortComments(comments);
+ assert.equal(sortedComments[0], comments[1]);
+ assert.equal(sortedComments[1], comments[2]);
+ assert.equal(sortedComments[2], comments[0]);
+ });
+
test('_createThreads', () => {
const comments = [
{
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 7275ae5..42fc4a6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -210,7 +210,9 @@
class$="[[_computeContainerClass(_editMode)]]"
floating-disabled="[[_panelFloatingDisabled]]"
keep-on-scroll
- ready-for-measure="[[!_loading]]">
+ ready-for-measure="[[!_loading]]"
+ on-floating-height-changed="_onChangeHeaderPanelHeightChanged"
+ >
<header>
<h3>
<a href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]">
@@ -357,7 +359,7 @@
</gr-diff-preferences-dialog>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
- <gr-diff-cursor id="cursor"></gr-diff-cursor>
+ <gr-diff-cursor id="cursor" scroll-top-margin="[[_scrollTopMargin]]"></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
<gr-reporting id="reporting"></gr-reporting>
</template>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 5087d74..cb89c0c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -193,6 +193,19 @@
type: Object,
value: () => new Set(),
},
+
+ /**
+ * gr-diff-view has gr-fixed-panel on top. The panel can
+ * intersect a main element and partially hides a content of
+ * the main element. To correctly calculates visibility of an
+ * element, the cursor must know how much height occuped by a fixed
+ * panel.
+ * The scrollTopMargin defines margin occuped by fixed panel.
+ */
+ _scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
@@ -216,6 +229,7 @@
[this.Shortcut.RIGHT_PANE]: '_handleRightPane',
[this.Shortcut.NEXT_LINE]: '_handleNextLineOrFileWithComments',
[this.Shortcut.PREV_LINE]: '_handlePrevLineOrFileWithComments',
+ [this.Shortcut.VISIBLE_LINE]: '_handleVisibleLine',
[this.Shortcut.NEXT_FILE_WITH_COMMENTS]:
'_handleNextLineOrFileWithComments',
[this.Shortcut.PREV_FILE_WITH_COMMENTS]:
@@ -290,6 +304,12 @@
return this.$.restAPI.getChangeFilePathsAsSpeciallySortedArray(
changeNum, patchRange).then(files => {
this._fileList = files;
+
+ // in case current file is not in changed files
+ // (file has no change but has comments)
+ if (this._path && !this._fileList.includes(this._path)) {
+ this._fileList.push(this._path);
+ }
});
}
@@ -369,6 +389,13 @@
this.$.cursor.moveUp();
}
+ _handleVisibleLine(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this.$.cursor.moveToVisibleArea();
+ }
+
_onOpenFixPreview(e) {
this.$.applyFixDialog.open(e);
}
@@ -1163,6 +1190,10 @@
_handleReloadingDiffPreference() {
this._getDiffPreferences();
}
+
+ _onChangeHeaderPanelHeightChanged(e) {
+ this._scrollTopMargin = e.detail.value;
+ }
}
customElements.define(GrDiffView.is, GrDiffView);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 29cc950..bd172a5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -433,6 +433,23 @@
});
});
+ suite('unchanged file', () => {
+ test('unchanged file should be included in _fileList', done => {
+ element._path = 'aaa.txt';
+ // trigger reload on files
+ element._changeNum = '123';
+ element._patchRange = {
+ basePatchNum: PARENT,
+ patchNum: '1',
+ };
+ assert.isFalse(element._fileList.includes('aaa.txt'));
+ flush(() => {
+ assert.isTrue(element._fileList.includes('aaa.txt'));
+ done();
+ });
+ });
+ });
+
suite('url params', () => {
setup(() => {
sandbox.stub(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index af07552..4301ac2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -82,6 +82,7 @@
}
.diff-row {
outline: none;
+ user-select: none;
}
.diff-row.target-row.target-side-left .lineNum.left,
.diff-row.target-row.target-side-right .lineNum.right,
@@ -176,7 +177,7 @@
.ignoredWhitespaceOnly .content.remove .contentText .intraline,
.delta.total.ignoredWhitespaceOnly .content.remove .contentText,
.ignoredWhitespaceOnly .content.remove .contentText {
- background: none;
+ background-color: var(--view-background-color);
}
.content .contentText:empty:after {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 5d32e9b..ef22188 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -815,9 +815,10 @@
// for each line from the start.
let lastEl;
for (const threadEl of addedThreadEls) {
+ const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
const commentSide = threadEl.getAttribute('comment-side');
- const lineEl = this._getLineElement(threadEl,
- commentSide);
+ const lineEl = this.$.diffBuilder.getLineElByNumber(
+ lineNumString, commentSide);
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const threadGroupEl = this._getOrCreateThreadGroup(
@@ -843,18 +844,6 @@
});
}
- _getLineElement(threadEl, commentSide) {
- const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
- const lineEl = this.$.diffBuilder.getLineElByNumber(
- lineNumString, commentSide);
- if (lineEl) {
- return lineEl;
- }
- // It is possible to add comment to non-existing line via API
- threadEl.invalidLineNumber = true;
- return this.$.diffBuilder.getLineElByNumber('FILE', commentSide);
- }
-
_unobserveIncrementalNodes() {
if (this._incrementalNodeObserver) {
Polymer.dom(this).unobserveNodes(this._incrementalNodeObserver);
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index d39ba58..c1e317a 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -240,6 +240,10 @@
this.Shortcut.NEXT_LINE, 'j', 'down');
this.bindShortcut(
this.Shortcut.PREV_LINE, 'k', 'up');
+ if (this._isCursorManagerSupportMoveToVisibleLine()) {
+ this.bindShortcut(
+ this.Shortcut.VISIBLE_LINE, '.');
+ }
this.bindShortcut(
this.Shortcut.NEXT_CHUNK, 'n');
this.bindShortcut(
@@ -303,6 +307,14 @@
this.Shortcut.SEARCH, '/');
}
+ _isCursorManagerSupportMoveToVisibleLine() {
+ // This method is a copy-paste from the
+ // method _isIntersectionObserverSupported of gr-cursor-manager.js
+ // It is better share this method with gr-cursor-manager,
+ // but doing it require a lot if changes instead of 1-line copied code
+ return 'IntersectionObserver' in window;
+ }
+
_accountChanged(account) {
if (!account) { return; }
diff --git a/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface.html b/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface.html
index 26ece30..a8bb06b 100644
--- a/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface.html
+++ b/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface.html
@@ -16,7 +16,6 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="gr-plugin-popup.html">
<dom-module id="gr-popup-interface">
diff --git a/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface_test.html b/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface_test.html
index 55b7ac5..20245e0 100644
--- a/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface_test.html
+++ b/polygerrit-ui/app/elements/plugins/gr-popup-interface/gr-popup-interface_test.html
@@ -25,6 +25,7 @@
<script src="/bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
<link rel="import" href="gr-popup-interface.html"/>
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<script>void(0);</script>
diff --git a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api.html b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api.html
index 8e6c053..b3f6aec 100644
--- a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api.html
+++ b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api.html
@@ -16,7 +16,6 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="gr-plugin-repo-command.html">
<dom-module id="gr-repo-api">
diff --git a/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api.html b/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api.html
index 20cc71b..999ecfa 100644
--- a/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api.html
+++ b/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api.html
@@ -19,8 +19,6 @@
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../settings/gr-settings-view/gr-settings-item.html">
<link rel="import" href="../../settings/gr-settings-view/gr-settings-menu-item.html">
-<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
-<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<dom-module id="gr-settings-api">
<script src="gr-settings-api.js"></script>
diff --git a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api.html b/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api.html
index d6e67fe..ef1c9d4 100644
--- a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api.html
+++ b/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api.html
@@ -16,7 +16,6 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="gr-custom-plugin-header.html">
<dom-module id="gr-theme-api">
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
index 7bc93b5..d615a7f 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -16,8 +16,6 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="/bower_components/iron-icon/iron-icon.html">
-<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -47,7 +45,7 @@
background-color: var(--comment-background-color);
color: var(--comment-text-color);
display: block;
- margin: 0 4px 4px 4px;
+ margin: 0 var(--spacing-s) var(--spacing-s);
white-space: normal;
box-shadow: var(--elevation-level-2);
border-radius: var(--border-radius);
@@ -75,23 +73,13 @@
.pathInfo {
display: flex;
align-items: baseline;
+ justify-content: space-between;
+ padding: 0 var(--spacing-s) var(--spacing-s);
}
.descriptionText {
margin-left: var(--spacing-m);
font-style: italic;
}
- .invalidLineNumber {
- padding: var(--spacing-m);
- }
- .invalidLineNumberText {
- color: var(--error-text-color);
- }
- .invalidLineNumberIcon {
- color: var(--error-text-color);
- vertical-align: top;
- margin-right: var(--spacing-s);
- }
-
</style>
<template is="dom-if" if="[[showFilePath]]">
<div class="pathInfo">
@@ -100,11 +88,6 @@
</div>
</template>
<div id="container" class$="[[_computeHostClass(unresolved, isRobotComment)]]">
- <template is="dom-if" if="[[invalidLineNumber]]">
- <div class="invalidLineNumber">
- <span class="invalidLineNumberText"><iron-icon icon="gr-icons:error" class="invalidLineNumberIcon"></iron-icon>This comment thread is attached to non-existing line [[lineNum]].</span>
- </div>
- </template>
<template id="commentList" is="dom-repeat" items="[[_orderedComments]]"
as="comment">
<gr-comment
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index 00ff03a..d8a56f8 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -124,11 +124,6 @@
type: Boolean,
value: false,
},
- /** It is possible to add comment to non-existing line via API */
- invalidLineNumber: {
- type: Number,
- reflectToAttribute: true,
- },
/** Necessary only if showFilePath is true or when used with gr-diff */
lineNum: {
type: Number,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index afd0e59..333ca9d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -264,6 +264,7 @@
id="deleteBtn"
link
class$="action delete [[_computeDeleteButtonClass(_isAdmin, draft)]]"
+ hidden$="[[isRobotComment]]"
on-click="_handleCommentDelete">
<iron-icon id="icon" icon="gr-icons:delete"></iron-icon>
</gr-button>
@@ -352,8 +353,7 @@
secondary
class="action show-fix"
hidden$="[[_hasNoFix(comment)]]"
- on-click="_handleShowFix"
- disabled="[[robotButtonDisabled]]">
+ on-click="_handleShowFix">
Show Fix
</gr-button>
<gr-endpoint-decorator name="robot-comment-controls">
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
index ae8f3f6..e3391a0 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
@@ -273,7 +273,7 @@
flush(() => {
element.confirmDeleteOverlay.open.lastCall.returnValue.then(() => {
const dialog =
- this.confirmDeleteOverlay.querySelector('#confirmDeleteComment');
+ window.confirmDeleteOverlay.querySelector('#confirmDeleteComment');
dialog.message = 'removal reason';
element._handleConfirmDeleteComment();
assert.isTrue(element.$.restAPI.deleteComment.calledWith(
@@ -440,6 +440,9 @@
assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
+ // Delete button is not hidden by default
+ assert.isFalse(element.shadowRoot.querySelector('#deleteBtn').hidden);
+
element.isRobotComment = true;
element.draft = true;
assert.isTrue(element.$$('.humanActions').hasAttribute('hidden'));
@@ -463,6 +466,9 @@
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(element.$$('.robotRun.link')).display,
'none');
+
+ // Delete button is hidden for robot comments
+ assert.isTrue(element.shadowRoot.querySelector('#deleteBtn').hidden);
});
test('collapsible drafts', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index fd70fd9..4f98e88 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
@@ -89,6 +89,16 @@
type: Boolean,
value: false,
},
+
+ /**
+ * The scrollTopMargin defines height of invisible area at the top
+ * of the page. If cursor locates inside this margin - it is
+ * not visible, because it is covered by some other element.
+ */
+ scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
@@ -121,6 +131,75 @@
}
/**
+ * Move the cursor to the row which is the closest to the viewport center
+ * in vertical direction.
+ * The method uses IntersectionObservers API. If browser
+ * doesn't support this API the method does nothing
+ *
+ * @param {!Function=} opt_condition Optional condition. If a condition
+ * is passed only stops which meet conditions are taken into account.
+ */
+ moveToVisibleArea(opt_condition) {
+ if (!this.stops || !this._isIntersectionObserverSupported()) {
+ return;
+ }
+ const filteredStops = opt_condition ? this.stops.filter(opt_condition)
+ : this.stops;
+ const dims = this._getWindowDims();
+ const windowCenter =
+ Math.round((dims.innerHeight + this.scrollTopMargin) / 2);
+
+ let closestToTheCenter = null;
+ let minDistanceToCenter = null;
+ let unobservedCount = filteredStops.length;
+
+ const observer = new IntersectionObserver(entries => {
+ // This callback is called for the first time immediately.
+ // Typically it gets all observed stops at once, but
+ // sometimes can get them in several chunks.
+ entries.forEach(entry => {
+ observer.unobserve(entry.target);
+
+ // In Edge it is recommended to use intersectionRatio instead of
+ // isIntersecting.
+ const isInsideViewport =
+ entry.isIntersecting || entry.intersectionRatio > 0;
+ if (!isInsideViewport) {
+ return;
+ }
+ const center = entry.boundingClientRect.top + Math.round(
+ entry.boundingClientRect.height / 2);
+ const distanceToWindowCenter = Math.abs(center - windowCenter);
+ if (minDistanceToCenter === null ||
+ distanceToWindowCenter < minDistanceToCenter) {
+ closestToTheCenter = entry.target;
+ minDistanceToCenter = distanceToWindowCenter;
+ }
+ });
+ unobservedCount -= entries.length;
+ if (unobservedCount == 0 && closestToTheCenter) {
+ // set cursor when all stops were observed.
+ // In most cases the target is visible, so scroll is not
+ // needed. But in rare cases the target can become invisible
+ // at this point (due to some scrolling in window).
+ // To avoid jumps set noScroll options.
+ this.setCursor(closestToTheCenter, true);
+ }
+ });
+ filteredStops.forEach(stop => {
+ observer.observe(stop);
+ });
+ }
+
+ _isIntersectionObserverSupported() {
+ // The copy of this method exists in gr-app-element.js under the
+ // name _isCursorManagerSupportMoveToVisibleLine
+ // If you update this method, you must update gr-app-element.js
+ // as well.
+ return 'IntersectionObserver' in window;
+ }
+
+ /**
* Set the cursor to an arbitrary element.
*
* @param {!HTMLElement} element
@@ -304,13 +383,14 @@
_targetIsVisible(top) {
const dims = this._getWindowDims();
return this.scrollBehavior === ScrollBehavior.KEEP_VISIBLE &&
- top > dims.pageYOffset &&
+ top > (dims.pageYOffset + this.scrollTopMargin) &&
top < dims.pageYOffset + dims.innerHeight;
}
_calculateScrollToValue(top, target) {
const dims = this._getWindowDims();
- return top - (dims.innerHeight / 3) + (target.offsetHeight / 2);
+ return top + this.scrollTopMargin - (dims.innerHeight / 3) +
+ (target.offsetHeight / 2);
}
_scrollToTarget() {
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
index f3ea177..ae5a945 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.html
@@ -31,7 +31,7 @@
}
</style>
<span>
- [[_computeDateStr(dateStr, _timeFormat, _relative, showDateAndTime)]]
+ [[_computeDateStr(dateStr, _timeFormat, _dateFormat, _relative, showDateAndTime)]]
</span>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
index 8c247e3..7be041b 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
@@ -27,8 +27,29 @@
TIME_12_WITH_SEC: 'h:mm:ss A', // 2:14:00 PM
TIME_24: 'HH:mm', // 14:14
TIME_24_WITH_SEC: 'HH:mm:ss', // 14:14:00
- MONTH_DAY: 'MMM DD', // Aug 29
- MONTH_DAY_YEAR: 'MMM DD, YYYY', // Aug 29, 1997
+ };
+
+ const DateFormats = {
+ STD: {
+ short: 'MMM DD', // Aug 29
+ full: 'MMM DD, YYYY', // Aug 29, 1997
+ },
+ US: {
+ short: 'MM/DD', // 08/29
+ full: 'MM/DD/YY', // 08/29/97
+ },
+ ISO: {
+ short: 'MM-DD', // 08-29
+ full: 'YYYY-MM-DD', // 1997-08-29
+ },
+ EURO: {
+ short: 'DD. MMM', // 29. Aug
+ full: 'DD.MM.YYYY', // 29.08.1997
+ },
+ UK: {
+ short: 'DD/MM', // 29/08
+ full: 'DD/MM/YYYY', // 29/08/1997
+ },
};
/**
@@ -66,9 +87,11 @@
title: {
type: String,
reflectToAttribute: true,
- computed: '_computeFullDateStr(dateStr, _timeFormat)',
+ computed: '_computeFullDateStr(dateStr, _timeFormat, _dateFormat)',
},
+ /** @type {?{short: string, full: string}} */
+ _dateFormat: Object,
_timeFormat: String, // No default value to prevent flickering.
_relative: Boolean, // No default value to prevent flickering.
};
@@ -88,6 +111,7 @@
return this._getLoggedIn().then(loggedIn => {
if (!loggedIn) {
this._timeFormat = TimeFormats.TIME_24;
+ this._dateFormat = DateFormats.STD;
this._relative = false;
return;
}
@@ -101,19 +125,47 @@
_loadTimeFormat() {
return this._getPreferences().then(preferences => {
const timeFormat = preferences && preferences.time_format;
- switch (timeFormat) {
- case 'HHMM_12':
- this._timeFormat = TimeFormats.TIME_12;
- break;
- case 'HHMM_24':
- this._timeFormat = TimeFormats.TIME_24;
- break;
- default:
- throw Error('Invalid time format: ' + timeFormat);
- }
+ const dateFormat = preferences && preferences.date_format;
+ this._decideTimeFormat(timeFormat);
+ this._decideDateFormat(dateFormat);
});
}
+ _decideTimeFormat(timeFormat) {
+ switch (timeFormat) {
+ case 'HHMM_12':
+ this._timeFormat = TimeFormats.TIME_12;
+ break;
+ case 'HHMM_24':
+ this._timeFormat = TimeFormats.TIME_24;
+ break;
+ default:
+ throw Error('Invalid time format: ' + timeFormat);
+ }
+ }
+
+ _decideDateFormat(dateFormat) {
+ switch (dateFormat) {
+ case 'STD':
+ this._dateFormat = DateFormats.STD;
+ break;
+ case 'US':
+ this._dateFormat = DateFormats.US;
+ break;
+ case 'ISO':
+ this._dateFormat = DateFormats.ISO;
+ break;
+ case 'EURO':
+ this._dateFormat = DateFormats.EURO;
+ break;
+ case 'UK':
+ this._dateFormat = DateFormats.UK;
+ break;
+ default:
+ throw Error('Invalid date format: ' + dateFormat);
+ }
+ }
+
_loadRelative() {
return this._getPreferences().then(prefs => {
// prefs.relative_date_in_change_table is not set when false.
@@ -143,11 +195,13 @@
_isWithinHalfYear(now, date) {
const diff = -date.diff(now);
return (date.day() !== now.getDay() || diff >= Duration.DAY) &&
- diff < 180 * Duration.DAY;
+ diff < 180 * Duration.DAY;
}
- _computeDateStr(dateStr, timeFormat, relative, showDateAndTime) {
- if (!dateStr) { return ''; }
+ _computeDateStr(
+ dateStr, timeFormat, dateFormat, relative, showDateAndTime
+ ) {
+ if (!dateStr || !timeFormat || !dateFormat) { return ''; }
const date = moment(util.parseDate(dateStr));
if (!date.isValid()) { return ''; }
if (relative) {
@@ -159,12 +213,12 @@
}
}
const now = new Date();
- let format = TimeFormats.MONTH_DAY_YEAR;
+ let format = dateFormat.full;
if (this._isWithinDay(now, date)) {
format = timeFormat;
} else {
if (this._isWithinHalfYear(now, date)) {
- format = TimeFormats.MONTH_DAY;
+ format = dateFormat.short;
}
if (this.showDateAndTime) {
format = `${format} ${timeFormat}`;
@@ -179,11 +233,12 @@
TimeFormats.TIME_24_WITH_SEC;
}
- _computeFullDateStr(dateStr, timeFormat) {
+ _computeFullDateStr(dateStr, timeFormat, dateFormat) {
// Polymer 2: check for undefined
if ([
dateStr,
timeFormat,
+ dateFormat,
].some(arg => arg === undefined)) {
return undefined;
}
@@ -191,7 +246,7 @@
if (!dateStr) { return ''; }
const date = moment(util.parseDate(dateStr));
if (!date.isValid()) { return ''; }
- let format = TimeFormats.MONTH_DAY_YEAR + ', ';
+ let format = dateFormat.full + ', ';
format += this._timeToSecondsFormat(timeFormat);
return date.format(format) + this._getUtcOffsetString();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
index fe2f110..d2f7489 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
@@ -88,10 +88,12 @@
return Promise.all([loggedInPromise, preferencesPromise]);
}
- suite('24 hours time format preference', () => {
- setup(() => stubRestAPI(
- {time_format: 'HHMM_24', relative_date_in_change_table: false}
- ).then(() => {
+ suite('STD + 24 hours time format preference', () => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_24',
+ date_format: 'STD',
+ relative_date_in_change_table: false,
+ }).then(() => {
element = fixture('basic');
sandbox.stub(element, '_getUtcOffsetString').returns('');
return element._loadPreferences();
@@ -135,11 +137,155 @@
});
});
- suite('12 hours time format preference', () => {
+ suite('US + 24 hours time format preference', () => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_24',
+ date_format: 'US',
+ relative_date_in_change_table: false,
+ }).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ }));
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '15:34',
+ '15:34',
+ '07/29/15, 15:34:14', done);
+ });
+
+ test('Within 24 hours on different days', done => {
+ testDates('2015-07-29 03:34:14.985000000',
+ '2015-07-28 20:25:14.985000000',
+ '07/28',
+ '07/28 20:25',
+ '07/28/15, 20:25:14', done);
+ });
+
+ test('More than 24 hours but less than six months', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-06-15 03:25:14.985000000',
+ '06/15',
+ '06/15 03:25',
+ '06/15/15, 03:25:14', done);
+ });
+ });
+
+ suite('ISO + 24 hours time format preference', () => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_24',
+ date_format: 'ISO',
+ relative_date_in_change_table: false,
+ }).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ }));
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '15:34',
+ '15:34',
+ '2015-07-29, 15:34:14', done);
+ });
+
+ test('Within 24 hours on different days', done => {
+ testDates('2015-07-29 03:34:14.985000000',
+ '2015-07-28 20:25:14.985000000',
+ '07-28',
+ '07-28 20:25',
+ '2015-07-28, 20:25:14', done);
+ });
+
+ test('More than 24 hours but less than six months', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-06-15 03:25:14.985000000',
+ '06-15',
+ '06-15 03:25',
+ '2015-06-15, 03:25:14', done);
+ });
+ });
+
+ suite('EURO + 24 hours time format preference', () => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_24',
+ date_format: 'EURO',
+ relative_date_in_change_table: false,
+ }).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ }));
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '15:34',
+ '15:34',
+ '29.07.2015, 15:34:14', done);
+ });
+
+ test('Within 24 hours on different days', done => {
+ testDates('2015-07-29 03:34:14.985000000',
+ '2015-07-28 20:25:14.985000000',
+ '28. Jul',
+ '28. Jul 20:25',
+ '28.07.2015, 20:25:14', done);
+ });
+
+ test('More than 24 hours but less than six months', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-06-15 03:25:14.985000000',
+ '15. Jun',
+ '15. Jun 03:25',
+ '15.06.2015, 03:25:14', done);
+ });
+ });
+
+ suite('UK + 24 hours time format preference', () => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_24',
+ date_format: 'UK',
+ relative_date_in_change_table: false,
+ }).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ }));
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '15:34',
+ '15:34',
+ '29/07/2015, 15:34:14', done);
+ });
+
+ test('Within 24 hours on different days', done => {
+ testDates('2015-07-29 03:34:14.985000000',
+ '2015-07-28 20:25:14.985000000',
+ '28/07',
+ '28/07 20:25',
+ '28/07/2015, 20:25:14', done);
+ });
+
+ test('More than 24 hours but less than six months', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-06-15 03:25:14.985000000',
+ '15/06',
+ '15/06 03:25',
+ '15/06/2015, 03:25:14', done);
+ });
+ });
+
+ suite('STD + 12 hours time format preference', () => {
setup(() =>
- // relative_date_in_change_table is not set when false.
+ // relative_date_in_change_table is not set when false.
stubRestAPI(
- {time_format: 'HHMM_12'}
+ {time_format: 'HHMM_12', date_format: 'STD'}
).then(() => {
element = fixture('basic');
sandbox.stub(element, '_getUtcOffsetString').returns('');
@@ -156,10 +302,96 @@
});
});
+ suite('US + 12 hours time format preference', () => {
+ setup(() =>
+ // relative_date_in_change_table is not set when false.
+ stubRestAPI(
+ {time_format: 'HHMM_12', date_format: 'US'}
+ ).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ })
+ );
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '3:34 PM',
+ '3:34 PM',
+ '07/29/15, 3:34:14 PM', done);
+ });
+ });
+
+ suite('ISO + 12 hours time format preference', () => {
+ setup(() =>
+ // relative_date_in_change_table is not set when false.
+ stubRestAPI(
+ {time_format: 'HHMM_12', date_format: 'ISO'}
+ ).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ })
+ );
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '3:34 PM',
+ '3:34 PM',
+ '2015-07-29, 3:34:14 PM', done);
+ });
+ });
+
+ suite('EURO + 12 hours time format preference', () => {
+ setup(() =>
+ // relative_date_in_change_table is not set when false.
+ stubRestAPI(
+ {time_format: 'HHMM_12', date_format: 'EURO'}
+ ).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ })
+ );
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '3:34 PM',
+ '3:34 PM',
+ '29.07.2015, 3:34:14 PM', done);
+ });
+ });
+
+ suite('UK + 12 hours time format preference', () => {
+ setup(() =>
+ // relative_date_in_change_table is not set when false.
+ stubRestAPI(
+ {time_format: 'HHMM_12', date_format: 'UK'}
+ ).then(() => {
+ element = fixture('basic');
+ sandbox.stub(element, '_getUtcOffsetString').returns('');
+ return element._loadPreferences();
+ })
+ );
+
+ test('Within 24 hours on same day', done => {
+ testDates('2015-07-29 20:34:14.985000000',
+ '2015-07-29 15:34:14.985000000',
+ '3:34 PM',
+ '3:34 PM',
+ '29/07/2015, 3:34:14 PM', done);
+ });
+ });
+
suite('relative date preference', () => {
- setup(() => stubRestAPI(
- {time_format: 'HHMM_12', relative_date_in_change_table: true}
- ).then(() => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_12',
+ date_format: 'STD',
+ relative_date_in_change_table: true,
+ }).then(() => {
element = fixture('basic');
sandbox.stub(element, '_getUtcOffsetString').returns('');
return element._loadPreferences();
@@ -183,15 +415,19 @@
});
suite('logged in', () => {
- setup(() => stubRestAPI(
- {time_format: 'HHMM_12', relative_date_in_change_table: true}
- ).then(() => {
+ setup(() => stubRestAPI({
+ time_format: 'HHMM_12',
+ date_format: 'US',
+ relative_date_in_change_table: true,
+ }).then(() => {
element = fixture('basic');
return element._loadPreferences();
}));
test('Preferences are respected', () => {
assert.equal(element._timeFormat, 'h:mm A');
+ assert.equal(element._dateFormat.short, 'MM/DD');
+ assert.equal(element._dateFormat.full, 'MM/DD/YY');
assert.isTrue(element._relative);
});
});
@@ -204,6 +440,8 @@
test('Default preferences are respected', () => {
assert.equal(element._timeFormat, 'HH:mm');
+ assert.equal(element._dateFormat.short, 'MMM DD');
+ assert.equal(element._dateFormat.full, 'MMM DD, YYYY');
assert.isFalse(element._relative);
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
index 77418df..475f6e2 100644
--- a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
+++ b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
@@ -27,6 +27,7 @@
color: var(--primary-text-color);
display: block;
max-height: 90vh;
+ overflow: auto;
}
.container {
display: flex;
@@ -42,6 +43,13 @@
display: flex;
flex-shrink: 1;
width: 100%;
+ flex: 1;
+ /* IMPORTANT: required for firefox */
+ min-height: 0px;
+ }
+ main .overflow-container {
+ flex: 1;
+ overflow: auto;
}
footer {
display: flex;
@@ -58,7 +66,11 @@
</style>
<div class="container" on-keydown="_handleKeydown">
<header class="font-h3"><slot name="header"></slot></header>
- <main><slot name="main"></slot></main>
+ <main>
+ <div class="overflow-container">
+ <slot name="main"></slot>
+ </div>
+ </main>
<footer>
<slot name="footer"></slot>
<gr-button id="cancel" class$="[[_computeCancelClass(cancelLabel)]]" link on-click="_handleCancelTap">
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
index f7349eb..7586876 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
@@ -59,8 +59,9 @@
cursor: pointer;
flex-direction: column;
font-size: inherit;
- // This variable was introduced in Dec 2019. We keep both min-height
- // rules around, because --paper-item-min-height is not yet upstreamed.
+ /* This variable was introduced in Dec 2019. We keep both min-height
+ * rules around, because --paper-item-min-height is not yet upstreamed.
+ */
--paper-item-min-height: 0;
--paper-item: {
min-height: 0;
diff --git a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
index 02e23e8..d4995cc 100644
--- a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
+++ b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
@@ -25,7 +25,10 @@
static get properties() {
return {
- floatingDisabled: Boolean,
+ floatingDisabled: {
+ type: Boolean,
+ value: false,
+ },
readyForMeasure: {
type: Boolean,
observer: '_readyForMeasureObserver',
@@ -58,10 +61,38 @@
type: Object,
value: null,
},
+ /**
+ * If place before any other content defines how much
+ * of the content below it is covered by this panel
+ */
+ floatingHeight: {
+ type: Number,
+ value: 0,
+ notify: true,
+ },
+
_webComponentsReady: Boolean,
};
}
+ static get observers() {
+ return [
+ '_updateFloatingHeight(floatingDisabled, _isMeasured, _headerHeight)',
+ ];
+ }
+
+ _updateFloatingHeight(floatingDisabled, isMeasured, headerHeight) {
+ if ([
+ floatingDisabled,
+ isMeasured,
+ headerHeight,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+ this.floatingHeight =
+ (!floatingDisabled && isMeasured) ? headerHeight : 0;
+ }
+
/** @override */
attached() {
super.attached();
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html
index 7b03308..a316fd5 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html
@@ -41,7 +41,7 @@
let sendStub;
setup(() => {
- this.clock = sinon.useFakeTimers();
+ window.clock = sinon.useFakeTimers();
sandbox = sinon.sandbox.create();
sendStub = sandbox.stub().returns(Promise.resolve({status: 200}));
stub('gr-rest-api-interface', {
@@ -56,7 +56,7 @@
});
teardown(() => {
- this.clock.restore();
+ window.clock.restore();
sandbox.restore();
element._removeEventCallbacks();
Gerrit._testOnly_resetPlugins();
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index 917c0a9..323a730 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -49,7 +49,7 @@
};
setup(() => {
- this.clock = sinon.useFakeTimers();
+ window.clock = sinon.useFakeTimers();
sandbox = sinon.sandbox.create();
getResponseObjectStub = sandbox.stub().returns(Promise.resolve());
sendStub = sandbox.stub().returns(Promise.resolve({status: 200}));
@@ -70,7 +70,7 @@
});
teardown(() => {
- this.clock.restore();
+ window.clock.restore();
sandbox.restore();
element._removeEventCallbacks();
plugin = null;
@@ -226,7 +226,7 @@
assert.isFalse(spy.called);
// Timeout on loading plugins
- this.clock.tick(PLUGIN_LOADING_TIMEOUT_MS * 2);
+ window.clock.tick(PLUGIN_LOADING_TIMEOUT_MS * 2);
flush(() => {
assert.isTrue(spy.called);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
index 7ddb666..85c23dd 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
@@ -43,7 +43,7 @@
let sendStub;
setup(() => {
- this.clock = sinon.useFakeTimers();
+ window.clock = sinon.useFakeTimers();
sandbox = sinon.sandbox.create();
sendStub = sandbox.stub().returns(Promise.resolve({status: 200}));
stub('gr-rest-api-interface', {
@@ -61,7 +61,7 @@
teardown(() => {
sandbox.restore();
- this.clock.restore();
+ window.clock.restore();
Gerrit._testOnly_resetPlugins();
});
@@ -110,7 +110,7 @@
Gerrit._loadPlugins(plugins);
assert.isFalse(Gerrit._arePluginsLoaded());
// Timeout on loading plugins
- this.clock.tick(PLUGIN_LOADING_TIMEOUT_MS * 2);
+ window.clock.tick(PLUGIN_LOADING_TIMEOUT_MS * 2);
flush(() => {
assert.isTrue(Gerrit._arePluginsLoaded());
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index e9477bf..b0186f0 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -50,7 +50,11 @@
element.config = {
ph: {
match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
- link: 'https://code.google.com/p/gerrit/issues/detail?id=$2',
+ link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
+ },
+ prefixsameinlinkandpattern: {
+ match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
+ link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
},
changeid: {
match: '(I[0-9a-f]{8,40})',
@@ -91,7 +95,7 @@
test('URL pattern was parsed and linked.', () => {
// Regular inline link.
- const url = 'https://code.google.com/p/gerrit/issues/detail?id=3650';
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
element.content = url;
const linkEl = element.$.output.childNodes[0];
assert.equal(linkEl.target, '_blank');
@@ -105,7 +109,7 @@
element.content = 'Issue 3650';
let linkEl = element.$.output.childNodes[0];
- const url = 'https://code.google.com/p/gerrit/issues/detail?id=3650';
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
assert.equal(linkEl.target, '_blank');
assert.equal(linkEl.href, url);
assert.equal(linkEl.textContent, 'Issue 3650');
@@ -118,6 +122,18 @@
assert.equal(linkEl.textContent, 'Bug 3650');
});
+ test('Pattern with same prefix as link was correctly parsed', () => {
+ // Pattern starts with the same prefix (`http`) as the url.
+ element.content = 'httpexample 3650';
+
+ assert.equal(element.$.output.childNodes.length, 1);
+ const linkEl = element.$.output.childNodes[0];
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, 'httpexample 3650');
+ });
+
test('Change-Id pattern was parsed and linked', () => {
// "Change-Id:" pattern.
const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
@@ -159,12 +175,12 @@
assert.equal(linkEl1.target, '_blank');
assert.equal(linkEl1.href,
- 'https://code.google.com/p/gerrit/issues/detail?id=3650');
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650');
assert.equal(linkEl1.textContent, 'Issue 3650');
assert.equal(linkEl2.target, '_blank');
assert.equal(linkEl2.href,
- 'https://code.google.com/p/gerrit/issues/detail?id=3450');
+ 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450');
assert.equal(linkEl2.textContent, 'Issue 3450');
});
@@ -177,7 +193,7 @@
const bug = 'Issue 3650';
const changeUrl = '/q/' + changeID;
- const bugUrl = 'https://code.google.com/p/gerrit/issues/detail?id=3650';
+ const bugUrl = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
element.content = prefix + changeID + bug;
@@ -262,16 +278,58 @@
let links = element.$.output.querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
+ assert.equal(links[0].innerHTML, 'mailto:test@google.com');
element.content = 'xx http://google.com yy';
links = element.$.output.querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'http://google.com');
+ assert.equal(links[0].innerHTML, 'http://google.com');
element.content = 'xx https://google.com yy';
links = element.$.output.querySelectorAll('a');
assert.equal(links.length, 1);
assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
+
+ element.content = 'xx ssh://google.com yy';
+ links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 0);
+
+ element.content = 'xx ftp://google.com yy';
+ links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 0);
+ });
+
+ test('links without leading whitespace are linkified', () => {
+ element.content = 'xx abcmailto:test@google.com yy';
+ assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx abc');
+ let links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
+ assert.equal(links[0].innerHTML, 'mailto:test@google.com');
+
+ element.content = 'xx defhttp://google.com yy';
+ assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx def');
+ links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'http://google.com');
+ assert.equal(links[0].innerHTML, 'http://google.com');
+
+ element.content = 'xx qwehttps://google.com yy';
+ assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx qwe');
+ links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
+
+ // Non-latin character
+ element.content = 'xx абвhttps://google.com yy';
+ assert.equal(element.$.output.innerHTML.substr(0, 6), 'xx абв');
+ links = element.$.output.querySelectorAll('a');
+ assert.equal(links.length, 1);
+ assert.equal(links[0].getAttribute('href'), 'https://google.com');
+ assert.equal(links[0].innerHTML, 'https://google.com');
element.content = 'xx ssh://google.com yy';
links = element.$.output.querySelectorAll('a');
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
index eb2b0d0..aac6d76 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
@@ -22,7 +22,7 @@
*
* @type {RegExp}
*/
- const URL_PROTOCOL_PATTERN = /^(https?:\/\/|mailto:)/;
+ const URL_PROTOCOL_PATTERN = /^(.*)(https?:\/\/|mailto:)/;
/**
* Construct a parser for linkifying text. Will linkify plain URLs that appear
@@ -257,13 +257,29 @@
// the source text does not include a protocol, the protocol will be added
// by ba-linkify. Create the link if the href is provided and its protocol
// matches the expected pattern.
- if (href && URL_PROTOCOL_PATTERN.test(href)) {
- this.addText(text, href);
- } else {
- // For the sections of text that lie between the links found by
- // ba-linkify, we search for the project-config-specified link patterns.
- this.parseLinks(text, this.linkConfig);
+ if (href) {
+ const result = URL_PROTOCOL_PATTERN.exec(href);
+ if (result) {
+ const prefixText = result[1];
+ if (prefixText.length > 0) {
+ // Fix for simple cases from
+ // https://bugs.chromium.org/p/gerrit/issues/detail?id=11697
+ // When leading whitespace is missed before link,
+ // linkify add this text before link as a schema name to href.
+ // We suppose, that prefixText just a single word
+ // before link and add this word as is, without processing
+ // any patterns in it.
+ this.parseLinks(prefixText, []);
+ text = text.substring(prefixText.length);
+ href = href.substring(prefixText.length);
+ }
+ this.addText(text, href);
+ return;
+ }
}
+ // For the sections of text that lie between the links found by
+ // ba-linkify, we search for the project-config-specified link patterns.
+ this.parseLinks(text, this.linkConfig);
};
/**
@@ -304,14 +320,15 @@
let result = match[0].replace(pattern,
patterns[p].html || patterns[p].link);
- let i;
- // Skip portion of replacement string that is equal to original.
- for (i = 0; i < result.length; i++) {
- if (result[i] !== match[0][i]) { break; }
- }
- result = result.slice(i);
-
if (patterns[p].html) {
+ let i;
+ // Skip portion of replacement string that is equal to original to
+ // allow overlapping patterns.
+ for (i = 0; i < result.length; i++) {
+ if (result[i] !== match[0][i]) { break; }
+ }
+ result = result.slice(i);
+
this.addHTML(
result,
susbtrIndex + match.index + i,
@@ -321,8 +338,8 @@
this.addLink(
match[0],
result,
- susbtrIndex + match.index + i,
- match[0].length - i,
+ susbtrIndex + match.index,
+ match[0].length,
outputArray);
} else {
throw Error('linkconfig entry ' + p +
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
index 5dee2fe..3a957ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
@@ -63,13 +63,13 @@
}
open(...args) {
- return new Promise(resolve => {
+ return new Promise((resolve, reject) => {
Polymer.IronOverlayBehaviorImpl.open.apply(this, args);
if (this._isMobile()) {
this.fire('fullscreen-overlay-opened');
this._fullScreenOpen = true;
}
- this._awaitOpen(resolve);
+ this._awaitOpen(resolve, reject);
});
}
@@ -96,7 +96,7 @@
* NOTE: (wyatta) Slightly hacky way to listen to the overlay actually
* opening. Eventually replace with a direct way to listen to the overlay.
*/
- _awaitOpen(fn) {
+ _awaitOpen(fn, reject) {
let iters = 0;
const step = () => {
this.async(() => {
@@ -105,11 +105,7 @@
} else if (iters++ < AWAIT_MAX_ITERS) {
step.call(this);
} else {
- // TODO(crbug.com/gerrit/10774): Once this is confirmed as the root
- // cause of the bug, fix it by either making sure to resolve the fn
- // function or find a better way to listen on the overlay being
- // shown.
- console.warn('gr-overlay _awaitOpen failed to resolve');
+ reject(new Error('gr-overlay _awaitOpen failed to resolve'));
}
}, AWAIT_STEP);
};
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 4aefe46..ce26bf6 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -990,6 +990,20 @@
* @param {function()=} opt_cancelCondition
*/
getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
+ return this.getConfig(false).then(config => {
+ const optionsHex = this._getChangeOptionsHex(config);
+ return this._getChangeDetail(
+ changeNum, optionsHex, opt_errFn, opt_cancelCondition)
+ .then(GrReviewerUpdatesParser.parse);
+ });
+ }
+
+ _getChangeOptionsHex(config) {
+ if (window.DEFAULT_DETAIL_HEXES && window.DEFAULT_DETAIL_HEXES.changePage
+ && !(config.receive && config.receive.enable_signed_push)) {
+ return window.DEFAULT_DETAIL_HEXES.changePage;
+ }
+
// This list MUST be kept in sync with
// ChangeIT#changeDetailsDoesNotRequireIndex
const options = [
@@ -1003,15 +1017,10 @@
this.ListChangesOption.WEB_LINKS,
this.ListChangesOption.SKIP_DIFFSTAT,
];
- return this.getConfig(false).then(config => {
- if (config.receive && config.receive.enable_signed_push) {
- options.push(this.ListChangesOption.PUSH_CERTIFICATES);
- }
- const optionsHex = this.listChangesOptionsToHex(...options);
- return this._getChangeDetail(
- changeNum, optionsHex, opt_errFn, opt_cancelCondition)
- .then(GrReviewerUpdatesParser.parse);
- });
+ if (config.receive && config.receive.enable_signed_push) {
+ options.push(this.ListChangesOption.PUSH_CERTIFICATES);
+ }
+ return this.listChangesOptionsToHex(...options);
}
/**
@@ -1020,11 +1029,16 @@
* @param {function()=} opt_cancelCondition
*/
getDiffChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
- const optionsHex = this.listChangesOptionsToHex(
- this.ListChangesOption.ALL_COMMITS,
- this.ListChangesOption.ALL_REVISIONS,
- this.ListChangesOption.SKIP_DIFFSTAT
- );
+ let optionsHex = '';
+ if (window.DEFAULT_DETAIL_HEXES && window.DEFAULT_DETAIL_HEXES.diffPage) {
+ optionsHex = window.DEFAULT_DETAIL_HEXES.diffPage;
+ } else {
+ optionsHex = this.listChangesOptionsToHex(
+ this.ListChangesOption.ALL_COMMITS,
+ this.ListChangesOption.ALL_REVISIONS,
+ this.ListChangesOption.SKIP_DIFFSTAT
+ );
+ }
return this._getChangeDetail(changeNum, optionsHex, opt_errFn,
opt_cancelCondition);
}
@@ -1040,7 +1054,7 @@
const urlWithParams = this._restApiHelper
.urlWithParams(url, optionsHex);
const params = {O: optionsHex};
- let req = {
+ const req = {
url,
errFn: opt_errFn,
cancelCondition: opt_cancelCondition,
@@ -1048,7 +1062,6 @@
fetchOptions: this._etags.getOptions(urlWithParams),
anonymizedUrl: '/changes/*~*/detail?O=' + optionsHex,
};
- req = this._restApiHelper.addAcceptJsonHeader(req);
return this._restApiHelper.fetchRawJSON(req).then(response => {
if (response && response.status === 304) {
return Promise.resolve(this._restApiHelper.parsePrefixedJSON(
@@ -2039,12 +2052,15 @@
* @param {string|number=} opt_patchNum
* @return {!Promise<!Object>} Diff comments response.
*/
+ // We don't want to add accept header, since preloading of comments is
+ // working only without accept header.
+ const noAcceptHeader = true;
const fetchComments = opt_patchNum => this._getChangeURLAndFetch({
changeNum,
endpoint,
patchNum: opt_patchNum,
reportEndpointAsIs: true,
- });
+ }, noAcceptHeader);
if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
return fetchComments();
@@ -2609,7 +2625,7 @@
* @param {Gerrit.ChangeFetchRequest} req
* @return {!Promise<!Object>}
*/
- _getChangeURLAndFetch(req) {
+ _getChangeURLAndFetch(req, noAcceptHeader) {
const anonymizedEndpoint = req.reportEndpointAsIs ?
req.endpoint : req.anonymizedEndpoint;
const anonymizedBaseUrl = req.patchNum ?
@@ -2622,7 +2638,7 @@
fetchOptions: req.fetchOptions,
anonymizedUrl: anonymizedEndpoint ?
(anonymizedBaseUrl + anonymizedEndpoint) : undefined,
- }));
+ }, noAcceptHeader));
}
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index ca4b246..ed7d29f 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -1036,16 +1036,6 @@
});
});
- test('_getChangeDetail accepts only json', () => {
- const authFetchStub = sandbox.stub(element._auth, 'fetch')
- .returns(Promise.resolve());
- const errFn = sinon.stub();
- element._getChangeDetail(123, '516714', errFn);
- assert.isTrue(authFetchStub.called);
- assert.equal(authFetchStub.lastCall.args[1].headers.get('Accept'),
- 'application/json');
- });
-
test('_getChangeDetail populates _projectLookup', () => {
sandbox.stub(element, 'getChangeActionURL')
.returns(Promise.resolve(''));
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
index 91fef29..2c326df 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js
@@ -204,9 +204,12 @@
* Same as {@link fetchRawJSON}, plus error handling.
*
* @param {Gerrit.FetchJSONRequest} req
+ * @param {boolean} noAcceptHeader - don't add default accept json header
*/
- fetchJSON(req) {
- req = this.addAcceptJsonHeader(req);
+ fetchJSON(req, noAcceptHeader) {
+ if (!noAcceptHeader) {
+ req = this.addAcceptJsonHeader(req);
+ }
return this.fetchRawJSON(req).then(response => {
if (!response) {
return;
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.html b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.html
index b4fefe1..ec56912 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.html
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.html
@@ -16,17 +16,20 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="../gr-icons/gr-icons.html">
<link rel="import" href="../../../behaviors/gr-tooltip-behavior/gr-tooltip-behavior.html">
<dom-module id="gr-tooltip-content">
<template>
<style>
- .arrow {
- color: var(--arrow-color);
+ iron-icon {
+ width: var(--line-height-normal);
+ height: var(--line-height-normal);
+ vertical-align: top;
}
</style>
<slot></slot><!--
- --><span class="arrow" hidden$="[[!showIcon]]">ⓘ</span>
+ --><iron-icon icon="gr-icons:info" hidden$="[[!showIcon]]"></iron-icon>
</template>
<script src="gr-tooltip-content.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.html b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.html
index 4276195..7098d7c 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.html
@@ -43,7 +43,7 @@
test('icon is not visible by default', () => {
assert.equal(Polymer.dom(element.root)
- .querySelector('.arrow').hidden, true);
+ .querySelector('iron-icon').hidden, true);
});
test('position-below attribute is reflected', () => {
@@ -55,7 +55,7 @@
test('icon is visible with showIcon property', () => {
element.showIcon = true;
assert.equal(Polymer.dom(element.root)
- .querySelector('.arrow').hidden, false);
+ .querySelector('iron-icon').hidden, false);
});
});
</script>
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index 5046a2a..b182309 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -28,6 +28,11 @@
{@param? polyfillSD: ?}
{@param? polyfillSC: ?}
{@param? useGoogleFonts: ?}
+ {@param? changeRequestsPath: ?}
+ {@param? defaultChangeDetailHex: ?}
+ {@param? defaultDiffDetailHex: ?}
+ {@param? preloadChangePage: ?}
+ {@param? preloadDiffPage: ?}
<!DOCTYPE html>{\n}
<html lang="en">{\n}
<meta charset="utf-8">{\n}
@@ -43,6 +48,14 @@
// Disable extra font load from paper-styles
window.polymerSkipLoadingFontRoboto = true;
window.CLOSURE_NO_DEPS = true;
+ window.DEFAULT_DETAIL_HEXES = {lb}
+ {if $defaultChangeDetailHex}
+ changePage: '{$defaultChangeDetailHex}',
+ {/if}
+ {if $defaultDiffDetailHex}
+ diffPage: '{$defaultDiffDetailHex}',
+ {/if}
+ {rb};
{if $canonicalPath != ''}window.CANONICAL_PATH = '{$canonicalPath}';{/if}
{if $versionInfo}window.VERSION_INFO = '{$versionInfo}';{/if}
{if $staticResourcePath != ''}window.STATIC_RESOURCE_PATH = '{$staticResourcePath}';{/if}
@@ -68,6 +81,16 @@
{else}
<link rel="icon" type="image/x-icon" href="{$canonicalPath}/favicon.ico">{\n}
{/if}
+ {if $changeRequestsPath}
+ {if $preloadChangePage and $defaultChangeDetailHex}
+ <link rel="preload" href="{$canonicalPath}/{$changeRequestsPath}/detail?O={$defaultChangeDetailHex}" as="fetch" type="application/json" crossorigin="anonymous"/>{\n}
+ {/if}
+ {if $preloadDiffPage and $defaultDiffDetailHex}
+ <link rel="preload" href="{$canonicalPath}/{$changeRequestsPath}/detail?O={$defaultDiffDetailHex}" as="fetch" type="application/json" crossorigin="anonymous"/>{\n}
+ {/if}
+ <link rel="preload" href="{$canonicalPath}/{$changeRequestsPath}/comments" as="fetch" type="application/json" crossorigin="anonymous"/>{\n}
+ <link rel="preload" href="{$canonicalPath}/{$changeRequestsPath}/robotcomments" as="fetch" type="application/json" crossorigin="anonymous"/>{\n}
+ {/if}
// RobotoMono fonts are used in styles/fonts.css
{if $useGoogleFonts}
diff --git a/resources/com/google/gerrit/server/mime/mime-types.properties b/resources/com/google/gerrit/server/mime/mime-types.properties
index 7c94f32..84eef77 100644
--- a/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -13,6 +13,7 @@
bzl = text/x-python
BUCK = text/x-python
BUILD = text/x-python
+BUILD.bazel = text/x-python
c = text/x-csrc
cfg = text/x-ttcn-cfg
cl = text/x-common-lisp
@@ -247,6 +248,7 @@
vtl = text/velocity
webidl = text/x-webidl
wsdl = application/xml
+WORKSPACE = text/x-python
xaml = application/xml
xhtml = text/html
xml = application/xml
diff --git a/tools/dev-hooks/pre-commit b/tools/dev-hooks/pre-commit
index b77f382..af87b7e 100755
--- a/tools/dev-hooks/pre-commit
+++ b/tools/dev-hooks/pre-commit
@@ -31,7 +31,7 @@
eslint=${gitroot}/node_modules/eslint/bin/eslint.js
# Run eslint over changed frontend code
-CHANGED_UI_FILES=$(git diff --cached --name-only -- '*.js' '*.html' | grep 'polygerrit-ui') && true
+CHANGED_UI_FILES=$(git diff --cached --name-only --diff-filter=ACM -- '*.js' '*.html' | grep 'polygerrit-ui') && true
if [ "${CHANGED_UI_FILES}" ]; then
if $eslint --fix ${CHANGED_UI_FILES}; then
# Add again in case lint fix modified some files
diff --git a/tools/js/download_bower.py b/tools/js/download_bower.py
index c9a5df6..c1d7d00 100755
--- a/tools/js/download_bower.py
+++ b/tools/js/download_bower.py
@@ -46,7 +46,8 @@
raise
out, err = p.communicate()
if p.returncode:
- sys.stderr.write(err)
+ # For python3 support we wrap str around err.
+ sys.stderr.write(str(err))
raise OSError('Command failed: %s' % ' '.join(cmd))
try:
