Merge "Shortcut for moving cursor to visible code"
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 9dd4d0f..86221eb 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -18,9 +18,14 @@
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.
+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]
+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 build the tests
@@ -36,9 +41,7 @@
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. Notice the
-plural form for the file name, expected by the warning (below), compared to the one from that linked
-example which is singular.
+locally. Dummy values for `user` and `password` in that file can be used initially.
----
[warn] Credentials file ~/.sbt/sonatype_credentials does not exist
@@ -67,26 +70,36 @@
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:
+
+----
+ssh-keyscan -t rsa -p 29418 localhost > ~/.ssh/known_hosts
+----
=== Input file
-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:
+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"
}
]
----
@@ -98,6 +111,8 @@
* `push`
* `clone`
+The example above assumes that the `loadtest-repo` project exists in the Gerrit under test.
+
== How to run tests
Run all tests:
@@ -107,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:
@@ -122,7 +137,7 @@
=== How to run using Docker
----
-docker run -it e2e-tests -s com.google.gerrit.scenarios.ReplayRecordsFromFeederScenario
+docker run -it e2e-tests -s com.google.gerrit.scenarios.CloneUsingBothProtocols
----
GERRIT
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 1379de7..284eb27 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7221,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/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/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..9e2aca0
--- /dev/null
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
@@ -0,0 +1,61 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.scenarios
+
+import java.io._
+
+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 io.gatling.core.structure.ScenarioBuilder
+import org.apache.commons.io.FileUtils
+import org.eclipse.jgit.hooks._
+
+import scala.concurrent.duration._
+
+class CloneUsingBothProtocols extends Simulation {
+
+ implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+ implicit val postMessageHook: Option[String] = Some(s"hooks/${CommitMsgHook.NAME}")
+
+ private val name: String = this.getClass.getSimpleName
+ private val file = s"data/$name.json"
+ private val data: FileBasedFeederBuilder[Any]#F = jsonFile(file).circular
+ private val request = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
+ private val protocol: GitProtocol = GitProtocol()
+
+ private val test: ScenarioBuilder = scenario(name)
+ .feed(data)
+ .exec(request)
+
+ setUp(
+ test.inject(
+ constantUsersPerSec(1) during (2 seconds)
+ )).protocols(protocol)
+
+ 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..5a3bb99 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,55 @@
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 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 io.gatling.core.structure.ScenarioBuilder
import org.apache.commons.io.FileUtils
-
-import scala.concurrent.duration._
import org.eclipse.jgit.hooks._
-class ReplayRecordsFromFeederScenario extends Simulation {
+import scala.concurrent.duration._
- val gitProtocol = GitProtocol()
- implicit val conf = GatlingGitConfiguration()
- implicit val postMessageHook: Option[String] = Some(
- s"hooks/${CommitMsgHook.NAME}")
+class ReplayRecordsFromFeeder extends Simulation {
- val feeder = jsonFile("data/requests.json").circular
+ implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+ implicit val postMessageHook: Option[String] = Some(s"hooks/${CommitMsgHook.NAME}")
- val replayCallsScenario: ScenarioBuilder =
- scenario("Git commands")
+ private val name: String = this.getClass.getSimpleName
+ private val file = s"data/$name.json"
+ private val data: FileBasedFeederBuilder[Any]#F = jsonFile(file).circular
+ private val request = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
+ private val protocol: GitProtocol = GitProtocol()
+
+ 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)
+ )).protocols(protocol)
+ .maxDuration(60 seconds)
after {
+ Thread.sleep(5000)
+ val path = conf.tmpBasePath
try {
- //After is often called too early. Some retries should be implemented.
- Thread.sleep(5000)
- FileUtils.deleteDirectory(new File(conf.tmpBasePath))
+ FileUtils.deleteDirectory(new File(path))
} catch {
- case e: IOException => {
- System.err.println(
- "Unable to delete temporary directory: " + conf.tmpBasePath)
- e.printStackTrace
- }
+ case e: IOException =>
+ System.err.println("Unable to delete temporary directory " + path)
+ e.printStackTrace()
}
}
}
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/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/CherryPickChangeInfo.java b/java/com/google/gerrit/extensions/common/CherryPickChangeInfo.java
deleted file mode 100644
index 8b02e23..0000000
--- a/java/com/google/gerrit/extensions/common/CherryPickChangeInfo.java
+++ /dev/null
@@ -1,24 +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;
-
-/**
- * {@link ChangeInfo} that is returned when a change was cherry-picked.
- *
- * <p>This class used to define additional fields to {@link ChangeInfo}, but those were pulled up
- * into {@link ChangeInfo}. Now this class is no longer needed, but we need to keep it for backwards
- * compatibility of the Java API.
- */
-public class CherryPickChangeInfo extends ChangeInfo {}
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/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/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/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/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/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 0d3aaac..60631d2 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -76,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;
@@ -461,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);
}
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/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 ae5066b..0eefe02 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4637,7 +4637,6 @@
ListChangesOption.ALL_COMMITS,
ListChangesOption.ALL_REVISIONS,
ListChangesOption.CHANGE_ACTIONS,
- ListChangesOption.CURRENT_ACTIONS,
ListChangesOption.DETAILED_LABELS,
ListChangesOption.DOWNLOAD_COMMANDS,
ListChangesOption.MESSAGES,
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/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/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 0e86746..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
@@ -640,6 +643,12 @@
</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]]"
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 71f8d6f..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
@@ -141,7 +141,7 @@
_robotCommentThreads: {
type: Array,
computed: '_computeRobotCommentThreads(_commentThreads,'
- + ' _selectedRevision)',
+ + ' _currentRobotCommentsPatchSet)',
},
/** @type {?} */
_serverConfig: {
@@ -181,6 +181,7 @@
type: Object,
computed: '_computeCurrentRevision(_change.current_revision, ' +
'_change.revisions)',
+ observer: '_handleCurrentRevisionUpdate',
},
_files: Object,
_changeNum: String,
@@ -318,6 +319,14 @@
_selectedFilesTabPluginEndpoint: {
type: String,
},
+ _robotCommentsPatchSetDropdownItems: {
+ type: Array,
+ value() { return []; },
+ computed: '_computeRobotCommentsPatchSetDropdownItems(_change)',
+ },
+ _currentRobotCommentsPatchSet: {
+ type: Number,
+ },
};
}
@@ -476,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) {
@@ -580,12 +593,35 @@
return false;
}
- _computeRobotCommentThreads(commentThreads, selectedRevision) {
- if (!commentThreads || !selectedRevision) return [];
+ _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
- === selectedRevision._number);
+ === currentRobotCommentsPatchSet);
});
}
@@ -795,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;
@@ -835,7 +868,6 @@
}
this._reloadPatchNumDependentResources().then(() => {
this._sendShowChangeEvent();
- this._setPrimaryTab();
});
return;
}
@@ -870,8 +902,6 @@
this._sendShowChangeEvent();
- this._setPrimaryTab();
-
this.async(() => {
if (this.viewState.scrollTop) {
document.documentElement.scrollTop =
@@ -1262,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 fe02876..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
@@ -645,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
@@ -665,8 +662,18 @@
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;
- element._selectedRevision = {_number: 4};
flush(() => {
done();
});
@@ -679,7 +686,7 @@
'rc2');
});
test('changing patchsets resets robot comments', done => {
- element._selectedRevision = {_number: 3};
+ element.set('_change.current_revision', 'rev3');
flush(() => {
assert.equal(element._robotCommentThreads.length, 0);
done();
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/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 33cd4cb..05bff9f 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
@@ -574,14 +574,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 867a067..589d7c2 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-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.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index d7eaabf..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
@@ -304,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);
+ }
});
}
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 dea9789..172a151 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/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-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-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:
