Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  Error Prone: Fix DefaultCharset in ListProjectsIT
  Bazel: Enable Error-Prone by default
  Parallelize account migration in schema 146 migration
  Fix _nextStepHandle not being assigned a value when scrolling
  Elasticsearch: Remove redundant handling of 'ignore_unmapped' property
  Elasticsearch: Fix support for V6 versions earlier than 6.7.*
  Clarify introduction to external IDs
  ExternalIdNotes: Improve grammar in checkState messages
  Update git submodules
  Add example of external ID key in external ID documentation
  Minor grammatical fixes in external ID documentation
  ElasticVersionTest: Add missing assertions on V6_7

Also update the replication plugin revision to include the fix:

  HttpResponse: Specify charset in constructor of InputStreamReader

which is needed due to enabling error prone checks on core plugins.

Change-Id: I90394f5c0d6aaf88b1169c26ceb65b699fddfe77
This commit is contained in:
David Pursehouse
2019-05-29 09:42:29 +09:00
12 changed files with 62 additions and 32 deletions

View File

@@ -3,6 +3,7 @@ build --repository_cache=~/.gerritcodereview/bazel-cache/repository
build --experimental_strict_action_env build --experimental_strict_action_env
build --action_env=PATH build --action_env=PATH
build --disk_cache=~/.gerritcodereview/bazel-cache/cas build --disk_cache=~/.gerritcodereview/bazel-cache/cas
build --java_toolchain //tools:error_prone_warnings_toolchain
test --build_tests_only test --build_tests_only
test --test_output=errors test --test_output=errors

View File

@@ -275,7 +275,7 @@ branch is fast.
To identify SSH keys in the REST API Gerrit uses To identify SSH keys in the REST API Gerrit uses
link:rest-api-accounts.html#ssh-key-id[sequence numbers per account]. link:rest-api-accounts.html#ssh-key-id[sequence numbers per account].
This is why the order of the keys in the `authorized_keys` file is This is why the order of the keys in the `authorized_keys` file is
used to determines the sequence numbers of the keys (the sequence used to determine the sequence numbers of the keys (the sequence
numbers start at 1). numbers start at 1).
To keep the sequence numbers intact when a key is deleted, a To keep the sequence numbers intact when a key is deleted, a
@@ -286,19 +286,21 @@ Invalid keys are marked with the prefix '# INVALID'.
[[external-ids]] [[external-ids]]
== External IDs == External IDs
External IDs are used to link external identities, such as an LDAP External IDs are used to link identities, such as the username and email
account or an OAUTH identity, to an account in Gerrit. addresses, and external identies such as an LDAP account or an OAUTH
identity, to an account in Gerrit.
External IDs are stored as Git Notes in the `All-Users` repository. The External IDs are stored as Git Notes in the `All-Users` repository. The
name of the notes branch is `refs/meta/external-ids`. name of the notes branch is `refs/meta/external-ids`.
As note key the SHA1 of the external ID key is used. This ensures that As note key the SHA1 of the external ID key is used, for example the key
an external ID is used only once (e.g. an external ID can never be for the external ID `username:jdoe` is `e0b751ae90ef039f320e097d7d212f490e933706`.
assigned to multiple accounts at a point in time). This ensures that an external ID is used only once (e.g. an external ID can
never be assigned to multiple accounts at a point in time).
[IMPORTANT] [IMPORTANT]
If the external ID key is changed manually you must adapt the note key If the external ID key is changed manually you must adapt the note key
to the new SHA1. Otherwise the external ID becomes inconsistent and is to the new SHA1, otherwise the external ID becomes inconsistent and is
ignored by Gerrit. ignored by Gerrit.
The note content is a Git config file: The note content is a Git config file:
@@ -310,14 +312,14 @@ The note content is a Git config file:
password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7 password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
---- ----
The config file has one `externalId` section. The external ID key which The config file has one `externalId` section. The external ID key, which
consists of scheme and ID in the format '<scheme>:<id>' is used as consists of scheme and ID in the format '<scheme>:<id>', is used as
subsection name. subsection name.
The `accountId` field is mandatory, the `email` and `password` fields The `accountId` field is mandatory. The `email` and `password` fields
are optional. are optional.
The external IDs are maintained by Gerrit, this means users are not The external IDs are maintained by Gerrit. This means users are not
allowed to manually edit their external IDs. Only users with the allowed to manually edit their external IDs. Only users with the
link:access-control.html#capability_accessDatabase[Access Database] link:access-control.html#capability_accessDatabase[Access Database]
global capability can push updates to the `refs/meta/external-ids` global capability can push updates to the `refs/meta/external-ids`

View File

@@ -305,7 +305,6 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
protected JsonArray getSortArray(String idFieldName) { protected JsonArray getSortArray(String idFieldName) {
JsonObject properties = new JsonObject(); JsonObject properties = new JsonObject();
properties.addProperty(ORDER, "asc"); properties.addProperty(ORDER, "asc");
client.adapter().setIgnoreUnmapped(properties);
JsonArray sortArray = new JsonArray(); JsonArray sortArray = new JsonArray();
addNamedElement(idFieldName, properties, sortArray); addNamedElement(idFieldName, properties, sortArray);

View File

@@ -164,7 +164,6 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
private JsonArray getSortArray() { private JsonArray getSortArray() {
JsonObject properties = new JsonObject(); JsonObject properties = new JsonObject();
properties.addProperty(ORDER, "desc"); properties.addProperty(ORDER, "desc");
client.adapter().setIgnoreUnmapped(properties);
JsonArray sortArray = new JsonArray(); JsonArray sortArray = new JsonArray();
addNamedElement(ChangeField.UPDATED.getName(), properties, sortArray); addNamedElement(ChangeField.UPDATED.getName(), properties, sortArray);

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.elasticsearch; package com.google.gerrit.elasticsearch;
import static com.google.gerrit.elasticsearch.ElasticVersion.V6_7;
import com.google.gson.JsonObject; import com.google.gson.JsonObject;
public class ElasticQueryAdapter { public class ElasticQueryAdapter {
@@ -22,7 +24,6 @@ public class ElasticQueryAdapter {
private static final String INCLUDE_TYPE = "include_type_name=true"; private static final String INCLUDE_TYPE = "include_type_name=true";
private static final String INDICES = "?allow_no_indices=false"; private static final String INDICES = "?allow_no_indices=false";
private final boolean ignoreUnmapped;
private final boolean useV5Type; private final boolean useV5Type;
private final boolean useV6Type; private final boolean useV6Type;
private final boolean omitType; private final boolean omitType;
@@ -37,24 +38,18 @@ public class ElasticQueryAdapter {
private final String includeTypeNameParam; private final String includeTypeNameParam;
ElasticQueryAdapter(ElasticVersion version) { ElasticQueryAdapter(ElasticVersion version) {
this.ignoreUnmapped = false;
this.useV5Type = !version.isV6OrLater(); this.useV5Type = !version.isV6OrLater();
this.useV6Type = version.isV6(); this.useV6Type = version.isV6();
this.omitType = version.isV7OrLater(); this.omitType = version.isV7OrLater();
this.versionDiscoveryUrl = version.isV6OrLater() ? "/%s*" : "/%s*/_aliases"; this.versionDiscoveryUrl = version.isV6OrLater() ? "/%s*" : "/%s*/_aliases";
this.searchFilteringName = "_source"; this.searchFilteringName = "_source";
this.indicesExistParams = version.isV6() ? INDICES + "&" + INCLUDE_TYPE : INDICES; this.indicesExistParams =
version.isAtLeastMinorVersion(V6_7) ? INDICES + "&" + INCLUDE_TYPE : INDICES;
this.exactFieldType = "keyword"; this.exactFieldType = "keyword";
this.stringFieldType = "text"; this.stringFieldType = "text";
this.indexProperty = "true"; this.indexProperty = "true";
this.rawFieldsKey = "_source"; this.rawFieldsKey = "_source";
this.includeTypeNameParam = version.isV6() ? "?" + INCLUDE_TYPE : ""; this.includeTypeNameParam = version.isAtLeastMinorVersion(V6_7) ? "?" + INCLUDE_TYPE : "";
}
void setIgnoreUnmapped(JsonObject properties) {
if (ignoreUnmapped) {
properties.addProperty("ignore_unmapped", true);
}
} }
public void setType(JsonObject properties, String type) { public void setType(JsonObject properties, String type) {

View File

@@ -71,14 +71,22 @@ public enum ElasticVersion {
return isAtLeastVersion(7); return isAtLeastVersion(7);
} }
private boolean isAtLeastVersion(int v) { private boolean isAtLeastVersion(int major) {
return getMajor() >= v; return getMajor() >= major;
}
public boolean isAtLeastMinorVersion(ElasticVersion version) {
return getMajor().equals(version.getMajor()) && getMinor() >= version.getMinor();
} }
private Integer getMajor() { private Integer getMajor() {
return Integer.valueOf(version.split("\\.")[0]); return Integer.valueOf(version.split("\\.")[0]);
} }
private Integer getMinor() {
return Integer.valueOf(version.split("\\.")[1]);
}
@Override @Override
public String toString() { public String toString() {
return version; return version;

View File

@@ -805,7 +805,7 @@ public class ExternalIdNotes extends VersionedMetaData {
} }
checkState( checkState(
accountId.equals(extId.accountId()), accountId.equals(extId.accountId()),
"external id %s belongs to account %s, expected account %s", "external id %s belongs to account %s, but expected account %s",
extId.key().get(), extId.key().get(),
extId.accountId().get(), extId.accountId().get(),
accountId.get()); accountId.get());
@@ -863,7 +863,7 @@ public class ExternalIdNotes extends VersionedMetaData {
ExternalId actualExtId = ExternalId.parse(noteId.name(), raw, noteDataId); ExternalId actualExtId = ExternalId.parse(noteId.name(), raw, noteDataId);
checkState( checkState(
extId.equals(actualExtId), extId.equals(actualExtId),
"external id %s should be removed, but it's not matching the actual external id %s", "external id %s should be removed, but it doesn't match the actual external id %s",
extId.toString(), extId.toString(),
actualExtId.toString()); actualExtId.toString());
noteMap.remove(noteId); noteMap.remove(noteId);

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertThatNameList; import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertThatNameList;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
import com.google.common.base.Splitter; import com.google.common.base.Splitter;
@@ -147,7 +148,9 @@ public class ListProjectsIT extends AbstractDaemonTest {
listProjects.displayToStream(displayOut); listProjects.displayToStream(displayOut);
List<String> lines = List<String> lines =
Splitter.on("\n").omitEmptyStrings().splitToList(new String(displayOut.toByteArray())); Splitter.on("\n")
.omitEmptyStrings()
.splitToList(new String(displayOut.toByteArray(), UTF_8));
assertThat(lines).isEqualTo(testProjects); assertThat(lines).isEqualTo(testProjects);
} }
} }
@@ -176,7 +179,7 @@ public class ListProjectsIT extends AbstractDaemonTest {
listProjects.setFormat(jsonFormat); listProjects.setFormat(jsonFormat);
listProjects.displayToStream(displayOut); listProjects.displayToStream(displayOut);
String projectsJsonOutput = new String(displayOut.toByteArray()); String projectsJsonOutput = new String(displayOut.toByteArray(), UTF_8);
Gson gson = jsonFormat.newGson(); Gson gson = jsonFormat.newGson();
Set<String> projectsJsonNames = gson.fromJson(projectsJsonOutput, JsonObject.class).keySet(); Set<String> projectsJsonNames = gson.fromJson(projectsJsonOutput, JsonObject.class).keySet();

View File

@@ -63,9 +63,22 @@ public class ElasticVersionTest extends GerritBaseTests {
assertThat(ElasticVersion.V6_4.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_4.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V6_5.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_5.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V6_6.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_6.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V6_7.isV6OrLater()).isTrue();
assertThat(ElasticVersion.V7_0.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_0.isV6OrLater()).isTrue();
} }
@Test
public void atLeastMinorVersion() throws Exception {
assertThat(ElasticVersion.V5_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
assertThat(ElasticVersion.V6_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isTrue();
assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
}
@Test @Test
public void version7() throws Exception { public void version7() throws Exception {
assertThat(ElasticVersion.V5_6.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V5_6.isV7OrLater()).isFalse();
@@ -74,6 +87,7 @@ public class ElasticVersionTest extends GerritBaseTests {
assertThat(ElasticVersion.V6_4.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_4.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_5.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_5.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_6.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_6.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V6_7.isV7OrLater()).isFalse();
assertThat(ElasticVersion.V7_0.isV7OrLater()).isTrue(); assertThat(ElasticVersion.V7_0.isV7OrLater()).isTrue();
} }
} }

View File

@@ -157,7 +157,7 @@
let currentBatch = 0; let currentBatch = 0;
const nextStep = () => { const nextStep = () => {
if (this._isScrolling) { if (this._isScrolling) {
this.async(nextStep, 100); this._nextStepHandle = this.async(nextStep, 100);
return; return;
} }
// If we are done, resolve the promise. // If we are done, resolve the promise.

View File

@@ -23,8 +23,10 @@ default_java_toolchain(
visibility = ["//visibility:public"], visibility = ["//visibility:public"],
) )
# This EP warnings list is based on: # Error Prone errors enabled by default; see ../.bazelrc for how this is
# enabled. This warnings list is originally based on:
# https://github.com/bazelbuild/BUILD_file_generator/blob/master/tools/bazel_defs/java.bzl # https://github.com/bazelbuild/BUILD_file_generator/blob/master/tools/bazel_defs/java.bzl
# However, feel free to add any additional errors. Thus far they have all been pretty useful.
java_package_configuration( java_package_configuration(
name = "error_prone", name = "error_prone",
javacopts = [ javacopts = [
@@ -96,5 +98,12 @@ package_group(
packages = [ packages = [
"//java/...", "//java/...",
"//javatests/...", "//javatests/...",
"//plugins/codemirror-editor/...",
"//plugins/commit-message-length-validator/...",
"//plugins/download-commands/...",
"//plugins/hooks/...",
"//plugins/replication/...",
"//plugins/reviewnotes/...",
"//plugins/singleusergroup/...",
], ],
) )