opendev/gerrit
Code Issues Proposed changes

elasticsearch: Always set up both change and account indexes

Browse Source 
Change query tests subtly depend on the account index for the account
setup step: omitting the account index caused
AccountManager#authenticate to throw an exception if an account
external ID was repeated, instead of returning the account with that
ID.

Always set up all indexes, using a helper in ElasticTestUtils. The
delete code can be simplified as well using the special string "_all"
to delete all indexes.

Change-Id: I7ab7be37a8cf953b6873745014e781e6b5e910f8
This commit is contained in:
Dave Borowitz 2016-12-16 11:19:29 -05:00
parent 4436902939
commit 49251588f2
4 changed files with 53 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gerrit-elasticsearch/BUILD
View File

@ -34,6 +34,7 @@ java_library(
testonly = 1,
srcs = glob(["src/test/java/**/ElasticTestUtils.java"]),
deps = [
":elasticsearch",
"//gerrit-extension-api:api",
"//gerrit-server:server",
"//lib:gson",

27
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
View File

@ -14,11 +14,7 @@
package com.google.gerrit.elasticsearch;
import static com.google.gerrit.elasticsearch.ElasticAccountIndex.ACCOUNTS_PREFIX;
import com.google.gerrit.elasticsearch.ElasticAccountIndex.AccountMapping;
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
import com.google.gerrit.server.query.account.AbstractQueryAccountsTest;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.inject.Guice;
@ -32,9 +28,6 @@ import org.junit.BeforeClass;
import java.util.concurrent.ExecutionException;
public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest {
private static final String INDEX_NAME =
String.format("%s%04d", ACCOUNTS_PREFIX,
AccountSchemaDefinitions.INSTANCE.getLatest().getVersion());
private static ElasticNodeInfo nodeInfo;
@BeforeClass
@ -45,21 +38,7 @@ public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest {
return;
}
nodeInfo = ElasticTestUtils.startElasticsearchNode();
createIndexes();
}
private static void createIndexes() {
AccountMapping accountMapping =
new AccountMapping(AccountSchemaDefinitions.INSTANCE.getLatest());
nodeInfo.node
.client()
.admin()
.indices()
.prepareCreate(INDEX_NAME)
.addMapping(ElasticAccountIndex.ACCOUNTS,
ElasticTestUtils.gson.toJson(accountMapping))
.execute()
.actionGet();
ElasticTestUtils.createAllIndexes(nodeInfo);
}
@AfterClass
@ -74,8 +53,8 @@ public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest {
@After
public void cleanupIndex() {
if (nodeInfo != null) {
ElasticTestUtils.deleteIndexes(nodeInfo.node, INDEX_NAME);
createIndexes();
ElasticTestUtils.deleteAllIndexes(nodeInfo);
ElasticTestUtils.createAllIndexes(nodeInfo);
}
}

35
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
View File

@ -14,13 +14,7 @@
package com.google.gerrit.elasticsearch;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CHANGES_PREFIX;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CLOSED_CHANGES;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.OPEN_CHANGES;
import com.google.gerrit.elasticsearch.ElasticChangeIndex.ChangeMapping;
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.query.change.AbstractQueryChangesTest;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.gerrit.testutil.InMemoryRepositoryManager.Repo;
@ -37,9 +31,6 @@ import org.junit.Test;
import java.util.concurrent.ExecutionException;
public class ElasticQueryChangesTest extends AbstractQueryChangesTest {
private static final String INDEX_NAME =
String.format("%s%04d", CHANGES_PREFIX,
ChangeSchemaDefinitions.INSTANCE.getLatest().getVersion());
private static ElasticNodeInfo nodeInfo;
@BeforeClass
@ -51,14 +42,14 @@ public class ElasticQueryChangesTest extends AbstractQueryChangesTest {
}
nodeInfo = ElasticTestUtils.startElasticsearchNode();
createIndexes();
ElasticTestUtils.createAllIndexes(nodeInfo);
}
@After
public void cleanupIndex() {
if (nodeInfo != null) {
ElasticTestUtils.deleteIndexes(nodeInfo.node, INDEX_NAME);
createIndexes();
ElasticTestUtils.deleteAllIndexes(nodeInfo);
ElasticTestUtils.createAllIndexes(nodeInfo);
}
}
@ -80,26 +71,6 @@ public class ElasticQueryChangesTest extends AbstractQueryChangesTest {
new InMemoryModule(elasticsearchConfig, notesMigration));
}
private static void createIndexes() {
ChangeMapping openChangesMapping =
new ChangeMapping(ChangeSchemaDefinitions.INSTANCE.getLatest());
ChangeMapping closedChangesMapping =
new ChangeMapping(ChangeSchemaDefinitions.INSTANCE.getLatest());
openChangesMapping.closedChanges = null;
closedChangesMapping.openChanges = null;
nodeInfo.node
.client()
.admin()
.indices()
.prepareCreate(INDEX_NAME)
.addMapping(OPEN_CHANGES,
ElasticTestUtils.gson.toJson(openChangesMapping))
.addMapping(CLOSED_CHANGES,
ElasticTestUtils.gson.toJson(closedChangesMapping))
.execute()
.actionGet();
}
@Test
public void byOwnerInvalidQuery() throws Exception {
TestRepository<Repo> repo = createProject("repo");

48
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java
View File

@ -15,10 +15,21 @@
package com.google.gerrit.elasticsearch;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.elasticsearch.ElasticAccountIndex.ACCOUNTS_PREFIX;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CHANGES_PREFIX;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CLOSED_CHANGES;
import static com.google.gerrit.elasticsearch.ElasticChangeIndex.OPEN_CHANGES;
import com.google.common.base.Strings;
import com.google.common.io.Files;
import com.google.gerrit.elasticsearch.ElasticAccountIndex.AccountMapping;
import com.google.gerrit.elasticsearch.ElasticChangeIndex.ChangeMapping;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.index.Schema;
import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
@ -100,8 +111,8 @@ final class ElasticTestUtils {
return new ElasticNodeInfo(node, elasticDir, getHttpPort(node));
}
static void deleteIndexes(Node node, String index) {
node.client().admin().indices().prepareDelete(index).execute();
static void deleteAllIndexes(ElasticNodeInfo nodeInfo) {
nodeInfo.node.client().admin().indices().prepareDelete("_all").execute();
}
static class NodeInfo {
@ -112,6 +123,39 @@ final class ElasticTestUtils {
Map<String, NodeInfo> nodes;
}
static void createAllIndexes(ElasticNodeInfo nodeInfo) {
Schema<ChangeData> changeSchema =
ChangeSchemaDefinitions.INSTANCE.getLatest();
ChangeMapping openChangesMapping = new ChangeMapping(changeSchema);
ChangeMapping closedChangesMapping = new ChangeMapping(changeSchema);
openChangesMapping.closedChanges = null;
closedChangesMapping.openChanges = null;
nodeInfo.node
.client()
.admin()
.indices()
.prepareCreate(
String.format("%s%04d", CHANGES_PREFIX, changeSchema.getVersion()))
.addMapping(OPEN_CHANGES, gson.toJson(openChangesMapping))
.addMapping(CLOSED_CHANGES, gson.toJson(closedChangesMapping))
.execute()
.actionGet();
Schema<AccountState> accountSchema =
AccountSchemaDefinitions.INSTANCE.getLatest();
AccountMapping accountMapping = new AccountMapping(accountSchema);
nodeInfo.node
.client()
.admin()
.indices()
.prepareCreate(
String.format(
"%s%04d", ACCOUNTS_PREFIX, accountSchema.getVersion()))
.addMapping(ElasticAccountIndex.ACCOUNTS, gson.toJson(accountMapping))
.execute()
.actionGet();
}
private static String getHttpPort(Node node)
throws InterruptedException, ExecutionException {
String nodes = node.client().admin().cluster()