From 2f9d6faec8fef0db777ebdb75038ac8a754fc732 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Wed, 30 May 2018 08:17:44 +0200 Subject: [PATCH 1/5] Add Bazel version check Bazel version check was implemented on master, and stable branches, that were already migrated to Bazel, missied that feature. We need the version check, to add bazel caching feature, that was only added in very recent Bazel versions. This is done in follow-up change. Change-Id: I901caa6f9c037be29d377dd9f1c451d57ecc252f --- WORKSPACE | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/WORKSPACE b/WORKSPACE index 591487c198..497ee8758d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,6 +4,13 @@ load("//tools/bzl:maven_jar.bzl", "maven_jar", "GERRIT", "MAVEN_LOCAL") load("//lib/codemirror:cm.bzl", "CM_VERSION", "DIFF_MATCH_PATCH_VERSION") load("//plugins:external_plugin_deps.bzl", "external_plugin_deps") +http_archive( + name = "bazel_skylib", + sha256 = "bbccf674aa441c266df9894182d80de104cabd19be98be002f6d478aaa31574d", + strip_prefix = "bazel-skylib-2169ae1c374aab4a09aa90e65efe1a3aad4e279b", + urls = ["https://github.com/bazelbuild/bazel-skylib/archive/2169ae1c374aab4a09aa90e65efe1a3aad4e279b.tar.gz"], +) + http_archive( name = "io_bazel_rules_closure", sha256 = "6691c58a2cd30a86776dd9bb34898b041e37136f2dc7e24cadaeaf599c95c657", @@ -20,6 +27,10 @@ http_file( url = "https://raw.githubusercontent.com/google/closure-compiler/775609aad61e14aef289ebec4bfc09ad88877f9e/contrib/externs/polymer-1.0.js", ) +load("@bazel_skylib//:lib.bzl", "versions") + +versions.check(minimum_bazel_version = "0.7.0") + load("@io_bazel_rules_closure//closure:defs.bzl", "closure_repositories") # Prevent redundant loading of dependencies. From 16e1dccac103a74558b6bfd35b7f0de5fa627ac0 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 1 Jun 2018 12:27:36 +0900 Subject: [PATCH 2/5] Elasticsearch: Add an adapter to support V5 Add an adapter that adjusts the query content depending on the Elasticsearch version. Bug: Issue 6094 Also-by: Marco Miller Helped-by: David Ostrovsky Change-Id: Ie3de3f5cec25c44a9c56eb3c430e63d0ff858894 --- .../elasticsearch/AbstractElasticIndex.java | 8 +- .../elasticsearch/ElasticAccountIndex.java | 8 +- .../elasticsearch/ElasticChangeIndex.java | 10 +-- .../elasticsearch/ElasticGroupIndex.java | 8 +- .../gerrit/elasticsearch/ElasticMapping.java | 32 ++++++-- .../elasticsearch/ElasticQueryAdapter.java | 74 +++++++++++++++++++ .../ElasticRestClientProvider.java | 7 ++ .../builders/SearchSourceBuilder.java | 10 ++- 8 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index 190c49875c..6f8fa061eb 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -50,7 +50,6 @@ import org.elasticsearch.client.Response; abstract class AbstractElasticIndex implements Index { protected static final String BULK = "_bulk"; - protected static final String IGNORE_UNMAPPED = "ignore_unmapped"; protected static final String ORDER = "order"; protected static final String SEARCH = "_search"; @@ -80,8 +79,8 @@ abstract class AbstractElasticIndex implements Index { private final Schema schema; private final SitePaths sitePaths; private final String indexNameRaw; - private final ElasticRestClientProvider client; + protected final ElasticRestClientProvider client; protected final String indexName; protected final Gson gson; protected final ElasticQueryBuilder queryBuilder; @@ -130,7 +129,8 @@ abstract class AbstractElasticIndex implements Index { @Override public void deleteAll() throws IOException { // Delete the index, if it exists. - Response response = client.get().performRequest("HEAD", indexName); + String endpoint = indexName + client.adapter().indicesExistParam(); + Response response = client.get().performRequest("HEAD", endpoint); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_OK) { response = client.get().performRequest("DELETE", indexName); @@ -182,7 +182,7 @@ abstract class AbstractElasticIndex implements Index { protected JsonArray getSortArray(String idFieldName) { JsonObject properties = new JsonObject(); properties.addProperty(ORDER, "asc"); - properties.addProperty(IGNORE_UNMAPPED, true); + client.adapter().setIgnoreUnmapped(properties); JsonArray sortArray = new JsonArray(); addNamedElement(idFieldName, properties, sortArray); diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index ba0ee58350..c5ab96741a 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -61,8 +61,8 @@ public class ElasticAccountIndex extends AbstractElasticIndex schema) { - this.accounts = ElasticMapping.createMapping(schema); + public AccountMapping(Schema schema, ElasticQueryAdapter adapter) { + this.accounts = ElasticMapping.createMapping(schema, adapter); } } @@ -83,7 +83,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex schema) { super(cfg, sitePaths, schema, client, ACCOUNTS); this.accountCache = accountCache; - this.mapping = new AccountMapping(schema); + this.mapping = new AccountMapping(schema, client.adapter()); this.schema = schema; } @@ -133,7 +133,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex schema) { - MappingProperties mapping = ElasticMapping.createMapping(schema); + public ChangeMapping(Schema schema, ElasticQueryAdapter adapter) { + MappingProperties mapping = ElasticMapping.createMapping(schema, adapter); this.openChanges = mapping; this.closedChanges = mapping; } @@ -117,7 +117,7 @@ public class ElasticChangeIndex extends AbstractElasticIndex schema) { - this.groups = ElasticMapping.createMapping(schema); + public GroupMapping(Schema schema, ElasticQueryAdapter adapter) { + this.groups = ElasticMapping.createMapping(schema, adapter); } } @@ -80,7 +80,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex schema) { super(cfg, sitePaths, schema, client, GROUPS); this.groupCache = groupCache; - this.mapping = new GroupMapping(schema); + this.mapping = new GroupMapping(schema, client.adapter()); this.schema = schema; } @@ -130,7 +130,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex schema) { - ElasticMapping.Builder mapping = new ElasticMapping.Builder(); + static MappingProperties createMapping(Schema schema, ElasticQueryAdapter adapter) { + ElasticMapping.Builder mapping = new ElasticMapping.Builder(adapter); for (FieldDef field : schema.getFields().values()) { String name = field.getName(); FieldType fieldType = field.getType(); - if (fieldType == FieldType.EXACT || fieldType == FieldType.KEYWORD) { + if (fieldType == FieldType.EXACT) { mapping.addExactField(name); + } else if (fieldType == FieldType.KEYWORD) { + mapping.addKeywordField(name); } else if (fieldType == FieldType.TIMESTAMP) { mapping.addTimestamp(name); } else if (fieldType == FieldType.INTEGER @@ -46,9 +48,14 @@ class ElasticMapping { } static class Builder { + private final ElasticQueryAdapter adapter; private final ImmutableMap.Builder fields = new ImmutableMap.Builder<>(); + Builder(ElasticQueryAdapter adapter) { + this.adapter = adapter; + } + MappingProperties build() { MappingProperties properties = new MappingProperties(); properties.properties = fields.build(); @@ -56,9 +63,20 @@ class ElasticMapping { } Builder addExactField(String name) { - FieldProperties key = new FieldProperties("string"); - key.index = "not_analyzed"; - FieldProperties properties = new FieldProperties("string"); + FieldProperties key = new FieldProperties(adapter.keywordFieldType()); + key.index = adapter.indexProperty(); + FieldProperties properties; + properties = new FieldProperties(adapter.stringFieldType()); + properties.fields = ImmutableMap.of("key", key); + fields.put(name, properties); + return this; + } + + Builder addKeywordField(String name) { + FieldProperties key = new FieldProperties(adapter.keywordFieldType()); + key.index = adapter.indexProperty(); + FieldProperties properties; + properties = new FieldProperties(adapter.keywordFieldType()); properties.fields = ImmutableMap.of("key", key); fields.put(name, properties); return this; @@ -78,7 +96,7 @@ class ElasticMapping { } Builder addString(String name) { - fields.put(name, new FieldProperties("string")); + fields.put(name, new FieldProperties(adapter.stringFieldType())); return this; } diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java new file mode 100644 index 0000000000..2ed75df464 --- /dev/null +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java @@ -0,0 +1,74 @@ +// 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.gson.JsonObject; + +public class ElasticQueryAdapter { + private final boolean ignoreUnmapped; + private final String searchFilteringName; + private final String indicesExistParam; + private final String keywordFieldType; + private final String stringFieldType; + private final String indexProperty; + + ElasticQueryAdapter(ElasticVersion version) { + this.ignoreUnmapped = version == ElasticVersion.V2_4; + switch (version) { + case V5_6: + case V6_2: + this.searchFilteringName = "_source"; + this.indicesExistParam = "?allow_no_indices=false"; + this.keywordFieldType = "keyword"; + this.stringFieldType = "text"; + this.indexProperty = "true"; + break; + case V2_4: + default: + this.searchFilteringName = "fields"; + this.indicesExistParam = ""; + this.keywordFieldType = "string"; + this.stringFieldType = "string"; + this.indexProperty = "not_analyzed"; + break; + } + } + + void setIgnoreUnmapped(JsonObject properties) { + if (ignoreUnmapped) { + properties.addProperty("ignore_unmapped", true); + } + } + + public String searchFilteringName() { + return searchFilteringName; + } + + String indicesExistParam() { + return indicesExistParam; + } + + String keywordFieldType() { + return keywordFieldType; + } + + String stringFieldType() { + return stringFieldType; + } + + String indexProperty() { + return indexProperty; + } +} diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java index 91f938c956..a69f851e0d 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java @@ -44,6 +44,7 @@ class ElasticRestClientProvider implements Provider, LifecycleListen private final String password; private RestClient client; + private ElasticQueryAdapter adapter; @Inject ElasticRestClientProvider(ElasticConfiguration cfg) { @@ -69,6 +70,7 @@ class ElasticRestClientProvider implements Provider, LifecycleListen client = build(); ElasticVersion version = getVersion(); log.info("Elasticsearch integration version {}", version); + adapter = new ElasticQueryAdapter(version); } } } @@ -89,6 +91,11 @@ class ElasticRestClientProvider implements Provider, LifecycleListen } } + ElasticQueryAdapter adapter() { + get(); // Make sure we're connected + return adapter; + } + public static class FailedToGetVersion extends ElasticException { private static final long serialVersionUID = 1L; private static final String MESSAGE = "Failed to get Elasticsearch version"; diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/SearchSourceBuilder.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/SearchSourceBuilder.java index e72e9fa74c..35cbea99b6 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/SearchSourceBuilder.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/SearchSourceBuilder.java @@ -14,6 +14,7 @@ package com.google.gerrit.elasticsearch.builders; +import com.google.gerrit.elasticsearch.ElasticQueryAdapter; import java.io.IOException; import java.util.List; @@ -23,6 +24,7 @@ import java.util.List; *

A trimmed down and modified version of org.elasticsearch.search.builder.SearchSourceBuilder. */ public class SearchSourceBuilder { + private final ElasticQueryAdapter adapter; private QuerySourceBuilder querySourceBuilder; @@ -33,7 +35,9 @@ public class SearchSourceBuilder { private List fieldNames; /** Constructs a new search source builder. */ - public SearchSourceBuilder() {} + public SearchSourceBuilder(ElasticQueryAdapter adapter) { + this.adapter = adapter; + } /** Constructs a new search source builder with a search query. */ public SearchSourceBuilder query(QueryBuilder query) { @@ -95,9 +99,9 @@ public class SearchSourceBuilder { if (fieldNames != null) { if (fieldNames.size() == 1) { - builder.field("fields", fieldNames.get(0)); + builder.field(adapter.searchFilteringName(), fieldNames.get(0)); } else { - builder.startArray("fields"); + builder.startArray(adapter.searchFilteringName()); for (String fieldName : fieldNames) { builder.value(fieldName); } From 33be879ea3b4ab1e6c05d696e4c395411b72dcc7 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 1 Jun 2018 15:04:45 -0400 Subject: [PATCH 3/5] MatchQueryBuilder: Don't use deprecated "match" query Before this fix, warnings such as [1] used to be present in the Elasticsearch V5 logs. Elasticsearch V2 did/does not show them. Properly factor the match phrase queries (both plain and prefix ones), fitting the way to prepare them as per Elasticsearch V5 documentation. Doing this way happens to also be supported by Elasticsearch V2, so no version-based api usage switching required here (in this specific case). [1] "Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]". Bug: Issue 6094 Change-Id: I2b970c426b425957634981c9f4de0cc5cd474d03 --- .../builders/MatchQueryBuilder.java | 19 ++++++++----------- .../elasticsearch/builders/QueryBuilders.java | 4 ++-- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/MatchQueryBuilder.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/MatchQueryBuilder.java index 7a120801f8..c0becd1e4b 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/MatchQueryBuilder.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/MatchQueryBuilder.java @@ -27,9 +27,14 @@ class MatchQueryBuilder extends QueryBuilder { enum Type { /** The text is analyzed and used as a phrase query. */ - PHRASE, + MATCH_PHRASE, /** The text is analyzed and used in a phrase query, with the last term acting as a prefix. */ - PHRASE_PREFIX + MATCH_PHRASE_PREFIX; + + @Override + public String toString() { + return name().toLowerCase(Locale.US); + } } private final String name; @@ -52,14 +57,6 @@ class MatchQueryBuilder extends QueryBuilder { @Override protected void doXContent(XContentBuilder builder) throws IOException { - builder.startObject("match"); - builder.startObject(name); - - builder.field("query", text); - if (type != null) { - builder.field("type", type.toString().toLowerCase(Locale.ENGLISH)); - } - builder.endObject(); - builder.endObject(); + builder.startObject(type.toString()).field(name, text).endObject(); } } diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/QueryBuilders.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/QueryBuilders.java index 26fac4cceb..940146f147 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/QueryBuilders.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/builders/QueryBuilders.java @@ -33,7 +33,7 @@ public abstract class QueryBuilders { * @param text The query text (to be analyzed). */ public static MatchQueryBuilder matchPhraseQuery(String name, Object text) { - return new MatchQueryBuilder(name, text).type(MatchQueryBuilder.Type.PHRASE); + return new MatchQueryBuilder(name, text).type(MatchQueryBuilder.Type.MATCH_PHRASE); } /** @@ -43,7 +43,7 @@ public abstract class QueryBuilders { * @param text The query text (to be analyzed). */ public static MatchQueryBuilder matchPhrasePrefixQuery(String name, Object text) { - return new MatchQueryBuilder(name, text).type(MatchQueryBuilder.Type.PHRASE_PREFIX); + return new MatchQueryBuilder(name, text).type(MatchQueryBuilder.Type.MATCH_PHRASE_PREFIX); } /** From 8c058bf8622f49332e8ad40ee01f36ad8927c941 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 25 May 2018 19:53:22 +0900 Subject: [PATCH 4/5] Elasticsearch: Add tests for queries against version 5 Add separate test classes to test queries with Elasticsearch version 5. The entire test classes are basically copied from the existing ones, with the only difference being the version passed into the config and into the container creation. This cannot be achieved using an abstract class due to the container being created in the @BeforeClass annotated method which is static and cannot be overridden by a derived class. The byOwnerInvalidQuery test is moved up from ElasticQueryChangesTest to AbstractQueryChangesTest to avoid having to repeat it in all the Elasticsearch test classes. Bug: Issue 6094 Change-Id: I9b91cbcbc24924dce43b1be9530543097b5fd174 --- .../ElasticQueryChangesTest.java | 11 --- .../ElasticV5QueryAccountsTest.java | 68 ++++++++++++++++++ .../ElasticV5QueryChangesTest.java | 72 +++++++++++++++++++ .../ElasticV5QueryGroupsTest.java | 68 ++++++++++++++++++ .../change/AbstractQueryChangesTest.java | 8 +++ .../query/change/LuceneQueryChangesTest.java | 1 + 6 files changed, 217 insertions(+), 11 deletions(-) create mode 100644 gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java create mode 100644 gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java create mode 100644 gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java index bf29cbbf44..fb21746920 100644 --- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java @@ -19,15 +19,12 @@ import com.google.gerrit.elasticsearch.testing.ElasticTestUtils; import com.google.gerrit.elasticsearch.testing.ElasticTestUtils.ElasticNodeInfo; import com.google.gerrit.server.query.change.AbstractQueryChangesTest; import com.google.gerrit.testutil.InMemoryModule; -import com.google.gerrit.testutil.InMemoryRepositoryManager.Repo; import com.google.inject.Guice; import com.google.inject.Injector; -import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Rule; -import org.junit.Test; import org.junit.rules.TestName; public class ElasticQueryChangesTest extends AbstractQueryChangesTest { @@ -72,12 +69,4 @@ public class ElasticQueryChangesTest extends AbstractQueryChangesTest { ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); } - - @Test - public void byOwnerInvalidQuery() throws Exception { - TestRepository repo = createProject("repo"); - insert(repo, newChange(repo), userId); - String nameEmail = user.asIdentifiedUser().getNameEmail(); - assertQuery("owner: \"" + nameEmail + "\"\\"); - } } diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java new file mode 100644 index 0000000000..e59824dc4d --- /dev/null +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java @@ -0,0 +1,68 @@ +// 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.elasticsearch.testing.ElasticContainer; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils.ElasticNodeInfo; +import com.google.gerrit.server.query.account.AbstractQueryAccountsTest; +import com.google.gerrit.testutil.InMemoryModule; +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 ElasticV5QueryAccountsTest extends AbstractQueryAccountsTest { + private static ElasticNodeInfo nodeInfo; + private static ElasticContainer container; + + @BeforeClass + public static void startIndexService() { + if (nodeInfo != null) { + // do not start Elasticsearch twice + return; + } + + container = ElasticContainer.createAndStart(ElasticVersion.V5_6); + nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); + } + + @AfterClass + public static void stopElasticsearchServer() { + if (container != null) { + container.stop(); + } + } + + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @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(); + ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); + return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); + } +} diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java new file mode 100644 index 0000000000..f9631009c0 --- /dev/null +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java @@ -0,0 +1,72 @@ +// 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.elasticsearch.testing.ElasticContainer; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils.ElasticNodeInfo; +import com.google.gerrit.server.query.change.AbstractQueryChangesTest; +import com.google.gerrit.testutil.InMemoryModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import org.eclipse.jgit.lib.Config; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.rules.TestName; + +public class ElasticV5QueryChangesTest extends AbstractQueryChangesTest { + @Rule public final TestName testName = new TestName(); + + private static ElasticNodeInfo nodeInfo; + private static ElasticContainer container; + + @BeforeClass + public static void startIndexService() { + if (nodeInfo != null) { + // do not start Elasticsearch twice + return; + } + + container = ElasticContainer.createAndStart(ElasticVersion.V5_6); + nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); + } + + @AfterClass + public static void stopElasticsearchServer() { + if (container != null) { + container.stop(); + } + } + + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @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(); + ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); + return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); + } +} diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java new file mode 100644 index 0000000000..6665801dea --- /dev/null +++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java @@ -0,0 +1,68 @@ +// 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.elasticsearch.testing.ElasticContainer; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils; +import com.google.gerrit.elasticsearch.testing.ElasticTestUtils.ElasticNodeInfo; +import com.google.gerrit.server.query.group.AbstractQueryGroupsTest; +import com.google.gerrit.testutil.InMemoryModule; +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 ElasticV5QueryGroupsTest extends AbstractQueryGroupsTest { + private static ElasticNodeInfo nodeInfo; + private static ElasticContainer container; + + @BeforeClass + public static void startIndexService() { + if (nodeInfo != null) { + // do not start Elasticsearch twice + return; + } + + container = ElasticContainer.createAndStart(ElasticVersion.V5_6); + nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); + } + + @AfterClass + public static void stopElasticsearchServer() { + if (container != null) { + container.stop(); + } + } + + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @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(); + ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); + return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration)); + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 79a4ab7634..7676e48de1 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -2197,6 +2197,14 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("query:query4"); } + @Test + public void byOwnerInvalidQuery() throws Exception { + TestRepository repo = createProject("repo"); + insert(repo, newChange(repo), userId); + String nameEmail = user.asIdentifiedUser().getNameEmail(); + assertQuery("owner: \"" + nameEmail + "\"\\"); + } + protected ChangeInserter newChange(TestRepository repo) throws Exception { return newChange(repo, null, null, null, null); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java index d3ecc296e9..9362ce9df7 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java @@ -51,6 +51,7 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest { } @Test + @Override public void byOwnerInvalidQuery() throws Exception { TestRepository repo = createProject("repo"); Change change1 = insert(repo, newChange(repo), userId); From 1492e86cd2439ed26fe9f5c25c15ef6e7fb2652d Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sun, 27 May 2018 15:00:56 +0900 Subject: [PATCH 5/5] ElasticReindexIT: Add tests against Elasticsearch version 5 Bug: Issue 6094 Change-Id: I324d099ee4cc484fcdfea8679e38cc2d3d2b5905 --- .../gerrit/acceptance/pgm/ElasticReindexIT.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java index f4ffc3c84d..683ec91699 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.pgm; import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.elasticsearch.ElasticVersion; import com.google.gerrit.elasticsearch.testing.ElasticContainer; import com.google.gerrit.elasticsearch.testing.ElasticTestUtils; import com.google.gerrit.elasticsearch.testing.ElasticTestUtils.ElasticNodeInfo; @@ -28,11 +29,10 @@ import org.junit.After; public class ElasticReindexIT extends AbstractReindexTests { private static ElasticContainer container; - @ConfigSuite.Default - public static Config elasticsearch() { + private static Config getConfig(ElasticVersion version) { ElasticNodeInfo elasticNodeInfo; try { - container = ElasticContainer.createAndStart(); + container = ElasticContainer.createAndStart(version); elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort()); } catch (Throwable t) { return null; @@ -43,6 +43,16 @@ public class ElasticReindexIT extends AbstractReindexTests { return cfg; } + @ConfigSuite.Default + public static Config elasticsearchV2() { + return getConfig(ElasticVersion.V2_4); + } + + @ConfigSuite.Config + public static Config elasticsearchV5() { + return getConfig(ElasticVersion.V5_6); + } + @Override public void configureIndex(Injector injector) throws Exception { ElasticTestUtils.createAllIndexes(injector);