From 3af5aed69ba4d6e729bffeac4d3440f37b4d9bdd Mon Sep 17 00:00:00 2001 From: Diego Zambelli Sessona Date: Wed, 26 Jul 2023 14:57:17 +0100 Subject: IndexType not honoured in AbstractDeamonTest AbstractDeamonTest was not setting the indexModule according to the configured index backend. Tests can be explicit about the indexing backend they want to use, even if it is not a known IndexType, for example elasticsearch [1]. [1]: https://gerrit-review.googlesource.com/c/modules/index-elasticsearch/+/379474 Bug: Issue 293262795 Release-Notes: skip Change-Id: I75ed59f3991fffdb82fd22191f32e73d57c0c20d --- .../com/google/gerrit/acceptance/GerritServer.java | 44 ++++++------ .../gerrit/index/testing/AbstractFakeIndex.java | 3 +- .../gerrit/acceptance/ssh/CustomIndexIT.java | 80 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 21 deletions(-) diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java index 998217214c..6d58a5490a 100644 --- a/java/com/google/gerrit/acceptance/GerritServer.java +++ b/java/com/google/gerrit/acceptance/GerritServer.java @@ -61,6 +61,7 @@ import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.experiments.ConfigExperimentFeatures.ConfigExperimentFeaturesModule; import com.google.gerrit.server.git.receive.AsyncReceiveCommits.AsyncReceiveCommitsModule; import com.google.gerrit.server.git.validators.CommitValidationListener; +import com.google.gerrit.server.index.AbstractIndexModule; import com.google.gerrit.server.index.options.AutoFlush; import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore; import com.google.gerrit.server.ssh.NoSshModule; @@ -459,7 +460,11 @@ public class GerritServer implements AutoCloseable { if (desc.memory()) { checkArgument(additionalArgs.length == 0, "cannot pass args to in-memory server"); - return startInMemory(desc, site, baseConfig, daemon, inMemoryRepoManager); + AbstractIndexModule testIndexModule = + (testSysModule instanceof AbstractIndexModule) + ? (AbstractIndexModule) testSysModule + : null; + return startInMemory(desc, site, baseConfig, daemon, inMemoryRepoManager, testIndexModule); } return startOnDisk(desc, site, daemon, serverStarted, additionalArgs); } @@ -469,7 +474,8 @@ public class GerritServer implements AutoCloseable { Path site, Config baseConfig, Daemon daemon, - @Nullable InMemoryRepositoryManager inMemoryRepoManager) + @Nullable InMemoryRepositoryManager inMemoryRepoManager, + @Nullable AbstractIndexModule testIndexModule) throws Exception { Config cfg = desc.buildConfig(baseConfig); mergeTestConfig(cfg); @@ -485,24 +491,11 @@ public class GerritServer implements AutoCloseable { "accountPatchReviewDb", null, "url", JdbcAccountPatchReviewStore.TEST_IN_MEMORY_URL); String configuredIndexBackend = cfg.getString("index", null, "type"); - IndexType indexType; - if (configuredIndexBackend != null) { - // Explicitly configured index backend from gerrit.config trumps any other ways to configure - // index backends so that Reindex tests can be explicit about the backend they want to test - // against. - indexType = new IndexType(configuredIndexBackend); - } else { - // Allow configuring the index backend based on sys/env variables so that integration tests - // can be run against different index backends. - indexType = IndexType.fromEnvironment().orElse(new IndexType("fake")); - } - if (indexType.isLucene()) { - daemon.setIndexModule( - LuceneIndexModule.singleVersionAllLatest( - 0, ReplicaUtil.isReplica(baseConfig), AutoFlush.ENABLED)); - } else { - daemon.setIndexModule(FakeIndexModule.latestVersion(false)); - } + IndexType indexType = + (configuredIndexBackend != null) + ? new IndexType(configuredIndexBackend) + : IndexType.fromEnvironment().orElse(new IndexType("fake")); + daemon.setIndexModule(createIndexModule(indexType, baseConfig, testIndexModule)); daemon.setEnableHttpd(desc.httpd()); daemon.setInMemory(true); @@ -522,6 +515,17 @@ public class GerritServer implements AutoCloseable { return new GerritServer(desc, null, createTestInjector(daemon), daemon, null); } + private static AbstractIndexModule createIndexModule( + IndexType indexType, Config baseConfig, @Nullable AbstractIndexModule testIndexModule) { + if (testIndexModule != null) { + return testIndexModule; + } + return indexType.isLucene() + ? LuceneIndexModule.singleVersionAllLatest( + 0, ReplicaUtil.isReplica(baseConfig), AutoFlush.ENABLED) + : FakeIndexModule.latestVersion(false); + } + private static GerritServer startOnDisk( Description desc, Path site, diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java index 8ecdae86e6..27c7fe8002 100644 --- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java +++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java @@ -236,7 +236,8 @@ public abstract class AbstractFakeIndex implements Index { private final boolean skipMergable; @Inject - FakeChangeIndex( + @VisibleForTesting + protected FakeChangeIndex( SitePaths sitePaths, ChangeData.Factory changeDataFactory, @Assisted Schema schema, diff --git a/javatests/com/google/gerrit/acceptance/ssh/CustomIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/CustomIndexIT.java index 434071ff9c..fee413ab8d 100644 --- a/javatests/com/google/gerrit/acceptance/ssh/CustomIndexIT.java +++ b/javatests/com/google/gerrit/acceptance/ssh/CustomIndexIT.java @@ -14,9 +14,29 @@ package com.google.gerrit.acceptance.ssh; +import static com.google.common.truth.Truth.assertThat; + import com.google.gerrit.index.IndexType; +import com.google.gerrit.index.Schema; +import com.google.gerrit.index.project.ProjectIndex; +import com.google.gerrit.index.testing.AbstractFakeIndex; +import com.google.gerrit.index.testing.FakeIndexVersionManager; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.index.AbstractIndexModule; +import com.google.gerrit.server.index.VersionManager; +import com.google.gerrit.server.index.account.AccountIndex; +import com.google.gerrit.server.index.change.ChangeIndex; +import com.google.gerrit.server.index.change.ChangeIndexCollection; +import com.google.gerrit.server.index.group.GroupIndex; +import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.testing.ConfigSuite; +import com.google.inject.Inject; +import com.google.inject.Module; +import com.google.inject.assistedinject.Assisted; +import java.util.Map; import org.eclipse.jgit.lib.Config; +import org.junit.Test; /** * Tests for a defaulted custom index configuration. This unknown type is the opposite of {@link @@ -24,10 +44,70 @@ import org.eclipse.jgit.lib.Config; */ public class CustomIndexIT extends AbstractIndexTests { + @Override + public Module createModule() { + return CustomIndexModule.latestVersion(false); + } + @ConfigSuite.Default public static Config customIndexType() { Config config = new Config(); config.setString("index", null, "type", "custom"); return config; } + + @Inject private ChangeIndexCollection changeIndex; + + @Test + public void customIndexModuleIsBound() throws Exception { + assertThat(changeIndex.getSearchIndex()).isInstanceOf(CustomModuleFakeIndexChange.class); + } +} + +class CustomIndexModule extends AbstractIndexModule { + + public static CustomIndexModule latestVersion(boolean secondary) { + return new CustomIndexModule(null, -1 /* direct executor */, secondary); + } + + private CustomIndexModule(Map singleVersions, int threads, boolean secondary) { + super(singleVersions, threads, secondary); + } + + @Override + protected Class getAccountIndex() { + return AbstractFakeIndex.FakeAccountIndex.class; + } + + @Override + protected Class getChangeIndex() { + return CustomModuleFakeIndexChange.class; + } + + @Override + protected Class getGroupIndex() { + return AbstractFakeIndex.FakeGroupIndex.class; + } + + @Override + protected Class getProjectIndex() { + return AbstractFakeIndex.FakeProjectIndex.class; + } + + @Override + protected Class getVersionManager() { + return FakeIndexVersionManager.class; + } +} + +class CustomModuleFakeIndexChange extends AbstractFakeIndex.FakeChangeIndex { + + @com.google.inject.Inject + CustomModuleFakeIndexChange( + SitePaths sitePaths, + ChangeData.Factory changeDataFactory, + @Assisted Schema schema, + @GerritServerConfig Config cfg) { + super(sitePaths, changeDataFactory, schema, cfg); + } } -- cgit v1.2.3