Merge "Bazel: Widen visibility of javax-annotation rule to plugins"
diff --git a/.bazelignore b/.bazelignore
index 30f1613..69c04b1 100644
--- a/.bazelignore
+++ b/.bazelignore
@@ -1 +1,2 @@
eclipse-out
+node_modules
diff --git a/.bazelversion b/.bazelversion
index 9084fa2..26aaba0 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-1.1.0
+1.2.0
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 452ef23..d126c96 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1391,9 +1391,15 @@
[[changeCleanup.abandonIfMergeable]]changeCleanup.abandonIfMergeable::
+
-Whether changes which are mergeable should be auto-abandoned.
+Whether changes which are mergeable should be auto-abandoned. When set
+to `false`, `-is:mergeable` is appended to the query used to find
+the changes to auto-abandon.
+
-By default `true`.
+By default `true`, meaning mergeable changes are auto-abandoned.
++
+If link:#index.change.indexMergeable[`index.change.indexMergeable`]
+is disabled, setting this option to `false` has no effect and it
+behaves as though it were set to `true`.
[[changeCleanup.cleanupAccountPatchReview]]changeCleanup.cleanupAccountPatchReview::
+
@@ -1670,7 +1676,7 @@
when handling very big binary files, such as device firmware or
CD-ROM ISO images.
+
-Defaults to 25% of the available JVM heap, limited to 2048m.
+Defaults to 25% of the available JVM heap, limited to 2g.
+
Common unit suffixes of 'k', 'm', or 'g' are supported.
@@ -2766,6 +2772,22 @@
If not set or set to a zero, defaults to the number of logical CPUs as returned
by the JVM. If set to a negative value, defaults to a direct executor.
+[[index.change.indexMergeable]]index.change.indexMergeable::
++
+Specifies if `mergeable` should be index or not. Indexing this field enables
+queries that contain the mergeability operator (`is:mergeable`). If enabled,
+Gerrit will check if the change is mergeable into the target branch when
+reindexing a change. This is an expensive operation.
++
+If true, Gerrit will reindex all open changes when the target ref advances.
+Depending on the frequency of updates to the ref and the number of open changes,
+this can be very expensive.
++
+When this setting is changed from `false` to `true`, all changes need to be
+reindexed.
++
+Defaults to true.
+
[[index.onlineUpgrade]]index.onlineUpgrade::
+
Whether to upgrade to new index schema versions while the server is
@@ -2813,19 +2835,6 @@
+
Defaults to 1024.
-[[index.reindexAfterRefUpdate]]index.reindexAfterRefUpdate::
-+
-Whether to reindex all affected open changes after a ref is updated. This
-includes reindexing all open changes to recompute the "mergeable" bit every time
-the destination branch moves, as well as reindexing changes to take into account
-new project configuration (e.g. label definitions).
-+
-Leaving this enabled may result in fresher results, but may cause performance
-problems if there are lots of open changes on a project whose branches advance
-frequently.
-+
-Defaults to true.
-
[[index.autoReindexIfStale]]index.autoReindexIfStale::
+
Whether to automatically check if a document became stale in the index
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 63dabc4..925d399 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -10,7 +10,8 @@
* Python 2 or 3
* link:https://github.com/nodesource/distributions/blob/master/README.md[Node.js (including npm)]
* Bower (`sudo npm install -g bower`)
-* link:https://docs.bazel.build/versions/master/install.html[Bazel]
+* link:https://docs.bazel.build/versions/master/install.html[Bazel] directly
+or through link:https://github.com/bazelbuild/bazelisk[Bazelisk]
* Maven
* zip, unzip
* gcc
diff --git a/Documentation/dev-crafting-changes.txt b/Documentation/dev-crafting-changes.txt
index bf4453c..eb9dee4 100644
--- a/Documentation/dev-crafting-changes.txt
+++ b/Documentation/dev-crafting-changes.txt
@@ -230,6 +230,13 @@
* Tests for new code will greatly help your change get approved.
+[[javadoc]]
+== Javadoc
+
+ * Javadocs for new code (especially public classes and
+ public/protected methods) will greatly help your change get
+ approved.
+
[[change-size]]
== Change Size/Number of Files Touched
diff --git a/Documentation/dev-eclipse.txt b/Documentation/dev-eclipse.txt
index a94dd7b..dfe6aa5 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -62,7 +62,7 @@
To format source code, Gerrit uses the
link:https://github.com/google/google-java-format[`google-java-format`]
tool (version 1.7), which automatically formats code to follow the
-style guide. See link:dev-contributing.html#style[Code Style] for the
+style guide. See link:dev-crafting-changes.html#style[Code Style] for the
instruction how to set up command line tool that uses this formatter.
The Eclipse plugin is provided that allows to format with the same
formatter from within the Eclipse IDE. See
diff --git a/Documentation/dev-intellij.txt b/Documentation/dev-intellij.txt
index 5077079..81790db 100644
--- a/Documentation/dev-intellij.txt
+++ b/Documentation/dev-intellij.txt
@@ -104,7 +104,7 @@
*Code -> Reformat Code*, keyboard shortcuts, or the commit dialog will use the
custom style defined by the `google-java-format` plugin.
-Please refer to the documentation on the <<dev-contributing#style,code style>>
+Please refer to the documentation on the <<dev-crafting-changes#style,code style>>
for which version of `google-java-format` is used with Gerrit.
==== Code style settings
@@ -159,7 +159,7 @@
plugin in IntelliJ IDEA.
To simplify the creation of commit messages which are compliant with the
-<<dev-contributing#commit-message,Commit Message>> format, do the following:
+<<dev-crafting-changes#commit-message,Commit Message>> format, do the following:
. Go to *File -> Settings -> Version Control -> Commit Dialog*.
. In the *Commit message inspections*, activate the three inspections:
@@ -171,7 +171,7 @@
right margin*.
In addition, you should follow the instructions of
-<<dev-contributing#git_commit_settings,this section>> (if you haven't
+<<dev-crafting-changes#git-commit-settings,this section>> (if you haven't
done so already):
* Install the Git commit message hook for the `Change-Id` line.
diff --git a/Documentation/js_licenses.txt b/Documentation/js_licenses.txt
index 8861266..c2bdfbb3 100644
--- a/Documentation/js_licenses.txt
+++ b/Documentation/js_licenses.txt
@@ -409,7 +409,7 @@
[[polymer]]
polymer
-* js:font-roboto
+* js:font-roboto-local
* js:iron-a11y-announcer
* js:iron-a11y-keys-behavior
* js:iron-autogrow-textarea
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt
index 394886d..949298f 100644
--- a/Documentation/licenses.txt
+++ b/Documentation/licenses.txt
@@ -2666,7 +2666,7 @@
[[polymer]]
polymer
-* js:font-roboto
+* js:font-roboto-local
* js:iron-a11y-announcer
* js:iron-a11y-keys-behavior
* js:iron-autogrow-textarea
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index 308e045..8725cee 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -192,3 +192,10 @@
of all changes in NoteDb is accurate, and so is only safe once all changes are
NoteDb primary. Otherwise, reading changes only from NoteDb might result in
inaccurate results, and writing to NoteDb would compound the problem. +
+
+== NoteDB to ReviewDB rollback
+
+In case of rollback from NoteDB to ReviewDB, all the meta refs and the
+sequence ref need to be removed.
+The [remove-notedb-refs.sh](https://gerrit.googlesource.com/gerrit/+/refs/heads/master/contrib/remove-notedb-refs.sh)
+script has been written to automate this process.
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 6ef4e20..85cdace 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -733,8 +733,8 @@
[
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -767,8 +767,8 @@
)]}'
{
"seq": 1,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
@@ -791,9 +791,9 @@
.Request
----
POST /accounts/self/sshkeys HTTP/1.0
- Content-Type: plain/text
+ Content-Type: text/plain
- AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d
+ ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com
----
As response an link:#ssh-key-info[SshKeyInfo] entity is returned that
@@ -808,8 +808,8 @@
)]}'
{
"seq": 2,
- "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d john.doe@example.com",
- "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw\u003d\u003d",
+ "ssh_public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw== john.doe@example.com",
+ "encoded_key": "AAAAB3NzaC1yc2EAAAABIwAAAQEA0T...YImydZAw==",
"algorithm": "ssh-rsa",
"comment": "john.doe@example.com",
"valid": true
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 69a7641..0fc733a 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -315,6 +315,11 @@
the `mergeable` field will always be omitted and `SKIP_MERGEABLE` has no
effect.
+
+When link:config-gerrit.html#index.change.indexMergeable[
+`index.change.indexMergeable`] is set to `false` in the `gerrit.config`,
+the `mergeable` field will always be omitted when querying changes and
+`SKIP_MERGEABLE` has no effect.
++
A change's mergeability can be requested separately by calling the
link:#get-mergeable[get-mergeable] endpoint.
--
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 021a1bb..affe1ea 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1572,6 +1572,21 @@
link:rest-api-changes.html#change-info[ChangeInfo] will never be set.
|=============================
+[[change-index-config-info]]
+=== ChangeIndexConfigInfo
+The `ChangeIndexConfigInfo` entity contains information about Gerrit
+configuration from the link:config-gerrit.html#index.change[index.change]
+section.
+
+[options="header",cols="1,^1,5"]
+|=============================
+|Field Name ||Description
+|`index_mergeable` |not set if `false`|
+Value of the link:config-gerrit.html#index.change.indexMergeable[
+configuration parameter] that controls whether the mergeability bit is
+indexed (hence queryable using `is:mergeable`).
+|=============================
+
[[check-account-external-ids-input]]
=== CheckAccountExternalIdsInput
The `CheckAccountExternalIdsInput` entity contains input for the
@@ -1821,6 +1836,21 @@
link:config-gerrit.html#gerrit.reportBugUrl[URL to report bugs].
|=================================
+[[index-config-info]]
+=== IndexConfigInfo
+The `IndexConfigInfo` entity contains information about Gerrit
+configuration from the link:config-gerrit.html#index[index]
+section.
+
+[options="header",cols="1,^1,5"]
+|=============================
+|Field Name ||Description
+|`change` ||
+Information about the configuration from the
+link:config-gerrit.html#index.change[index.change] section as
+link:#index.change[ChangeIndexConfigInfo] entity.
+|=============================
+
[[hit-ration-info]]
=== HitRatioInfo
The `HitRatioInfo` entity contains information about the hit ratio of a
@@ -1947,6 +1977,10 @@
Information about the configuration from the
link:config-gerrit.html#gerrit[gerrit] section as link:#gerrit-info[
GerritInfo] entity.
+|`index` ||
+Information about the configuration from the
+link:config-gerrit.html#index[index] section as link:#index[
+IndexConfigInfo] entity.
|`note_db_enabled` |not set if `false`|
Whether the NoteDb storage backend is fully enabled.
|`plugin` ||
diff --git a/Jenkinsfile b/Jenkinsfile
index 257b53c..f21c7897 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -154,18 +154,19 @@
}
def prepareBuildsForMode(buildName, mode="notedb", retryTimes = 1) {
- def propagate = retryTimes == 1 ? false : true
return {
stage("${buildName}/${mode}") {
- catchError{
- retry(retryTimes){
- def slaveBuild = build job: "${buildName}", parameters: [
+ def slaveBuild = null
+ for (int i = 1; i <= retryTimes; i++) {
+ try {
+ slaveBuild = build job: "${buildName}", parameters: [
string(name: 'REFSPEC', value: Change.ref),
string(name: 'BRANCH', value: Change.sha1),
string(name: 'CHANGE_URL', value: Change.url),
string(name: 'MODE', value: mode),
string(name: 'TARGET_BRANCH', value: Change.branch)
- ], propagate: propagate
+ ], propagate: false
+ } finally {
if (buildName == "Gerrit-codestyle"){
Builds.codeStyle = new Build(
slaveBuild.getAbsoluteUrl(), slaveBuild.getResult())
@@ -173,6 +174,9 @@
Builds.verification[mode] = new Build(
slaveBuild.getAbsoluteUrl(), slaveBuild.getResult())
}
+ if (slaveBuild.getResult() == "SUCCESS") {
+ break
+ }
}
}
}
@@ -181,9 +185,17 @@
def collectBuilds() {
def builds = [:]
- builds["Gerrit-codestyle"] = prepareBuildsForMode("Gerrit-codestyle")
- Builds.modes.each {
- builds["Gerrit-verification(${it})"] = prepareBuildsForMode("Gerrit-verifier-bazel", it)
+ if (env.GERRIT_CHANGE_NUMBER == "") {
+ builds["java8"] = { -> build "Gerrit-bazel-${env.BRANCH_NAME}" }
+
+ if (env.BRANCH_NAME == "master") {
+ builds["java11"] = { -> build "Gerrit-bazel-java11-${env.BRANCH_NAME}" }
+ }
+ } else {
+ builds["Gerrit-codestyle"] = prepareBuildsForMode("Gerrit-codestyle")
+ Builds.modes.each {
+ builds["Gerrit-verification(${it})"] = prepareBuildsForMode("Gerrit-verifier-bazel", it)
+ }
}
return builds
}
@@ -270,44 +282,48 @@
node ('master') {
- stage('Preparing'){
- gerritReview labels: ['Verified': 0, 'Code-Style': 0]
+ if (env.GERRIT_CHANGE_NUMBER != "") {
+ stage('Preparing'){
+ gerritReview labels: ['Verified': 0, 'Code-Style': 0]
- getChangeMetaData()
- collectBuildModes()
+ getChangeMetaData()
+ collectBuildModes()
+ }
}
parallel(collectBuilds())
- stage('Retry Flaky Builds'){
- def flakyBuildsModes = findFlakyBuilds()
- if (flakyBuildsModes.size() > 0){
- parallel flakyBuildsModes.collectEntries {
- ["Gerrit-verification(${it})" :
- prepareBuildsForMode("Gerrit-verifier-bazel", it, 3)]
+ if (env.GERRIT_CHANGE_NUMBER != "") {
+ stage('Retry Flaky Builds'){
+ def flakyBuildsModes = findFlakyBuilds()
+ if (flakyBuildsModes.size() > 0){
+ parallel flakyBuildsModes.collectEntries {
+ ["Gerrit-verification(${it})" :
+ prepareBuildsForMode("Gerrit-verifier-bazel", it, 3)]
+ }
}
}
- }
- stage('Report to Gerrit'){
- resCodeStyle = getLabelValue(1, Builds.codeStyle.result)
- gerritReview(
- labels: ['Code-Style': resCodeStyle],
- message: createCodeStyleMsgBody(Builds.codeStyle, resCodeStyle))
- postCheck(new GerritCheck("codestyle", Change.number, Change.sha1, Builds.codeStyle))
+ stage('Report to Gerrit'){
+ resCodeStyle = getLabelValue(1, Builds.codeStyle.result)
+ gerritReview(
+ labels: ['Code-Style': resCodeStyle],
+ message: createCodeStyleMsgBody(Builds.codeStyle, resCodeStyle))
+ postCheck(new GerritCheck("codestyle", Change.number, Change.sha1, Builds.codeStyle))
- def verificationResults = Builds.verification.collect { k, v -> v }
- def resVerify = verificationResults.inject(1) {
- acc, build -> getLabelValue(acc, build.result)
+ def verificationResults = Builds.verification.collect { k, v -> v }
+ def resVerify = verificationResults.inject(1) {
+ acc, build -> getLabelValue(acc, build.result)
+ }
+ gerritReview(
+ labels: ['Verified': resVerify],
+ message: createVerifyMsgBody(Builds.verification))
+
+ Builds.verification.each { type, build -> postCheck(
+ new GerritCheck(type, Change.number, Change.sha1, build)
+ )}
+
+ setResult(resVerify, resCodeStyle)
}
- gerritReview(
- labels: ['Verified': resVerify],
- message: createVerifyMsgBody(Builds.verification))
-
- Builds.verification.each { type, build -> postCheck(
- new GerritCheck(type, Change.number, Change.sha1, build)
- )}
-
- setResult(resVerify, resCodeStyle)
}
}
diff --git a/WORKSPACE b/WORKSPACE
index 7529d8a..e370f69 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -748,7 +748,7 @@
sha1 = "f7be08ec23c21485b9b5a1cf1654c2ec8c58168d",
)
-GITILES_VERS = "0.3-6"
+GITILES_VERS = "0.3-7"
GITILES_REPO = GERRIT
@@ -757,14 +757,14 @@
artifact = "com.google.gitiles:blame-cache:" + GITILES_VERS,
attach_source = False,
repository = GITILES_REPO,
- sha1 = "bd1ec86570b8a6e4b68c5af6311c8cd10aa3f295",
+ sha1 = "af6212a62363906c63d367f8276ae1645f83bf93",
)
maven_jar(
name = "gitiles-servlet",
artifact = "com.google.gitiles:gitiles-servlet:" + GITILES_VERS,
repository = GITILES_REPO,
- sha1 = "98bf06ca9abc871beb3d6c01e6f053243d4e911a",
+ sha1 = "6a53f722f8572a2f1bcb7d86e5692168844bab76",
)
# prettify must match the version used in Gitiles
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index b68130b..cb3524a 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -122,7 +122,6 @@
"//java/com/google/gerrit/lucene",
"//java/com/google/gerrit/mail",
"//java/com/google/gerrit/metrics",
- "//java/com/google/gerrit/pgm/init",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/git/receive",
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 6e736a0..f90df67 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -269,7 +269,7 @@
private static final ImmutableMap<String, Level> LOG_LEVELS =
ImmutableMap.<String, Level>builder()
- .put("com.google.gerrit", Level.INFO)
+ .put("com.google.gerrit", getGerritLogLevel())
// Silence non-critical messages from MINA SSHD.
.put("org.apache.mina", Level.WARN)
@@ -307,6 +307,14 @@
.put("org.eclipse.jgit.util.FS", Level.WARN)
.build();
+ private static Level getGerritLogLevel() {
+ String value = Strings.nullToEmpty(System.getenv("GERRIT_LOG_LEVEL"));
+ if (value.isEmpty()) {
+ value = Strings.nullToEmpty(System.getProperty("gerrit.logLevel"));
+ }
+ return Level.toLevel(value, Level.INFO);
+ }
+
private static boolean forceLocalDisk() {
String value = Strings.nullToEmpty(System.getenv("GERRIT_FORCE_LOCAL_DISK"));
if (value.isEmpty()) {
@@ -563,8 +571,8 @@
cfg.setInt("sshd", null, "commandStartThreads", 1);
cfg.setInt("receive", null, "threadPoolSize", 1);
cfg.setInt("index", null, "threads", 1);
- if (cfg.getString("index", null, "reindexAfterRefUpdate") == null) {
- cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
+ if (cfg.getString("index", "change", "indexMergeable") == null) {
+ cfg.setBoolean("index", "change", "indexMergeable", false);
}
}
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index 864839a..5f412ce 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -26,6 +26,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.common.io.CharStreams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
@@ -68,7 +69,6 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
-import org.apache.commons.codec.binary.Base64;
import org.apache.http.HttpEntity;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
@@ -88,7 +88,7 @@
protected static final String SETTINGS = "settings";
protected static byte[] decodeBase64(String base64String) {
- return Base64.decodeBase64(base64String);
+ return BaseEncoding.base64().decode(base64String);
}
protected static <T> List<T> decodeProtos(
@@ -268,7 +268,7 @@
} else if (type == FieldType.TIMESTAMP) {
rawFields.put(element.getKey(), new Timestamp(inner.getAsLong()));
} else if (type == FieldType.STORED_ONLY) {
- rawFields.put(element.getKey(), Base64.decodeBase64(inner.getAsString()));
+ rawFields.put(element.getKey(), decodeBase64(inner.getAsString()));
} else {
throw FieldType.badFieldType(type);
}
diff --git a/java/com/google/gerrit/elasticsearch/BUILD b/java/com/google/gerrit/elasticsearch/BUILD
index edbd82c..8bab80b 100644
--- a/java/com/google/gerrit/elasticsearch/BUILD
+++ b/java/com/google/gerrit/elasticsearch/BUILD
@@ -19,7 +19,6 @@
"//lib:guava",
"//lib:jgit",
"//lib:protobuf",
- "//lib/commons:codec",
"//lib/commons:lang",
"//lib/elasticsearch-rest-client",
"//lib/flogger:api",
diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
index a06f90f..c3e3264 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
@@ -14,6 +14,7 @@
package com.google.gerrit.elasticsearch;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
import com.google.gerrit.elasticsearch.bulk.BulkRequest;
import com.google.gerrit.elasticsearch.bulk.IndexRequest;
@@ -74,7 +75,7 @@
public void replace(AccountState as) {
BulkRequest bulk =
new IndexRequest(getId(as), indexName, type, client.adapter())
- .add(new UpdateRequest<>(schema, as));
+ .add(new UpdateRequest<>(schema, as, ImmutableSet.of()));
String uri = getURI(type, BULK);
Response response = postRequest(uri, bulk, getRefreshParam());
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 37184cc..084c2ec 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -22,6 +22,7 @@
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -48,6 +49,7 @@
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.IndexUtils;
import com.google.gerrit.server.index.change.ChangeField;
@@ -65,6 +67,7 @@
import java.util.Optional;
import java.util.Set;
import org.apache.http.HttpStatus;
+import org.eclipse.jgit.lib.Config;
import org.elasticsearch.client.Response;
/** Secondary index implementation using Elasticsearch. */
@@ -91,6 +94,7 @@
private final ChangeData.Factory changeDataFactory;
private final Schema<ChangeData> schema;
private final FieldDef<ChangeData, ?> idField;
+ private final ImmutableSet<String> skipFields;
@Inject
ElasticChangeIndex(
@@ -98,6 +102,7 @@
ChangeData.Factory changeDataFactory,
SitePaths sitePaths,
ElasticRestClientProvider clientBuilder,
+ @GerritServerConfig Config gerritConfig,
@Assisted Schema<ChangeData> schema) {
super(cfg, sitePaths, schema, clientBuilder, CHANGES);
this.changeDataFactory = changeDataFactory;
@@ -105,6 +110,10 @@
this.mapping = new ChangeMapping(schema, client.adapter());
this.idField =
this.schema.useLegacyNumericFields() ? ChangeField.LEGACY_ID : ChangeField.LEGACY_ID_STR;
+ this.skipFields =
+ gerritConfig.getBoolean("index", "change", "indexMergeable", true)
+ ? ImmutableSet.of()
+ : ImmutableSet.of(ChangeField.MERGEABLE.getName());
}
@Override
@@ -123,7 +132,7 @@
ElasticQueryAdapter adapter = client.adapter();
BulkRequest bulk =
new IndexRequest(getId(cd), indexName, adapter.getType(insertIndex), adapter)
- .add(new UpdateRequest<>(schema, cd));
+ .add(new UpdateRequest<>(schema, cd, skipFields));
if (adapter.deleteToReplace()) {
bulk.add(new DeleteRequest(cd.getId().toString(), indexName, deleteIndex, adapter));
}
@@ -263,7 +272,7 @@
// Mergeable.
JsonElement mergeableElement = source.get(ChangeField.MERGEABLE.getName());
- if (mergeableElement != null) {
+ if (mergeableElement != null && !skipFields.contains(ChangeField.MERGEABLE.getName())) {
String mergeable = mergeableElement.getAsString();
if ("1".equals(mergeable)) {
cd.setMergeable(true);
diff --git a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
index c215132..ce2025f 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
@@ -14,6 +14,7 @@
package com.google.gerrit.elasticsearch;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
import com.google.gerrit.elasticsearch.bulk.BulkRequest;
import com.google.gerrit.elasticsearch.bulk.IndexRequest;
@@ -74,7 +75,7 @@
public void replace(InternalGroup group) {
BulkRequest bulk =
new IndexRequest(getId(group), indexName, type, client.adapter())
- .add(new UpdateRequest<>(schema, group));
+ .add(new UpdateRequest<>(schema, group, ImmutableSet.of()));
String uri = getURI(type, BULK);
Response response = postRequest(uri, bulk, getRefreshParam());
diff --git a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
index 29f8507..b636706 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java
@@ -14,6 +14,7 @@
package com.google.gerrit.elasticsearch;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
import com.google.gerrit.elasticsearch.bulk.BulkRequest;
import com.google.gerrit.elasticsearch.bulk.IndexRequest;
@@ -74,7 +75,7 @@
public void replace(ProjectData projectState) {
BulkRequest bulk =
new IndexRequest(projectState.getProject().getName(), indexName, type, client.adapter())
- .add(new UpdateRequest<>(schema, projectState));
+ .add(new UpdateRequest<>(schema, projectState, ImmutableSet.of()));
String uri = getURI(type, BULK);
Response response = postRequest(uri, bulk, getRefreshParam());
diff --git a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
index 2f0bd01..196b8d6 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
@@ -16,6 +16,7 @@
import static java.util.stream.Collectors.toList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.elasticsearch.builders.XContentBuilder;
@@ -27,17 +28,19 @@
private final Schema<V> schema;
private final V v;
+ private final ImmutableSet<String> skipFields;
- public UpdateRequest(Schema<V> schema, V v) {
+ public UpdateRequest(Schema<V> schema, V v, ImmutableSet<String> skipFields) {
this.schema = schema;
this.v = v;
+ this.skipFields = skipFields;
}
@Override
protected String getRequest() {
try (XContentBuilder closeable = new XContentBuilder()) {
XContentBuilder builder = closeable.startObject();
- for (Values<V> values : schema.buildFields(v)) {
+ for (Values<V> values : schema.buildFields(v, skipFields)) {
String name = values.getField().getName();
if (values.getField().isRepeatable()) {
builder.field(name, Streams.stream(values.getValues()).collect(toList()));
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index 739bd38..04e97dc 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -26,6 +26,7 @@
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;
@@ -116,13 +117,30 @@
@AutoValue
public abstract static class Id {
- /** Parse a Change.Id out of a string representation. */
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @deprecated use {@link #tryParse(String)} instead.
+ */
+ @Deprecated
public static Id parse(String str) {
Integer id = Ints.tryParse(str);
checkArgument(id != null, "invalid change ID: %s", str);
return Change.id(id);
}
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @param str the string to parse
+ * @return Optional containing the Change.Id, or {@code Optional.empty()} if str does not
+ * represent a valid Change.Id.
+ */
+ public static Optional<Id> tryParse(String str) {
+ Integer id = Ints.tryParse(str);
+ return id != null ? Optional.of(Change.id(id)) : Optional.empty();
+ }
+
public static Id fromRef(String ref) {
if (RefNames.isRefsEdit(ref)) {
return fromEditRefPart(ref);
diff --git a/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java b/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java
new file mode 100644
index 0000000..7bca79e
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/ChangeIndexConfigInfo.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class ChangeIndexConfigInfo {
+ public Boolean indexMergeable;
+}
diff --git a/java/com/google/gerrit/extensions/common/IndexConfigInfo.java b/java/com/google/gerrit/extensions/common/IndexConfigInfo.java
new file mode 100644
index 0000000..084c53a
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/IndexConfigInfo.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class IndexConfigInfo {
+ public ChangeIndexConfigInfo change;
+}
diff --git a/java/com/google/gerrit/extensions/common/ServerInfo.java b/java/com/google/gerrit/extensions/common/ServerInfo.java
index 82d5bc8..9cf1ec1 100644
--- a/java/com/google/gerrit/extensions/common/ServerInfo.java
+++ b/java/com/google/gerrit/extensions/common/ServerInfo.java
@@ -20,6 +20,7 @@
public ChangeConfigInfo change;
public DownloadInfo download;
public GerritInfo gerrit;
+ public IndexConfigInfo index;
public Boolean noteDbEnabled;
public PluginConfigInfo plugin;
public SshdInfo sshd;
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD
index bcb2a2a..d6a43b91 100644
--- a/java/com/google/gerrit/httpd/BUILD
+++ b/java/com/google/gerrit/httpd/BUILD
@@ -38,7 +38,6 @@
"//lib:soy",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
- "//lib/commons:codec",
"//lib/commons:lang",
"//lib/flogger:api",
"//lib/guice",
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
index 89ad878..77c5381 100644
--- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -24,6 +24,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -46,17 +47,15 @@
if (idString.endsWith("/")) {
idString = idString.substring(0, idString.length() - 1);
}
- Change.Id id;
- try {
- id = Change.Id.parse(idString);
- } catch (IllegalArgumentException e) {
+ Optional<Change.Id> id = Change.Id.tryParse(idString);
+ if (!id.isPresent()) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
ChangeResource changeResource;
try {
- changeResource = changesCollection.parse(id);
+ changeResource = changesCollection.parse(id.get());
} catch (ResourceConflictException | ResourceNotFoundException e) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index 88a3f0a..d43fcc7 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -20,6 +20,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -48,7 +49,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
-import org.apache.commons.codec.binary.Base64;
/**
* Authenticates the current user by HTTP basic authentication.
@@ -110,7 +110,7 @@
return true;
}
- final byte[] decoded = Base64.decodeBase64(hdr.substring(LIT_BASIC.length()));
+ final byte[] decoded = BaseEncoding.base64().decode(hdr.substring(LIT_BASIC.length()));
String usernamePassword = new String(decoded, encoding(req));
int splitPos = usernamePassword.indexOf(':');
if (splitPos < 1) {
diff --git a/java/com/google/gerrit/httpd/ProjectOAuthFilter.java b/java/com/google/gerrit/httpd/ProjectOAuthFilter.java
index 4b5742d..693232f 100644
--- a/java/com/google/gerrit/httpd/ProjectOAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectOAuthFilter.java
@@ -22,6 +22,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.auth.oauth.OAuthLoginProvider;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -53,7 +54,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
-import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.lib.Config;
/**
@@ -225,7 +225,7 @@
private AuthInfo extractAuthInfo(String hdr, String encoding)
throws UnsupportedEncodingException {
- byte[] decoded = Base64.decodeBase64(hdr.substring(BASIC.length()));
+ byte[] decoded = BaseEncoding.base64().decode(hdr.substring(BASIC.length()));
String usernamePassword = new String(decoded, encoding);
int splitPos = usernamePassword.indexOf(':');
if (splitPos < 1 || splitPos == usernamePassword.length() - 1) {
diff --git a/java/com/google/gerrit/httpd/RemoteUserUtil.java b/java/com/google/gerrit/httpd/RemoteUserUtil.java
index a02b5a0..84954dc 100644
--- a/java/com/google/gerrit/httpd/RemoteUserUtil.java
+++ b/java/com/google/gerrit/httpd/RemoteUserUtil.java
@@ -18,8 +18,8 @@
import static com.google.common.net.HttpHeaders.AUTHORIZATION;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.io.BaseEncoding;
import javax.servlet.http.HttpServletRequest;
-import org.eclipse.jgit.util.Base64;
public class RemoteUserUtil {
/**
@@ -70,7 +70,7 @@
} else if (auth.startsWith("Basic ")) {
auth = auth.substring("Basic ".length());
- auth = new String(Base64.decode(auth), UTF_8);
+ auth = new String(BaseEncoding.base64().decode(auth), UTF_8);
final int c = auth.indexOf(':');
return c > 0 ? auth.substring(0, c) : null;
diff --git a/java/com/google/gerrit/httpd/auth/oauth/BUILD b/java/com/google/gerrit/httpd/auth/oauth/BUILD
index dd4549e..11c9295 100644
--- a/java/com/google/gerrit/httpd/auth/oauth/BUILD
+++ b/java/com/google/gerrit/httpd/auth/oauth/BUILD
@@ -16,7 +16,6 @@
"//lib:guava",
"//lib:jgit",
"//lib:servlet-api",
- "//lib/commons:codec",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-servlet",
diff --git a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
index 84dee6e..c7b65d0 100644
--- a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
+++ b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
@@ -19,6 +19,7 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.auth.oauth.OAuthServiceProvider;
import com.google.gerrit.extensions.auth.oauth.OAuthToken;
@@ -45,7 +46,6 @@
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.errors.ConfigInvalidException;
@SessionScoped
@@ -244,7 +244,7 @@
private static String generateRandomState() {
byte[] state = new byte[32];
randomState.nextBytes(state);
- return Base64.encodeBase64URLSafeString(state);
+ return BaseEncoding.base64Url().encode(state);
}
@Override
diff --git a/java/com/google/gerrit/httpd/auth/openid/BUILD b/java/com/google/gerrit/httpd/auth/openid/BUILD
index 94f436b..29841aa 100644
--- a/java/com/google/gerrit/httpd/auth/openid/BUILD
+++ b/java/com/google/gerrit/httpd/auth/openid/BUILD
@@ -17,7 +17,6 @@
"//java/com/google/gerrit/server",
"//lib:guava",
"//lib:servlet-api",
- "//lib/commons:codec",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-servlet",
diff --git a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
index bbdb0c4..f9e6286 100644
--- a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
+++ b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.auth.oauth.OAuthServiceProvider;
import com.google.gerrit.extensions.auth.oauth.OAuthToken;
@@ -43,7 +44,6 @@
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.errors.ConfigInvalidException;
/** OAuth protocol implementation */
@@ -229,7 +229,7 @@
private static String generateRandomState() {
byte[] state = new byte[32];
randomState.nextBytes(state);
- return Base64.encodeBase64URLSafeString(state);
+ return BaseEncoding.base64Url().encode(state);
}
@Override
diff --git a/java/com/google/gerrit/httpd/init/BUILD b/java/com/google/gerrit/httpd/init/BUILD
index 149dee8..222041a 100644
--- a/java/com/google/gerrit/httpd/init/BUILD
+++ b/java/com/google/gerrit/httpd/init/BUILD
@@ -6,7 +6,6 @@
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/elasticsearch",
- "//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/gpg",
"//java/com/google/gerrit/httpd",
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java b/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
index a378fa4..aa1b921 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java
@@ -19,7 +19,7 @@
import com.google.gerrit.extensions.registration.PluginName;
import com.google.gerrit.httpd.restapi.RestApiServlet.ViewData;
import com.google.gerrit.metrics.Counter1;
-import com.google.gerrit.metrics.Counter2;
+import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.Field;
@@ -37,7 +37,7 @@
};
final Counter1<String> count;
- final Counter2<String, Integer> errorCount;
+ final Counter3<String, Integer, String> errorCount;
final Timer1<String> serverLatency;
final Histogram1<String> responseBytes;
@@ -60,6 +60,9 @@
viewField,
Field.ofInteger("error_code", Metadata.Builder::httpStatus)
.description("HTTP status code")
+ .build(),
+ Field.ofString("cause", Metadata.Builder::cause)
+ .description("The cause of the error.")
.build());
serverLatency =
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 7700740..0535397 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.flogger.LazyArgs.lazy;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
@@ -103,6 +104,7 @@
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.OptionUtil;
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
@@ -173,7 +175,7 @@
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
-import java.util.function.Predicate;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.zip.GZIPOutputStream;
@@ -246,6 +248,7 @@
final DynamicSet<PerformanceLogger> performanceLoggers;
final ChangeFinder changeFinder;
final RetryHelper retryHelper;
+ final PluginSetContext<ExceptionHook> exceptionHooks;
@Inject
Globals(
@@ -260,7 +263,8 @@
@GerritServerConfig Config config,
DynamicSet<PerformanceLogger> performanceLoggers,
ChangeFinder changeFinder,
- RetryHelper retryHelper) {
+ RetryHelper retryHelper,
+ PluginSetContext<ExceptionHook> exceptionHooks) {
this.currentUser = currentUser;
this.webSession = webSession;
this.paramParser = paramParser;
@@ -273,6 +277,7 @@
this.performanceLoggers = performanceLoggers;
this.changeFinder = changeFinder;
this.retryHelper = retryHelper;
+ this.exceptionHooks = exceptionHooks;
allowOrigin = makeAllowOrigin(config);
}
@@ -287,7 +292,6 @@
private final Globals globals;
private final Provider<RestCollection<RestResource, RestResource>> members;
- private Optional<String> traceId = Optional.empty();
public RestApiServlet(
Globals globals, RestCollection<? extends RestResource, ? extends RestResource> members) {
@@ -313,6 +317,7 @@
res.setHeader("X-Content-Type-Options", "nosniff");
int status = SC_OK;
long responseBytes = -1;
+ Optional<Exception> cause = Optional.empty();
Response<?> response = null;
QueryParams qp = null;
Object inputRequestBody = null;
@@ -594,22 +599,28 @@
}
}
} catch (MalformedJsonException | JsonParseException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req, res, status = SC_BAD_REQUEST, "Invalid " + JSON_TYPE + " in request", e);
} catch (BadRequestException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req, res, status = SC_BAD_REQUEST, messageOr(e, "Bad Request"), e.caching(), e);
} catch (AuthException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(req, res, status = SC_FORBIDDEN, messageOr(e, "Forbidden"), e.caching(), e);
} catch (AmbiguousViewException e) {
+ cause = Optional.of(e);
responseBytes = replyError(req, res, status = SC_NOT_FOUND, messageOr(e, "Ambiguous"), e);
} catch (ResourceNotFoundException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(req, res, status = SC_NOT_FOUND, messageOr(e, "Not Found"), e.caching(), e);
} catch (MethodNotAllowedException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req,
@@ -619,9 +630,11 @@
e.caching(),
e);
} catch (ResourceConflictException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(req, res, status = SC_CONFLICT, messageOr(e, "Conflict"), e.caching(), e);
} catch (PreconditionFailedException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req,
@@ -631,6 +644,7 @@
e.caching(),
e);
} catch (UnprocessableEntityException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req,
@@ -640,19 +654,22 @@
e.caching(),
e);
} catch (NotImplementedException e) {
+ cause = Optional.of(e);
logger.atSevere().withCause(e).log("Error in %s %s", req.getMethod(), uriForLogging(req));
responseBytes =
replyError(req, res, status = SC_NOT_IMPLEMENTED, messageOr(e, "Not Implemented"), e);
} catch (UpdateException e) {
+ cause = Optional.of(e);
Throwable t = e.getCause();
if (t instanceof LockFailureException) {
logger.atSevere().withCause(t).log("Error in %s %s", req.getMethod(), uriForLogging(req));
responseBytes = replyError(req, res, status = SC_SERVICE_UNAVAILABLE, "Lock failure", e);
} else {
status = SC_INTERNAL_SERVER_ERROR;
- responseBytes = handleException(e, req, res);
+ responseBytes = handleException(traceContext, e, req, res);
}
} catch (QuotaException e) {
+ cause = Optional.of(e);
responseBytes =
replyError(
req,
@@ -662,13 +679,15 @@
e.caching(),
e);
} catch (Exception e) {
+ cause = Optional.of(e);
status = SC_INTERNAL_SERVER_ERROR;
- responseBytes = handleException(e, req, res);
+ responseBytes = handleException(traceContext, e, req, res);
} finally {
String metric = getViewName(viewData);
+ String formattedCause = cause.map(globals.retryHelper::formatCause).orElse("_none");
globals.metrics.count.increment(metric);
if (status >= SC_BAD_REQUEST) {
- globals.metrics.errorCount.increment(metric, status);
+ globals.metrics.errorCount.increment(metric, status, formattedCause);
}
if (responseBytes != -1) {
globals.metrics.responseBytes.record(metric, responseBytes);
@@ -704,8 +723,7 @@
traceContext,
globals.metrics.view(restCollection.getClass(), pluginName) + "#parse",
ActionType.REST_READ_REQUEST,
- () -> restCollection.parse(parentResource, id),
- noRetry());
+ () -> restCollection.parse(parentResource, id));
}
private Response<?> invokeRestReadViewWithRetry(
@@ -720,8 +738,7 @@
traceContext,
getViewName(viewData),
ActionType.REST_READ_REQUEST,
- () -> view.apply(rsrc),
- noRetry());
+ () -> view.apply(rsrc));
}
private Response<?> invokeRestModifyViewWithRetry(
@@ -737,8 +754,7 @@
traceContext,
getViewName(viewData),
ActionType.REST_WRITE_REQUEST,
- () -> view.apply(rsrc, inputRequestBody),
- retryOnLockFailure());
+ () -> view.apply(rsrc, inputRequestBody));
}
private Response<?> invokeRestCollectionCreateViewWithRetry(
@@ -755,8 +771,7 @@
traceContext,
getViewName(viewData),
ActionType.REST_WRITE_REQUEST,
- () -> view.apply(rsrc, path, inputRequestBody),
- retryOnLockFailure());
+ () -> view.apply(rsrc, path, inputRequestBody));
}
private Response<?> invokeRestCollectionDeleteMissingViewWithRetry(
@@ -773,8 +788,7 @@
traceContext,
getViewName(viewData),
ActionType.REST_WRITE_REQUEST,
- () -> view.apply(rsrc, path, inputRequestBody),
- retryOnLockFailure());
+ () -> view.apply(rsrc, path, inputRequestBody));
}
private Response<?> invokeRestCollectionModifyViewWithRetry(
@@ -790,8 +804,7 @@
traceContext,
getViewName(viewData),
ActionType.REST_WRITE_REQUEST,
- () -> view.apply(rsrc, inputRequestBody),
- retryOnLockFailure());
+ () -> view.apply(rsrc, inputRequestBody));
}
private <T> T invokeRestEndpointWithRetry(
@@ -799,9 +812,9 @@
TraceContext traceContext,
String caller,
ActionType actionType,
- Action<T> action,
- Predicate<Throwable> retryExceptionPredicate)
+ Action<T> action)
throws Exception {
+ AtomicReference<Optional<String>> traceId = new AtomicReference<>(Optional.empty());
RetryHelper.Options.Builder retryOptionsBuilder = RetryHelper.options().caller(caller);
if (!traceContext.isTracing()) {
// enable automatic retry with tracing in case of non-recoverable failure
@@ -810,36 +823,28 @@
.retryWithTrace(t -> !(t instanceof RestApiException))
.onAutoTrace(
autoTraceId -> {
- traceId = Optional.of(autoTraceId);
+ traceId.set(Optional.of(autoTraceId));
// Include details of the request into the trace.
traceRequestData(req);
});
}
try {
+ // ExceptionHookImpl controls on which exceptions we retry.
+ // The passed in exceptionPredicate allows to define additional exceptions on which retry
+ // should happen, but here we have none (hence pass in "t -> false" as exceptionPredicate).
return globals.retryHelper.execute(
- actionType, action, retryOptionsBuilder.build(), retryExceptionPredicate);
+ actionType, action, retryOptionsBuilder.build(), t -> false);
} finally {
// If auto-tracing got triggered due to a non-recoverable failure, also trace the rest of
// this request. This means logging is forced for all further log statements and the logs are
// associated with the same trace ID.
- traceId.ifPresent(tid -> traceContext.addTag(RequestId.Type.TRACE_ID, tid).forceLogging());
+ traceId
+ .get()
+ .ifPresent(tid -> traceContext.addTag(RequestId.Type.TRACE_ID, tid).forceLogging());
}
}
- private static Predicate<Throwable> noRetry() {
- return t -> false;
- }
-
- private static Predicate<Throwable> retryOnLockFailure() {
- return t -> {
- if (t instanceof UpdateException) {
- t = t.getCause();
- }
- return t instanceof LockFailureException;
- };
- }
-
private String getViewName(ViewData viewData) {
return viewData != null && viewData.view != null ? globals.metrics.view(viewData) : "_unknown";
}
@@ -1658,13 +1663,25 @@
}
}
- private long handleException(Throwable err, HttpServletRequest req, HttpServletResponse res)
+ private long handleException(
+ TraceContext traceContext, Throwable err, HttpServletRequest req, HttpServletResponse res)
throws IOException {
logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uriForLogging(req));
if (!res.isCommitted()) {
res.reset();
- traceId.ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- return replyError(req, res, SC_INTERNAL_SERVER_ERROR, "Internal server error", err);
+ traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+ StringBuilder msg = new StringBuilder("Internal server error");
+ ImmutableList<String> userMessages =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getUserMessage(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(toImmutableList());
+ if (!userMessages.isEmpty()) {
+ msg.append("\n");
+ userMessages.forEach(m -> msg.append("\n* ").append(m));
+ }
+ return replyError(req, res, SC_INTERNAL_SERVER_ERROR, msg.toString(), err);
}
return 0;
}
diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java
index f9f8c48..0aa374b 100644
--- a/java/com/google/gerrit/index/Schema.java
+++ b/java/com/google/gerrit/index/Schema.java
@@ -15,11 +15,12 @@
package com.google.gerrit.index;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.base.MoreObjects;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import java.util.ArrayList;
import java.util.Arrays;
@@ -181,12 +182,17 @@
* <p>Null values are omitted, as are fields which cause errors, which are logged.
*
* @param obj input object.
+ * @param skipFields set of field names to skip when indexing the document
* @return all non-null field values from the object.
*/
- public final Iterable<Values<T>> buildFields(T obj) {
- return FluentIterable.from(fields.values())
- .transform(
+ public final Iterable<Values<T>> buildFields(T obj, ImmutableSet<String> skipFields) {
+ return fields.values().stream()
+ .map(
f -> {
+ if (skipFields.contains(f.getName())) {
+ return null;
+ }
+
Object v;
try {
v = f.get(obj);
@@ -203,7 +209,8 @@
return new Values<>(f, Collections.singleton(v));
}
})
- .filter(Objects::nonNull);
+ .filter(Objects::nonNull)
+ .collect(toImmutableList());
}
@Override
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index deb3203..5392ab4 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -21,6 +21,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
@@ -98,6 +99,7 @@
private final SitePaths sitePaths;
private final Directory dir;
private final String name;
+ private final ImmutableSet<String> skipFields;
private final ListeningExecutorService writerThread;
private final IndexWriter writer;
private final ReferenceManager<IndexSearcher> searcherManager;
@@ -110,6 +112,7 @@
SitePaths sitePaths,
Directory dir,
String name,
+ ImmutableSet<String> skipFields,
String subIndex,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
@@ -118,6 +121,7 @@
this.sitePaths = sitePaths;
this.dir = dir;
this.name = name;
+ this.skipFields = skipFields;
String index = Joiner.on('_').skipNulls().join(name, subIndex);
long commitPeriod = writerConfig.getCommitWithinMs();
@@ -311,7 +315,7 @@
Document toDocument(V obj) {
Document result = new Document();
- for (Values<V> vs : schema.buildFields(obj)) {
+ for (Values<V> vs : schema.buildFields(obj, skipFields)) {
if (vs.getValues() != null) {
add(result, vs);
}
diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java
index fd439f1..e51a91a7 100644
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.lucene.LuceneChangeIndex.UPDATED_SORT_FIELD;
import static com.google.gerrit.server.index.change.ChangeSchemaDefinitions.NAME;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Change;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
@@ -48,6 +49,7 @@
Schema<ChangeData> schema,
SitePaths sitePaths,
Path path,
+ ImmutableSet<String> skipFields,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
throws IOException {
@@ -56,6 +58,7 @@
sitePaths,
FSDirectory.open(path),
path.getFileName().toString(),
+ skipFields,
writerConfig,
searcherFactory);
}
@@ -65,10 +68,11 @@
SitePaths sitePaths,
Directory dir,
String subIndex,
+ ImmutableSet<String> skipFields,
GerritIndexWriterConfig writerConfig,
SearcherFactory searcherFactory)
throws IOException {
- super(schema, sitePaths, dir, NAME, subIndex, writerConfig, searcherFactory);
+ super(schema, sitePaths, dir, NAME, skipFields, subIndex, writerConfig, searcherFactory);
}
@Override
diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
index efd7ea3..242cffd 100644
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.server.index.account.AccountField.ID_STR;
import static com.google.gerrit.server.index.account.AccountField.PREFERRED_EMAIL_EXACT;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
@@ -100,6 +101,7 @@
sitePaths,
dir(schema, cfg, sitePaths),
ACCOUNTS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, ACCOUNTS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 16d66b6..3b277dd 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -29,6 +29,7 @@
import com.google.common.collect.Collections2;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
@@ -167,6 +168,7 @@
private final String idSortFieldName;
private final IdTerm idTerm;
private final ChangeIdExtractor extractor;
+ private final ImmutableSet<String> skipFields;
@Inject
LuceneChangeIndex(
@@ -179,6 +181,10 @@
this.executor = executor;
this.changeDataFactory = changeDataFactory;
this.schema = schema;
+ this.skipFields =
+ cfg.getBoolean("index", "change", "indexMergeable", true)
+ ? ImmutableSet.of()
+ : ImmutableSet.of(ChangeField.MERGEABLE.getName());
GerritIndexWriterConfig openConfig = new GerritIndexWriterConfig(cfg, "changes_open");
GerritIndexWriterConfig closedConfig = new GerritIndexWriterConfig(cfg, "changes_closed");
@@ -189,18 +195,40 @@
if (LuceneIndexModule.isInMemoryTest(cfg)) {
openIndex =
new ChangeSubIndex(
- schema, sitePaths, new RAMDirectory(), "ramOpen", openConfig, searcherFactory);
+ schema,
+ sitePaths,
+ new RAMDirectory(),
+ "ramOpen",
+ skipFields,
+ openConfig,
+ searcherFactory);
closedIndex =
new ChangeSubIndex(
- schema, sitePaths, new RAMDirectory(), "ramClosed", closedConfig, searcherFactory);
+ schema,
+ sitePaths,
+ new RAMDirectory(),
+ "ramClosed",
+ skipFields,
+ closedConfig,
+ searcherFactory);
} else {
Path dir = LuceneVersionManager.getDir(sitePaths, CHANGES, schema);
openIndex =
new ChangeSubIndex(
- schema, sitePaths, dir.resolve(CHANGES_OPEN), openConfig, searcherFactory);
+ schema,
+ sitePaths,
+ dir.resolve(CHANGES_OPEN),
+ skipFields,
+ openConfig,
+ searcherFactory);
closedIndex =
new ChangeSubIndex(
- schema, sitePaths, dir.resolve(CHANGES_CLOSED), closedConfig, searcherFactory);
+ schema,
+ sitePaths,
+ dir.resolve(CHANGES_CLOSED),
+ skipFields,
+ closedConfig,
+ searcherFactory);
}
idField = this.schema.useLegacyNumericFields() ? LEGACY_ID : LEGACY_ID_STR;
@@ -565,7 +593,7 @@
private void decodeMergeable(ListMultimap<String, IndexableField> doc, ChangeData cd) {
IndexableField f = Iterables.getFirst(doc.get(MERGEABLE_FIELD), null);
- if (f != null) {
+ if (f != null && !skipFields.contains(MERGEABLE_FIELD)) {
String mergeable = f.stringValue();
if ("1".equals(mergeable)) {
cd.setMergeable(true);
diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
index 99cd40d..3d1d471 100644
--- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.server.index.group.GroupField.UUID;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
@@ -90,6 +91,7 @@
sitePaths,
dir(schema, cfg, sitePaths),
GROUPS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, GROUPS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
index 97454c7..a3a0d9c 100644
--- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.index.project.ProjectField.NAME;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
@@ -90,6 +91,7 @@
sitePaths,
dir(schema, cfg, sitePaths),
PROJECTS,
+ ImmutableSet.of(),
null,
new GerritIndexWriterConfig(cfg, PROJECTS),
new SearcherFactory());
diff --git a/java/com/google/gerrit/pgm/init/AccountsOnInit.java b/java/com/google/gerrit/pgm/init/AccountsOnInit.java
index 4ffe942..536ddcd 100644
--- a/java/com/google/gerrit/pgm/init/AccountsOnInit.java
+++ b/java/com/google/gerrit/pgm/init/AccountsOnInit.java
@@ -14,9 +14,8 @@
package com.google.gerrit.pgm.init;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Objects.requireNonNull;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
@@ -129,9 +128,10 @@
private File getPath() {
Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
- checkArgument(basePath != null, "gerrit.basePath must be configured");
- File file = FileKey.resolve(basePath.resolve(allUsers).toFile(), FS.DETECTED);
- checkState(file != null, "%s does not exist", file.getAbsolutePath());
- return file;
+ requireNonNull(basePath, "gerrit.basePath must be configured");
+ File file = basePath.resolve(allUsers).toFile();
+ File resolvedFile = FileKey.resolve(file, FS.DETECTED);
+ requireNonNull(resolvedFile, () -> String.format("%s does not exist", file.getAbsolutePath()));
+ return resolvedFile;
}
}
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index dbaf9c3..45d037a 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -107,12 +107,12 @@
"//lib/auto:auto-value-annotations",
"//lib/bouncycastle:bcpkix-neverlink",
"//lib/bouncycastle:bcprov-neverlink",
- "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:dbcp",
"//lib/commons:lang",
"//lib/commons:net",
"//lib/commons:validator",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
diff --git a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
index 996257c..5e7919f 100644
--- a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
+++ b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
@@ -115,11 +115,12 @@
.collect(toList()));
config.replace(createGroupAccessSection);
} else {
- Permission createGroupPermission = new Permission(Permission.CREATE);
- createGroupAccessSection.addPermission(createGroupPermission);
- createGroupsGlobal.forEach(createGroupPermission::add);
// The create permission is managed by Gerrit at this point only so there is no concern of
// overwriting user-defined permissions here.
+ Permission createGroupPermission = new Permission(Permission.CREATE);
+ createGroupAccessSection.remove(createGroupPermission);
+ createGroupAccessSection.addPermission(createGroupPermission);
+ createGroupsGlobal.forEach(createGroupPermission::add);
config.replace(createGroupAccessSection);
}
diff --git a/java/com/google/gerrit/server/ExceptionHook.java b/java/com/google/gerrit/server/ExceptionHook.java
index 6f05814..db44b4b 100644
--- a/java/com/google/gerrit/server/ExceptionHook.java
+++ b/java/com/google/gerrit/server/ExceptionHook.java
@@ -53,4 +53,15 @@
default Optional<String> formatCause(Throwable throwable) {
return Optional.empty();
}
+
+ /**
+ * Returns an error message that should be returned to the user.
+ *
+ * @param throwable throwable that was thrown while executing an operation
+ * @return error message that should be returned to the user, {@link Optional#empty()} if no
+ * message should be returned to the user
+ */
+ default Optional<String> getUserMessage(Throwable throwable) {
+ return Optional.empty();
+ }
}
diff --git a/java/com/google/gerrit/server/ExceptionHookImpl.java b/java/com/google/gerrit/server/ExceptionHookImpl.java
new file mode 100644
index 0000000..9613b29
--- /dev/null
+++ b/java/com/google/gerrit/server/ExceptionHookImpl.java
@@ -0,0 +1,68 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import com.google.common.base.Predicate;
+import com.google.common.base.Throwables;
+import com.google.gerrit.git.LockFailureException;
+import java.util.Optional;
+import org.eclipse.jgit.lib.RefUpdate;
+
+/**
+ * Class to detect and handle exceptions that are caused by temporary errors, and hence should cause
+ * a retry of the failed operation.
+ */
+public class ExceptionHookImpl implements ExceptionHook {
+ private static final String LOCK_FAILURE_USER_MESSAGE =
+ "Updating a ref failed with LOCK_FAILURE.\n"
+ + "This may be a temporary issue due to concurrent updates.\n"
+ + "Please retry later.";
+
+ @Override
+ public boolean shouldRetry(Throwable throwable) {
+ return isLockFailure(throwable);
+ }
+
+ @Override
+ public Optional<String> formatCause(Throwable throwable) {
+ if (isLockFailure(throwable)) {
+ return Optional.of(RefUpdate.Result.LOCK_FAILURE.name());
+ }
+ return Optional.empty();
+ }
+
+ @Override
+ public Optional<String> getUserMessage(Throwable throwable) {
+ if (isLockFailure(throwable)) {
+ return Optional.of(LOCK_FAILURE_USER_MESSAGE);
+ }
+ return Optional.empty();
+ }
+
+ private static boolean isLockFailure(Throwable throwable) {
+ return isMatching(throwable, t -> t instanceof LockFailureException);
+ }
+
+ /**
+ * Check whether the given exception or any of its causes matches the given predicate.
+ *
+ * @param throwable Exception that should be tested
+ * @param predicate predicate to check if a throwable matches
+ * @return {@code true} if the given exception or any of its causes matches the given predicate
+ */
+ private static boolean isMatching(Throwable throwable, Predicate<Throwable> predicate) {
+ return Throwables.getCausalChain(throwable).stream().anyMatch(predicate);
+ }
+}
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index c446c92..f1c2f82 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -125,8 +125,8 @@
ImmutableList.Builder<CommentValidationFailure> commentValidationFailures =
new ImmutableList.Builder<>();
commentValidators.runEach(
- listener ->
- commentValidationFailures.addAll(listener.validateComments(commentsForValidation)));
+ validator ->
+ commentValidationFailures.addAll(validator.validateComments(commentsForValidation)));
return commentValidationFailures.build();
}
}
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 988d871..a41a36c 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -516,12 +516,22 @@
* @throws IOException if an error occurs.
*/
public Result resolve(String input) throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate());
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
public Result resolve(String input, Predicate<AccountState> accountActivityPredicate)
throws ConfigInvalidException, IOException {
- return searchImpl(input, searchers, visibilitySupplier(), accountActivityPredicate);
+ return searchImpl(input, searchers, visibilitySupplierCanSee(), accountActivityPredicate);
+ }
+
+ public Result resolveIgnoreVisibility(String input) throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate());
+ }
+
+ public Result resolveIgnoreVisibility(
+ String input, Predicate<AccountState> accountActivityPredicate)
+ throws ConfigInvalidException, IOException {
+ return searchImpl(input, searchers, visibilitySupplierAll(), accountActivityPredicate);
}
/**
@@ -550,13 +560,23 @@
@Deprecated
public Result resolveByNameOrEmail(String input) throws ConfigInvalidException, IOException {
return searchImpl(
- input, nameOrEmailSearchers, visibilitySupplier(), accountActivityPredicate());
+ input, nameOrEmailSearchers, visibilitySupplierCanSee(), accountActivityPredicate());
}
- private Supplier<Predicate<AccountState>> visibilitySupplier() {
+ private Supplier<Predicate<AccountState>> visibilitySupplierCanSee() {
return () -> accountControlFactory.get()::canSee;
}
+ private Supplier<Predicate<AccountState>> visibilitySupplierAll() {
+ return () -> all();
+ }
+
+ private Predicate<AccountState> all() {
+ return accountState -> {
+ return true;
+ };
+ }
+
private Predicate<AccountState> accountActivityPredicate() {
return (AccountState accountState) -> accountState.account().isActive();
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
index 25420ee..8887e06 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -71,6 +71,7 @@
private final Counter1<Boolean> reloadCounter;
private final Timer0 reloadDifferential;
private final boolean enablePartialReloads;
+ private final boolean isPersistentCache;
@Inject
ExternalIdCacheLoader(
@@ -101,6 +102,8 @@
.setUnit(Units.MILLISECONDS));
this.enablePartialReloads =
config.getBoolean("cache", ExternalIdCacheImpl.CACHE_NAME, "enablePartialReloads", true);
+ this.isPersistentCache =
+ config.getInt("cache", ExternalIdCacheImpl.CACHE_NAME, "diskLimit", 0) > 0;
}
@Override
@@ -156,8 +159,11 @@
}
}
if (oldExternalIds == null) {
- logger.atWarning().log(
- "Unable to find an old ExternalId cache state, falling back to full reload");
+ if (isPersistentCache) {
+ // If there is no persistence, this is normal. Don't upset admins reading the logs.
+ logger.atWarning().log(
+ "Unable to find an old ExternalId cache state, falling back to full reload");
+ }
return reloadAllExternalIds(notesRev);
}
diff --git a/java/com/google/gerrit/server/audit/BUILD b/java/com/google/gerrit/server/audit/BUILD
index 95929d3..a7ecde4 100644
--- a/java/com/google/gerrit/server/audit/BUILD
+++ b/java/com/google/gerrit/server/audit/BUILD
@@ -57,7 +57,6 @@
"//lib/auto:auto-value-annotations",
"//lib/bouncycastle:bcpkix-neverlink",
"//lib/bouncycastle:bcprov-neverlink",
- "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:dbcp",
"//lib/commons:lang",
diff --git a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
index 4d41ed7..afd159c 100644
--- a/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
+++ b/java/com/google/gerrit/server/config/ChangeCleanupConfig.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.config;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.server.config.ScheduleConfig.Schedule;
import com.google.inject.Inject;
@@ -25,6 +26,8 @@
@Singleton
public class ChangeCleanupConfig {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private static String SECTION = "changeCleanup";
private static String KEY_ABANDON_AFTER = "abandonAfter";
private static String KEY_ABANDON_IF_MERGEABLE = "abandonIfMergeable";
@@ -48,12 +51,26 @@
this.urlFormatter = urlFormatter;
schedule = ScheduleConfig.createSchedule(cfg, SECTION);
abandonAfter = readAbandonAfter(cfg);
- abandonIfMergeable = cfg.getBoolean(SECTION, null, KEY_ABANDON_IF_MERGEABLE, true);
+ boolean indexMergeable = cfg.getBoolean("index", "change", "indexMergeable", true);
+ if (!indexMergeable) {
+ if (!readAbandonIfMergeable(cfg)) {
+ logger.atWarning().log(
+ "index.change.indexMergeable is disabled; %s.%s=false will be ineffective",
+ SECTION, KEY_ABANDON_IF_MERGEABLE);
+ }
+ abandonIfMergeable = true;
+ } else {
+ abandonIfMergeable = readAbandonIfMergeable(cfg);
+ }
cleanupAccountPatchReview =
cfg.getBoolean(SECTION, null, KEY_CLEANUP_ACCOUNT_PATCH_REVIEW, false);
abandonMessage = readAbandonMessage(cfg);
}
+ private boolean readAbandonIfMergeable(Config cfg) {
+ return cfg.getBoolean(SECTION, null, KEY_ABANDON_IF_MERGEABLE, true);
+ }
+
private long readAbandonAfter(Config cfg) {
long abandonAfter =
ConfigUtil.getTimeUnit(cfg, SECTION, null, KEY_ABANDON_AFTER, 0, TimeUnit.MILLISECONDS);
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 3b9c40e..2a0466f 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -79,6 +79,7 @@
import com.google.gerrit.server.CreateGroupPermissionSyncer;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.ExceptionHook;
+import com.google.gerrit.server.ExceptionHookImpl;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.TraceRequestListener;
@@ -393,6 +394,7 @@
DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
DynamicSet.setOf(binder(), ChangeETagComputation.class);
DynamicSet.setOf(binder(), ExceptionHook.class);
+ DynamicSet.bind(binder(), ExceptionHook.class).to(ExceptionHookImpl.class);
DynamicSet.setOf(binder(), MailSoyTemplateProvider.class);
DynamicMap.mapOf(binder(), MailFilter.class);
diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java
index c8b5e3f..9aebebf 100644
--- a/java/com/google/gerrit/server/git/MergedByPushOp.java
+++ b/java/com/google/gerrit/server/git/MergedByPushOp.java
@@ -52,6 +52,7 @@
MergedByPushOp create(
RequestScopePropagator requestScopePropagator,
PatchSet.Id psId,
+ @Assisted RequestId submissionId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId);
}
@@ -65,6 +66,7 @@
private final ChangeMerged changeMerged;
private final PatchSet.Id psId;
+ private final RequestId submissionId;
private final String refName;
private final String mergeResultRevId;
@@ -84,6 +86,7 @@
ChangeMerged changeMerged,
@Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
+ @Assisted RequestId submissionId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId) {
this.patchSetInfoFactory = patchSetInfoFactory;
@@ -93,6 +96,7 @@
this.sendEmailExecutor = sendEmailExecutor;
this.changeMerged = changeMerged;
this.requestScopePropagator = requestScopePropagator;
+ this.submissionId = submissionId;
this.psId = psId;
this.refName = refName;
this.mergeResultRevId = mergeResultRevId;
@@ -133,7 +137,6 @@
}
change.setCurrentPatchSet(info);
change.setStatus(Change.Status.MERGED);
- RequestId submissionId = new RequestId(change.getId().toString());
change.setSubmissionId(submissionId.toStringForStorage());
// we cannot reconstruct the submit records for when this change was
// submitted, this is why we must fix the status and other details.
diff --git a/java/com/google/gerrit/server/git/TracingHook.java b/java/com/google/gerrit/server/git/TracingHook.java
index 63d8bc6..56eded0 100644
--- a/java/com/google/gerrit/server/git/TracingHook.java
+++ b/java/com/google/gerrit/server/git/TracingHook.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.git;
-import static com.google.common.base.Preconditions.checkState;
-
import com.google.gerrit.server.logging.TraceContext;
import java.util.List;
import java.util.Optional;
@@ -59,7 +57,10 @@
* @param serverOptionList list of provided server options
*/
private void maybeStartTrace(List<String> serverOptionList) {
- checkState(traceContext == null, "Trace was already started.");
+ if (traceContext != null) {
+ // Trace was already started
+ return;
+ }
Optional<String> traceOption = parseTraceOption(serverOptionList);
traceContext =
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index 7038736..7767fe2 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -310,7 +310,8 @@
allRefsWatcher = new AllRefsWatcher();
receivePack.setAdvertiseRefsHook(
- ReceiveCommitsAdvertiseRefsHookChain.create(allRefsWatcher, queryProvider, projectName));
+ ReceiveCommitsAdvertiseRefsHookChain.create(
+ allRefsWatcher, queryProvider, projectName, user.getAccountId()));
resultChangeIds = new ResultChangeIds();
receiveCommits =
factory.create(
diff --git a/java/com/google/gerrit/server/git/receive/BUILD b/java/com/google/gerrit/server/git/receive/BUILD
index 2b04d4d..7402a37 100644
--- a/java/com/google/gerrit/server/git/receive/BUILD
+++ b/java/com/google/gerrit/server/git/receive/BUILD
@@ -15,6 +15,7 @@
"//java/com/google/gerrit/exceptions",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/git",
+ "//java/com/google/gerrit/index",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 7dd21e1..cec9e4e 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -3281,6 +3281,7 @@
int existingPatchSets = 0;
int newPatchSets = 0;
+ RequestId submissionId = null;
COMMIT:
for (RevCommit c; (c = rw.next()) != null; ) {
rw.parseBody(c);
@@ -3289,13 +3290,20 @@
receivePackRefCache.tipsFromObjectId(c.copy(), RefNames.REFS_CHANGES)) {
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
Optional<ChangeNotes> notes = getChangeNotes(psId.changeId());
+ if (submissionId == null) {
+ submissionId = new RequestId(psId.changeId().toString());
+ }
if (notes.isPresent() && notes.get().getChange().getDest().equals(branch)) {
existingPatchSets++;
bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null));
bu.addOp(
psId.changeId(),
mergedByPushOpFactory.create(
- requestScopePropagator, psId, refName, newTip.getId().getName()));
+ requestScopePropagator,
+ psId,
+ submissionId,
+ refName,
+ newTip.getId().getName()));
continue COMMIT;
}
}
@@ -3324,13 +3332,20 @@
logger.atFine().log("Not closing %s because validation failed", id);
continue;
}
+ if (submissionId == null) {
+ submissionId = new RequestId(id.toString());
+ }
req.addOps(bu, null);
bu.addOp(id, setPrivateOpFactory.create(false, null));
bu.addOp(
id,
mergedByPushOpFactory
.create(
- requestScopePropagator, req.psId, refName, newTip.getId().getName())
+ requestScopePropagator,
+ req.psId,
+ submissionId,
+ refName,
+ newTip.getId().getName())
.setPatchSetProvider(req.replaceOp::getPatchSet));
bu.addOp(id, new ChangeProgressOp(progress));
ids.add(id);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
index 83bf554..6c1f097 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java
@@ -18,14 +18,19 @@
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.git.HookUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeStatusPredicate;
import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.query.change.OwnerPredicate;
+import com.google.gerrit.server.query.change.ProjectPredicate;
import com.google.gerrit.server.util.MagicBranch;
import com.google.inject.Provider;
import java.io.IOException;
@@ -65,11 +70,13 @@
private final Provider<InternalChangeQuery> queryProvider;
private final Project.NameKey projectName;
+ private final Account.Id user;
public ReceiveCommitsAdvertiseRefsHook(
- Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName) {
+ Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, Account.Id user) {
this.queryProvider = queryProvider;
this.projectName = projectName;
+ this.user = user;
}
@Override
@@ -90,7 +97,9 @@
private Set<ObjectId> advertiseOpenChanges(Repository repo)
throws ServiceMayNotContinueException {
- // Advertise some recent open changes, in case a commit is based on one.
+ // Advertise the user's most recent open changes. It's likely that the user has one of these in
+ // their local repo and they can serve as starting points to figure out the common ancestor of
+ // what the client and server have in common.
int limit = 32;
try {
Set<ObjectId> r = Sets.newHashSetWithExpectedSize(limit);
@@ -105,7 +114,11 @@
ChangeField.PATCH_SET)
.enforceVisibility(true)
.setLimit(limit)
- .byProjectOpen(projectName)) {
+ .query(
+ Predicate.and(
+ new ProjectPredicate(projectName.get()),
+ ChangeStatusPredicate.open(),
+ new OwnerPredicate(user)))) {
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
// Ensure we actually observed a patch set ref pointing to this
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHookChain.java b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHookChain.java
index 76f6b04..fae1401 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHookChain.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHookChain.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.git.receive;
import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Provider;
@@ -35,8 +36,9 @@
public static AdvertiseRefsHook create(
AllRefsWatcher allRefsWatcher,
Provider<InternalChangeQuery> queryProvider,
- Project.NameKey projectName) {
- return create(allRefsWatcher, queryProvider, projectName, false);
+ Project.NameKey projectName,
+ Account.Id user) {
+ return create(allRefsWatcher, queryProvider, projectName, user, false);
}
/**
@@ -47,18 +49,19 @@
*/
@VisibleForTesting
public static AdvertiseRefsHook createForTest(
- Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName) {
- return create(new AllRefsWatcher(), queryProvider, projectName, true);
+ Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, Account.Id user) {
+ return create(new AllRefsWatcher(), queryProvider, projectName, user, true);
}
private static AdvertiseRefsHook create(
AllRefsWatcher allRefsWatcher,
Provider<InternalChangeQuery> queryProvider,
Project.NameKey projectName,
+ Account.Id user,
boolean skipHackPushNegotiateHook) {
List<AdvertiseRefsHook> advHooks = new ArrayList<>();
advHooks.add(allRefsWatcher);
- advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName));
+ advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName, user));
if (!skipHackPushNegotiateHook) {
advHooks.add(new HackPushNegotiateHook());
}
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index e95cf3b..6c0d5d3 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -61,6 +61,7 @@
import com.google.gerrit.server.extensions.events.RevisionCreated;
import com.google.gerrit.server.git.MergedByPushOp;
import com.google.gerrit.server.git.receive.ReceiveCommits.MagicBranchInput;
+import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -236,7 +237,11 @@
if (mergedInto != null) {
mergedByPushOp =
mergedByPushOpFactory.create(
- requestScopePropagator, patchSetId, mergedInto, mergeResultRevId);
+ requestScopePropagator,
+ patchSetId,
+ new RequestId(patchSetId.changeId().toString()),
+ mergedInto,
+ mergeResultRevId);
}
}
diff --git a/java/com/google/gerrit/server/index/StalenessCheckResult.java b/java/com/google/gerrit/server/index/StalenessCheckResult.java
index cd3f592..fe35e6e 100644
--- a/java/com/google/gerrit/server/index/StalenessCheckResult.java
+++ b/java/com/google/gerrit/server/index/StalenessCheckResult.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.index;
import com.google.auto.value.AutoValue;
+import com.google.errorprone.annotations.FormatMethod;
import java.util.Optional;
/** Structured result of a staleness check. */
@@ -29,6 +30,7 @@
return new AutoValue_StalenessCheckResult(true, Optional.of(reason));
}
+ @FormatMethod
public static StalenessCheckResult stale(String reason, Object... args) {
return stale(String.format(reason, args));
}
diff --git a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
index f6d3b6f..5efa065 100644
--- a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
+++ b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
@@ -76,7 +76,7 @@
this.accountCache = accountCache;
this.indexer = indexer;
this.executor = executor;
- this.enabled = cfg.getBoolean("index", null, "reindexAfterRefUpdate", true);
+ this.enabled = cfg.getBoolean("index", "change", "indexMergeable", true);
}
@Override
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 64d156f..d312530 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -95,9 +95,6 @@
// The version of a secondary index.
public abstract Optional<Integer> indexVersion();
- // The number of inputs to an operation, eg. Reachable.fromRefs.
- public abstract Optional<Integer> inputSize();
-
// The name of the implementation method.
public abstract Optional<String> methodName();
@@ -303,8 +300,6 @@
public abstract Builder indexVersion(int indexVersion);
- public abstract Builder inputSize(int size);
-
public abstract Builder methodName(@Nullable String methodName);
public abstract Builder multiple(boolean multiple);
diff --git a/java/com/google/gerrit/server/mail/SignedToken.java b/java/com/google/gerrit/server/mail/SignedToken.java
index 436b854..ba064df 100644
--- a/java/com/google/gerrit/server/mail/SignedToken.java
+++ b/java/com/google/gerrit/server/mail/SignedToken.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail;
+import com.google.common.io.BaseEncoding;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
@@ -21,7 +22,6 @@
import javax.crypto.Mac;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.SecretKeySpec;
-import org.apache.commons.codec.binary.Base64;
/**
* Utility function to compute and verify XSRF tokens.
@@ -164,11 +164,11 @@
}
private static byte[] decodeBase64(final String s) {
- return Base64.decodeBase64(toBytes(s));
+ return BaseEncoding.base64().decode(s);
}
private static String encodeBase64(final byte[] buf) {
- return toString(Base64.encodeBase64(buf));
+ return BaseEncoding.base64().encode(buf);
}
private static void encodeInt(final byte[] buf, final int o, final int v) {
@@ -202,12 +202,4 @@
}
return r;
}
-
- private static String toString(final byte[] b) {
- final StringBuilder r = new StringBuilder(b.length);
- for (int i = 0; i < b.length; i++) {
- r.append((char) b[i]);
- }
- return r.toString();
- }
}
diff --git a/java/com/google/gerrit/server/mail/SignedTokenEmailTokenVerifier.java b/java/com/google/gerrit/server/mail/SignedTokenEmailTokenVerifier.java
index 8b5cc92..8ae06f8 100644
--- a/java/com/google/gerrit/server/mail/SignedTokenEmailTokenVerifier.java
+++ b/java/com/google/gerrit/server/mail/SignedTokenEmailTokenVerifier.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.mail.send.RegisterNewEmailSender;
@@ -25,7 +26,6 @@
import com.google.inject.Singleton;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import org.eclipse.jgit.util.Base64;
/** Verifies the token sent by {@link RegisterNewEmailSender}. */
@Singleton
@@ -50,7 +50,7 @@
try {
String payload = String.format("%s:%s", accountId, emailAddress);
byte[] utf8 = payload.getBytes(UTF_8);
- String base64 = Base64.encodeBytes(utf8);
+ String base64 = BaseEncoding.base64().encode(utf8);
return emailRegistrationToken.newToken(base64);
} catch (XsrfException e) {
throw new IllegalArgumentException(e);
@@ -70,7 +70,7 @@
throw new InvalidTokenException();
}
- String payload = new String(Base64.decode(token.getData()), UTF_8);
+ String payload = new String(BaseEncoding.base64().decode(token.getData()), UTF_8);
Matcher matcher = Pattern.compile("^([0-9]+):(.+@.+)$").matcher(payload);
if (!matcher.matches()) {
throw new InvalidTokenException();
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 36b3c20..d7f09d2 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -180,6 +180,7 @@
setChangeSubjectHeader();
setHeader(MailHeader.CHANGE_ID.fieldName(), "" + change.getKey().get());
setHeader(MailHeader.CHANGE_NUMBER.fieldName(), "" + change.getChangeId());
+ setHeader(MailHeader.PROJECT.fieldName(), "" + change.getProject());
setChangeUrlHeader();
setCommitIdHeader();
diff --git a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
index bdfe2e8..6a34786 100644
--- a/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
+++ b/java/com/google/gerrit/server/mail/send/FromAddressGeneratorProvider.java
@@ -16,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.entities.Account;
import com.google.gerrit.mail.Address;
@@ -32,7 +33,6 @@
import java.security.NoSuchAlgorithmException;
import java.util.Optional;
import java.util.regex.Pattern;
-import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
@@ -232,7 +232,7 @@
try {
MessageDigest hash = MessageDigest.getInstance("MD5");
byte[] bytes = hash.digest(data.getBytes(UTF_8));
- return Base64.encodeBase64URLSafeString(bytes);
+ return BaseEncoding.base64Url().encode(bytes);
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException("No MD5 available", e);
}
diff --git a/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index f928bf0..0fb5c6f 100644
--- a/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -122,7 +122,7 @@
soyContext.put("branch", branchData);
footers.add(MailHeader.PROJECT.withDelimiter() + branch.project().get());
- footers.add("Gerrit-Branch: " + branch.shortName());
+ footers.add(MailHeader.BRANCH.withDelimiter() + branch.shortName());
}
@VisibleForTesting
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 7d2fa0a..e81f7f4 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -211,12 +211,12 @@
}
}
- Set<Address> intersection = Sets.intersection(smtpRcptTo, smtpRcptToPlaintextOnly);
+ Set<Address> intersection = Sets.intersection(va.smtpRcptTo, smtpRcptToPlaintextOnly);
if (!intersection.isEmpty()) {
logger.atSevere().log("Email '%s' will be sent twice to %s", messageClass, intersection);
}
- if (!smtpRcptTo.isEmpty()) {
+ if (!va.smtpRcptTo.isEmpty()) {
// Send multipart message
logger.atFine().log(
"Sending multipart '%s' from %s to %s",
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index ce88f07..d301d34 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -51,11 +51,11 @@
private final Change change;
protected final PersonIdent serverIdent;
- protected PatchSet.Id psId;
+ @Nullable protected PatchSet.Id psId;
private ObjectId result;
- protected boolean rootOnly;
+ boolean rootOnly;
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNotes notes,
CurrentUser user,
PersonIdent serverIdent,
@@ -72,7 +72,7 @@
this.when = when;
}
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNoteUtil noteUtil,
PersonIdent serverIdent,
@Nullable ChangeNotes notes,
@@ -172,7 +172,7 @@
public abstract boolean isEmpty();
/** Wether this update can only be a root commit. */
- public boolean isRootOnly() {
+ boolean isRootOnly() {
return rootOnly;
}
@@ -256,7 +256,7 @@
protected abstract CommitBuilder applyImpl(RevWalk rw, ObjectInserter ins, ObjectId curr)
throws IOException;
- protected static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
+ static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
ObjectId getResult() {
return result;
@@ -270,7 +270,7 @@
return ins.insert(Constants.OBJ_TREE, new byte[] {});
}
- protected void verifyComment(Comment c) {
+ void verifyComment(Comment c) {
checkArgument(c.getCommitId() != null, "commit ID required for comment: %s", c);
checkArgument(
c.author.getId().equals(getAccountId()),
diff --git a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
index 5d909d0..030cfb2 100644
--- a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
@@ -91,7 +91,7 @@
executor.submit(
() -> {
try (OpenRepo allUsersRepo = OpenRepo.open(repoManager, allUsersName)) {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
allUsersRepo.flush();
BatchRefUpdate bru = allUsersRepo.repo.getRefDatabase().newBatchUpdate();
bru.setPushCertificate(pushCert);
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 7ca0b86..72a460c 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -20,7 +20,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.Sets;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
@@ -56,8 +55,6 @@
* <p>This class is not thread safe.
*/
public class ChangeDraftUpdate extends AbstractChangeUpdate {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
public interface Factory {
ChangeDraftUpdate create(
ChangeNotes notes,
@@ -209,7 +206,6 @@
// keyed by commit ID.
Map<ObjectId, RevisionNoteBuilder> builders = cache.getBuilders();
boolean touchedAnyRevs = false;
- boolean hasComments = false;
for (Map.Entry<ObjectId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedCommits.add(e.getKey());
ObjectId id = e.getKey();
@@ -220,7 +216,6 @@
if (data.length == 0) {
rnm.noteMap.remove(id);
} else {
- hasComments = true;
ObjectId dataBlob = ins.insert(OBJ_BLOB, data);
rnm.noteMap.set(id, dataBlob);
}
diff --git a/java/com/google/gerrit/server/notedb/LimitExceededException.java b/java/com/google/gerrit/server/notedb/LimitExceededException.java
new file mode 100644
index 0000000..69f9241
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/LimitExceededException.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import com.google.gerrit.exceptions.StorageException;
+
+/**
+ * A write operation was rejected because a limit would be exceeded. Limits are currently imposed
+ * on:
+ *
+ * <ul>
+ * <li>The number of NoteDb updates per change.
+ * <li>The number of patch sets per change.
+ * </ul>
+ */
+public class LimitExceededException extends StorageException {
+ private static final long serialVersionUID = 1L;
+
+ LimitExceededException(String message) {
+ super(message);
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 886e02b..1b92c0e 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -64,6 +64,10 @@
* {@link #stage()}.
*/
public class NoteDbUpdateManager implements AutoCloseable {
+ private static final int MAX_UPDATES_DEFAULT = 1000;
+ /** Limits the number of patch sets that can be created. Can be overridden in the config. */
+ private static final int MAX_PATCH_SETS_DEFAULT = 1500;
+
public interface Factory {
NoteDbUpdateManager create(Project.NameKey projectName);
}
@@ -74,6 +78,7 @@
private final NoteDbMetrics metrics;
private final Project.NameKey projectName;
private final int maxUpdates;
+ private final int maxPatchSets;
private final ListMultimap<String, ChangeUpdate> changeUpdates;
private final ListMultimap<String, ChangeDraftUpdate> draftUpdates;
private final ListMultimap<String, RobotCommentUpdate> robotCommentUpdates;
@@ -103,7 +108,8 @@
this.metrics = metrics;
this.updateAllUsersAsync = updateAllUsersAsync;
this.projectName = projectName;
- maxUpdates = cfg.getInt("change", null, "maxUpdates", 1000);
+ maxUpdates = cfg.getInt("change", null, "maxUpdates", MAX_UPDATES_DEFAULT);
+ maxPatchSets = cfg.getInt("change", null, "maxPatchSets", MAX_PATCH_SETS_DEFAULT);
changeUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
draftUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
robotCommentUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
@@ -351,17 +357,17 @@
}
private void addCommands() throws IOException {
- changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates));
+ changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates), Optional.of(maxPatchSets));
if (!draftUpdates.isEmpty()) {
boolean publishOnly = draftUpdates.values().stream().allMatch(ChangeDraftUpdate::canRunAsync);
if (publishOnly) {
updateAllUsersAsync.setDraftUpdates(draftUpdates);
} else {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
}
}
if (!robotCommentUpdates.isEmpty()) {
- changeRepo.addUpdates(robotCommentUpdates);
+ changeRepo.addUpdatesNoLimits(robotCommentUpdates);
}
if (!rewriters.isEmpty()) {
addRewrites(rewriters, changeRepo);
@@ -375,17 +381,16 @@
private void doDelete(Change.Id id) throws IOException {
String metaRef = RefNames.changeMetaRef(id);
Optional<ObjectId> old = changeRepo.cmds.get(metaRef);
- if (old.isPresent()) {
- changeRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), metaRef));
- }
+ old.ifPresent(
+ objectId -> changeRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), metaRef)));
// Just scan repo for ref names, but get "old" values from cmds.
for (Ref r :
allUsersRepo.repo.getRefDatabase().getRefsByPrefix(RefNames.refsDraftCommentsPrefix(id))) {
old = allUsersRepo.cmds.get(r.getName());
- if (old.isPresent()) {
- allUsersRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), r.getName()));
- }
+ old.ifPresent(
+ objectId ->
+ allUsersRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), r.getName())));
}
}
diff --git a/java/com/google/gerrit/server/notedb/OpenRepo.java b/java/com/google/gerrit/server/notedb/OpenRepo.java
index de88684..351f31d 100644
--- a/java/com/google/gerrit/server/notedb/OpenRepo.java
+++ b/java/com/google/gerrit/server/notedb/OpenRepo.java
@@ -43,6 +43,15 @@
* objects that are jointly closed when invoking {@link #close}.
*/
class OpenRepo implements AutoCloseable {
+ final Repository repo;
+ final RevWalk rw;
+ final ChainedReceiveCommands cmds;
+ final ObjectInserter tempIns;
+
+ private final InMemoryInserter inMemIns;
+ @Nullable private final ObjectInserter finalIns;
+ private final boolean close;
+
/** Returns a {@link OpenRepo} wrapping around an open {@link Repository}. */
static OpenRepo open(GitRepositoryManager repoManager, Project.NameKey project)
throws IOException {
@@ -60,15 +69,6 @@
}
}
- final Repository repo;
- final RevWalk rw;
- final ChainedReceiveCommands cmds;
- final ObjectInserter tempIns;
-
- private final InMemoryInserter inMemIns;
- @Nullable private final ObjectInserter finalIns;
- private final boolean close;
-
OpenRepo(
Repository repo,
RevWalk rw,
@@ -125,12 +125,15 @@
return updates.iterator().next().allowWriteToNewRef();
}
- <U extends AbstractChangeUpdate> void addUpdates(ListMultimap<String, U> all) throws IOException {
- addUpdates(all, Optional.empty());
+ <U extends AbstractChangeUpdate> void addUpdatesNoLimits(ListMultimap<String, U> all)
+ throws IOException {
+ addUpdates(
+ all, Optional.empty() /* unlimited updates */, Optional.empty() /* unlimited patch sets */);
}
<U extends AbstractChangeUpdate> void addUpdates(
- ListMultimap<String, U> all, Optional<Integer> maxUpdates) throws IOException {
+ ListMultimap<String, U> all, Optional<Integer> maxUpdates, Optional<Integer> maxPatchSets)
+ throws IOException {
for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
String refName = e.getKey();
Collection<U> updates = e.getValue();
@@ -142,29 +145,43 @@
continue;
}
- int updateCount;
+ int updateCount = 0;
U first = updates.iterator().next();
if (maxUpdates.isPresent()) {
checkState(first.getNotes() != null, "expected ChangeNotes on %s", first);
updateCount = first.getNotes().getUpdateCount();
- } else {
- updateCount = 0;
}
ObjectId curr = old;
- for (U u : updates) {
- if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) {
+ for (U update : updates) {
+ if (maxPatchSets.isPresent() && update.psId != null) {
+ // Patch set IDs are assigned consecutively. Patch sets may have been deleted, but the ID
+ // is still a good estimate and an upper bound.
+ if (update.psId.get() > maxPatchSets.get()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %d may not exceed %d patch sets. To continue working on this change, "
+ + "recreate it with a new Change-Id, then abandon this one.",
+ update.getId().get(), maxPatchSets.get()));
+ }
+ }
+ if (update.isRootOnly() && !old.equals(ObjectId.zeroId())) {
throw new StorageException("Given ChangeUpdate is only allowed on initial commit");
}
- ObjectId next = u.apply(rw, tempIns, curr);
+ ObjectId next = update.apply(rw, tempIns, curr);
if (next == null) {
continue;
}
if (maxUpdates.isPresent()
&& !Objects.equals(next, curr)
&& ++updateCount > maxUpdates.get()
- && !u.bypassMaxUpdates()) {
- throw new TooManyUpdatesException(u.getId(), maxUpdates.get());
+ && !update.bypassMaxUpdates()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %s may not exceed %d updates. It may still be abandoned or submitted. To"
+ + " continue working on this change, recreate it with a new Change-Id, then"
+ + " abandon this one.",
+ update.getId(), maxUpdates.get()));
}
curr = next;
}
diff --git a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java b/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
deleted file mode 100644
index 9c6faaf..0000000
--- a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright (C) 2019 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.notedb;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.exceptions.StorageException;
-
-/**
- * Exception indicating that the change has received too many updates. Further actions apart from
- * {@code abandon} or {@code submit} are blocked.
- */
-public class TooManyUpdatesException extends StorageException {
- @VisibleForTesting
- public static String message(Change.Id id, int maxUpdates) {
- return "Change "
- + id
- + " may not exceed "
- + maxUpdates
- + " updates. It may still be abandoned or submitted. To continue working on this "
- + "change, recreate it with a new Change-Id, then abandon this one.";
- }
-
- private static final long serialVersionUID = 1L;
-
- TooManyUpdatesException(Change.Id id, int maxUpdates) {
- super(message(id, maxUpdates));
- }
-}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 0f228fe..8b6b91a 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -272,6 +272,12 @@
return;
}
+ if (edits.isEmpty() && (aSize != bSize)) {
+ // Only edits due to rebase were present. If we now added the edits for the newlines, the
+ // code which later assembles the file contents would fail.
+ return;
+ }
+
Optional<Edit> lastEdit = getLast(edits);
if (isNewlineAtEndDeleted(a, b)) {
Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 4d551a2..fa877af 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -923,9 +923,18 @@
lowerNames.put(lower, name);
List<LabelValue> values = new ArrayList<>();
+ Set<Short> allValues = new HashSet<>();
for (String value : rc.getStringList(LABEL, name, KEY_VALUE)) {
try {
- values.add(parseLabelValue(value));
+ LabelValue labelValue = parseLabelValue(value);
+ if (allValues.add(labelValue.getValue())) {
+ values.add(labelValue);
+ } else {
+ error(
+ new ValidationError(
+ PROJECT_CONFIG,
+ String.format("Duplicate %s \"%s\" for label \"%s\"", KEY_VALUE, value, name)));
+ }
} catch (IllegalArgumentException notValue) {
error(
new ValidationError(
diff --git a/java/com/google/gerrit/server/project/Reachable.java b/java/com/google/gerrit/server/project/Reachable.java
index c30378b..6d28646a 100644
--- a/java/com/google/gerrit/server/project/Reachable.java
+++ b/java/com/google/gerrit/server/project/Reachable.java
@@ -67,7 +67,7 @@
try (TraceTimer timer =
TraceContext.newTimer(
"IncludedInResolver.includedInAny",
- Metadata.builder().projectName(project.get()).inputSize(refs.size()).build())) {
+ Metadata.builder().projectName(project.get()).resourceCount(refs.size()).build())) {
return IncludedInResolver.includedInAny(repo, rw, commit, filtered.values());
}
} catch (IOException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index c6beac4..78ca0fc 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -598,7 +598,11 @@
committer = c.getCommitterIdent();
parentCount = c.getParentCount();
} catch (IOException e) {
- throw new StorageException(e);
+ throw new StorageException(
+ String.format(
+ "Loading commit %s for ps %d of change %d failed.",
+ ps.commitId(), ps.id().get(), ps.id().changeId().get()),
+ e);
}
return true;
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index c86bd94..df5d291 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -63,6 +63,7 @@
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.OperatorAliasConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
@@ -94,6 +95,7 @@
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
/** Parses a query string meant to be applied to change objects. */
@@ -221,6 +223,7 @@
final GroupMembers groupMembers;
final Provider<AnonymousUser> anonymousUserProvider;
final OperatorAliasConfig operatorAliasConfig;
+ final boolean indexMergeable;
private final Provider<CurrentUser> self;
@@ -253,7 +256,8 @@
AccountCache accountCache,
GroupMembers groupMembers,
Provider<AnonymousUser> anonymousUserProvider,
- OperatorAliasConfig operatorAliasConfig) {
+ OperatorAliasConfig operatorAliasConfig,
+ @GerritServerConfig Config gerritConfig) {
this(
queryProvider,
rewriter,
@@ -281,7 +285,8 @@
accountCache,
groupMembers,
anonymousUserProvider,
- operatorAliasConfig);
+ operatorAliasConfig,
+ gerritConfig.getBoolean("index", "change", "indexMergeable", true));
}
private Arguments(
@@ -311,7 +316,8 @@
AccountCache accountCache,
GroupMembers groupMembers,
Provider<AnonymousUser> anonymousUserProvider,
- OperatorAliasConfig operatorAliasConfig) {
+ OperatorAliasConfig operatorAliasConfig,
+ boolean indexMergeable) {
this.queryProvider = queryProvider;
this.rewriter = rewriter;
this.opFactories = opFactories;
@@ -339,6 +345,7 @@
this.groupMembers = groupMembers;
this.anonymousUserProvider = anonymousUserProvider;
this.operatorAliasConfig = operatorAliasConfig;
+ this.indexMergeable = indexMergeable;
}
Arguments asUser(CurrentUser otherUser) {
@@ -369,7 +376,8 @@
accountCache,
groupMembers,
anonymousUserProvider,
- operatorAliasConfig);
+ operatorAliasConfig,
+ indexMergeable);
}
Arguments asUser(Account.Id otherId) {
@@ -570,6 +578,9 @@
}
if ("mergeable".equalsIgnoreCase(value)) {
+ if (!args.indexMergeable) {
+ throw new QueryParseException("'is:mergeable' operator is not supported by server");
+ }
return new BooleanPredicate(ChangeField.MERGEABLE);
}
@@ -1383,7 +1394,11 @@
private List<Change> parseChange(String value) throws QueryParseException {
if (PAT_LEGACY_ID.matcher(value).matches()) {
- return asChanges(args.queryProvider.get().byLegacyChangeId(Change.Id.parse(value)));
+ Optional<Change.Id> id = Change.Id.tryParse(value);
+ if (!id.isPresent()) {
+ throw error("Invalid change id " + value);
+ }
+ return asChanges(args.queryProvider.get().byLegacyChangeId(id.get()));
} else if (PAT_CHANGE_ID.matcher(value).matches()) {
List<Change> changes = asChanges(args.queryProvider.get().byKeyPrefix(parseChangeId(value)));
if (changes.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index fd341e9..720e7d2 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -34,7 +34,6 @@
"//lib:servlet-api",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
- "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:lang",
"//lib/flogger:api",
diff --git a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
index 7b89b9c..36a0c71 100644
--- a/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
+++ b/java/com/google/gerrit/server/restapi/account/PutHttpPassword.java
@@ -15,9 +15,11 @@
package com.google.gerrit.server.restapi.account;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.exceptions.EmailException;
import com.google.gerrit.extensions.common.HttpPasswordInput;
@@ -43,7 +45,6 @@
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Optional;
-import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.errors.ConfigInvalidException;
public class PutHttpPassword implements RestModifyView<AccountResource, HttpPasswordInput> {
@@ -142,7 +143,7 @@
byte[] rand = new byte[LEN];
rng.nextBytes(rand);
- byte[] enc = Base64.encodeBase64(rand, false);
+ byte[] enc = BaseEncoding.base64().encode(rand).getBytes(UTF_8);
StringBuilder r = new StringBuilder(enc.length);
for (int i = 0; i < enc.length; i++) {
if (enc[i] == '=') {
diff --git a/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java b/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
index 8040847..af65483 100644
--- a/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
+++ b/java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toMap;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
@@ -76,7 +77,15 @@
// Map of all patch sets, keyed by commit SHA-1.
Map<ObjectId, PatchSetData> byId = collectById(in);
PatchSetData start = byId.get(startPs.commitId());
- checkArgument(start != null, "%s not found in %s", startPs, in);
+ requireNonNull(
+ start,
+ () ->
+ String.format(
+ "commit %s of patch set %s not found in %s",
+ startPs.commitId().name(),
+ startPs.id(),
+ byId.entrySet().stream()
+ .collect(toMap(e -> e.getKey().name(), e -> e.getValue().patchSet().id()))));
// Map of patch set -> immediate parent.
ListMultimap<PatchSetData, PatchSetData> parents =
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 47c6a7e..b44bb29 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -95,11 +95,12 @@
String.format("change is %s.", ChangeUtil.status(changeResource.getChange())));
}
- String submissionId =
- requireNonNull(
- changeResource.getChange().getSubmissionId(),
- String.format("merged change %s has no submission ID", changeResource.getId()));
-
+ String submissionId = changeResource.getChange().getSubmissionId();
+ if (submissionId == null) {
+ throw new ResourceConflictException(
+ "This change is merged but doesn't have a submission id,"
+ + " meaning it was not submitted through Gerrit.");
+ }
List<ChangeData> changeDatas = queryProvider.get().bySubmissionId(submissionId);
for (ChangeData changeData : changeDatas) {
@@ -160,10 +161,21 @@
"Revert this change and all changes that have been submitted together with this change")
.setVisible(
and(
- change.isMerged() && change.getSubmissionId() != null && projectStatePermitsWrite,
+ change.isMerged()
+ && change.getSubmissionId() != null
+ && isChangePartOfSubmission(change.getSubmissionId())
+ && projectStatePermitsWrite,
permissionBackend
.user(rsrc.getUser())
.ref(change.getDest())
.testCond(CREATE_CHANGE)));
}
+
+ /**
+ * @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);
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/change/Reviewers.java b/java/com/google/gerrit/server/restapi/change/Reviewers.java
index b2714da..d702142 100644
--- a/java/com/google/gerrit/server/restapi/change/Reviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/Reviewers.java
@@ -21,12 +21,11 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.restapi.account.AccountsCollection;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -37,22 +36,22 @@
public class Reviewers implements ChildCollection<ChangeResource, ReviewerResource> {
private final DynamicMap<RestView<ReviewerResource>> views;
private final ApprovalsUtil approvalsUtil;
- private final AccountsCollection accounts;
private final ReviewerResource.Factory resourceFactory;
private final ListReviewers list;
+ private final AccountResolver accountResolver;
@Inject
Reviewers(
ApprovalsUtil approvalsUtil,
- AccountsCollection accounts,
ReviewerResource.Factory resourceFactory,
DynamicMap<RestView<ReviewerResource>> views,
- ListReviewers list) {
+ ListReviewers list,
+ AccountResolver accountResolver) {
this.approvalsUtil = approvalsUtil;
- this.accounts = accounts;
this.resourceFactory = resourceFactory;
this.views = views;
this.list = list;
+ this.accountResolver = accountResolver;
}
@Override
@@ -68,22 +67,18 @@
@Override
public ReviewerResource parse(ChangeResource rsrc, IdString id)
throws ResourceNotFoundException, AuthException, IOException, ConfigInvalidException {
- Address address = Address.tryParse(id.get());
-
- Account.Id accountId = null;
try {
- accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
- } catch (ResourceNotFoundException e) {
- if (address == null) {
- throw e;
+
+ AccountResolver.Result result = accountResolver.resolveIgnoreVisibility(id.get());
+ if (fetchAccountIds(rsrc).contains(result.asUniqueUser().getAccountId())) {
+ return resourceFactory.create(rsrc, result.asUniqueUser().getAccountId());
+ }
+ } catch (AccountResolver.UnresolvableAccountException e) {
+ if (e.isSelf()) {
+ throw new AuthException(e.getMessage(), e);
}
}
- // See if the id exists as a reviewer for this change
- if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
- return resourceFactory.create(rsrc, accountId);
- }
-
- // See if the address exists as a reviewer on the change
+ Address address = Address.tryParse(id.get());
if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
return new ReviewerResource(rsrc, address);
}
diff --git a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
index 2d504c7..2d4bfe6 100644
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -23,9 +23,11 @@
import com.google.gerrit.extensions.common.AccountsInfo;
import com.google.gerrit.extensions.common.AuthInfo;
import com.google.gerrit.extensions.common.ChangeConfigInfo;
+import com.google.gerrit.extensions.common.ChangeIndexConfigInfo;
import com.google.gerrit.extensions.common.DownloadInfo;
import com.google.gerrit.extensions.common.DownloadSchemeInfo;
import com.google.gerrit.extensions.common.GerritInfo;
+import com.google.gerrit.extensions.common.IndexConfigInfo;
import com.google.gerrit.extensions.common.PluginConfigInfo;
import com.google.gerrit.extensions.common.ReceiveInfo;
import com.google.gerrit.extensions.common.ServerInfo;
@@ -141,6 +143,7 @@
info.change = getChangeInfo();
info.download = getDownloadInfo();
info.gerrit = getGerritInfo();
+ info.index = getIndexInfo();
info.noteDbEnabled = true;
info.plugin = getPluginInfo();
info.defaultTheme = getDefaultTheme();
@@ -296,6 +299,14 @@
return info;
}
+ private IndexConfigInfo getIndexInfo() {
+ ChangeIndexConfigInfo change = new ChangeIndexConfigInfo();
+ change.indexMergeable = toBoolean(config.getBoolean("index", "change", "indexMergeable", true));
+ IndexConfigInfo index = new IndexConfigInfo();
+ index.change = change;
+ return index;
+ }
+
private String getDocUrl() {
String docUrl = config.getString("gerrit", null, "docUrl");
if (Strings.isNullOrEmpty(docUrl)) {
diff --git a/java/com/google/gerrit/server/restapi/group/DeleteMembers.java b/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
index 9bae2e2..fa1c5c6 100644
--- a/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
+++ b/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.group;
+import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.entities.Account;
@@ -68,6 +69,9 @@
Set<Account.Id> membersToRemove = new HashSet<>();
for (String nameOrEmail : input.members) {
+ if (Strings.isNullOrEmpty(nameOrEmail)) {
+ continue;
+ }
membersToRemove.add(accountResolver.resolve(nameOrEmail).asUnique().account().id());
}
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
diff --git a/java/com/google/gerrit/server/restapi/group/GroupsCollection.java b/java/com/google/gerrit/server/restapi/group/GroupsCollection.java
index b92a464..65a7f4f 100644
--- a/java/com/google/gerrit/server/restapi/group/GroupsCollection.java
+++ b/java/com/google/gerrit/server/restapi/group/GroupsCollection.java
@@ -14,10 +14,13 @@
package com.google.gerrit.server.restapi.group;
+import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.NeedsParams;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestCollection;
import com.google.gerrit.extensions.restapi.RestView;
@@ -30,21 +33,27 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-public class GroupsCollection implements RestCollection<TopLevelResource, GroupResource> {
+public class GroupsCollection
+ implements RestCollection<TopLevelResource, GroupResource>, NeedsParams {
private final DynamicMap<RestView<GroupResource>> views;
+ private final Provider<ListGroups> list;
private final Provider<QueryGroups> queryGroups;
private final GroupControl.Factory groupControlFactory;
private final GroupResolver groupResolver;
private final Provider<CurrentUser> self;
+ private boolean hasQuery;
+
@Inject
public GroupsCollection(
DynamicMap<RestView<GroupResource>> views,
+ Provider<ListGroups> list,
Provider<QueryGroups> queryGroups,
GroupControl.Factory groupControlFactory,
GroupResolver groupResolver,
Provider<CurrentUser> self) {
this.views = views;
+ this.list = list;
this.queryGroups = queryGroups;
this.groupControlFactory = groupControlFactory;
this.groupResolver = groupResolver;
@@ -52,6 +61,11 @@
}
@Override
+ public void setParams(ListMultimap<String, String> params) throws BadRequestException {
+ this.hasQuery = params.containsKey("query");
+ }
+
+ @Override
public RestView<TopLevelResource> list() throws ResourceNotFoundException, AuthException {
final CurrentUser user = self.get();
if (user instanceof AnonymousUser) {
@@ -59,7 +73,12 @@
} else if (!(user.isIdentifiedUser())) {
throw new ResourceNotFoundException();
}
- return queryGroups.get();
+
+ if (hasQuery) {
+ return queryGroups.get();
+ }
+
+ return list.get();
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
index a45c67f..1e288f4 100644
--- a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
+++ b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
@@ -24,10 +24,12 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.project.RefPattern;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
+import java.util.Set;
public class LabelDefinitionInputParser {
public static LabelFunction parseFunction(String functionString) throws BadRequestException {
@@ -39,6 +41,7 @@
public static List<LabelValue> parseValues(Map<String, String> values)
throws BadRequestException {
List<LabelValue> valueList = new ArrayList<>();
+ Set<Short> allValues = new HashSet<>();
for (Entry<String, String> e : values.entrySet()) {
short value;
try {
@@ -46,6 +49,9 @@
} catch (NumberFormatException ex) {
throw new BadRequestException("invalid value: " + e.getKey(), ex);
}
+ if (!allValues.add(value)) {
+ throw new BadRequestException("duplicate value: " + value);
+ }
String valueDescription = e.getValue().trim();
if (valueDescription.isEmpty()) {
throw new BadRequestException("description for value '" + e.getKey() + "' cannot be empty");
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index e240f6a..1e86d52 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -52,8 +52,8 @@
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.LimitExceededException;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -181,7 +181,7 @@
private static void wrapAndThrowException(Exception e) throws UpdateException, RestApiException {
// Convert common non-REST exception types with user-visible messages to corresponding REST
// exception types.
- if (e instanceof InvalidChangeOperationException || e instanceof TooManyUpdatesException) {
+ if (e instanceof InvalidChangeOperationException || e instanceof LimitExceededException) {
throw new ResourceConflictException(e.getMessage(), e);
} else if (e instanceof NoSuchChangeException
|| e instanceof NoSuchRefException
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index f8cf4cf..4f9a67c 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -34,9 +34,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.git.LockFailureException;
-import com.google.gerrit.metrics.Counter1;
-import com.google.gerrit.metrics.Counter2;
import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Field;
@@ -57,7 +54,6 @@
import java.util.function.Consumer;
import java.util.function.Predicate;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.RefUpdate;
@Singleton
public class RetryHelper {
@@ -130,8 +126,8 @@
@VisibleForTesting
@Singleton
public static class Metrics {
- final Counter2<ActionType, String> attemptCounts;
- final Counter1<ActionType> timeoutCount;
+ final Counter3<ActionType, String, String> attemptCounts;
+ final Counter3<ActionType, String, String> timeoutCount;
final Counter3<ActionType, String, String> autoRetryCount;
final Counter3<ActionType, String, String> failuresOnAutoRetryCount;
@@ -139,6 +135,19 @@
Metrics(MetricMaker metricMaker) {
Field<ActionType> actionTypeField =
Field.ofEnum(ActionType.class, "action_type", Metadata.Builder::actionType).build();
+ Field<String> operationNameField =
+ Field.ofString("operation_name", Metadata.Builder::operationName)
+ .description("The name of the operation that was retried.")
+ .build();
+ Field<String> lastAttemptCauseField =
+ Field.ofString("cause", Metadata.Builder::cause)
+ .description("The cause for the last attempt.")
+ .build();
+ Field<String> causeField =
+ Field.ofString("cause", Metadata.Builder::cause)
+ .description("The cause for the retry.")
+ .build();
+
attemptCounts =
metricMaker.newCounter(
"action/retry_attempt_count",
@@ -148,9 +157,8 @@
.setCumulative()
.setUnit("attempts"),
actionTypeField,
- Field.ofString("cause", Metadata.Builder::cause)
- .description("The cause for the last attempt.")
- .build());
+ operationNameField,
+ lastAttemptCauseField);
timeoutCount =
metricMaker.newCounter(
"action/retry_timeout_count",
@@ -158,7 +166,9 @@
"Number of action executions of RetryHelper that ultimately timed out")
.setCumulative()
.setUnit("timeouts"),
- actionTypeField);
+ actionTypeField,
+ operationNameField,
+ lastAttemptCauseField);
autoRetryCount =
metricMaker.newCounter(
"action/auto_retry_count",
@@ -166,12 +176,8 @@
.setCumulative()
.setUnit("retries"),
actionTypeField,
- Field.ofString("operation_name", Metadata.Builder::operationName)
- .description("The name of the operation that was retried.")
- .build(),
- Field.ofString("cause", Metadata.Builder::cause)
- .description("The cause for the retry.")
- .build());
+ operationNameField,
+ causeField);
failuresOnAutoRetryCount =
metricMaker.newCounter(
"action/failures_on_auto_retry_count",
@@ -179,12 +185,8 @@
.setCumulative()
.setUnit("failures"),
actionTypeField,
- Field.ofString("operation_name", Metadata.Builder::operationName)
- .description("The name of the operation that was retried.")
- .build(),
- Field.ofString("cause", Metadata.Builder::cause)
- .description("The cause for the retry.")
- .build());
+ operationNameField,
+ causeField);
}
}
@@ -284,15 +286,7 @@
throws RestApiException, UpdateException {
try {
return execute(
- ActionType.CHANGE_UPDATE,
- () -> changeAction.call(updateFactory),
- opts,
- t -> {
- if (t instanceof UpdateException || t instanceof StorageException) {
- t = t.getCause();
- }
- return t instanceof LockFailureException;
- });
+ ActionType.CHANGE_UPDATE, () -> changeAction.call(updateFactory), opts, t -> false);
} catch (Throwable t) {
Throwables.throwIfUnchecked(t);
Throwables.throwIfInstanceOf(t, UpdateException.class);
@@ -365,19 +359,20 @@
return false;
});
retryerBuilder.withRetryListener(listener);
- return executeWithTimeoutCount(actionType, action, retryerBuilder.build());
+ return executeWithTimeoutCount(actionType, action, opts, retryerBuilder.build());
} finally {
if (listener.getAttemptCount() > 1) {
logger.atFine().log("%s was attempted %d times", actionType, listener.getAttemptCount());
metrics.attemptCounts.incrementBy(
actionType,
+ opts.caller().orElse("N/A"),
listener.getCause().map(this::formatCause).orElse("_unknown"),
listener.getAttemptCount() - 1);
}
}
}
- private String formatCause(Throwable t) {
+ public String formatCause(Throwable t) {
while ((t instanceof UpdateException
|| t instanceof StorageException
|| t instanceof ExecutionException)
@@ -390,9 +385,6 @@
return formattedCause.get();
}
- if (t instanceof LockFailureException) {
- return RefUpdate.Result.LOCK_FAILURE.name();
- }
return t.getClass().getSimpleName();
}
@@ -409,18 +401,22 @@
*
* @param actionType the type of the action
* @param action the action which should be executed and retried on failure
+ * @param opts options for retrying the action on failure
* @param retryer the retryer
* @return the result of executing the action
* @throws Throwable any error or exception that made the action fail, callers are expected to
* catch and inspect this Throwable to decide carefully whether it should be re-thrown
*/
- private <T> T executeWithTimeoutCount(ActionType actionType, Action<T> action, Retryer<T> retryer)
- throws Throwable {
+ private <T> T executeWithTimeoutCount(
+ ActionType actionType, Action<T> action, Options opts, Retryer<T> retryer) throws Throwable {
try {
return retryer.call(action::call);
} catch (ExecutionException | RetryException e) {
if (e instanceof RetryException) {
- metrics.timeoutCount.increment(actionType);
+ metrics.timeoutCount.increment(
+ actionType,
+ opts.caller().orElse("N/A"),
+ e.getCause() != null ? formatCause(e.getCause()) : "_unknown");
}
if (e.getCause() != null) {
throw e.getCause();
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index bc9a0ee..f567a3a 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -32,7 +32,6 @@
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/bouncycastle:bcprov-neverlink",
- "//lib/commons:codec",
"//lib/dropwizard:dropwizard-core",
"//lib/flogger:api",
"//lib/guice",
diff --git a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
index 6c0f3af..916775d 100644
--- a/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
+++ b/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java
@@ -19,6 +19,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.FileUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PeerDaemonUser;
@@ -39,7 +40,6 @@
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
-import org.apache.commons.codec.binary.Base64;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
@@ -198,7 +198,8 @@
}
try {
- byte[] bin = Base64.decodeBase64(line.getBytes(ISO_8859_1));
+ byte[] bin =
+ BaseEncoding.base64().decode(new String(line.getBytes(ISO_8859_1), ISO_8859_1));
keys.add(new ByteArrayBuffer(bin).getRawPublicKey());
} catch (RuntimeException | SshException e) {
logBadKey(path, line, e);
diff --git a/java/com/google/gerrit/sshd/SshUtil.java b/java/com/google/gerrit/sshd/SshUtil.java
index 39366f0..eac9737 100644
--- a/java/com/google/gerrit/sshd/SshUtil.java
+++ b/java/com/google/gerrit/sshd/SshUtil.java
@@ -14,6 +14,7 @@
package com.google.gerrit.sshd;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -28,12 +29,10 @@
import java.security.interfaces.DSAPublicKey;
import java.security.interfaces.RSAPublicKey;
import java.security.spec.InvalidKeySpecException;
-import org.apache.commons.codec.binary.Base64;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.server.session.ServerSession;
-import org.eclipse.jgit.lib.Constants;
/** Utilities to support SSH operations. */
public class SshUtil {
@@ -53,7 +52,7 @@
if (s == null) {
throw new InvalidKeySpecException("No key string");
}
- final byte[] bin = Base64.decodeBase64(Constants.encodeASCII(s));
+ final byte[] bin = BaseEncoding.base64().decode(s);
return new ByteArrayBuffer(bin).getRawPublicKey();
} catch (RuntimeException | SshException e) {
throw new InvalidKeySpecException("Cannot parse key", e);
@@ -91,8 +90,7 @@
}
final PublicKey key =
- new ByteArrayBuffer(Base64.decodeBase64(Constants.encodeASCII(strBuf.toString())))
- .getRawPublicKey();
+ new ByteArrayBuffer(BaseEncoding.base64().decode(strBuf.toString())).getRawPublicKey();
if (key instanceof RSAPublicKey) {
strBuf.insert(0, KeyPairProvider.SSH_RSA + " ");
diff --git a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index e0c4624..df1e3ed 100644
--- a/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -275,7 +275,7 @@
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
for (String sshKey : sshKeys) {
SshKeyInput in = new SshKeyInput();
- in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "plain/text");
+ in.raw = RawInputUtil.create(sshKey.getBytes(UTF_8), "text/plain");
addSshKey.apply(rsrc, in);
}
}
diff --git a/java/com/google/gerrit/testing/IndexConfig.java b/java/com/google/gerrit/testing/IndexConfig.java
index 21c49dd..fb6c926 100644
--- a/java/com/google/gerrit/testing/IndexConfig.java
+++ b/java/com/google/gerrit/testing/IndexConfig.java
@@ -23,7 +23,9 @@
public static Config createFromExistingConfig(Config cfg) {
cfg.setInt("index", null, "maxPages", 10);
- cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
+ // To avoid this flakiness indexMergeable is switched off for the tests as it incurs background
+ // reindex calls.
+ cfg.setBoolean("index", "change", "indexMergeable", false);
cfg.setString("trackingid", "query-bug", "footer", "Bug:");
cfg.setString("trackingid", "query-bug", "match", "QUERY\\d{2,8}");
cfg.setString("trackingid", "query-bug", "system", "querytests");
diff --git a/java/org/apache/commons/net/BUILD b/java/org/apache/commons/net/BUILD
index c322ecd..d83d8ec 100644
--- a/java/org/apache/commons/net/BUILD
+++ b/java/org/apache/commons/net/BUILD
@@ -6,7 +6,7 @@
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/util/ssl",
- "//lib/commons:codec",
+ "//lib:guava",
"//lib/commons:net",
],
)
diff --git a/java/org/apache/commons/net/smtp/AuthSMTPClient.java b/java/org/apache/commons/net/smtp/AuthSMTPClient.java
index 33dd609..85e4dbf 100644
--- a/java/org/apache/commons/net/smtp/AuthSMTPClient.java
+++ b/java/org/apache/commons/net/smtp/AuthSMTPClient.java
@@ -16,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.util.ssl.BlindSSLSocketFactory;
import java.io.BufferedReader;
import java.io.BufferedWriter;
@@ -33,7 +34,6 @@
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
-import org.apache.commons.codec.binary.Base64;
public class AuthSMTPClient extends SMTPClient {
private String authTypes;
@@ -134,7 +134,7 @@
}
final String enc = getReplyStrings()[0].split(" ", 2)[1];
- final byte[] nonce = Base64.decodeBase64(enc.getBytes(UTF_8));
+ final byte[] nonce = BaseEncoding.base64().decode(enc);
final String sec;
try {
Mac mac = Mac.getInstance(macName);
@@ -187,6 +187,6 @@
}
private static String encodeBase64(byte[] data) {
- return new String(Base64.encodeBase64(data), UTF_8);
+ return BaseEncoding.base64().encode(data);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index 0090ed1..abfc23d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.stream.Collectors.toList;
+import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -32,9 +33,11 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.AbandonUtil;
@@ -45,6 +48,7 @@
import java.util.List;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class AbandonIT extends AbstractDaemonTest {
@@ -136,6 +140,93 @@
}
@Test
+ @UseClockStep
+ @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
+ @GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void notAbandonedIfMergeableWhenMergeableOperatorIsEnabled() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+
+ // create 2 changes
+ int id1 = createChange().getChange().getId().get();
+ int id2 = createChange().getChange().getId().get();
+
+ // create 2 changes that conflict with each other
+ testRepo.reset(initial);
+ int id3 = createChange("change 3", "file.txt", "content").getChange().getId().get();
+ testRepo.reset(initial);
+ int id4 = createChange("change 4", "file.txt", "other content").getChange().getId().get();
+
+ // make all 4 previously created changes older than 1 week
+ TestTimeUtil.incrementClock(7 * 24, HOURS);
+
+ // create 1 new change that will not be abandoned because it is not older than 1 week
+ testRepo.reset(initial);
+ ChangeData cd = createChange().getChange();
+ int id5 = cd.getId().get();
+
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id1, id2, id3, id4, id5);
+ assertThat(query("is:abandoned")).isEmpty();
+
+ // submit one of the conflicting changes
+ gApi.changes().id(id3).current().review(ReviewInput.approve());
+ gApi.changes().id(id3).current().submit();
+ assertThat(toChangeNumbers(query("is:merged"))).containsExactly(id3);
+ assertThat(toChangeNumbers(query("-is:mergeable"))).containsExactly(id4);
+
+ abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5, id2, id1);
+ assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4);
+ }
+
+ @Test
+ @UseClockStep
+ @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
+ @GerritConfig(name = "changeCleanup.abandonIfMergeable", value = "false")
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ /**
+ * When indexMergeable is disabled then the abandonIfMergeable option is ineffective and the auto
+ * abandon behaves as though it were set to its default value (true).
+ */
+ public void abandonedIfMergeableWhenMergeableOperatorIsDisabled() throws Exception {
+ ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+
+ // create 2 changes
+ int id1 = createChange().getChange().getId().get();
+ int id2 = createChange().getChange().getId().get();
+
+ // create 2 changes that conflict with each other
+ testRepo.reset(initial);
+ int id3 = createChange("change 3", "file.txt", "content").getChange().getId().get();
+ testRepo.reset(initial);
+ int id4 = createChange("change 4", "file.txt", "other content").getChange().getId().get();
+
+ // make all 4 previously created changes older than 1 week
+ TestTimeUtil.incrementClock(7 * 24, HOURS);
+
+ // create 1 new change that will not be abandoned because it is not older than 1 week
+ testRepo.reset(initial);
+ ChangeData cd = createChange().getChange();
+ int id5 = cd.getId().get();
+
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id1, id2, id3, id4, id5);
+ assertThat(query("is:abandoned")).isEmpty();
+
+ // submit one of the conflicting changes
+ gApi.changes().id(id3).current().review(ReviewInput.approve());
+ gApi.changes().id(id3).current().submit();
+ assertThat(toChangeNumbers(query("is:merged"))).containsExactly(id3);
+
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> query("-is:mergeable"));
+ assertThat(thrown).hasMessageThat().contains("operator is not supported");
+
+ abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5);
+ assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4, id2, id1);
+ }
+
+ @Test
public void changeCleanupConfigDefaultAbandonMessage() throws Exception {
assertThat(cleanupConfig.getAbandonMessage())
.startsWith(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index e38babb..74f753d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2204,7 +2204,7 @@
gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) 2));
requestScopeOperations.setApiUser(user.id());
gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.dislike());
@@ -2212,7 +2212,26 @@
m = gApi.changes().id(r.getChangeId()).reviewer(user.id().toString()).votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short) -1));
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short) -1));
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "NONE")
+ public void listVotesEvenWhenAccountsAreNotVisible() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
+
+ requestScopeOperations.setApiUser(user.id());
+
+ // check finding by address works
+ Map<String, Short> m = gApi.changes().id(r.getChangeId()).reviewer(admin.email()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
+
+ // check finding by id works
+ m = gApi.changes().id(r.getChangeId()).reviewer(admin.id().toString()).votes();
+ assertThat(m).hasSize(1);
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 2));
}
@Test
@@ -3573,6 +3592,28 @@
}
@Test
+ public void checkSubmissionIdForAutoClosedChange() throws Exception {
+ PushOneCommit.Result first = createChange();
+ PushOneCommit.Result second = createChange();
+
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+
+ PushOneCommit.Result result = push.to("refs/heads/master");
+ result.assertOkStatus();
+
+ ChangeInfo firstChange = gApi.changes().id(first.getChangeId()).get();
+ assertThat(firstChange.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(firstChange.submissionId).isNotNull();
+
+ ChangeInfo secondChange = gApi.changes().id(second.getChangeId()).get();
+ assertThat(secondChange.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(secondChange.submissionId).isNotNull();
+
+ assertThat(secondChange.submissionId).isEqualTo(firstChange.submissionId);
+ assertThat(gApi.changes().id(second.getChangeId()).submittedTogether()).hasSize(2);
+ }
+
+ @Test
public void maxPermittedValueAllowed() throws Exception {
final int minPermittedValue = -2;
final int maxPermittedValue = +2;
@@ -4396,6 +4437,20 @@
}
}
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void changeQueryReturnsMergeableWhenGerritIndexMergeable() throws Exception {
+ String changeId = createChange().getChangeId();
+ assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ public void changeQueryDoesNotReturnMergeableWhenGerritDoesNotIndexMergeable() throws Exception {
+ String changeId = createChange().getChangeId();
+ assertThat(gApi.changes().query(changeId).get().get(0).mergeable).isNull();
+ }
+
private PushOneCommit.Result createWorkInProgressChange() throws Exception {
return pushTo("refs/for/master%wip");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/BUILD b/javatests/com/google/gerrit/acceptance/api/group/BUILD
index 9e08069..1ba1138 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/BUILD
+++ b/javatests/com/google/gerrit/acceptance/api/group/BUILD
@@ -19,7 +19,6 @@
name = "util",
srcs = ["GroupAssert.java"],
deps = [
- "//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
"//lib/truth",
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index e543976..988580e 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -187,6 +187,24 @@
}
@Test
+ public void removeMember_nullInMemberInputDoesNotCauseFailure() throws Exception {
+ AccountGroup.UUID group =
+ groupOperations.newGroup().addMember(admin.id()).addMember(user.id()).create();
+ gApi.groups().id(group.get()).removeMembers(user.id().toString(), null);
+ ImmutableSet<Account.Id> members = groupOperations.group(group).get().members();
+ assertThat(members).containsExactly(admin.id());
+ }
+
+ @Test
+ public void removeMember_emptyStringInMemberInputDoesNotCauseFailure() throws Exception {
+ AccountGroup.UUID group =
+ groupOperations.newGroup().addMember(admin.id()).addMember(user.id()).create();
+ gApi.groups().id(group.get()).removeMembers(user.id().toString(), "");
+ ImmutableSet<Account.Id> members = groupOperations.group(group).get().members();
+ assertThat(members).containsExactly(admin.id());
+ }
+
+ @Test
public void cachedGroupsForMemberAreUpdatedOnMemberAdditionAndRemoval() throws Exception {
String username = name("user");
Account.Id accountId = accountOperations.newAccount().username(username).create();
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index 78c8209..2801b36 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -69,11 +69,16 @@
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.project.CommentLinkInfoImpl;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Module;
import java.util.HashMap;
import java.util.Map;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
@@ -851,6 +856,58 @@
assertCommentLinks(getConfig(project), expected);
}
+ @Test
+ public void cannotPushLabelDefinitionWithDuplicateValues() throws Exception {
+ Config cfg = new Config();
+ cfg.fromText(projectOperations.project(allProjects).getConfig().toText());
+ cfg.setStringList(
+ "label",
+ "Code-Review",
+ "value",
+ ImmutableList.of("+1 LGTM", "1 LGTM", "0 No Value", "-1 Looks Bad"));
+
+ TestRepository<InMemoryRepository> repo = cloneProject(allProjects);
+ GitUtil.fetch(repo, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
+ repo.reset(RefNames.REFS_CONFIG);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(admin.newIdent(), repo, "Subject", "project.config", cfg.toText())
+ .to(RefNames.REFS_CONFIG);
+ r.assertErrorStatus("invalid project configuration");
+ r.assertMessage("project.config: duplicate value \"1 lgtm\" for label \"code-review\"");
+ }
+
+ @Test
+ public void getProjectThatHasLabelDefinitionWithDuplicateValues() throws Exception {
+ // Update the definition of the Code-Review label so that it has the value "+1 LGTM" twice.
+ // This update bypasses all validation checks so that the duplicate label value doesn't get
+ // rejected.
+ Config cfg = new Config();
+ cfg.fromText(projectOperations.project(allProjects).getConfig().toText());
+ cfg.setStringList(
+ "label",
+ "Code-Review",
+ "value",
+ ImmutableList.of("+1 LGTM", "1 LGTM", "0 No Value", "-1 Looks Bad"));
+
+ try (TestRepository<Repository> repo =
+ new TestRepository<>(repoManager.openRepository(allProjects))) {
+ repo.update(
+ RefNames.REFS_CONFIG,
+ repo.commit()
+ .message("Set label with duplicate value")
+ .parent(getHead(repo.getRepository(), RefNames.REFS_CONFIG))
+ .add(ProjectConfig.PROJECT_CONFIG, cfg.toText()));
+ }
+
+ // Verify that project info can be retrieved and that the label value "+1 LGTM" appears only
+ // once.
+ ProjectInfo projectInfo = gApi.projects().name(allProjects.get()).get();
+ assertThat(projectInfo.labels.keySet()).containsExactly("Code-Review");
+ assertThat(projectInfo.labels.get("Code-Review").values)
+ .containsExactly("+1", "LGTM", " 0", "No Value", "-1", "Looks Bad");
+ }
+
private CommentLinkInfo commentLinkInfo(String name, String match, String link) {
return new CommentLinkInfoImpl(name, match, link, null /*html*/, null /*enabled*/);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 24ac209..2a6b42c 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -41,6 +41,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
@@ -578,6 +579,109 @@
}
@Test
+ public void addedNewlineAtEndOfFileIsMarkedInDiffWhenOtherwiseOnlyEditsDueToRebaseExist()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70 modification
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(102);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void addedNewlineAtEndOfFileIsNotIdentifiedAsDueToRebaseEvenThoughItShould()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ // Add a comment so that file contents are not 'skipped'. To be able to add a comment, touch
+ // (= modify) the file in the change.
+ addModifiedPatchSet(
+ changeId, FILE_NAME, fileContent -> fileContent.replace("Line 2\n", "Line two\n"));
+ CommentInput comment = createCommentInput(3, 0, 4, 0, "Comment to not skip file content.");
+ addCommentTo(changeId, CURRENT, FILE_NAME, comment);
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).linesOfA().isNull();
+ assertThat(diffInfo).content().element(1).linesOfB().containsExactly("");
+ // This should actually be isDueToRebase().
+ assertThat(diffInfo).content().element(1).isNotDueToRebase();
+ }
+
+ @Test
+ public void
+ addedNewlineAtEndOfFileIsMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceConsidered()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(1).isNotNull(); // Line 70.5 insertion
+ assertThat(diffInfo).content().element(2).commonLines().isNotEmpty();
+ assertThat(diffInfo).content().element(3).linesOfA().containsExactly("Line 101");
+ assertThat(diffInfo).content().element(3).linesOfB().containsExactly("Line 101", "");
+
+ assertThat(diffInfo).metaA().totalLineCount().isEqualTo(101);
+ assertThat(diffInfo).metaB().totalLineCount().isEqualTo(103);
+ }
+
+ @Test
+ // TODO: Fix this issue. This test documents the current behavior and ensures that we at least
+ // don't run into an internal server error.
+ public void
+ addedNewlineAtEndOfFileIsNotMarkedWhenEditDueToRebaseIncreasedLineCountAndWhitespaceIgnoredEvenThoughItShould()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line 70\nLine 70.5\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("\n"));
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .withBase(previousPatchSetId)
+ .get();
+ assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
+ }
+
+ @Test
public void addedLastLineWithoutNewlineBeforeAndAfterwardsIsMarkedInDiff() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, fileContent -> fileContent.concat("Line 101"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
@@ -2374,9 +2478,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2395,6 +2497,112 @@
}
@Test
+ public void
+ diffOfFileWithOnlyRebaseHunksAndWithCommentAndConsideringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void diffOfFileWithOnlyRebaseHunksAndWithCommentAndIgnoringWhitespaceReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.replace("Line 70\n", "Line seventy\n");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_ALL)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ String baseFileContent = FILE_CONTENT.concat("Line 101");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, baseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = baseFileContent.concat("\nLine 102\nLine 103\n");
+ ObjectId commit3 = addCommit(commit2, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit3);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
+ public void
+ diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents()
+ throws Exception {
+ addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
+ String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
+ String newBaseFileContent = FILE_CONTENT.concat("Line 101\nLine 103\nLine 104");
+ ObjectId commit2 = addCommit(commit1, FILE_NAME, newBaseFileContent);
+ rebaseChangeOn(changeId, commit2);
+ CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
+
+ DiffInfo diffInfo =
+ getDiffRequest(changeId, CURRENT, FILE_NAME)
+ .withBase(previousPatchSetId)
+ .withWhitespace(DiffPreferencesInfo.Whitespace.IGNORE_NONE)
+ .get();
+ // We don't list the full file contents here as that is not the focus of this test.
+ assertThat(diffInfo)
+ .content()
+ .element(0)
+ .commonLines()
+ .containsAtLeast("Line 1", "Line two", "Line 3", "Line 4", "Line 5")
+ .inOrder();
+ }
+
+ @Test
public void diffOfNonExistentFileIsAnEmptyDiffResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
@@ -2432,9 +2640,7 @@
changeId, FILE_NAME, content -> content.replace("Line 20\n", "Line twenty\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(20, 0, 21, 0, "Should be 'Line 20'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
addModifiedPatchSet(
changeId, FILE_NAME2, content -> content.replace("2nd line\n", "Second line\n"));
@@ -2476,9 +2682,7 @@
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
CommentInput comment = createCommentInput(2, 0, 3, 0, "Should be 'Line 2'.");
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.comments = ImmutableMap.of(FILE_NAME, ImmutableList.of(comment));
- gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ addCommentTo(changeId, previousPatchSetId, FILE_NAME, comment);
String newFilePath = "a_new_file.txt";
gApi.changes().id(changeId).edit().renameFile(FILE_NAME, newFilePath);
gApi.changes().id(changeId).edit().publish();
@@ -2507,6 +2711,14 @@
return comment;
}
+ private void addCommentTo(
+ String changeId, String previousPatchSetId, String fileName, CommentInput comment)
+ throws RestApiException {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.comments = ImmutableMap.of(fileName, ImmutableList.of(comment));
+ gApi.changes().id(changeId).revision(previousPatchSetId).review(reviewInput);
+ }
+
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index ad73e0f..61d0fd5 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -46,6 +46,7 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
@@ -1062,6 +1063,7 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void mergeable() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index f0312de..d1d197b 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -1446,7 +1446,7 @@
private TestRefAdvertiser.Result getReceivePackRefs() throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
AdvertiseRefsHook adv =
- ReceiveCommitsAdvertiseRefsHookChain.createForTest(queryProvider, project);
+ ReceiveCommitsAdvertiseRefsHookChain.createForTest(queryProvider, project, admin.id());
ReceivePack rp = new ReceivePack(repo);
rp.setAdvertiseRefsHook(adv);
TestRefAdvertiser advertiser = new TestRefAdvertiser(repo);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index aaf52a1..eff98b3 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -1205,7 +1205,7 @@
}
@Test
- @GerritConfig(name = "index.reindexAfterRefUpdate", value = "true")
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void submitSchedulesOpenChangesOfSameBranchForReindexing() throws Throwable {
// Create a merged change.
PushOneCommit push =
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 8b51e7f..911a04d 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -72,13 +72,28 @@
}
@Test
- public void changeActionOneMergedChangeHasReverts() throws Exception {
+ public void changeActionOneMergedChangeHasOnlyNormalRevert() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
gApi.changes().id(changeId).current().review(ReviewInput.approve());
gApi.changes().id(changeId).current().submit();
Map<String, ActionInfo> actions = getChangeActions(changeId);
assertThat(actions).containsKey("revert");
- assertThat(actions).containsKey("revert_submission");
+ assertThat(actions).doesNotContainKey("revert_submission");
+ }
+
+ @Test
+ public void changeActionTwoMergedChangesHaveReverts() throws Exception {
+ String changeId1 = createChangeWithTopic().getChangeId();
+ String changeId2 = createChangeWithTopic().getChangeId();
+ gApi.changes().id(changeId1).current().review(ReviewInput.approve());
+ gApi.changes().id(changeId2).current().review(ReviewInput.approve());
+ gApi.changes().id(changeId2).current().submit();
+ Map<String, ActionInfo> actions1 = getChangeActions(changeId1);
+ assertThat(actions1).containsKey("revert");
+ assertThat(actions1).containsKey("revert_submission");
+ Map<String, ActionInfo> actions2 = getChangeActions(changeId2);
+ assertThat(actions2).containsKey("revert");
+ assertThat(actions2).containsKey("revert_submission");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
index 3030b02..ea3a6a0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
@@ -22,6 +22,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -58,6 +59,7 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void indexChangeAfterOwnerLosesVisibility() throws Exception {
// Create a test group with 2 users as members
TestAccount user2 = accountCreator.user2();
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java b/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java
index ae17be0..8baeffc 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java
@@ -18,6 +18,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.Ordering;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo;
@@ -26,7 +27,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
-import org.eclipse.jgit.util.Base64;
import org.junit.Test;
public class ListCachesIT extends AbstractDaemonTest {
@@ -78,7 +78,7 @@
public void listCacheNamesTextList() throws Exception {
RestResponse r = adminRestSession.get("/config/server/caches/?format=TEXT_LIST");
r.assertOK();
- String result = new String(Base64.decode(r.getEntityContent()), UTF_8.name());
+ String result = new String(BaseEncoding.base64().decode(r.getEntityContent()), UTF_8);
List<String> list = Arrays.asList(result.split("\n"));
assertThat(list).contains("accounts");
assertThat(list).contains("projects");
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
index 996119d..9573eb0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java
@@ -180,6 +180,9 @@
assertThat(i.gerrit.allUsers).isEqualTo(AllUsersNameProvider.DEFAULT);
assertThat(i.gerrit.reportBugUrl).isNull();
+ // index
+ assertThat(i.index.change.indexMergeable).isNull(); // false in all tests
+
// plugin
assertThat(i.plugin.jsResourcePaths).isEmpty();
@@ -206,4 +209,11 @@
ServerInfo i = gApi.config().server().getInfo();
assertThat(i.change.excludeMergeableInChangeInfo).isTrue();
}
+
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
+ public void indexMergeableIsTrueWhenTrueInConfig() throws Exception {
+ ServerInfo i = gApi.config().server().getInfo();
+ assertThat(i.index.change.indexMergeable).isTrue();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/group/ListGroupsIT.java b/javatests/com/google/gerrit/acceptance/rest/group/ListGroupsIT.java
new file mode 100644
index 0000000..d8132b7
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/group/ListGroupsIT.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.group;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.common.GroupInfo;
+import com.google.gson.reflect.TypeToken;
+import java.util.Map;
+import org.junit.Test;
+
+public class ListGroupsIT extends AbstractDaemonTest {
+ @Test
+ public void listAllGroups() throws Exception {
+ RestResponse response = adminRestSession.get("/groups/");
+ response.assertOK();
+
+ Map<String, GroupInfo> groupMap =
+ newGson()
+ .fromJson(response.getReader(), new TypeToken<Map<String, GroupInfo>>() {}.getType());
+ assertThat(groupMap.keySet()).containsExactly("Administrators", "Non-Interactive Users");
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 91a10ca..3e9b1f6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -604,7 +604,7 @@
}
@Test
- public void syncCreateGroupPermission() throws Exception {
+ public void syncCreateGroupPermission_addAndRemoveCreateGroupCapability() throws Exception {
// Grant CREATE_GROUP to Registered Users
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSection = newAccessSectionInfo();
@@ -642,6 +642,44 @@
}
@Test
+ public void syncCreateGroupPermission_addCreateGroupCapabilityToMultipleGroups()
+ throws Exception {
+ PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+
+ // Grant CREATE_GROUP to Registered Users
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSection = newAccessSectionInfo();
+ PermissionInfo createGroup = newPermissionInfo();
+ createGroup.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
+ accessSection.permissions.put(GlobalCapability.CREATE_GROUP, createGroup);
+ accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSection);
+ gApi.projects().name(allProjects.get()).access(accessInput);
+
+ // Grant CREATE_GROUP to Administrators
+ accessInput = newProjectAccessInput();
+ accessSection = newAccessSectionInfo();
+ createGroup = newPermissionInfo();
+ createGroup.rules.put(adminGroupUuid().get(), pri);
+ accessSection.permissions.put(GlobalCapability.CREATE_GROUP, createGroup);
+ accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSection);
+ gApi.projects().name(allProjects.get()).access(accessInput);
+
+ // Assert that the permissions were synced from All-Projects (global) to All-Users (ref)
+ Map<String, AccessSectionInfo> local = gApi.projects().name("All-Users").access().local;
+ assertThat(local).isNotNull();
+ assertThat(local).containsKey(RefNames.REFS_GROUPS + "*");
+ Map<String, PermissionInfo> permissions = local.get(RefNames.REFS_GROUPS + "*").permissions;
+ assertThat(permissions).hasSize(2);
+ // READ is the default permission and should be preserved by the syncer
+ assertThat(permissions.keySet()).containsExactly(Permission.READ, Permission.CREATE);
+ Map<String, PermissionRuleInfo> rules = permissions.get(Permission.CREATE).rules;
+ assertThat(rules.keySet())
+ .containsExactly(SystemGroupBackend.REGISTERED_USERS.get(), adminGroupUuid().get());
+ assertThat(rules.get(SystemGroupBackend.REGISTERED_USERS.get())).isEqualTo(pri);
+ assertThat(rules.get(adminGroupUuid().get())).isEqualTo(pri);
+ }
+
+ @Test
public void addAccessSectionForInvalidRef() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
index 28e8b14..57a1e56 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java
@@ -169,6 +169,21 @@
}
@Test
+ public void cannotCreateLabelWithDuplicateValues() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ // Positive values can be specified as '<value>' or '+<value>'.
+ input.values =
+ ImmutableMap.of(
+ "+1", "Looks Good", "1", "Looks Good", "0", "Don't Know", "-1", "Looks Bad");
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(allProjects.get()).label("Foo").create(input));
+ assertThat(thrown).hasMessageThat().contains("duplicate value: 1");
+ }
+
+ @Test
public void cannotCreateLabelWithInvalidDefaultValue() throws Exception {
LabelDefinitionInput input = new LabelDefinitionInput();
input.values = ImmutableMap.of("+1", "Looks Good", "0", "Don't Know", "-1", "Looks Bad");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
index 9cba930..97b795f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
@@ -323,6 +323,21 @@
}
@Test
+ public void cannotSetDuplicateValues() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ // Positive values can be specified as '<value>' or '+<value>'.
+ input.values =
+ ImmutableMap.of(
+ "+1", "Looks Good", "1", "Looks Good", "0", "Don't Know", "-1", "Looks Bad");
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(allProjects.get()).label("Code-Review").update(input));
+ assertThat(thrown).hasMessageThat().contains("duplicate value: 1");
+ }
+
+ @Test
public void updateDefaultValue() throws Exception {
LabelDefinitionInput input = new LabelDefinitionInput();
input.defaultValue = 1;
diff --git a/javatests/com/google/gerrit/acceptance/rest/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
index 220254b..df6a264 100644
--- a/javatests/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/revision/RevisionIT.java
@@ -19,13 +19,13 @@
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.io.BaseEncoding;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.util.Base64;
import org.junit.Test;
public class RevisionIT extends AbstractDaemonTest {
@@ -50,7 +50,7 @@
+ FILE_NAME
+ "/content?parent=1");
response.assertOK();
- assertThat(new String(Base64.decode(response.getEntityContent()), UTF_8))
+ assertThat(new String(BaseEncoding.base64().decode(response.getEntityContent()), UTF_8))
.isEqualTo(parentContent);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 65f8aa0..ba41d7e 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -319,8 +319,6 @@
@Test
public void postCommentsUnreachableData() throws Exception {
- requestScopeOperations.setApiUser(admin.id());
-
String file = "file";
PushOneCommit push =
pushFactory.create(admin.newIdent(), testRepo, "first subject", file, "l1\nl2\n");
diff --git a/javatests/com/google/gerrit/common/BUILD b/javatests/com/google/gerrit/common/BUILD
index 4ec9581..c7b21a3 100644
--- a/javatests/com/google/gerrit/common/BUILD
+++ b/javatests/com/google/gerrit/common/BUILD
@@ -7,7 +7,6 @@
deps = [
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/common:version",
- "//java/com/google/gerrit/entities",
"//java/com/google/gerrit/launcher",
"//lib:guava",
"//lib/auto:auto-value",
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 73e7eca..c15f1a7 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -51,7 +51,7 @@
case V6_7:
return "blacktop/elasticsearch:6.7.2";
case V6_8:
- return "blacktop/elasticsearch:6.8.4";
+ return "blacktop/elasticsearch:6.8.5";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
diff --git a/javatests/com/google/gerrit/mail/BUILD b/javatests/com/google/gerrit/mail/BUILD
index bd2c478..3d63844 100644
--- a/javatests/com/google/gerrit/mail/BUILD
+++ b/javatests/com/google/gerrit/mail/BUILD
@@ -15,7 +15,6 @@
"//lib:guava-retrying",
"//lib:jgit",
"//lib:jgit-junit",
- "//lib/commons:codec",
"//lib/guice",
"//lib/truth",
"//lib/truth:truth-java8-extension",
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index fd6c512..59ed018 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -74,7 +74,6 @@
"//lib:protobuf",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
- "//lib/commons:codec",
"//lib/flogger:api",
"//lib/guice",
"//lib/mockito",
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index 9a48a68..3dfbefe 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -18,6 +18,7 @@
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import org.eclipse.jgit.lib.Config;
import org.junit.Ignore;
@Ignore
@@ -26,8 +27,34 @@
super(
new ChangeQueryBuilder.Definition<>(FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(
- null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, null, indexes, null, null, null, null, null, null, null, null));
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ indexes,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ new Config()));
}
@Operator
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 231340d..9c12052 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -41,6 +41,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Streams;
import com.google.common.truth.ThrowableSubject;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
@@ -79,6 +80,7 @@
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
@@ -2023,6 +2025,7 @@
}
@Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "true")
public void mergeable() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create());
@@ -2040,7 +2043,7 @@
// If a change gets submitted, the remaining open changes get reindexed asynchronously to update
// their mergeability information. If the further assertions in this test are done before the
// asynchronous reindex completed they fail because the mergeability information in the index
- // was not updated yet. To avoid this flakiness reindexAfterRefUpdate is switched off for the
+ // was not updated yet. To avoid this flakiness indexMergeable is switched off for the
// tests and we index change2 synchronously here.
gApi.changes().id(change2.getChangeId()).index();
@@ -3079,6 +3082,20 @@
}
}
+ @Test
+ @GerritConfig(name = "index.change.indexMergeable", value = "false")
+ public void mergeableFailsWhenNotIndexed() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ RevCommit commit1 = repo.parseBody(repo.commit().add("file1", "contents1").create());
+ insert(repo, newChangeForCommit(repo, commit1));
+
+ Throwable thrown = assertThrows(Throwable.class, () -> assertQuery("status:open is:mergeable"));
+ assertThat(thrown.getCause()).isInstanceOf(QueryParseException.class);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("'is:mergeable' operator is not supported by server");
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
return newChange(repo, null, null, null, null, false);
}
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index d0162d3..e5b51e7 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -16,12 +16,14 @@
"//prolog:gerrit-prolog-common",
],
deps = [
+ "//java/com/google/gerrit/acceptance/config",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/index",
+ "//java/com/google/gerrit/index:query_exception",
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/project/testing:project-test-util",
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index a2f800a..5860c48 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.InMemoryTestEnvironment;
import com.google.inject.Inject;
@@ -51,6 +50,7 @@
public class BatchUpdateTest {
private static final int MAX_UPDATES = 4;
+ private static final int MAX_PATCH_SETS = 3;
@Rule
public InMemoryTestEnvironment testEnvironment =
@@ -58,6 +58,7 @@
() -> {
Config cfg = new Config();
cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
+ cfg.setInt("change", null, "maxPatchSets", MAX_PATCH_SETS);
return cfg;
});
@@ -106,11 +107,10 @@
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Excessive update"));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -118,17 +118,16 @@
@Test
public void cannotExceedMaxUpdatesCountingMultipleChangeUpdatesInSingleBatch() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Update on PS1", PatchSet.id(id, 1)));
bu.addOp(id, new AddMessageOp("Update on PS2", PatchSet.id(id, 2)));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES - 1);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -187,7 +186,7 @@
@Test
public void exceedingMaxUpdatesAllowedWithSubmitAfterOtherOp() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Message on PS1", PatchSet.id(id, 1)));
@@ -222,6 +221,28 @@
assertThat(getMetaId(id)).isNotEqualTo(oldMetaId);
}
+ @Test
+ public void limitPatchSetCount_exceed() throws Exception {
+ Change.Id changeId = createChangeWithPatchSets(MAX_PATCH_SETS);
+ ObjectId oldMetaId = getMetaId(changeId);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("kaboom").create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, MAX_PATCH_SETS + 1), commitId)
+ .setMessage("kaboom"));
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Change " + changeId + " may not exceed " + MAX_PATCH_SETS + " patch sets");
+ }
+ assertThat(changeNotesFactory.create(project, changeId).getPatchSets()).hasSize(MAX_PATCH_SETS);
+ assertThat(getMetaId(changeId)).isEqualTo(oldMetaId);
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);
@@ -243,21 +264,22 @@
return id;
}
- private Change.Id createChangeWithTwoPatchSets(int totalUpdates) throws Exception {
- Change.Id id = createChangeWithUpdates(totalUpdates - 1);
+ private Change.Id createChangeWithPatchSets(int patchSets) throws Exception {
+ checkArgument(patchSets >= 2);
+ Change.Id id = createChangeWithUpdates(MAX_UPDATES - 2);
ChangeNotes notes = changeNotesFactory.create(project, id);
-
- try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
- ObjectId commitId = repo.amend(notes.getCurrentPatchSet().commitId()).message("PS2").create();
- bu.addOp(
- id,
- patchSetInserterFactory
- .create(notes, PatchSet.id(id, 2), commitId)
- .setMessage("Add PS2"));
- bu.execute();
+ for (int i = 2; i <= patchSets; ++i) {
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("PS" + i).create();
+ bu.addOp(
+ id,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(id, i), commitId)
+ .setMessage("Add PS" + i));
+ bu.execute();
+ }
}
-
- assertThat(getUpdateCount(id)).isEqualTo(totalUpdates);
return id;
}
@@ -291,7 +313,7 @@
}
}
- private int getUpdateCount(Change.Id changeId) throws Exception {
+ private int getUpdateCount(Change.Id changeId) {
return changeNotesFactory.create(project, changeId).getUpdateCount();
}
diff --git a/javatests/com/google/gerrit/server/util/git/BUILD b/javatests/com/google/gerrit/server/util/git/BUILD
index 883898f..b789dff 100644
--- a/javatests/com/google/gerrit/server/util/git/BUILD
+++ b/javatests/com/google/gerrit/server/util/git/BUILD
@@ -18,7 +18,6 @@
"//lib:protobuf",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
- "//lib/commons:codec",
"//lib/guice",
"//lib/truth",
"//lib/truth:truth-java8-extension",
diff --git a/lib/js/bower_archives.bzl b/lib/js/bower_archives.bzl
index 34742db..0b2fdef9 100644
--- a/lib/js/bower_archives.bzl
+++ b/lib/js/bower_archives.bzl
@@ -26,10 +26,10 @@
sha1 = "849ad3ee7c77506548b7b5db603a4e150b9431aa",
)
bower_archive(
- name = "font-roboto",
- package = "PolymerElements/font-roboto",
+ name = "font-roboto-local",
+ package = "PolymerElements/font-roboto-local",
version = "1.1.0",
- sha1 = "ab4218d87b9ce569d6282b01f7642e551879c3d5",
+ sha1 = "de651abf9b1b2d0935f7b264d48131677196412f",
)
bower_archive(
name = "iron-a11y-announcer",
@@ -136,8 +136,10 @@
bower_archive(
name = "paper-styles",
package = "PolymerElements/paper-styles",
- version = "2.1.0",
- sha1 = "c143c5491571a6922c06ffe7fdf64ec009ec2eb1",
+ # Basically 2.1.0 but with
+ # https://github.com/PolymerElements/paper-styles/pull/165 applied
+ version = "a6c207e6eee3402fd7a6550e6f9c387ca22ec4c4",
+ sha1 = "6bd17410578b5d4017ccef330393a4b41b1c716e",
)
bower_archive(
name = "shadycss",
diff --git a/lib/js/bower_components.bzl b/lib/js/bower_components.bzl
index 658074e..7fd61c7 100644
--- a/lib/js/bower_components.bzl
+++ b/lib/js/bower_components.bzl
@@ -30,7 +30,7 @@
seed = True,
)
bower_component(
- name = "font-roboto",
+ name = "font-roboto-local",
license = "//lib:LICENSE-polymer",
)
bower_component(
@@ -283,7 +283,7 @@
name = "paper-styles",
license = "//lib:LICENSE-polymer",
deps = [
- ":font-roboto",
+ ":font-roboto-local",
":iron-flex-layout",
":polymer",
],
diff --git a/plugins/codemirror-editor b/plugins/codemirror-editor
index 92ce310..9508693 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit 92ce310ecf717133601b9e824c38bc5e5eafecba
+Subproject commit 9508693bd0a85560674143256314beac08f2d8ca
diff --git a/plugins/delete-project b/plugins/delete-project
index 38f4fde..39dd25c 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 38f4fde24ce24cf1bf8d1e4d074f1d784ed983b8
+Subproject commit 39dd25c822be14a69282d38e5205f0ca4f2902c1
diff --git a/plugins/hooks b/plugins/hooks
index f4bf0ff..6316be2 160000
--- a/plugins/hooks
+++ b/plugins/hooks
@@ -1 +1 @@
-Subproject commit f4bf0ffbd13a748cc46a3368a8fadcc2cbab6e21
+Subproject commit 6316be2828808dafc546ecd11c055396d0b4951b
diff --git a/plugins/replication b/plugins/replication
index d7c09fb..4689b41 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit d7c09fbb4c18b1743d6060d361171c2a5237f22b
+Subproject commit 4689b419eab61ea204daf2dfca47296667ac317c
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 5ff820b..3a70ac53 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -55,7 +55,6 @@
exclude = [
"bower_components/**",
"**/*_test.html",
- "embed/test.html",
"test/**",
"samples/**",
],
@@ -166,33 +165,6 @@
],
) for directory in DIRECTORIES]
-# Embed bundle
-polygerrit_bundle(
- name = "polygerrit_embed_ui",
- srcs = glob(
- [
- "**/*.html",
- "**/*.js",
- ],
- exclude = [
- "bower_components/**",
- "test/**",
- "**/*_test.html",
- ],
- ),
- outs = ["polygerrit_embed_ui.zip"],
- app = "embed/embed.html",
-)
-
-filegroup(
- name = "embed_test_files",
- srcs = glob(
- [
- "embed/**/*_test.html",
- ],
- ),
-)
-
filegroup(
name = "template_test_srcs",
srcs = [
@@ -200,21 +172,3 @@
"template_test_srcs/template_test.js",
],
)
-
-sh_test(
- name = "embed_test",
- size = "small",
- srcs = ["embed_test.sh"],
- data = [
- "embed/test.html",
- "test/common-test-setup.html",
- ":embed_test_files",
- ":pg_code.zip",
- ":test_components.zip",
- ],
- # Should not run sandboxed.
- tags = [
- "local",
- "manual",
- ],
-)
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
index ffc1b9b..1d6e706 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
@@ -43,11 +43,6 @@
.value {
width: 32em;
}
- gr-autocomplete {
- --gr-autocomplete: {
- padding: 0 var(--spacing-xs);
- }
- }
.hide {
display: none;
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
index 8d03595..b78090c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
@@ -40,14 +40,7 @@
width: 20em;
}
gr-autocomplete {
- border: none;
- --gr-autocomplete: {
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- height: 2em;
- padding: 0 var(--spacing-xs);
- width: 20em;
- }
+ width: 20em;
}
</style>
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
index bc7a109..cf24793 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
@@ -48,10 +48,6 @@
}
gr-autocomplete {
width: 20em;
- --gr-autocomplete: {
- height: 2em;
- width: 20em;
- }
}
a {
color: var(--primary-text-color);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index b7e615c..f1338fb 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -92,7 +92,6 @@
a {
color: inherit;
cursor: pointer;
- display: inline-block;
text-decoration: none;
}
a:hover {
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
index 3652e70..cbc7bcd 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
+++ b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
@@ -50,9 +50,13 @@
this.$.createOverlay.close();
}
- _pickerConfirm() {
+ _pickerConfirm(e) {
this.$.createOverlay.close();
const detail = {repo: this._repo, branch: this._branch};
+ // e is a 'confirm' event from gr-dialog. We want to fire a more detailed
+ // 'confirm' event here, so let's stop propagation of the bare event.
+ e.preventDefault();
+ e.stopPropagation();
this.dispatchEvent(new CustomEvent('confirm', {detail, bubbles: false}));
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 0817762..6703651 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -269,6 +269,25 @@
Do you really want to delete the edit?
</div>
</gr-dialog>
+ <gr-dialog
+ id="showRevertSubmissionChangesDialog"
+ class="confirmDialog"
+ confirm-label="Close"
+ cancel-label=''
+ on-confirm="_handleShowRevertSubmissionChangesConfirm">
+ <div class="header" slot="header">
+ Reverted Changes
+ </div>
+ <div class="main" slot="main">
+ <template is="dom-repeat" items="[[_revertChanges]]">
+ <div>
+ <a href$="[[item.link]]" target="_blank">
+ Change [[item._number]]
+ </a>
+ </div>
+ </template>
+ </div>
+ </gr-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 2ce0d7d..8ad2a06 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -420,6 +420,10 @@
type: Boolean,
value: true,
},
+ _revertChanges: {
+ type: Array,
+ value: [],
+ },
};
}
@@ -1258,6 +1262,7 @@
_handleResponse(action, response) {
if (!response) { return; }
return this.$.restAPI.getResponseObject(response).then(obj => {
+ let revertChanges = [];
switch (action.__key) {
case ChangeActions.REVERT:
this._waitForChangeReachable(obj._number)
@@ -1282,6 +1287,29 @@
case ChangeActions.REBASE_EDIT:
Gerrit.Nav.navigateToChange(this.change);
break;
+ case ChangeActions.REVERT_SUBMISSION:
+ revertChanges = obj.revert_changes || [];
+ revertChanges = revertChanges.map(change => {
+ change.link = '/q/' + encodeURIComponent(change.change_id);
+ return change;
+ });
+ // list of reverted changes can never be 0
+ if (revertChanges.length === 1) {
+ // redirect to the change if only 1 change is reverted
+ const change = revertChanges[0];
+ this._waitForChangeReachable(change._number).then(success => {
+ if (success) {
+ Gerrit.Nav.navigateToChange(change);
+ } else {
+ console.error('Change ' + change._number + ' not reachable');
+ }
+ });
+ } else {
+ // show multiple reverted changes in a dialog
+ this._revertChanges = revertChanges;
+ this._showActionDialog(this.$.showRevertSubmissionChangesDialog);
+ }
+ break;
default:
this.dispatchEvent(new CustomEvent('reload-change',
{detail: {action: action.__key}, bubbles: false}));
@@ -1290,6 +1318,10 @@
});
}
+ _handleShowRevertSubmissionChangesConfirm() {
+ this._hideAllDialogs();
+ }
+
_handleResponseError(action, response, body) {
if (action && action.__key === RevisionActions.CHERRYPICK) {
if (response && response.status === 409 &&
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index a10f028..532c573 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -1422,6 +1422,7 @@
let payload;
let onShowError;
let onShowAlert;
+ let getResponseObjectStub;
setup(() => {
cleanup = sinon.stub();
@@ -1437,12 +1438,18 @@
suite('happy path', () => {
let sendStub;
-
+ let waitForChangeReachableStub;
setup(() => {
sandbox.stub(element, 'fetchChangeUpdates')
.returns(Promise.resolve({isLatest: true}));
sendStub = sandbox.stub(element.$.restAPI, 'executeChangeAction')
.returns(Promise.resolve({}));
+ getResponseObjectStub = sandbox.stub(element.$.restAPI,
+ 'getResponseObject');
+ waitForChangeReachableStub = sandbox.stub(element,
+ '_waitForChangeReachable').returns(Promise.resolve(true));
+ sandbox.stub(Gerrit.Nav,
+ 'navigateToChange').returns(Promise.resolve(true));
});
test('change action', () => {
@@ -1455,6 +1462,49 @@
});
});
+ suite('single changes revert', () => {
+ setup(() => {
+ getResponseObjectStub
+ .returns(Promise.resolve({revert_changes: [
+ {change_id: 12345},
+ ]}));
+ showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ });
+
+ test('revert submission single change', done => {
+ element._send('POST', {message: 'Revert submission'},
+ '/revert_submission', false, cleanup).then(res => {
+ element._handleResponse({__key: 'revert_submission'}, {}).
+ then(() => {
+ assert.isTrue(waitForChangeReachableStub.called);
+ done();
+ });
+ });
+ });
+ });
+
+ suite('multiple changes revert', () => {
+ let showActionDialogStub;
+ setup(() => {
+ getResponseObjectStub
+ .returns(Promise.resolve({revert_changes: [
+ {change_id: 12345}, {change_id: 23456},
+ ]}));
+ showActionDialogStub = sandbox.stub(element, '_showActionDialog');
+ });
+
+ test('revert submission multiple change', done => {
+ element._send('POST', {message: 'Revert submission'},
+ '/revert_submission', false, cleanup).then(res => {
+ element._handleResponse({__key: 'revert_submission'}, {}).then(
+ () => {
+ assert.isTrue(showActionDialogStub.called);
+ done();
+ });
+ });
+ });
+ });
+
test('revision action', () => {
return element._send('DELETE', payload, '/endpoint', true, cleanup)
.then(() => {
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 a5a5aa5..539b1b6 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
@@ -177,9 +177,10 @@
<span class="title">Assignee</span>
<span class="value">
<gr-account-list
- max-count="1"
id="assigneeValue"
placeholder="Set assignee..."
+ max-count="1"
+ skip-suggest-on-empty
accounts="{{_assignee}}"
readonly="[[_computeAssigneeReadOnly(_mutable, change)]]"
suggestions-provider="[[_getReviewerSuggestionsProvider(change)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.html b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.html
index e20bbd7..4ddc876 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.html
@@ -27,16 +27,13 @@
<template>
<style include="shared-styles">
:host {
- background-color: var(--dialog-background-color);
display: block;
+ padding: var(--spacing-m) 0;
}
section {
display: flex;
padding: var(--spacing-m) var(--spacing-xl);
}
- section:not(:first-of-type) {
- border-top: 1px solid var(--border-color);
- }
.flexContainer {
display: flex;
justify-content: space-between;
@@ -76,9 +73,9 @@
}
</style>
<section>
- <span class="title">
+ <h3 class="title">
Patch set [[patchNum]] of [[_computePatchSetQuantity(change.revisions)]]
- </span>
+ </h3>
</section>
<section class$="[[_computeShowDownloadCommands(_schemes)]]">
<gr-download-commands
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
index fe8384e..d29858e 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
@@ -90,7 +90,7 @@
}
gr-smart-search {
flex-grow: 1;
- margin-left: var(--spacing-m);
+ margin: 0 var(--spacing-m);
max-width: 500px;
}
gr-dropdown,
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
index 0cdef8c..1b57ddb 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
@@ -29,11 +29,9 @@
}
gr-autocomplete {
background-color: var(--view-background-color);
- border: 1px solid var(--border-color);
border-radius: var(--border-radius);
flex: 1;
outline: none;
- padding: var(--spacing-xs);
}
</style>
<form>
@@ -45,7 +43,6 @@
on-commit="_handleInputCommit"
allow-non-suggested-values
multi
- borderless
threshold="[[_threshold]]"
tab-complete
vertical-offset="30"></gr-autocomplete>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js
index 533eccd..c729bcb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js
@@ -69,7 +69,7 @@
for (let node of childNodes) {
const initialNodeLength = this.getLength(node);
// If the current node is completely before the offset.
- if (initialNodeLength <= offset) {
+ if (offset > 0 && initialNodeLength <= offset) {
offset -= initialNodeLength;
continue;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html
index a032bc2..a2c271b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html
@@ -249,6 +249,19 @@
'0<test-wrapper>1234567890</test-wrapper>123456789');
});
+ test('handles zero-length nodes', () => {
+ const container = document.createElement('div');
+ container.appendChild(document.createTextNode('0123456789'));
+ container.appendChild(document.createElement('span'));
+ container.appendChild(document.createTextNode('0123456789'));
+ GrAnnotation.annotateWithElement(
+ container, 1, 10, {tagName: 'test-wrapper'});
+
+ assert.equal(
+ container.innerHTML,
+ '0<test-wrapper>123456789<span></span>0</test-wrapper>123456789');
+ });
+
test('sets sanitized attributes', () => {
const container = document.createElement('div');
container.textContent = fullText;
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
index b0947cd..8ef946c 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
@@ -100,6 +100,7 @@
'gr-diff gr-syntax gr-syntax-attribute': true,
'gr-diff gr-syntax gr-syntax-built_in': true,
'gr-diff gr-syntax gr-syntax-comment': true,
+ 'gr-diff gr-syntax gr-syntax-doctag': true,
'gr-diff gr-syntax gr-syntax-function': true,
'gr-diff gr-syntax gr-syntax-keyword': true,
'gr-diff gr-syntax gr-syntax-link': true,
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.html b/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.html
index c9f8eb3..e5ae06d 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.html
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-themes/gr-syntax-theme.html
@@ -99,8 +99,11 @@
.gr-syntax-template-tag {
color: var(--syntax-template-tag-color);
}
- .gr-syntax-param {
- color: var(--syntax-param-color);
+ .gr-syntax-params {
+ color: var(--syntax-params-color);
+ }
+ .gr-syntax-doctag {
+ font-weight: var(--syntax-doctag-weight);
}
</style>
</template>
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
index 4b9fbac..cb950da 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
@@ -55,21 +55,13 @@
gr-dialog .main > iron-input{
width: 100%;
}
- gr-autocomplete {
- --gr-autocomplete: {
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- height: 2em;
- padding: 0 var(--spacing-xs);
- }
- }
input {
border: 1px solid var(--border-color);
border-radius: var(--border-radius);
- height: 2em;
margin: var(--spacing-m) 0;
- padding: 0 var(--spacing-xs);
+ padding: var(--spacing-s);
width: 100%;
+ box-sizing: content-box;
}
@media screen and (max-width: 50em) {
gr-dialog {
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index 4ed0d3a..1e86acc 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -211,7 +211,7 @@
this.bindShortcut(
this.Shortcut.TOGGLE_CHANGE_STAR, 's:keyup');
this.bindShortcut(
- this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r');
+ this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r:keyup');
this.bindShortcut(
this.Shortcut.EDIT_TOPIC, 't');
@@ -224,7 +224,7 @@
this.bindShortcut(
this.Shortcut.COLLAPSE_ALL_MESSAGES, 'z');
this.bindShortcut(
- this.Shortcut.REFRESH_CHANGE, 'shift+r');
+ this.Shortcut.REFRESH_CHANGE, 'shift+r:keyup');
this.bindShortcut(
this.Shortcut.UP_TO_DASHBOARD, 'u');
this.bindShortcut(
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
index fb5ab15..fb5d64f 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
@@ -80,10 +80,10 @@
data-name$="[[item.name]]"
data-url$="[[item.url]]"
on-click="_handleShowAgreement"
- disabled$="[[_disableAggreements(item, _groups, _signedAgreements)]]">
+ disabled$="[[_disableAgreements(item, _groups, _signedAgreements)]]">
<label id="claNewAgreementsLabel">[[item.name]]</label>
</span>
- <div class$="alreadySubmittedText [[_hideAggreements(item, _groups, _signedAgreements)]]">
+ <div class$="alreadySubmittedText [[_hideAgreements(item, _groups, _signedAgreements)]]">
Agreement already submitted.
</div>
<div class="agreementsUrl">
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
index 98f1413..863577c 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
@@ -117,7 +117,8 @@
return agreements ? 'show' : '';
}
- _disableAggreements(item, groups, signedAgreements) {
+ _disableAgreements(item, groups, signedAgreements) {
+ if (!groups) return false;
for (const group of groups) {
if ((item && item.auto_verify_group &&
item.auto_verify_group.id === group.id) ||
@@ -128,8 +129,8 @@
return false;
}
- _hideAggreements(item, groups, signedAgreements) {
- return this._disableAggreements(item, groups, signedAgreements) ?
+ _hideAgreements(item, groups, signedAgreements) {
+ return this._disableAgreements(item, groups, signedAgreements) ?
'' : 'hide';
}
@@ -141,6 +142,7 @@
// if specified it returns 'hideAgreementsTextBox' which
// then hides the text box and submit button.
_computeHideAgreementClass(name, config) {
+ if (!config) return '';
for (const key in config) {
if (!config.hasOwnProperty(key)) {
continue;
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
index 833fa39..13c4de4 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
@@ -142,28 +142,31 @@
'none');
});
- test('_disableAggreements', () => {
+ test('_disableAgreements', () => {
// In the auto verify group and have not yet signed agreement
assert.isTrue(
- element._disableAggreements(auth, groups, signedAgreements));
+ element._disableAgreements(auth, groups, signedAgreements));
// Not in the auto verify group and have not yet signed agreement
assert.isFalse(
- element._disableAggreements(auth2, groups, signedAgreements));
+ element._disableAgreements(auth2, groups, signedAgreements));
// Not in the auto verify group, have signed agreement
assert.isTrue(
- element._disableAggreements(auth3, groups, signedAgreements));
+ element._disableAgreements(auth3, groups, signedAgreements));
+ // Make sure the undefined check works
+ assert.isFalse(
+ element._disableAgreements(auth, undefined, signedAgreements));
});
- test('_hideAggreements', () => {
+ test('_hideAgreements', () => {
// Not in the auto verify group and have not yet signed agreement
assert.equal(
- element._hideAggreements(auth, groups, signedAgreements), '');
+ element._hideAgreements(auth, groups, signedAgreements), '');
// In the auto verify group
assert.equal(
- element._hideAggreements(auth2, groups, signedAgreements), 'hide');
+ element._hideAgreements(auth2, groups, signedAgreements), 'hide');
// Not in the auto verify group, have signed agreement
assert.equal(
- element._hideAggreements(auth3, groups, signedAgreements), '');
+ element._hideAgreements(auth3, groups, signedAgreements), '');
});
test('_disableAgreementsText', () => {
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html
index 2027119..22ba457 100644
--- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html
+++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.html
@@ -18,6 +18,7 @@
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
+<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -37,6 +38,9 @@
#generatedPasswordDisplay {
margin: var(--spacing-l) 0;
}
+ #generatedPasswordDisplay .title {
+ width: unset;
+ }
#generatedPasswordDisplay .value {
font-family: var(--monospace-font-family);
font-size: var(--font-size-mono);
@@ -79,6 +83,12 @@
<section id="generatedPasswordDisplay">
<span class="title">New Password:</span>
<span class="value">[[_generatedPassword]]</span>
+ <gr-copy-clipboard
+ has-tooltip
+ button-title="Copy password to clipboard"
+ hide-input
+ text="[[_generatedPassword]]">
+ </gr-copy-clipboard>
</section>
<section id="passwordWarning">
This password will not be displayed again.<br>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.html b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.html
index ae656fd..992ea8407 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.html
@@ -40,7 +40,8 @@
on-commit="_handleInputCommit"
clear-on-commit
warn-uncommitted
- text="{{_inputText}}">
+ text="{{_inputText}}"
+ vertical-offset="24">
</gr-autocomplete>
</template>
<script src="gr-account-entry.js"></script>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.js b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.js
index 48dddb0..3caec32 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.js
@@ -51,16 +51,17 @@
},
/**
- * Returns suggestions and convert them to list item
- * @type {Gerrit.GrSuggestionsProvider}
- */
+ * Returns suggestions and convert them to list item
+ * @type {Gerrit.GrSuggestionsProvider}
+ */
suggestionsProvider: {
type: Object,
},
/**
- * Needed for template checking since value is initially set to null.
- * @type {?Object} */
+ * Needed for template checking since value is initially set to null.
+ * @type {?Object}
+ */
pendingConfirmation: {
type: Object,
value: null,
@@ -71,32 +72,41 @@
value: false,
},
/**
- * When true, allows for non-suggested inputs to be added.
- */
+ * When true, allows for non-suggested inputs to be added.
+ */
allowAnyInput: {
type: Boolean,
value: false,
},
/**
- * Array of values (groups/accounts) that are removable. When this prop is
- * undefined, all values are removable.
- */
+ * Array of values (groups/accounts) that are removable. When this prop is
+ * undefined, all values are removable.
+ */
removableValues: Array,
maxCount: {
type: Number,
value: 0,
},
- /** Returns suggestion items
- * @type {!function(string): Promise<Array<Gerrit.GrSuggestionItem>>}
- */
+ /**
+ * Returns suggestion items
+ * @type {!function(string): Promise<Array<Gerrit.GrSuggestionItem>>}
+ */
_querySuggestions: {
type: Function,
value() {
return this._getSuggestions.bind(this);
},
},
+
+ /**
+ * Set to true to disable suggestions on empty input.
+ */
+ skipSuggestOnEmpty: {
+ type: Boolean,
+ value: false,
+ },
};
}
@@ -116,6 +126,9 @@
}
_getSuggestions(input) {
+ if (this.skipSuggestOnEmpty && !input) {
+ return Promise.resolve([]);
+ }
const provider = this.suggestionsProvider;
if (!provider) {
return Promise.resolve([]);
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.html b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.html
index 9521c5f..7fea243 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.html
@@ -382,6 +382,57 @@
});
});
+ test('suggestion on empty', done => {
+ element.skipSuggestOnEmpty = false;
+ const suggestions = [
+ {
+ email: 'abc@example.com',
+ text: 'abcd',
+ },
+ {
+ email: 'qwe@example.com',
+ text: 'qwer',
+ },
+ ];
+ const getSuggestionsStub =
+ sandbox.stub(suggestionsProvider, 'getSuggestions')
+ .returns(Promise.resolve(suggestions));
+
+ const makeSuggestionItemStub =
+ sandbox.stub(suggestionsProvider, 'makeSuggestionItem', item => item);
+
+ const input = element.$.entry.$.input;
+
+ input.text = '';
+ MockInteractions.focus(input.$.input);
+ input.noDebounce = true;
+ flushAsynchronousOperations();
+ flush(() => {
+ assert.isTrue(getSuggestionsStub.calledOnce);
+ assert.equal(getSuggestionsStub.lastCall.args[0], '');
+ assert.equal(makeSuggestionItemStub.getCalls().length, 2);
+ done();
+ });
+ });
+
+ test('skip suggestion on empty', done => {
+ element.skipSuggestOnEmpty = true;
+ const getSuggestionsStub =
+ sandbox.stub(suggestionsProvider, 'getSuggestions')
+ .returns(Promise.resolve([]));
+
+ const input = element.$.entry.$.input;
+
+ input.text = '';
+ MockInteractions.focus(input.$.input);
+ input.noDebounce = true;
+ flushAsynchronousOperations();
+ flush(() => {
+ assert.isTrue(getSuggestionsStub.notCalled);
+ done();
+ });
+ });
+
suite('allowAnyInput', () => {
setup(() => {
element.allowAnyInput = true;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.html b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.html
index 64950f2..cf32e28 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.html
@@ -60,6 +60,7 @@
.dropdown-content {
background: var(--dropdown-background-color);
box-shadow: rgba(0, 0, 0, 0.3) 0 1px 3px;
+ border-radius: var(--border-radius);
max-height: 50vh;
overflow: auto;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
index c9d12ce..e47b661 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
@@ -36,13 +36,14 @@
margin: 0 var(--spacing-xs);
vertical-align: top;
}
- paper-input:not(.borderless) {
- border: 1px solid var(--border-color);
+ paper-input.borderless {
+ border: none;
+ padding: 0;
}
paper-input {
- height: var(--line-height-normal);
- width: 100%;
- @apply --gr-autocomplete;
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ padding: var(--spacing-s);
--paper-input-container: {
padding: 0;
};
@@ -50,13 +51,25 @@
font-size: var(--font-size-normal);
line-height: var(--line-height-normal);
};
+ /* This is a hack for not being able to set height:0 on the underline
+ of a paper-input 2.2.3 element. All the underline fixes below only
+ actually work in 3.x.x, so the height must be adjusted directly as
+ a workaround until we are on Polymer 3. */
+ height: var(--line-height-normal);
+ --paper-input-container-underline-height: 0;
+ --paper-input-container-underline-wrapper-height: 0;
+ --paper-input-container-underline-focus-height: 0;
+ --paper-input-container-underline-legacy-height: 0;
--paper-input-container-underline: {
+ height: 0;
display: none;
};
--paper-input-container-underline-focus: {
+ height: 0;
display: none;
};
--paper-input-container-underline-disabled: {
+ height: 0;
display: none;
};
}
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index a2c908a..3a60255 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -74,9 +74,9 @@
},
/**
- * The number of characters that must be typed before suggestions are
- * made. If threshold is zero, default suggestions are enabled.
- */
+ * The number of characters that must be typed before suggestions are
+ * made. If threshold is zero, default suggestions are enabled.
+ */
threshold: {
type: Number,
value: 1,
@@ -89,11 +89,15 @@
type: Boolean,
value: false,
},
- // Vertical offset needed for a 1em font-size with no vertical padding.
- // Inputs with additional padding will need to increase vertical offset.
+ /**
+ * Vertical offset needed for an element with 20px line-height, 4px
+ * padding and 1px border (30px height total). Plus 1px spacing between
+ * input and dropdown. Inputs with different line-height or padding will
+ * need to tweak vertical offset.
+ */
verticalOffset: {
type: Number,
- value: 20,
+ value: 31,
},
text: {
@@ -110,10 +114,10 @@
},
/**
- * When true, tab key autocompletes but does not fire the commit event.
- * When false, tab key not caught, and focus is removed from the element.
- * See Issue 4556, Issue 6645.
- */
+ * When true, tab key autocompletes but does not fire the commit event.
+ * When false, tab key not caught, and focus is removed from the element.
+ * See Issue 4556, Issue 6645.
+ */
tabComplete: {
type: Boolean,
value: false,
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 68338a4..39329e5 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -335,7 +335,7 @@
});
test('vertical offset overridden by param if it exists', () => {
- assert.equal(element.$.suggestions.verticalOffset, 20);
+ assert.equal(element.$.suggestions.verticalOffset, 31);
element.verticalOffset = 30;
assert.equal(element.$.suggestions.verticalOffset, 30);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index 1de704a..8c9ba50 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -91,7 +91,7 @@
_disabledChanged(disabled) {
if (disabled) {
- this._enabledTabindex = this.getAttribute('tabindex');
+ this._enabledTabindex = this.getAttribute('tabindex') || '0';
}
this.setAttribute('tabindex', disabled ? '-1' : this._enabledTabindex);
this.updateStyles();
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
index d8a5443..bcb560a 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
@@ -68,6 +68,21 @@
assert.isTrue(element.$$('paper-button').disabled);
});
+ test('tabindex should be -1 if disabled', () => {
+ element.disabled = true;
+ assert.isTrue(element.getAttribute('tabindex') === '-1');
+ });
+
+ // Regression tests for Issue: 11969
+ test('tabindex should be reset to 0 if enabled', () => {
+ element.disabled = false;
+ assert.isTrue(element.getAttribute('tabindex') === '0');
+ element.disabled = true;
+ assert.isTrue(element.getAttribute('tabindex') === '-1');
+ element.disabled = false;
+ assert.isTrue(element.getAttribute('tabindex') === '0');
+ });
+
// 'tap' event is tested so we don't loose backward compatibility with older
// plugins who didn't move to on-click which is faster and well supported.
for (const eventName of ['tap', 'click']) {
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 d7510ec..89b28d5 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
@@ -58,6 +58,9 @@
#container.unresolved {
background-color: var(--unresolved-comment-background-color);
}
+ #container.robotComment {
+ background-color: var(--robot-comment-background-color);
+ }
#commentInfoContainer {
border-top: 1px dotted var(--border-color);
display: flex;
@@ -82,7 +85,7 @@
<span class="descriptionText">Patchset [[patchNum]]</span>
</div>
</template>
- <div id="container" class$="[[_computeHostClass(unresolved)]]">
+ <div id="container" class$="[[_computeHostClass(unresolved, isRobotComment)]]">
<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 72fc45d..8687e77 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
@@ -137,6 +137,11 @@
_lastComment: Object,
_orderedComments: Array,
_projectConfig: Object,
+ isRobotComment: {
+ type: Boolean,
+ value: false,
+ reflectToAttribute: true,
+ },
};
}
@@ -221,6 +226,7 @@
this._lastComment = this._getLastComment();
this.unresolved = this._lastComment.unresolved;
this.hasDraft = this._lastComment.__draft;
+ this.isRobotComment = !!(this._lastComment.robot_id);
}
}
@@ -498,6 +504,9 @@
}
_computeHostClass(unresolved) {
+ if (this.isRobotComment) {
+ return 'robotComment';
+ }
return unresolved ? 'unresolved' : '';
}
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 2ef5539..e754a55 100644
--- a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
+++ b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
@@ -32,36 +32,32 @@
display: flex;
flex-direction: column;
max-height: 90vh;
+ padding: var(--spacing-xl);
}
header {
- border-bottom: 1px solid var(--border-color);
flex-shrink: 0;
- font-weight: var(--font-weight-bold);
+ padding-bottom: var(--spacing-xl);
}
main {
display: flex;
flex-shrink: 1;
width: 100%;
}
- header,
- main,
- footer {
- padding: var(--spacing-m) var(--spacing-xl);
- }
- gr-button {
- margin-left: var(--spacing-l);
- }
footer {
display: flex;
flex-shrink: 0;
justify-content: flex-end;
+ padding-top: var(--spacing-xl);
+ }
+ gr-button {
+ margin-left: var(--spacing-l);
}
.hidden {
display: none;
}
</style>
<div class="container" on-keydown="_handleKeydown">
- <header><slot name="header"></slot></header>
+ <header class="font-h3"><slot name="header"></slot></header>
<main><slot name="main"></slot></main>
<footer>
<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 f5f25d2..7fffcff 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
@@ -138,7 +138,7 @@
id="dropdown"
vertical-align="top"
allow-outside-scroll="true"
- on-tap="_handleDropdownTap">
+ on-click="_handleDropdownClick">
<paper-listbox
class="dropdown-content"
slot="dropdown-content"
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
index 2197733..6272a67 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
@@ -84,7 +84,7 @@
* Handle a click on the iron-dropdown element.
* @param {!Event} e
*/
- _handleDropdownTap(e) {
+ _handleDropdownClick(e) {
// async is needed so that that the click event is fired before the
// dropdown closes (This was a bug for touch devices).
this.async(() => {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
index 2a05050..d76721f 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -109,7 +109,7 @@
vertical-offset="[[verticalOffset]]"
allow-outside-scroll="true"
horizontal-align="[[horizontalAlign]]"
- on-tap="_handleDropdownTap">
+ on-click="_handleDropdownClick">
<div class="dropdown-content" slot="dropdown-content">
<ul>
<template is="dom-if" if="[[topContent]]">
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
index 9725f0c..fc92e76 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -159,7 +159,7 @@
* Handle a click on the iron-dropdown element.
* @param {!Event} e
*/
- _handleDropdownTap(e) {
+ _handleDropdownClick(e) {
this._close();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-labeled-autocomplete/gr-labeled-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-labeled-autocomplete/gr-labeled-autocomplete.html
index da0b93f..47be6f7 100644
--- a/polygerrit-ui/app/elements/shared/gr-labeled-autocomplete/gr-labeled-autocomplete.html
+++ b/polygerrit-ui/app/elements/shared/gr-labeled-autocomplete/gr-labeled-autocomplete.html
@@ -38,14 +38,7 @@
#body {
display: flex;
}
- gr-autocomplete {
- height: 1.5em;
- --gr-autocomplete: {
- border: none;
- }
- }
#trigger {
- border-left: 1px solid var(--deemphasized-text-color);
color: var(--deemphasized-text-color);
cursor: pointer;
padding-left: var(--spacing-s);
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
index 2b4b982..46b8fb6 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
@@ -25,7 +25,10 @@
<style include="shared-styles">
:host {
background: var(--dialog-background-color);
- box-shadow: rgba(0, 0, 0, 0.3) 0 1px 3px;
+ border-radius: var(--border-radius);
+ box-shadow: 0 11px 15px -7px rgba(0,0,0,.2),
+ 0 24px 38px 3px rgba(0,0,0,.14),
+ 0 9px 46px 8px rgba(0,0,0,.12);
}
@media screen and (max-width: 50em) {
@@ -35,6 +38,8 @@
position: fixed;
right: 0;
top: 0;
+ border-radius: 0;
+ box-shadow: none;
}
}
</style>
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 eae25ae..75593e8 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
@@ -2354,7 +2354,7 @@
method: 'POST',
url: '/accounts/self/sshkeys',
body: key,
- contentType: 'plain/text',
+ contentType: 'text/plain',
reportUrlAsIs: true,
};
return this._restApiHelper.send(req)
diff --git a/polygerrit-ui/app/embed/embed.html b/polygerrit-ui/app/embed/embed.html
deleted file mode 100644
index 64e0137..0000000
--- a/polygerrit-ui/app/embed/embed.html
+++ /dev/null
@@ -1,27 +0,0 @@
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-<script>
- window.Gerrit = window.Gerrit || {};
-</script>
-<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="../elements/change/gr-change-view/gr-change-view.html">
-<link rel="import" href="../elements/core/gr-search-bar/gr-search-bar.html">
-<link rel="import" href="../elements/diff/gr-diff-view/gr-diff-view.html">
-<link rel="import" href="../elements/change-list/gr-change-list-view/gr-change-list-view.html">
-<link rel="import" href="../elements/change-list/gr-dashboard-view/gr-dashboard-view.html">
-<link rel="import" href="../elements/change-list/gr-embed-dashboard/gr-embed-dashboard.html">
-<link rel="import" href="../styles/themes/app-theme.html">
diff --git a/polygerrit-ui/app/embed/embed_test.html b/polygerrit-ui/app/embed/embed_test.html
deleted file mode 100644
index 5d81b7e..0000000
--- a/polygerrit-ui/app/embed/embed_test.html
+++ /dev/null
@@ -1,98 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>embed_test</title>
-
-<script src="/bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
-
-<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/bower_components/web-component-tester/browser.js"></script>
-<link rel="import" href="embed.html"/>
-
-<script>void(0);</script>
-
-<test-fixture id="change-view">
- <template>
- <gr-change-view></gr-change-view>
- </template>
-</test-fixture>
-
-<test-fixture id="diff-view">
- <template>
- <gr-diff-view></gr-diff-view>
- </template>
-</test-fixture>
-
-<test-fixture id="dashboard-view">
- <template>
- <gr-dashboard-view></gr-dashboard-view>
- </template>
-</test-fixture>
-
-<test-fixture id="change-list-view">
- <template>
- <gr-change-list-view></gr-change-list-view>
- </template>
-</test-fixture>
-
-<test-fixture id="change-list">
- <template>
- <gr-change-list></gr-change-list>
- </template>
-</test-fixture>
-
-<test-fixture id="search-bar">
- <template>
- <gr-search-bar></gr-search-bar>
- </template>
-</test-fixture>
-
-<script>
- suite('embed test', () => {
- test('gr-change-view is embedded', () => {
- const element = fixture('change-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-view');
- });
-
- test('diff-view is embedded', () => {
- const element = fixture('diff-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-diff-view');
- });
-
- test('dashboard-view is embedded', () => {
- const element = fixture('dashboard-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-dashboard-view');
- });
-
- test('change-list-view is embedded', () => {
- const element = fixture('change-list-view');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-list-view');
- });
-
- test('change-list is embedded', () => {
- const element = fixture('change-list');
- assert.equal(element.tagName.toLowerCase(), 'gr-change-list');
- });
-
- test('search-bar is embedded', () => {
- const element = fixture('search-bar');
- assert.equal(element.tagName.toLowerCase(), 'gr-search-bar');
- });
- });
-</script>
diff --git a/polygerrit-ui/app/embed/test.html b/polygerrit-ui/app/embed/test.html
deleted file mode 100644
index 955eaee..0000000
--- a/polygerrit-ui/app/embed/test.html
+++ /dev/null
@@ -1,26 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>Embed Test Runner</title>
-<meta charset="utf-8">
-<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/bower_components/web-component-tester/browser.js"></script>
-<script>
- WCT.loadSuites(['../embed/embed_test.html']);
-</script>
diff --git a/polygerrit-ui/app/embed_test.sh b/polygerrit-ui/app/embed_test.sh
deleted file mode 100755
index 0d8f58f..0000000
--- a/polygerrit-ui/app/embed_test.sh
+++ /dev/null
@@ -1,69 +0,0 @@
-#!/bin/sh
-
-set -ex
-
-t=$(mktemp -d || mktemp -d -t wct-XXXXXXXXXX)
-components=$TEST_SRCDIR/gerrit/polygerrit-ui/app/test_components.zip
-code=$TEST_SRCDIR/gerrit/polygerrit-ui/app/pg_code.zip
-
-echo $t
-unzip -qd $t $components
-unzip -qd $t $code
-# Purge test/ directory contents coming from pg_code.zip.
-rm -rf $t/test
-mkdir -p $t/test
-cp $TEST_SRCDIR/gerrit/polygerrit-ui/app/embed/test.html $t/test/
-
-if [ "${WCT_HEADLESS_MODE:-0}" != "0" ]; then
- CHROME_OPTIONS=[\'start-maximized\',\'headless\',\'disable-gpu\',\'no-sandbox\']
- FIREFOX_OPTIONS=[\'-headless\']
-else
- CHROME_OPTIONS=[\'start-maximized\']
- FIREFOX_OPTIONS=[\'\']
-fi
-
-# For some reason wct tries to install selenium into its node_modules
-# directory on first run. If you've installed into /usr/local and
-# aren't running wct as root, you're screwed. Turning this option off
-# through skipSeleniumInstall seems to still work, so there's that.
-
-# Sauce tests are disabled by default in order to run local tests
-# only. Run it with (saucelabs.com account required; free for open
-# source): WCT_ARGS='--plugin sauce' ./polygerrit-ui/app/embed_test.sh
-
-cat <<EOF > $t/wct.conf.js
-module.exports = {
- 'suites': ['test'],
- 'webserver': {
- 'pathMappings': [
- {'/components/bower_components': 'bower_components'}
- ]
- },
- 'plugins': {
- 'local': {
- 'skipSeleniumInstall': true,
- 'browserOptions': {
- 'chrome': ${CHROME_OPTIONS},
- 'firefox': ${FIREFOX_OPTIONS}
- }
- },
- 'sauce': {
- 'disabled': true,
- 'browsers': [
- 'OS X 10.12/chrome',
- 'Windows 10/chrome',
- 'Linux/firefox',
- 'OS X 10.12/safari',
- 'Windows 10/microsoftedge'
- ]
- }
- }
- };
-EOF
-
-export PATH="$(dirname $NPM):$PATH"
-
-cd $t
-test -n "${WCT}"
-
-${WCT} ${WCT_ARGS}
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 16c0f29..e1304e6 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -89,6 +89,8 @@
# we extract from the zip, but depend on the component for license checking.
"@webcomponentsjs//:zipfile",
"//lib/js:webcomponentsjs",
+ "@font-roboto-local//:zipfile",
+ "//lib/js:font-roboto-local",
],
outs = outs,
cmd = " && ".join([
@@ -100,6 +102,7 @@
"for f in $(locations " + name + "_theme_sources); do cp $$f $$TMP/polygerrit_ui/styles/themes; done",
"for f in $(locations //lib/js:highlightjs_files); do cp $$f $$TMP/polygerrit_ui/bower_components/highlightjs/ ; done",
"unzip -qd $$TMP/polygerrit_ui/bower_components $(location @webcomponentsjs//:zipfile) webcomponentsjs/webcomponents-lite.js",
+ "unzip -qd $$TMP/polygerrit_ui/bower_components $(location @font-roboto-local//:zipfile) font-roboto-local/fonts/\*/\*.ttf",
"cd $$TMP",
"find . -exec touch -t 198001010000 '{}' ';'",
"zip -qr $$ROOT/$@ *",
diff --git a/polygerrit-ui/app/run_test.sh b/polygerrit-ui/app/run_test.sh
index e9be18d..76a4fa1 100755
--- a/polygerrit-ui/app/run_test.sh
+++ b/polygerrit-ui/app/run_test.sh
@@ -48,5 +48,4 @@
--test_env="DISPLAY=${DISPLAY}" \
--test_env="WCT_HEADLESS_MODE=${WCT_HEADLESS_MODE}" \
"$@" \
- //polygerrit-ui/app:embed_test \
//polygerrit-ui/app:wct_test
diff --git a/polygerrit-ui/app/styles/gr-form-styles.html b/polygerrit-ui/app/styles/gr-form-styles.html
index 3fe0a72..7c9ae0d 100644
--- a/polygerrit-ui/app/styles/gr-form-styles.html
+++ b/polygerrit-ui/app/styles/gr-form-styles.html
@@ -61,7 +61,6 @@
}
.gr-form-styles td,
.gr-form-styles tfoot th {
- height: 2em;
padding: var(--spacing-s) 0;
vertical-align: middle;
}
@@ -84,8 +83,7 @@
.gr-form-styles textarea {
border: 1px solid var(--border-color);
border-radius: var(--border-radius);
- height: 2em;
- padding: 0 var(--spacing-xs);
+ padding: var(--spacing-s);
}
.gr-form-styles td:last-child {
width: 5em;
@@ -97,23 +95,16 @@
.gr-form-styles iron-autogrow-textarea {
border: none;
height: auto;
- min-height: 2em;
+ min-height: 4em;
--iron-autogrow-textarea: {
border: 1px solid var(--border-color);
border-radius: var(--border-radius);
box-sizing: border-box;
- padding: var(--spacing-s) var(--spacing-xs) 0 var(--spacing-xs);
+ padding: var(--spacing-s);
}
}
.gr-form-styles gr-autocomplete {
- border: none;
- --gr-autocomplete: {
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- height: 2em;
- padding: 0 var(--spacing-xs);
- width: 14em;
- }
+ width: 14em;
}
@media only screen and (max-width: 40em) {
.gr-form-styles section {
diff --git a/polygerrit-ui/app/styles/shared-styles.html b/polygerrit-ui/app/styles/shared-styles.html
index 5314741..51b92e1 100644
--- a/polygerrit-ui/app/styles/shared-styles.html
+++ b/polygerrit-ui/app/styles/shared-styles.html
@@ -42,10 +42,11 @@
input {
background-color: inherit;
border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
box-sizing: border-box;
color: var(--primary-text-color);
margin: 0;
- padding: 0;
+ padding: var(--spacing-s);
}
iron-autogrow-textarea {
background-color: inherit;
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index ccdde2b..3a620d2 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -45,6 +45,7 @@
--assignee-highlight-color: #fcfad6;
--chip-background-color: #eee;
--comment-background-color: #fcfad6;
+ --robot-comment-background-color: #e8f0fe;
--default-button-background-color: white;
--dialog-background-color: white;
--dropdown-background-color: white;
@@ -74,7 +75,7 @@
/* fonts */
--font-family: 'Roboto', -apple-system, BlinkMacSystemFont, 'Segoe UI', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
- --monospace-font-family: 'Roboto Mono', Menlo, 'Lucida Console', Monaco, monospace;
+ --monospace-font-family: 'Roboto Mono', 'SF Mono', 'Lucida Console', Monaco, monospace;
--font-size-code: 12px; /* 12px mono */
--font-size-mono: .929rem; /* 13px mono */
--font-size-small: .857rem; /* 12px */
@@ -142,6 +143,7 @@
--syntax-built_in-color: #30a;
--syntax-comment-color: #3f7f5f;
--syntax-default-color: var(--primary-text-color);
+ --syntax-doctag-weight: bold;
--syntax-function-color: var(--primary-text-color);
--syntax-keyword-color: #9e0069;
--syntax-link-color: #219;
@@ -149,7 +151,7 @@
--syntax-meta-color: #ff1717;
--syntax-meta-keyword-color: #219;
--syntax-number-color: #164;
- --syntax-param-color: var(--primary-text-color);
+ --syntax-params-color: var(--primary-text-color);
--syntax-regexp-color: #fa8602;
--syntax-selector-attr-color: #fa8602;
--syntax-selector-class-color: #164;
@@ -162,10 +164,10 @@
--syntax-title-color: #0000c0;
--syntax-type-color: #2a66d9;
--syntax-variable-color: var(--primary-text-color);
-
/* misc */
--border-radius: 4px;
--reply-overlay-z-index: 1000;
+ --iron-overlay-backdrop-opacity: 0.32;
--iron-overlay-backdrop: {
transition: none;
};
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index f3d5e78..4a91774f 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -45,6 +45,7 @@
--assignee-highlight-color: #3a361c;
--chip-background-color: #131416;
--comment-background-color: #0b162b;
+ --robot-comment-background-color: #e8f0fe;
--default-button-background-color: #3c4043;
--dialog-background-color: #131416;
--dropdown-background-color: #131416;
@@ -112,6 +113,7 @@
--syntax-built_in-color: #f7c369;
--syntax-comment-color: var(--deemphasized-text-color);
--syntax-default-color: var(--primary-text-color);
+ --syntax-doctag-weight: bold;
--syntax-function-color: var(--primary-text-color);
--syntax-keyword-color: #cd4cf0;
--syntax-link-color: #c792ea;
@@ -119,7 +121,7 @@
--syntax-meta-color: #6d7eee;
--syntax-meta-keyword-color: #eefff7;
--syntax-number-color: #00998a;
- --syntax-param-color: var(--primary-text-color);
+ --syntax-params-color: var(--primary-text-color);
--syntax-regexp-color: #f77669;
--syntax-selector-attr-color: #80cbbf;
--syntax-selector-class-color: #ffcb68;
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index 35bdefd..3b699c2 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -86,6 +86,9 @@
log.Println("Local plugins from", "../plugins")
} else {
http.HandleFunc("/plugins/", handleProxy)
+ // Serve local plugins from `plugins_`
+ http.Handle("/plugins_/", http.StripPrefix("/plugins_/",
+ http.FileServer(http.Dir("../plugins"))))
}
log.Println("Serving on port", *port)
log.Fatal(http.ListenAndServe(*port, &server{}))
diff --git a/tools/BUILD b/tools/BUILD
index 5531c3e..f0a4ffa 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -53,6 +53,7 @@
"-Xep:ExpectedExceptionChecker:ERROR",
"-Xep:Finally:ERROR",
"-Xep:FloatingPointLiteralPrecision:ERROR",
+ "-Xep:FormatStringAnnotation:ERROR",
"-Xep:FragmentInjection:ERROR",
"-Xep:FragmentNotInstantiable:ERROR",
"-Xep:FunctionalInterfaceClash:ERROR",
diff --git a/tools/js/bower2bazel.py b/tools/js/bower2bazel.py
index e728cc3..a03f764 100755
--- a/tools/js/bower2bazel.py
+++ b/tools/js/bower2bazel.py
@@ -38,7 +38,7 @@
"codemirror-minified": "codemirror-minified",
"es6-promise": "es6-promise",
"fetch": "fetch",
- "font-roboto": "polymer",
+ "font-roboto-local": "polymer",
"iron-a11y-announcer": "polymer",
"iron-a11y-keys-behavior": "polymer",
"iron-autogrow-textarea": "polymer",
@@ -77,7 +77,7 @@
"paper-behaviors": "polymer",
"paper-ripple": "polymer",
"iron-checked-element-behavior": "polymer",
- "font-roboto": "polymer",
+ "font-roboto-local": "polymer",
}
diff --git a/tools/maven/package.bzl b/tools/maven/package.bzl
index 11e569d..b25656d 100644
--- a/tools/maven/package.bzl
+++ b/tools/maven/package.bzl
@@ -17,6 +17,14 @@
"echo \"# this script should run from the root of your workspace.\" >> $@",
"echo \"set -e\" >> $@",
"echo \"\" >> $@",
+ "echo 'function bazel_cmd() {' >> $@",
+ "echo ' if [[ `which bazelisk` ]]; then' >> $@",
+ "echo ' bazelisk \"$$@\"' >> $@",
+ "echo ' else' >> $@",
+ "echo ' bazel \"$$@\"' >> $@",
+ "echo ' fi' >> $@",
+ "echo '}' >> $@",
+ "echo \"\" >> $@",
"echo 'if [[ \"$$VERBOSE\" ]]; then set -x ; fi' >> $@",
"echo \"\" >> $@",
"echo %s >> $@",
@@ -32,7 +40,7 @@
src = {},
doc = {},
war = {}):
- build_cmd = ["bazel", "build"]
+ build_cmd = ["bazel_cmd", "build"]
mvn_cmd = ["python", "tools/maven/mvn.py", "-v", version]
api_cmd = mvn_cmd[:]
api_targets = []
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 357faa4..1dfc2f4 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -170,18 +170,18 @@
sha1 = "3e83394258ae2089be7219b971ec21a8288528ad",
)
- TESTCONTAINERS_VERSION = "1.12.3"
+ TESTCONTAINERS_VERSION = "1.12.4"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "e424a4549640e120acceac641ac909fcda58bf62",
+ sha1 = "456b6facac12c4b67130d9056a43c011679e9f0c",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "c0796de5032070b8768ce78c78949b48f13c30db",
+ sha1 = "9e210c277a35a95a76d03a79e2812575bd07391c",
)
maven_jar(
