From 5e4a4e7e4855bf1aa1166db55bf0d43c3a8f10a1 Mon Sep 17 00:00:00 2001 From: Mark Bekhet Date: Thu, 13 Jun 2019 14:08:36 -0400 Subject: [PATCH 01/14] Avoid logging "length=0" exception Avoid logging ssh exception for "stream is already closed" when length=0 if present in the stacktrace. This will reduce noise and we will be able to see if we get this exception when length is not 0. This exception is related to ssh streams being prematurely closed by the connected slave frontends. This exception can be safely omitted from the logs because it is thrown when sshd.flush is called and the channel is closed. Length = 0 means that there was nothing to be flushed and no new information is provided by this stack trace. Change-Id: I85f77cab03453ec51d33b226804ed7f2a454ae6f --- java/com/google/gerrit/sshd/BaseCommand.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/sshd/BaseCommand.java b/java/com/google/gerrit/sshd/BaseCommand.java index a027dd197d..85d9eb24bd 100644 --- a/java/com/google/gerrit/sshd/BaseCommand.java +++ b/java/com/google/gerrit/sshd/BaseCommand.java @@ -51,6 +51,7 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.StringWriter; import java.nio.charset.Charset; +import java.util.Arrays; import java.util.concurrent.Future; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.atomic.AtomicReference; @@ -356,7 +357,7 @@ public abstract class BaseCommand implements Command { } m.append(" during "); m.append(context.getCommandLine()); - logger.atSevere().withCause(e).log(m.toString()); + logCauseIfRelevant(e, m); } if (e instanceof Failure) { @@ -383,6 +384,20 @@ public abstract class BaseCommand implements Command { return 128; } + private void logCauseIfRelevant(Throwable e, StringBuilder message) { + String zeroLength = "length=0"; + String streamAlreadyClosed = "stream is already closed"; + boolean isZeroLength = false; + + if (streamAlreadyClosed.equals(e.getMessage())) { + StackTraceElement[] stackTrace = e.getStackTrace(); + isZeroLength = Arrays.stream(stackTrace).anyMatch(s -> s.toString().contains(zeroLength)); + } + if (!isZeroLength) { + logger.atSevere().withCause(e).log(message.toString()); + } + } + protected UnloggedFailure die(String msg) { return new UnloggedFailure(1, "fatal: " + msg); } From cfd99e4be6da02d7a6397496e3b22856d34f8af5 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Mon, 7 Dec 2020 08:40:59 -0700 Subject: [PATCH 02/14] Update git submodules * Update plugins/replication from branch 'stable-2.16' to ba2c8e16b798c2eaf4e56dd66d8c1cd00999e096 - Fix replication to retry on lock errors Versions of Git released since 2014 have created a new status "failed to update ref" which replaces the two statuses "failed to lock" and "failed to write". So, we now see the newer status when the remote is unable to lock a ref. Refer Git commit: https://github.com/git/git/commit/6629ea2d4a5faa0a84367f6d4aedba53cb0f26b4 Config 'lockErrorMaxRetries' is not removed as part of this change as folks who have it configured currently don't run into unexpected behavior with retries when they upgrade to a newer version of the plugin. Also, the "failed to lock" check is not removed for folks still using a version of Git older than 2014. Change-Id: I9b3b15bebd55df30cbee50a0e0c2190d04f2f443 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 8fdb0f9ac0..ba2c8e16b7 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 8fdb0f9ac0a7f68b3f942cb4a9fd4c94e488ab57 +Subproject commit ba2c8e16b798c2eaf4e56dd66d8c1cd00999e096 From 9caa81a31b8cee4727a27230ec4891f718d60c4e Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Sun, 6 Dec 2020 16:10:20 -0500 Subject: [PATCH 03/14] Elasticsearch: Discontinue EOL version 6.8 thus whole V6 support - Remove the whole V6 support as 6.8 was the last supported 6.x version. - Remove the no longer necessary documentation parts about that version. - Update the related tests accordingly, including removing the V6 ones. [1] https://www.elastic.co/support/eol Change-Id: I4748c9718eec246479a2fae78e5f9b1ef912fb93 --- Documentation/config-gerrit.txt | 6 +- .../elasticsearch/AbstractElasticIndex.java | 42 +++----- .../elasticsearch/ElasticAccountIndex.java | 3 +- .../elasticsearch/ElasticChangeIndex.java | 35 +------ .../elasticsearch/ElasticConfiguration.java | 7 +- .../elasticsearch/ElasticGroupIndex.java | 4 +- .../elasticsearch/ElasticProjectIndex.java | 4 +- .../elasticsearch/ElasticQueryAdapter.java | 45 +-------- .../ElasticRestClientProvider.java | 2 +- .../gerrit/elasticsearch/ElasticSetting.java | 8 +- .../gerrit/elasticsearch/ElasticVersion.java | 29 ------ .../acceptance/pgm/ElasticReindexIT.java | 5 - .../gerrit/acceptance/ssh/ElasticIndexIT.java | 5 - .../com/google/gerrit/elasticsearch/BUILD | 10 -- .../elasticsearch/ElasticContainer.java | 2 - .../ElasticV6QueryAccountsTest.java | 64 ------------- .../ElasticV6QueryChangesTest.java | 95 ------------------- .../ElasticV6QueryGroupsTest.java | 64 ------------- .../ElasticV6QueryProjectsTest.java | 64 ------------- .../elasticsearch/ElasticVersionTest.java | 45 --------- 20 files changed, 31 insertions(+), 508 deletions(-) delete mode 100644 javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java delete mode 100644 javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java delete mode 100644 javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java delete mode 100644 javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index f35dfc7284..e63e0bea39 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -3047,10 +3047,6 @@ WARNING: Support for Elasticsearch is still experimental and is not recommended for production use. For compatibility information, please refer to the link:https://www.gerritcodereview.com/elasticsearch.html[project homepage]. -In Elasticsearch version 6.2 or later, the open and closed changes are merged -into the default `_doc` type. The latter is also used for the accounts and groups -indices starting with Elasticsearch 6.2. - Note that when Gerrit is configured to use Elasticsearch, the Elasticsearch server(s) must be reachable during the site initialization. @@ -3080,7 +3076,7 @@ Sets the number of shards to use per index. Refer to the link:https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#_static_index_settings[ Elasticsearch documentation] for details. + -Defaults to 5 for Elasticsearch versions 5 and 6, and to 1 starting with Elasticsearch 7. +Defaults to 1. [[elasticsearch.numberOfReplicas]]elasticsearch.numberOfReplicas:: + diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index 0d1f9cd50a..ed32ce5b28 100644 --- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -127,7 +127,6 @@ abstract class AbstractElasticIndex implements Index { private final SitePaths sitePaths; private final String indexNameRaw; - protected final String type; protected final ElasticRestClientProvider client; protected final String indexName; protected final Gson gson; @@ -147,7 +146,6 @@ abstract class AbstractElasticIndex implements Index { this.indexName = config.getIndexName(indexName, schema.getVersion()); this.indexNameRaw = indexName; this.client = client; - this.type = client.adapter().getType(); } @Override @@ -167,7 +165,7 @@ abstract class AbstractElasticIndex implements Index { @Override public void delete(K id) { - String uri = getURI(type, BULK); + String uri = getURI(BULK); Response response = postRequest(uri, getDeleteActions(id), getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { @@ -192,10 +190,8 @@ abstract class AbstractElasticIndex implements Index { } // Recreate the index. - String indexCreationFields = concatJsonString(getSettings(client.adapter()), getMappings()); - response = - performRequest( - "PUT", indexName + client.adapter().includeTypeNameParam(), indexCreationFields); + String indexCreationFields = concatJsonString(getSettings(), getMappings()); + response = performRequest("PUT", indexName, indexCreationFields); statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { String error = String.format("Failed to create index %s: %s", indexName, statusCode); @@ -207,26 +203,20 @@ abstract class AbstractElasticIndex implements Index { protected abstract String getMappings(); - private String getSettings(ElasticQueryAdapter adapter) { - return gson.toJson(ImmutableMap.of(SETTINGS, ElasticSetting.createSetting(config, adapter))); + private String getSettings() { + return gson.toJson(ImmutableMap.of(SETTINGS, ElasticSetting.createSetting(config))); } protected abstract String getId(V v); protected String getMappingsForSingleType(MappingProperties properties) { - return getMappingsFor(client.adapter().getType(), properties); + return getMappingsFor(properties); } - protected String getMappingsFor(String type, MappingProperties properties) { + protected String getMappingsFor(MappingProperties properties) { JsonObject mappings = new JsonObject(); - if (client.adapter().omitType()) { - mappings.add(MAPPINGS, gson.toJsonTree(properties)); - } else { - JsonObject mappingType = new JsonObject(); - mappingType.add(type, gson.toJsonTree(properties)); - mappings.add(MAPPINGS, gson.toJsonTree(mappingType)); - } + mappings.add(MAPPINGS, gson.toJsonTree(properties)); return gson.toJson(mappings); } @@ -305,15 +295,9 @@ abstract class AbstractElasticIndex implements Index { return sortArray; } - protected String getURI(String type, String request) { + protected String getURI(String request) { try { - String encodedIndexName = URLEncoder.encode(indexName, UTF_8.toString()); - if (SEARCH.equals(request) && client.adapter().omitType()) { - return encodedIndexName + "/" + request; - } - String encodedTypeIfAny = - client.adapter().omitType() ? "" : "/" + URLEncoder.encode(type, UTF_8.toString()); - return encodedIndexName + encodedTypeIfAny + "/" + request; + return URLEncoder.encode(indexName, UTF_8.toString()) + "/" + request; } catch (UnsupportedEncodingException e) { throw new StorageException(e); } @@ -359,12 +343,10 @@ abstract class AbstractElasticIndex implements Index { protected class ElasticQuerySource implements DataSource { private final QueryOptions opts; private final String search; - private final String index; - ElasticQuerySource(Predicate p, QueryOptions opts, String index, JsonArray sortArray) + ElasticQuerySource(Predicate p, QueryOptions opts, JsonArray sortArray) throws QueryParseException { this.opts = opts; - this.index = index; QueryBuilder qb = queryBuilder.toQueryBuilder(p); SearchSourceBuilder searchSource = new SearchSourceBuilder(client.adapter()) @@ -392,7 +374,7 @@ abstract class AbstractElasticIndex implements Index { private ResultSet readImpl(Function mapper) { try { - String uri = getURI(index, SEARCH); + String uri = getURI(SEARCH); Response response = performRequest(HttpPost.METHOD_NAME, uri, search, Collections.emptyMap()); StatusLine statusLine = response.getStatusLine(); diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index 374120e3e4..bde3ad50b6 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -74,7 +74,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex(schema, as)); - String uri = getURI(type, BULK); + String uri = getURI(BULK); Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { @@ -96,7 +96,6 @@ public class ElasticAccountIndex extends AbstractElasticIndex IndexUtils.accountFields(o, schema.useLegacyNumericFields())), - type, sortArray); } diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index a65840044e..2be1585f06 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -14,8 +14,6 @@ package com.google.gerrit.elasticsearch; -import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES; -import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; @@ -23,7 +21,6 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; -import com.google.common.collect.Lists; import com.google.common.collect.MultimapBuilder; import com.google.common.collect.Sets; import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties; @@ -50,7 +47,6 @@ import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.index.change.ChangeIndex; -import com.google.gerrit.server.index.change.ChangeIndexRewriter; import com.google.gerrit.server.project.SubmitRuleOptions; import com.google.gerrit.server.query.change.ChangeData; import com.google.gson.JsonArray; @@ -59,7 +55,6 @@ import com.google.gson.JsonObject; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; import java.util.Collections; -import java.util.List; import java.util.Optional; import java.util.Set; import org.apache.http.HttpStatus; @@ -82,8 +77,6 @@ class ElasticChangeIndex extends AbstractElasticIndex } private static final String CHANGES = "changes"; - private static final String OPEN_CHANGES = "open_" + CHANGES; - private static final String CLOSED_CHANGES = "closed_" + CHANGES; private final ChangeMapping mapping; private final ChangeData.Factory changeDataFactory; @@ -109,7 +102,7 @@ class ElasticChangeIndex extends AbstractElasticIndex public void replace(ChangeData cd) { BulkRequest bulk = new IndexRequest(getId(cd), indexName).add(new UpdateRequest<>(schema, cd)); - String uri = getURI(type, BULK); + String uri = getURI(BULK); Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { @@ -122,27 +115,9 @@ class ElasticChangeIndex extends AbstractElasticIndex @Override public DataSource getSource(Predicate p, QueryOptions opts) throws QueryParseException { - Set statuses = ChangeIndexRewriter.getPossibleStatus(p); - List indexes = Lists.newArrayListWithCapacity(2); - if (!client.adapter().omitType()) { - if (client.adapter().useV6Type()) { - if (!Sets.intersection(statuses, OPEN_STATUSES).isEmpty() - || !Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) { - indexes.add(ElasticQueryAdapter.V6_TYPE); - } - } else { - if (!Sets.intersection(statuses, OPEN_STATUSES).isEmpty()) { - indexes.add(OPEN_CHANGES); - } - if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) { - indexes.add(CLOSED_CHANGES); - } - } - } - QueryOptions filteredOpts = opts.filterFields(o -> IndexUtils.changeFields(o, schema.useLegacyNumericFields())); - return new ElasticQuerySource(p, filteredOpts, getURI(indexes), getSortArray()); + return new ElasticQuerySource(p, filteredOpts, getSortArray()); } private JsonArray getSortArray() { @@ -155,10 +130,6 @@ class ElasticChangeIndex extends AbstractElasticIndex return sortArray; } - private String getURI(List types) { - return String.join(",", types); - } - @Override protected String getDeleteActions(Change.Id c) { return getDeleteRequest(c); @@ -166,7 +137,7 @@ class ElasticChangeIndex extends AbstractElasticIndex @Override protected String getMappings() { - return getMappingsFor(client.adapter().getType(), mapping.changes); + return getMappingsFor(mapping.changes); } @Override diff --git a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java index 35c33cb9b4..06b128c102 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java +++ b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java @@ -44,7 +44,7 @@ class ElasticConfiguration { static final String DEFAULT_PORT = "9200"; static final String DEFAULT_USERNAME = "elastic"; - static final int DEFAULT_NUMBER_OF_SHARDS = 0; + static final int DEFAULT_NUMBER_OF_SHARDS = 1; static final int DEFAULT_NUMBER_OF_REPLICAS = 1; static final int DEFAULT_MAX_RESULT_WINDOW = 10000; @@ -107,10 +107,7 @@ class ElasticConfiguration { return String.format("%s%s_%04d", prefix, name, schemaVersion); } - int getNumberOfShards(ElasticQueryAdapter adapter) { - if (numberOfShards == DEFAULT_NUMBER_OF_SHARDS) { - return adapter.getDefaultNumberOfShards(); - } + int getNumberOfShards() { return numberOfShards; } } diff --git a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java index 3922f89616..241e7fddd9 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java @@ -75,7 +75,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex(schema, group)); - String uri = getURI(type, BULK); + String uri = getURI(BULK); Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { @@ -90,7 +90,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex getSource(Predicate p, QueryOptions opts) throws QueryParseException { JsonArray sortArray = getSortArray(GroupField.UUID.getName()); - return new ElasticQuerySource(p, opts.filterFields(IndexUtils::groupFields), type, sortArray); + return new ElasticQuerySource(p, opts.filterFields(IndexUtils::groupFields), sortArray); } @Override diff --git a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java index 7e45f4fc23..36eeca1083 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticProjectIndex.java @@ -76,7 +76,7 @@ public class ElasticProjectIndex extends AbstractElasticIndex(schema, projectState)); - String uri = getURI(type, BULK); + String uri = getURI(BULK); Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { @@ -91,7 +91,7 @@ public class ElasticProjectIndex extends AbstractElasticIndex getSource(Predicate p, QueryOptions opts) throws QueryParseException { JsonArray sortArray = getSortArray(ProjectField.NAME.getName()); - return new ElasticQuerySource(p, opts.filterFields(IndexUtils::projectFields), type, sortArray); + return new ElasticQuerySource(p, opts.filterFields(IndexUtils::projectFields), sortArray); } @Override diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java index 779d433615..19d990181d 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java +++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java @@ -14,42 +14,23 @@ package com.google.gerrit.elasticsearch; -import static com.google.gerrit.elasticsearch.ElasticVersion.V6_8; - public class ElasticQueryAdapter { - static final String V6_TYPE = "_doc"; - - private static final String INCLUDE_TYPE = "include_type_name=true"; private static final String INDICES = "?allow_no_indices=false"; - private final boolean useV6Type; - private final boolean omitType; - private final int defaultNumberOfShards; - private final String searchFilteringName; - private final String indicesExistParams; private final String exactFieldType; private final String stringFieldType; private final String indexProperty; private final String rawFieldsKey; private final String versionDiscoveryUrl; - private final String includeTypeNameParam; - ElasticQueryAdapter(ElasticVersion version) { - this.useV6Type = version.isV6(); - this.omitType = version.isV7OrLater(); - this.defaultNumberOfShards = version.isV7OrLater() ? 1 : 5; - this.versionDiscoveryUrl = version.isV6OrLater() ? "/%s*" : "/%s*/_aliases"; + ElasticQueryAdapter() { + this.versionDiscoveryUrl = "/%s*"; this.searchFilteringName = "_source"; this.exactFieldType = "keyword"; this.stringFieldType = "text"; this.indexProperty = "true"; this.rawFieldsKey = "_source"; - - // Since v6.7 (end-of-life), in fact, for these two parameters: - this.indicesExistParams = - version.isAtLeastMinorVersion(V6_8) ? INDICES + "&" + INCLUDE_TYPE : INDICES; - this.includeTypeNameParam = version.isAtLeastMinorVersion(V6_8) ? "?" + INCLUDE_TYPE : ""; } public String searchFilteringName() { @@ -57,7 +38,7 @@ public class ElasticQueryAdapter { } String indicesExistParams() { - return indicesExistParams; + return INDICES; } String exactFieldType() { @@ -76,27 +57,7 @@ public class ElasticQueryAdapter { return rawFieldsKey; } - boolean useV6Type() { - return useV6Type; - } - - boolean omitType() { - return omitType; - } - - int getDefaultNumberOfShards() { - return defaultNumberOfShards; - } - - String getType() { - return useV6Type() ? V6_TYPE : ""; - } - String getVersionDiscoveryUrl(String name) { return String.format(versionDiscoveryUrl, name); } - - String includeTypeNameParam() { - return includeTypeNameParam; - } } diff --git a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java index a67de44d0d..f635b23c53 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java +++ b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java @@ -65,7 +65,7 @@ class ElasticRestClientProvider implements Provider, LifecycleListen client = build(); ElasticVersion version = getVersion(); logger.atInfo().log("Elasticsearch integration version %s", version); - adapter = new ElasticQueryAdapter(version); + adapter = new ElasticQueryAdapter(); } } } diff --git a/java/com/google/gerrit/elasticsearch/ElasticSetting.java b/java/com/google/gerrit/elasticsearch/ElasticSetting.java index e016efb94e..7ec0566a88 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticSetting.java +++ b/java/com/google/gerrit/elasticsearch/ElasticSetting.java @@ -22,18 +22,18 @@ class ElasticSetting { private static final ImmutableMap CUSTOM_CHAR_MAPPING = ImmutableMap.of("\\u002E", "\\u0020", "\\u005F", "\\u0020"); - static SettingProperties createSetting(ElasticConfiguration config, ElasticQueryAdapter adapter) { - return new ElasticSetting.Builder().addCharFilter().addAnalyzer().build(config, adapter); + static SettingProperties createSetting(ElasticConfiguration config) { + return new ElasticSetting.Builder().addCharFilter().addAnalyzer().build(config); } static class Builder { private final ImmutableMap.Builder fields = new ImmutableMap.Builder<>(); - SettingProperties build(ElasticConfiguration config, ElasticQueryAdapter adapter) { + SettingProperties build(ElasticConfiguration config) { SettingProperties properties = new SettingProperties(); properties.analysis = fields.build(); - properties.numberOfShards = config.getNumberOfShards(adapter); + properties.numberOfShards = config.getNumberOfShards(); properties.numberOfReplicas = config.numberOfReplicas; properties.maxResultWindow = config.maxResultWindow; return properties; diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index b3f1471517..225ecaac1f 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java @@ -18,7 +18,6 @@ import com.google.common.base.Joiner; import java.util.regex.Pattern; public enum ElasticVersion { - V6_8("6.8.*"), V7_0("7.0.*"), V7_1("7.1.*"), V7_2("7.2.*"), @@ -67,34 +66,6 @@ public enum ElasticVersion { return Joiner.on(", ").join(ElasticVersion.values()); } - public boolean isV6() { - return getMajor() == 6; - } - - public boolean isV6OrLater() { - return isAtLeastVersion(6); - } - - public boolean isV7OrLater() { - return isAtLeastVersion(7); - } - - private boolean isAtLeastVersion(int major) { - return getMajor() >= major; - } - - public boolean isAtLeastMinorVersion(ElasticVersion version) { - return getMajor().equals(version.getMajor()) && getMinor() >= version.getMinor(); - } - - private Integer getMajor() { - return Integer.valueOf(version.split("\\.")[0]); - } - - private Integer getMinor() { - return Integer.valueOf(version.split("\\.")[1]); - } - @Override public String toString() { return version; diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java index 7b99a55e57..f23cc10cb8 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java @@ -25,11 +25,6 @@ import org.eclipse.jgit.lib.Config; public class ElasticReindexIT extends AbstractReindexTests { @ConfigSuite.Default - public static Config elasticsearchV6() { - return getConfig(ElasticVersion.V6_8); - } - - @ConfigSuite.Config public static Config elasticsearchV7() { return getConfig(ElasticVersion.V7_8); } diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java index 43a5cebf1b..f35bcb79eb 100644 --- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java +++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java @@ -27,11 +27,6 @@ import org.eclipse.jgit.lib.Config; public class ElasticIndexIT extends AbstractIndexTests { @ConfigSuite.Default - public static Config elasticsearchV6() { - return getConfig(ElasticVersion.V6_8); - } - - @ConfigSuite.Config public static Config elasticsearchV7() { return getConfig(ElasticVersion.V7_8); } diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD index e269fc25a0..be35d5a552 100644 --- a/javatests/com/google/gerrit/elasticsearch/BUILD +++ b/javatests/com/google/gerrit/elasticsearch/BUILD @@ -50,8 +50,6 @@ TYPES = [ SUFFIX = "sTest.java" -ELASTICSEARCH_TESTS_V6 = {i: "ElasticV6Query" + i.capitalize() + SUFFIX for i in TYPES} - ELASTICSEARCH_TESTS_V7 = {i: "ElasticV7Query" + i.capitalize() + SUFFIX for i in TYPES} ELASTICSEARCH_TAGS = [ @@ -60,14 +58,6 @@ ELASTICSEARCH_TAGS = [ "exclusive", ] -[junit_tests( - name = "elasticsearch_query_%ss_test_V6" % name, - size = "large", - srcs = [src], - tags = ELASTICSEARCH_TAGS, - deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + HTTP_TEST_DEPS, -) for name, src in ELASTICSEARCH_TESTS_V6.items()] - [junit_tests( name = "elasticsearch_query_%ss_test_V7" % name, size = "large", diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index 48295ea5f9..35c32f3ccf 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -39,8 +39,6 @@ public class ElasticContainer extends ElasticsearchContainer { private static String getImageName(ElasticVersion version) { switch (version) { - case V6_8: - return "blacktop/elasticsearch:6.8.13"; case V7_0: return "blacktop/elasticsearch:7.0.1"; case V7_1: diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java deleted file mode 100644 index 15d8dd63fe..0000000000 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (C) 2018 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.elasticsearch; - -import com.google.gerrit.server.query.account.AbstractQueryAccountsTest; -import com.google.gerrit.testing.ConfigSuite; -import com.google.gerrit.testing.InMemoryModule; -import com.google.gerrit.testing.IndexConfig; -import com.google.inject.Guice; -import com.google.inject.Injector; -import org.eclipse.jgit.lib.Config; -import org.junit.AfterClass; -import org.junit.BeforeClass; - -public class ElasticV6QueryAccountsTest extends AbstractQueryAccountsTest { - @ConfigSuite.Default - public static Config defaultConfig() { - return IndexConfig.createForElasticsearch(); - } - - private static ElasticContainer container; - - @BeforeClass - public static void startIndexService() { - if (container == null) { - // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V6_8); - } - } - - @AfterClass - public static void stopElasticsearchServer() { - if (container != null) { - container.stop(); - } - } - - @Override - protected void initAfterLifecycleStart() throws Exception { - super.initAfterLifecycleStart(); - ElasticTestUtils.createAllIndexes(injector); - } - - @Override - protected Injector createInjector() { - Config elasticsearchConfig = new Config(config); - InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getSanitizedMethodName(); - ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix); - return Guice.createInjector(new InMemoryModule(elasticsearchConfig)); - } -} diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java deleted file mode 100644 index d734f1eb6f..0000000000 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright (C) 2018 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.elasticsearch; - -import static java.util.concurrent.TimeUnit.MINUTES; - -import com.google.gerrit.server.query.change.AbstractQueryChangesTest; -import com.google.gerrit.testing.ConfigSuite; -import com.google.gerrit.testing.GerritTestName; -import com.google.gerrit.testing.InMemoryModule; -import com.google.gerrit.testing.IndexConfig; -import com.google.inject.Guice; -import com.google.inject.Injector; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; -import org.apache.http.impl.nio.client.HttpAsyncClients; -import org.eclipse.jgit.lib.Config; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Rule; - -public class ElasticV6QueryChangesTest extends AbstractQueryChangesTest { - @ConfigSuite.Default - public static Config defaultConfig() { - return IndexConfig.createForElasticsearch(); - } - - private static ElasticContainer container; - private static CloseableHttpAsyncClient client; - - @BeforeClass - public static void startIndexService() { - if (container == null) { - // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V6_8); - client = HttpAsyncClients.createDefault(); - client.start(); - } - } - - @AfterClass - public static void stopElasticsearchServer() { - if (container != null) { - container.stop(); - } - } - - @Rule public final GerritTestName testName = new GerritTestName(); - - @After - public void closeIndex() throws Exception { - // Close the index after each test to prevent exceeding Elasticsearch's - // shard limit (see Issue 10120). - client - .execute( - new HttpPost( - String.format( - "http://%s:%d/%s*/_close", - container.getHttpHost().getHostName(), - container.getHttpHost().getPort(), - testName.getSanitizedMethodName())), - HttpClientContext.create(), - null) - .get(5, MINUTES); - } - - @Override - protected void initAfterLifecycleStart() throws Exception { - super.initAfterLifecycleStart(); - ElasticTestUtils.createAllIndexes(injector); - } - - @Override - protected Injector createInjector() { - Config elasticsearchConfig = new Config(config); - InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getSanitizedMethodName(); - ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix); - return Guice.createInjector(new InMemoryModule(elasticsearchConfig)); - } -} diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java deleted file mode 100644 index 28d798e7c4..0000000000 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (C) 2018 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.elasticsearch; - -import com.google.gerrit.server.query.group.AbstractQueryGroupsTest; -import com.google.gerrit.testing.ConfigSuite; -import com.google.gerrit.testing.InMemoryModule; -import com.google.gerrit.testing.IndexConfig; -import com.google.inject.Guice; -import com.google.inject.Injector; -import org.eclipse.jgit.lib.Config; -import org.junit.AfterClass; -import org.junit.BeforeClass; - -public class ElasticV6QueryGroupsTest extends AbstractQueryGroupsTest { - @ConfigSuite.Default - public static Config defaultConfig() { - return IndexConfig.createForElasticsearch(); - } - - private static ElasticContainer container; - - @BeforeClass - public static void startIndexService() { - if (container == null) { - // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V6_8); - } - } - - @AfterClass - public static void stopElasticsearchServer() { - if (container != null) { - container.stop(); - } - } - - @Override - protected void initAfterLifecycleStart() throws Exception { - super.initAfterLifecycleStart(); - ElasticTestUtils.createAllIndexes(injector); - } - - @Override - protected Injector createInjector() { - Config elasticsearchConfig = new Config(config); - InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getSanitizedMethodName(); - ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix); - return Guice.createInjector(new InMemoryModule(elasticsearchConfig)); - } -} diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java deleted file mode 100644 index 6658d7244e..0000000000 --- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (C) 2018 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.elasticsearch; - -import com.google.gerrit.server.query.project.AbstractQueryProjectsTest; -import com.google.gerrit.testing.ConfigSuite; -import com.google.gerrit.testing.InMemoryModule; -import com.google.gerrit.testing.IndexConfig; -import com.google.inject.Guice; -import com.google.inject.Injector; -import org.eclipse.jgit.lib.Config; -import org.junit.AfterClass; -import org.junit.BeforeClass; - -public class ElasticV6QueryProjectsTest extends AbstractQueryProjectsTest { - @ConfigSuite.Default - public static Config defaultConfig() { - return IndexConfig.createForElasticsearch(); - } - - private static ElasticContainer container; - - @BeforeClass - public static void startIndexService() { - if (container == null) { - // Only start Elasticsearch once - container = ElasticContainer.createAndStart(ElasticVersion.V6_8); - } - } - - @AfterClass - public static void stopElasticsearchServer() { - if (container != null) { - container.stop(); - } - } - - @Override - protected void initAfterLifecycleStart() throws Exception { - super.initAfterLifecycleStart(); - ElasticTestUtils.createAllIndexes(injector); - } - - @Override - protected Injector createInjector() { - Config elasticsearchConfig = new Config(config); - InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getSanitizedMethodName(); - ElasticTestUtils.configure(elasticsearchConfig, container, indicesPrefix); - return Guice.createInjector(new InMemoryModule(elasticsearchConfig)); - } -} diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index 9325a1b7bc..5d3d3e06a5 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java @@ -22,9 +22,6 @@ import org.junit.Test; public class ElasticVersionTest { @Test public void supportedVersion() throws Exception { - assertThat(ElasticVersion.forVersion("6.8.0")).isEqualTo(ElasticVersion.V6_8); - assertThat(ElasticVersion.forVersion("6.8.1")).isEqualTo(ElasticVersion.V6_8); - assertThat(ElasticVersion.forVersion("7.0.0")).isEqualTo(ElasticVersion.V7_0); assertThat(ElasticVersion.forVersion("7.0.1")).isEqualTo(ElasticVersion.V7_0); @@ -64,46 +61,4 @@ public class ElasticVersionTest { "Unsupported version: [4.0.0]. Supported versions: " + ElasticVersion.supportedVersions()); } - - @Test - public void atLeastMinorVersion() throws Exception { - assertThat(ElasticVersion.V6_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isTrue(); - assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_1.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - assertThat(ElasticVersion.V7_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse(); - } - - @Test - public void version6OrLater() throws Exception { - assertThat(ElasticVersion.V6_8.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_0.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_1.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_2.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_3.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_4.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_5.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_6.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_7.isV6OrLater()).isTrue(); - assertThat(ElasticVersion.V7_8.isV6OrLater()).isTrue(); - } - - @Test - public void version7OrLater() throws Exception { - assertThat(ElasticVersion.V6_8.isV7OrLater()).isFalse(); - assertThat(ElasticVersion.V7_0.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_1.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_2.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_3.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_4.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_5.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_6.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_7.isV7OrLater()).isTrue(); - assertThat(ElasticVersion.V7_8.isV7OrLater()).isTrue(); - } } From 52c143fabc606857f641c132e3dd62d0f0bd7f3f Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Sun, 6 Dec 2020 16:32:58 -0500 Subject: [PATCH 04/14] Elasticsearch: Discontinue EOL versions 7.0 and 7.1 support Remove support for elasticsearch versions 7.0 and 7.1, as both became EOL recently [1]. [1] https://www.elastic.co/support/eol Change-Id: I57fff58fbeb36c824608bc384bb32c8869145cb9 --- java/com/google/gerrit/elasticsearch/ElasticVersion.java | 2 -- .../com/google/gerrit/elasticsearch/ElasticContainer.java | 4 ---- .../com/google/gerrit/elasticsearch/ElasticVersionTest.java | 6 ------ 3 files changed, 12 deletions(-) diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index 225ecaac1f..bba1577147 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java @@ -18,8 +18,6 @@ import com.google.common.base.Joiner; import java.util.regex.Pattern; public enum ElasticVersion { - V7_0("7.0.*"), - V7_1("7.1.*"), V7_2("7.2.*"), V7_3("7.3.*"), V7_4("7.4.*"), diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index 35c32f3ccf..e8cf3e90ea 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -39,10 +39,6 @@ public class ElasticContainer extends ElasticsearchContainer { private static String getImageName(ElasticVersion version) { switch (version) { - case V7_0: - return "blacktop/elasticsearch:7.0.1"; - case V7_1: - return "blacktop/elasticsearch:7.1.1"; case V7_2: return "blacktop/elasticsearch:7.2.1"; case V7_3: diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index 5d3d3e06a5..1ec8a5dadd 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java @@ -22,12 +22,6 @@ import org.junit.Test; public class ElasticVersionTest { @Test public void supportedVersion() throws Exception { - assertThat(ElasticVersion.forVersion("7.0.0")).isEqualTo(ElasticVersion.V7_0); - assertThat(ElasticVersion.forVersion("7.0.1")).isEqualTo(ElasticVersion.V7_0); - - assertThat(ElasticVersion.forVersion("7.1.0")).isEqualTo(ElasticVersion.V7_1); - assertThat(ElasticVersion.forVersion("7.1.1")).isEqualTo(ElasticVersion.V7_1); - assertThat(ElasticVersion.forVersion("7.2.0")).isEqualTo(ElasticVersion.V7_2); assertThat(ElasticVersion.forVersion("7.2.1")).isEqualTo(ElasticVersion.V7_2); From 8ae8584c30dd2a0be4d01e0c741afc32f443f62b Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 4 Dec 2020 10:16:30 +0000 Subject: [PATCH 05/14] Compact the REST-API output JSON unconditionally The output JSON was initially compacted only when the Accept header was set to 'application/json', from the very initial Change-Id: I2014a2209. The ability to get a pretty-printed JSON is still available using the 'pp=1' query string, therefore it is best to always compress the JSON output by default. Bug: Issue 13781 Change-Id: I2f1cb91e95e10b27b328764834d58d0799e01ed4 (cherry picked from commit 9c09efbc48038c3b03d5f388f95c845ae8b409c1) --- .../gerrit/httpd/restapi/RestApiServlet.java | 23 ++---- .../acceptance/rest/RestApiServletIT.java | 76 +++++++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 88c5106e67..a7c0e300fd 100644 --- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -1071,7 +1071,7 @@ public class RestApiServlet extends HttpServlet { TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE); buf.write(JSON_MAGIC); Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8)); - Gson gson = newGson(config, req); + Gson gson = newGson(config); if (result instanceof JsonElement) { gson.toJson((JsonElement) result, w); } else { @@ -1098,25 +1098,18 @@ public class RestApiServlet extends HttpServlet { req, res, asBinaryResult(buf).setContentType(JSON_TYPE).setCharacterEncoding(UTF_8)); } - private static Gson newGson( - ListMultimap config, @Nullable HttpServletRequest req) { + private static Gson newGson(ListMultimap config) { GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder(); - enablePrettyPrint(gb, config, req); + enablePrettyPrint(gb, config); enablePartialGetFields(gb, config); return gb.create(); } - private static void enablePrettyPrint( - GsonBuilder gb, ListMultimap config, @Nullable HttpServletRequest req) { - String pp = Iterables.getFirst(config.get("pp"), null); - if (pp == null) { - pp = Iterables.getFirst(config.get("prettyPrint"), null); - if (pp == null && req != null) { - pp = acceptsJson(req) ? "0" : "1"; - } - } + private static void enablePrettyPrint(GsonBuilder gb, ListMultimap config) { + String pp = + Iterables.getFirst(config.get("pp"), Iterables.getFirst(config.get("prettyPrint"), "0")); if ("1".equals(pp) || "true".equals(pp)) { gb.setPrettyPrinting(); } @@ -1573,10 +1566,6 @@ public class RestApiServlet extends HttpServlet { return CharMatcher.anyOf("<&").matchesAnyOf(text); } - private static boolean acceptsJson(HttpServletRequest req) { - return req != null && isType(JSON_TYPE, req.getHeader(HttpHeaders.ACCEPT)); - } - private static boolean acceptsGzip(HttpServletRequest req) { if (req != null) { String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING); diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java new file mode 100644 index 0000000000..bc454605ab --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java @@ -0,0 +1,76 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.rest; + +import static com.google.common.truth.Truth.assertThat; +import static org.apache.http.HttpStatus.SC_OK; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.httpd.restapi.RestApiServlet; +import java.io.IOException; +import java.util.regex.Pattern; +import org.apache.http.message.BasicHeader; +import org.junit.Test; + +public class RestApiServletIT extends AbstractDaemonTest { + private static String ANY_REST_API = "/accounts/self/capabilities"; + private static BasicHeader ACCEPT_STAR_HEADER = new BasicHeader("Accept", "*/*"); + private static Pattern ANY_SPACE = Pattern.compile("\\s"); + + @Test + public void restResponseBodyShouldBeCompactWithoutSpaces() throws Exception { + RestResponse response = adminRestSession.getWithHeader(ANY_REST_API, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + assertThat(contentWithoutMagicJson(response)).doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPPIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBeCompactWithoutSpacesWhenPrerryPrintIsZero() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 0))) + .doesNotContainMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPPIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 1))).containsMatch(ANY_SPACE); + } + + @Test + public void restResponseBodyShouldBePrettyfiedWhenPrettyPrintIsOne() throws Exception { + assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 1))) + .containsMatch(ANY_SPACE); + } + + private RestResponse prettyJsonRestResponse(String ppArgument, int ppValue) throws Exception { + RestResponse response = + adminRestSession.getWithHeader( + ANY_REST_API + "?" + ppArgument + "=" + ppValue, ACCEPT_STAR_HEADER); + assertThat(response.getStatusCode()).isEqualTo(SC_OK); + + return response; + } + + private String contentWithoutMagicJson(RestResponse response) throws IOException { + return response.getEntityContent().substring(RestApiServlet.JSON_MAGIC.length); + } +} From e746bd26b5830c998cd5091d11c542416b062e5f Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Tue, 8 Dec 2020 08:35:10 -0700 Subject: [PATCH 06/14] Update git submodules * Update plugins/replication from branch 'stable-3.0' to 2a600dede934b348173bff26e00f373367a3d142 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Fix replication to retry on lock errors Change-Id: I6e262d2c22d2dcd49b341b3c752d6d8b6c93b32c - Fix replication to retry on lock errors Versions of Git released since 2014 have created a new status "failed to update ref" which replaces the two statuses "failed to lock" and "failed to write". So, we now see the newer status when the remote is unable to lock a ref. Refer Git commit: https://github.com/git/git/commit/6629ea2d4a5faa0a84367f6d4aedba53cb0f26b4 Config 'lockErrorMaxRetries' is not removed as part of this change as folks who have it configured currently don't run into unexpected behavior with retries when they upgrade to a newer version of the plugin. Also, the "failed to lock" check is not removed for folks still using a version of Git older than 2014. Change-Id: I9b3b15bebd55df30cbee50a0e0c2190d04f2f443 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index 4604b01e43..2a600dede9 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 4604b01e43c3b4aee3d03cb84945b3238c150ede +Subproject commit 2a600dede934b348173bff26e00f373367a3d142 From 81841ec2856f62a8cadbc308b82293895c63edb2 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Tue, 8 Dec 2020 08:35:17 -0700 Subject: [PATCH 07/14] Update git submodules * Update plugins/replication from branch 'stable-3.1' to c4df2f1eb9fe470debf4958e05cf29c2ec314b43 - Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: Fix replication to retry on lock errors Change-Id: Ib4b2c1fcac5da6551f72bce68a101b93e9b43b19 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Fix replication to retry on lock errors Change-Id: I6e262d2c22d2dcd49b341b3c752d6d8b6c93b32c - Fix replication to retry on lock errors Versions of Git released since 2014 have created a new status "failed to update ref" which replaces the two statuses "failed to lock" and "failed to write". So, we now see the newer status when the remote is unable to lock a ref. Refer Git commit: https://github.com/git/git/commit/6629ea2d4a5faa0a84367f6d4aedba53cb0f26b4 Config 'lockErrorMaxRetries' is not removed as part of this change as folks who have it configured currently don't run into unexpected behavior with retries when they upgrade to a newer version of the plugin. Also, the "failed to lock" check is not removed for folks still using a version of Git older than 2014. Change-Id: I9b3b15bebd55df30cbee50a0e0c2190d04f2f443 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index e09b5a08e1..c4df2f1eb9 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit e09b5a08e183d4f92410bfa06289a1784cefb43b +Subproject commit c4df2f1eb9fe470debf4958e05cf29c2ec314b43 From 18a859fd8b811448f11dbd8493fa111d6fddfa9c Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Tue, 8 Dec 2020 12:13:39 -0700 Subject: [PATCH 08/14] Update git submodules * Update plugins/replication from branch 'stable-3.2' to 5bd0d7e8e5704b11a95a0f2ca4c67eb392002484 - Merge branch 'stable-3.1' into stable-3.2 * stable-3.1: Fix replication to retry on lock errors Change-Id: Icacd9095feaefd240803405c5b0a16cc0b3a9ed8 - Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: Fix replication to retry on lock errors Change-Id: Ib4b2c1fcac5da6551f72bce68a101b93e9b43b19 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Fix replication to retry on lock errors Change-Id: I6e262d2c22d2dcd49b341b3c752d6d8b6c93b32c - Fix replication to retry on lock errors Versions of Git released since 2014 have created a new status "failed to update ref" which replaces the two statuses "failed to lock" and "failed to write". So, we now see the newer status when the remote is unable to lock a ref. Refer Git commit: https://github.com/git/git/commit/6629ea2d4a5faa0a84367f6d4aedba53cb0f26b4 Config 'lockErrorMaxRetries' is not removed as part of this change as folks who have it configured currently don't run into unexpected behavior with retries when they upgrade to a newer version of the plugin. Also, the "failed to lock" check is not removed for folks still using a version of Git older than 2014. Change-Id: I9b3b15bebd55df30cbee50a0e0c2190d04f2f443 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index ada8228a73..5bd0d7e8e5 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit ada8228a735d80a0a8652761621ca1d8c5ba986d +Subproject commit 5bd0d7e8e5704b11a95a0f2ca4c67eb392002484 From a936f9cd4e049b21356a382d5b7f5acb513c091e Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Tue, 8 Dec 2020 12:13:44 -0700 Subject: [PATCH 09/14] Update git submodules * Update plugins/replication from branch 'stable-3.3' to 5c2a69917080999f77a02541a753078c7729232c - Merge branch 'stable-3.2' into stable-3.3 * stable-3.2: Fix replication to retry on lock errors Change-Id: Iab364714135d693e011e8abbf7782ae620d009c4 - Merge branch 'stable-3.1' into stable-3.2 * stable-3.1: Fix replication to retry on lock errors Change-Id: Icacd9095feaefd240803405c5b0a16cc0b3a9ed8 - Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: Fix replication to retry on lock errors Change-Id: Ib4b2c1fcac5da6551f72bce68a101b93e9b43b19 - Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Fix replication to retry on lock errors Change-Id: I6e262d2c22d2dcd49b341b3c752d6d8b6c93b32c - Fix replication to retry on lock errors Versions of Git released since 2014 have created a new status "failed to update ref" which replaces the two statuses "failed to lock" and "failed to write". So, we now see the newer status when the remote is unable to lock a ref. Refer Git commit: https://github.com/git/git/commit/6629ea2d4a5faa0a84367f6d4aedba53cb0f26b4 Config 'lockErrorMaxRetries' is not removed as part of this change as folks who have it configured currently don't run into unexpected behavior with retries when they upgrade to a newer version of the plugin. Also, the "failed to lock" check is not removed for folks still using a version of Git older than 2014. Change-Id: I9b3b15bebd55df30cbee50a0e0c2190d04f2f443 --- plugins/replication | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/replication b/plugins/replication index a6ca76650e..5c2a699170 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit a6ca76650e4befab43f9fac288e6f27a38708dd1 +Subproject commit 5c2a69917080999f77a02541a753078c7729232c From 70a7a75e74fcdea231234c3da0254298b68b3fb6 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 3 Dec 2020 17:58:05 +0100 Subject: [PATCH 10/14] Bump commons-io version to 2.4 The currently used version is from 2012. This dependency is not used in gerrit core, but only in delete-project plugin. Bug: Issue 13779 Change-Id: Ie15285654e473d35b7c118282cee71cb6b62d30b --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 9981218b6d..f88729eb02 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -921,8 +921,8 @@ maven_jar( maven_jar( name = "commons-io", - artifact = "commons-io:commons-io:2.2", - sha1 = "83b5b8a7ba1c08f9e8c8ff2373724e33d3c1e22a", + artifact = "commons-io:commons-io:2.4", + sha1 = "b1b6ea3b7e4aa4f492509a4952029cd8e48019ad", ) maven_jar( From d07ec9c4badac7032a7345d8ae4c3978d64cc437 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 10 Dec 2020 17:33:05 +0100 Subject: [PATCH 11/14] Exempt commons-io from library compliance Commons-io is not used in gerrit core, but only in delete-project core plugin and other non-core plugins: find-owners and uploadvalidator. Change-Id: I189aec6abf175aa9edf9006e285071048d4cb7a9 --- WORKSPACE | 6 ------ lib/nongoogle_test.sh | 1 + tools/nongoogle.bzl | 6 ++++++ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index f88729eb02..0a7d635483 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -919,12 +919,6 @@ maven_jar( sha1 = "c88807f210ab216aa831b48569ef50bd797384bc", ) -maven_jar( - name = "commons-io", - artifact = "commons-io:commons-io:2.4", - sha1 = "b1b6ea3b7e4aa4f492509a4952029cd8e48019ad", -) - maven_jar( name = "asciidoctor", artifact = "org.asciidoctor:asciidoctorj:1.5.7", diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh index 8369024c3b..9ec19a3a2c 100755 --- a/lib/nongoogle_test.sh +++ b/lib/nongoogle_test.sh @@ -12,6 +12,7 @@ grep 'name = "[^"]*"' ${bzl} | sed 's|^[^"]*"||g;s|".*$||g' | sort > $TMP/names cat << EOF > $TMP/want cglib-3_2 +commons-io docker-java-api docker-java-transport dropwizard-core diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index 459143d153..1868e37b73 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl @@ -106,6 +106,12 @@ def declare_nongoogle_deps(): sha1 = "c2351800432bdbdd8284c3f5a7f0782a352aa84a", ) + maven_jar( + name = "commons-io", + artifact = "commons-io:commons-io:2.4", + sha1 = "b1b6ea3b7e4aa4f492509a4952029cd8e48019ad", + ) + # Google internal dependencies: these are developed at Google, so there is # no concern about version skew. From eefcdbf2dc39c94dc0a73e9c5aa0f718ad6cf51a Mon Sep 17 00:00:00 2001 From: Ivo List Date: Wed, 25 Nov 2020 11:19:53 +0000 Subject: [PATCH 12/14] Move java_tools javac argument into tools parameter. Bazel@HEAD has javac parameter removed. The javac package can be listed next to other tools. Change-Id: I15aaad773d81d8ccfc4374252abfc5da0420e46f (cherry picked from commit b4d318e27536da0c8c4c17a69592923bc6427054) --- tools/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/BUILD b/tools/BUILD index be12735c7c..545a20607b 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -44,7 +44,6 @@ default_java_toolchain( header_compiler_direct = ["@bazel_tools//tools/jdk:turbine_direct"], ijar = ["@bazel_tools//tools/jdk:ijar"], javabuilder = ["@bazel_tools//tools/jdk:javabuilder"], - javac = ["@bazel_tools//tools/jdk:javac_jar"], javac_supports_workers = True, jvm_opts = JDK11_JVM_OPTS, misc = [ @@ -60,6 +59,7 @@ default_java_toolchain( target_version = "11", tools = [ "@bazel_tools//tools/jdk:java_compiler_jar", + "@bazel_tools//tools/jdk:javac_jar", "@bazel_tools//tools/jdk:jdk_compiler_jar", ], visibility = ["//visibility:public"], From c2fadfdb1751992f6809445c513f7012115a32f9 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 14 Dec 2020 18:55:15 +0100 Subject: [PATCH 13/14] Update JGit to 5d925ecbb This JGit update is needed to fix unnecessary Bazel rebuilds. This update includes the following commit: 5d925ecbb Fix stamping to produce stable file timestamps Change-Id: Iebdf4f87a54fefe6b824d86f482059f36173869c --- modules/jgit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jgit b/modules/jgit index 9a1065afec..5d925ecbb3 160000 --- a/modules/jgit +++ b/modules/jgit @@ -1 +1 @@ -Subproject commit 9a1065afec18c5a76d3a95e77aa70d7a24252056 +Subproject commit 5d925ecbb3d0977c586f0001baf20aff12823de9 From 2a9ab5156f6ab3ee770abbd9933fc008007f51b8 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 14 Dec 2020 19:13:12 +0100 Subject: [PATCH 14/14] Bazel: Perform some clean ups after rules_closure removal rules_closure dependency was removed in I8f99133eb0. This change removes some left overs: 1. Remove now no-op closure worker strategy from .bazelrc. 2. Remove now unused compatibility layer in generated external repository BUILD file. Change-Id: I7b70820e0ef57ea900a04fb821f53eef2c5eb4b9 --- .bazelrc | 2 +- tools/bzl/maven_jar.bzl | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.bazelrc b/.bazelrc index b9189c1dfe..6e264845ee 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,4 +1,4 @@ -build --workspace_status_command="python ./tools/workspace_status.py" --strategy=Closure=worker +build --workspace_status_command="python ./tools/workspace_status.py" build --repository_cache=~/.gerritcodereview/bazel-cache/repository build --action_env=PATH build --disk_cache=~/.gerritcodereview/bazel-cache/cas diff --git a/tools/bzl/maven_jar.bzl b/tools/bzl/maven_jar.bzl index 9908ee8136..15b1797b84 100644 --- a/tools/bzl/maven_jar.bzl +++ b/tools/bzl/maven_jar.bzl @@ -126,18 +126,6 @@ java_import( """.format(srcjar = srcjar) ctx.file("%s/BUILD" % ctx.path("jar"), contents, False) - # Compatibility layer for java_import_external from rules_closure - contents = """ -{header} -package(default_visibility = ['//visibility:public']) - -alias( - name = "{rule_name}", - actual = "@{rule_name}//jar", -) -\n""".format(rule_name = ctx.name, header = header) - ctx.file("BUILD", contents, False) - def _maven_jar_impl(ctx): """rule to download a Maven archive.""" coordinates = _create_coordinates(ctx.attr.artifact)