From 6331ad4afa7ab90d4f1f22869f283ac0fd2332d1 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 2 Aug 2026 21:59:57 -0400 Subject: [PATCH] SOLR-18150: SolrClientTestRule now uses SolrBackend Includes some minor tweaks to SolrClientTestRule like supporting general properties in collection/core creation. --- .../search/TestDocValuesIteratorCache.java | 4 +- .../apache/solr/update/CustomTLogDirTest.java | 13 +- .../solr/embedded/EmbeddedSolrBackend.java | 8 + .../apache/solr/embedded/JettySolrRunner.java | 3 +- .../solr/util/EmbeddedSolrServerTestRule.java | 31 +-- .../apache/solr/util/SolrClientTestRule.java | 179 +++++++++++------- .../apache/solr/util/SolrJettyTestRule.java | 53 +++--- 7 files changed, 163 insertions(+), 128 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java index e71090e0efc2..eeb5f718d5a4 100644 --- a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java +++ b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java @@ -28,7 +28,6 @@ import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; import org.apache.solr.common.SolrDocument; import org.apache.solr.core.SolrCore; import org.apache.solr.index.NoMergePolicyFactory; @@ -83,8 +82,7 @@ public void test() throws Exception { Random r = random(); String[][] expectVals = indexDocs(client, r); - try (SolrCore core = - ((EmbeddedSolrServer) client).getCoreContainer().getCore(DEFAULT_TEST_CORENAME)) { + try (SolrCore core = solrTestRule.getCoreContainer().getCore(DEFAULT_TEST_CORENAME)) { RefCounted sref = core.getSearcher(); try (Closeable c = sref::decref) { SolrIndexSearcher s = sref.get(); diff --git a/solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java b/solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java index 334b8c3525fb..5d0047b07254 100644 --- a/solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java +++ b/solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java @@ -25,7 +25,6 @@ import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; import org.apache.solr.core.CoreContainer; import org.apache.solr.util.EmbeddedSolrServerTestRule; import org.apache.solr.util.SolrClientTestRule; @@ -49,7 +48,7 @@ public void testExternal() throws Exception { String collectionName = "coll" + collectionIdx.getAndIncrement(); SolrClient client = solrTestRule.getSolrClient(collectionName); - Path coreRootDir = ((EmbeddedSolrServer) client).getCoreContainer().getCoreRootDirectory(); + Path coreRootDir = solrTestRule.getCoreContainer().getCoreRootDirectory(); Path instanceDir = FilterPath.unwrap(coreRootDir.resolve(collectionName)); @@ -65,7 +64,7 @@ public void testRelative() throws Exception { String collectionName = "coll" + collectionIdx.getAndIncrement(); SolrClient client = solrTestRule.getSolrClient(collectionName); - Path coreRootDir = ((EmbeddedSolrServer) client).getCoreContainer().getCoreRootDirectory(); + Path coreRootDir = solrTestRule.getCoreContainer().getCoreRootDirectory(); Path instanceDir = FilterPath.unwrap(coreRootDir.resolve(collectionName)); @@ -96,7 +95,7 @@ public void testAbsoluteSubdir() throws Exception { String collectionName = "coll" + collectionIdx.getAndIncrement(); SolrClient client = solrTestRule.getSolrClient(collectionName); - Path coreRootDir = ((EmbeddedSolrServer) client).getCoreContainer().getCoreRootDirectory(); + Path coreRootDir = solrTestRule.getCoreContainer().getCoreRootDirectory(); Path instanceDir = FilterPath.unwrap(coreRootDir.resolve(collectionName)); @@ -112,7 +111,7 @@ public void testDefault() throws Exception { String collectionName = "coll" + collectionIdx.getAndIncrement(); SolrClient client = solrTestRule.getSolrClient(collectionName); - Path coreRootDir = ((EmbeddedSolrServer) client).getCoreContainer().getCoreRootDirectory(); + Path coreRootDir = solrTestRule.getCoreContainer().getCoreRootDirectory(); Path instanceDir = FilterPath.unwrap(coreRootDir.resolve(collectionName)); @@ -126,7 +125,7 @@ public void testExplicitDefault() throws Exception { String collectionName = "coll" + collectionIdx.getAndIncrement(); SolrClient client = solrTestRule.getSolrClient(collectionName); - Path coreRootDir = ((EmbeddedSolrServer) client).getCoreContainer().getCoreRootDirectory(); + Path coreRootDir = solrTestRule.getCoreContainer().getCoreRootDirectory(); Path instanceDir = FilterPath.unwrap(coreRootDir.resolve(collectionName)); @@ -169,7 +168,7 @@ private static void validateTlogPath( .toList(); assertNotNull(list); assertEquals(1, list.size()); - CoreContainer cc = ((EmbeddedSolrServer) client).getCoreContainer(); + CoreContainer cc = solrTestRule.getCoreContainer(); cc.unload(collectionName, true, true, true); assertFalse(Files.exists(resolvedTlogDir)); } diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/EmbeddedSolrBackend.java b/solr/test-framework/src/java/org/apache/solr/embedded/EmbeddedSolrBackend.java index 6b70539afcf1..1b94697bde2f 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/EmbeddedSolrBackend.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/EmbeddedSolrBackend.java @@ -54,6 +54,14 @@ public EmbeddedSolrBackend(EmbeddedSolrServer solrServer) { this.adminClient = solrServer; } + /** + * @lucene.internal + */ + public EmbeddedSolrBackend(CoreContainer container) { + this.coreContainer = container; + this.adminClient = new EmbeddedSolrServer(container, null); + } + @Override public CoreContainer getCoreContainer() { return coreContainer; diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 653a001ff1d6..b27651251f3d 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -43,7 +43,6 @@ import java.util.concurrent.TimeoutException; import org.apache.solr.SolrBackend; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.jetty.SSLConfig; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -832,7 +831,7 @@ private EmbeddedSolrBackend getEmbeddedSolrBackend() { throw new IllegalStateException( "Don't call SolrBackend methods in SolrCloud on JettySolrRunner"); } - return new EmbeddedSolrBackend(new EmbeddedSolrServer(container, null)); // cheap + return new EmbeddedSolrBackend(container); // cheap } @Override diff --git a/solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java b/solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java index 1d4cd23b9a5b..de8d574138b3 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java +++ b/solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java @@ -16,16 +16,19 @@ */ package org.apache.solr.util; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Properties; import java.util.Set; import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.NodeConfig; import org.apache.solr.core.SolrPaths; import org.apache.solr.core.SolrXmlConfig; +import org.apache.solr.embedded.EmbeddedSolrBackend; import org.apache.solr.update.UpdateShardHandlerConfig; /** @@ -35,16 +38,6 @@ public class EmbeddedSolrServerTestRule extends SolrClientTestRule { private static final String CORE_DIR_PROP = "coreRootDirectory"; - private EmbeddedSolrServer adminClient = null; - - /** - * Shuts down the EmbeddedSolrServer instance and clears the coreRootDirectory system property if - * necessary - */ - @Override - protected void after() { - if (adminClient != null) adminClient.getCoreContainer().shutdown(); - } /** * Starts the Solr server with the given solrHome. If solrHome contains a solr.xml file, it is @@ -80,8 +73,8 @@ public void startSolr(Path solrHome) { /** Starts Solr with custom NodeConfig */ public void startSolr(NodeConfig nodeConfig) { var container = new CoreContainer(nodeConfig); - adminClient = new EmbeddedSolrServer(container, null); - container.load(); // do after setting adminClient so that after() can shutdown the container + backend = new EmbeddedSolrBackend(container); + container.load(); // do after setting this.backend so that after() can shutdown the container } /** Returns a NodeConfigBuilder with default settings for test configuration */ @@ -93,21 +86,13 @@ public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) { .setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString()); } - /** Provides an EmbeddedSolrServer instance for administration actions */ - @Override - public EmbeddedSolrServer getAdminClient() { - if (adminClient == null) { - throw new RuntimeException("Solr must be started first"); - } - return adminClient; - } - @Override public EmbeddedSolrServer getSolrClient(String collection) { return new EmbeddedSolrServer(getCoreContainer(), collection); } - public CoreContainer getCoreContainer() { - return getAdminClient().getCoreContainer(); + @Override + protected void createColl(NewCollectionBuilder b) throws SolrServerException, IOException { + createCollStandalone(b); } } diff --git a/solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java b/solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java index ad37a04741d3..cd87ef88fbfb 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java +++ b/solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java @@ -20,23 +20,46 @@ import java.io.IOException; import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Random; import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.solr.SolrBackend; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.request.CoreAdminRequest; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; import org.junit.rules.ExternalResource; /** * Provides access to a {@link SolrClient} instance and a running Solr in tests. Implementations * could run Solr in different ways (e.g. strictly embedded, adding HTTP/Jetty, adding SolrCloud, or * an external process). It's a JUnit {@link ExternalResource} (a {@code TestRule}), and thus closes - * the client and Solr itself when the test completes. This test utility is encouraged to be used by - * external projects that wish to test communicating with Solr, especially for plugin providers. + * the client and Solr itself when the test completes. Wraps a {@link SolrBackend}. This test + * utility is encouraged to be used by external projects that wish to test communicating with Solr, + * especially for plugin providers. */ public abstract class SolrClientTestRule extends ExternalResource { + protected SolrBackend backend; + + /** Returns the underlying {@link SolrBackend}. */ + public SolrBackend getBackend() { + return backend; + } + + @Override + protected void after() { + if (backend != null) { + backend.close(); + backend = null; + } + } + /** Starts the Solr server with empty solrHome. */ public void startSolr() { startSolr(LuceneTestCase.createTempDir("solrhome")); @@ -48,62 +71,63 @@ public void startSolr() { */ public abstract void startSolr(Path solrHome); + /** + * Returns a builder for creating a collection/core. Not for testing collection creation itself. + */ public NewCollectionBuilder newCollection(String name) { return new NewCollectionBuilder(name); } + /** + * Returns a builder for creating a collection/core. Not for testing collection creation itself. + */ public NewCollectionBuilder newCollection() { return new NewCollectionBuilder(DEFAULT_TEST_COLLECTION_NAME); } public class NewCollectionBuilder { - private String name; + private final String name; private String configSet; - private String configFile; - private String schemaFile; - private String basicAuthUser; - private String basicAuthPwd; + private Path configSetPath; + private final Map properties = new LinkedHashMap<>(); public NewCollectionBuilder(String name) { this.name = name; } + /** Chooses the configSet by name. */ public NewCollectionBuilder withConfigSet(String configSet) { - // Chop off "/conf" if found -- configSet can be a path. - // This is a hack so that we can continue to use ExternalPaths.DEFAULT_CONFIGSET etc. as-is. - // Without this, managed resources might be written to - // conf/conf/_schema_analysis_stopwords_english.json because SolrResourceLoader points to the - // wrong dir. - if (configSet != null) { - final var confSuffix = FileSystems.getDefault().getSeparator() + "conf"; - if (configSet.endsWith(confSuffix)) { - configSet = configSet.substring(0, configSet.length() - confSuffix.length()); - } - this.configSet = configSet; + assert configSetPath == null; + if (configSet.contains(FileSystems.getDefault().getSeparator())) { + throw new IllegalArgumentException("wrong overload"); } + this.configSet = configSet; return this; } - public NewCollectionBuilder withConfigSet(Path configSet) { - withConfigSet(configSet.toString()); + /** Choose the configSet path directly containing the files. */ + public NewCollectionBuilder withConfigSet(Path configSetPath) { + // get configSet name & normalize path + assert this.configSet == null; + configSet = configSetPath.getFileName().toString(); + if (configSet.equals("conf")) { + configSet = configSetPath.getParent().getFileName().toString(); + } else if (Files.exists(configSetPath.resolve("conf"))) { + configSetPath = configSetPath.resolve("conf"); + } + this.configSetPath = configSetPath; return this; } + /** A {@code solrconfig.xml} alternative. */ public NewCollectionBuilder withConfigFile(String configFile) { - this.configFile = configFile; - return this; + return withProperty(CoreDescriptor.CORE_CONFIG, configFile); } + /** A {@code schema.xml} alternative. */ public NewCollectionBuilder withSchemaFile(String schemaFile) { - this.schemaFile = schemaFile; - return this; - } - - public NewCollectionBuilder withBasicAuthCredentials(String user, String password) { - this.basicAuthUser = user; - this.basicAuthPwd = password; - return this; + return withProperty(CoreDescriptor.CORE_SCHEMA, schemaFile); } public String getName() { @@ -114,57 +138,68 @@ public String getConfigSet() { return configSet; } - public String getConfigFile() { - return configFile; + public Path getConfigSetPath() { + return configSetPath; } - public String getSchemaFile() { - return schemaFile; - } - - public void create() throws SolrServerException, IOException { - SolrClientTestRule.this.create(this); + public NewCollectionBuilder withProperty(String key, String value) { + properties.put(key, value); + return this; } - public String getBasicAuthUser() { - return basicAuthUser; + public Map getProperties() { + return properties; } - public String getBasicAuthPwd() { - return basicAuthPwd; + public void create() throws SolrServerException, IOException { + SolrClientTestRule.this.createColl(this); } } - protected void create(NewCollectionBuilder b) throws SolrServerException, IOException { - - CoreAdminRequest.Create req = new CoreAdminRequest.Create(); - req.setCoreName(b.getName()); - req.setInstanceDir(b.getName()); - - if (b.getConfigSet() != null) { - req.setConfigSet(b.getConfigSet()); - } - - if (b.getConfigFile() != null) { - req.setConfigName(b.getConfigFile()); + /** + * Override to call either {@link #createCollSolrCloud(NewCollectionBuilder)} or {@link + * #createCollStandalone(NewCollectionBuilder)} + */ + protected abstract void createColl(NewCollectionBuilder b) + throws SolrServerException, IOException; + + protected void createCollSolrCloud(NewCollectionBuilder b) + throws SolrServerException, IOException { + if (b.getConfigSetPath() != null) { + if (!backend.hasConfigSet(b.getConfigSet())) { + backend.uploadConfigSet(b.getConfigSetPath(), b.getConfigSet()); + } } - if (b.getSchemaFile() != null) { - req.setSchemaName(b.getSchemaFile()); - } + var create = CollectionAdminRequest.createCollection(b.getName(), b.getConfigSet(), 1, 1); + create.setProperties(b.getProperties()); + backend.createCollection(create); + } - if (b.getBasicAuthUser() != null) { - req.setBasicAuthCredentials(b.getBasicAuthUser(), b.getBasicAuthPwd()); + protected void createCollStandalone(NewCollectionBuilder b) + throws SolrServerException, IOException { + // If there's a configSet path, we can reference it directly and not actually bother with + // ConfigSetService + String configSet = b.getConfigSet(); + Path configSetPath = b.getConfigSetPath(); + if (configSetPath != null) { + // Chop off "/conf" if found. Without this, managed resources might be written to + // conf/conf/_schema_analysis_stopwords_english.json because SolrResourceLoader points to the + // wrong dir. + if (configSetPath.endsWith("conf")) { + configSetPath = configSetPath.getParent(); + } + configSet = configSetPath.toString(); } - req.process(getAdminClient()); + var create = CollectionAdminRequest.createCollection(b.getName(), configSet, 1, 1); + create.setProperties(b.getProperties()); + backend.createCollection(create); } - /** - * Provides a SolrClient instance for administration actions. The caller doesn't need to close it - */ + /** Provides a SolrClient instance for administration actions. The caller must not close it. */ public SolrClient getAdminClient() { - return getSolrClient(null); + return backend.getSolrClient(); } /** Provides a SolrClient instance for collection1. The caller doesn't need to close it */ @@ -181,4 +216,18 @@ public SolrClient getSolrClient() { public void clearIndex() throws SolrServerException, IOException { new UpdateRequest().deleteByQuery("*:*").commit(getSolrClient(), null); } + + /** + * @see SolrBackend#getCoreContainer() + */ + public CoreContainer getCoreContainer() { + return backend.getCoreContainer(); + } + + /** + * @see SolrBackend#getBaseUrl(Random) + */ + public String getBaseUrl(Random r) { + return backend.getBaseUrl(r); + } } diff --git a/solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java b/solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java index f540f823bbb4..0640c02bcd1e 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java +++ b/solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java @@ -18,14 +18,15 @@ import static org.apache.solr.SolrTestCaseJ4.DEFAULT_TEST_COLLECTION_NAME; +import java.io.IOException; import java.lang.invoke.MethodHandles; import java.nio.file.Path; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; -import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.common.util.IOUtils; -import org.apache.solr.core.CoreContainer; import org.apache.solr.embedded.JettyConfig; import org.apache.solr.embedded.JettySolrRunner; import org.slf4j.Logger; @@ -38,30 +39,18 @@ public class SolrJettyTestRule extends SolrClientTestRule { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private JettySolrRunner jetty; - - private final ConcurrentHashMap clients = new ConcurrentHashMap<>(); + private final ConcurrentHashMap clients = new ConcurrentHashMap<>(); private boolean enableProxy; @Override protected void after() { - for (SolrClient solrClient : clients.values()) { + for (var solrClient : clients.values()) { IOUtils.closeQuietly(solrClient); } clients.clear(); + enableProxy = false; - if (jetty != null) { - try { - jetty.stop(); - } catch (RuntimeException e) { - throw e; - } catch (Exception e) { - throw new RuntimeException(e); - } finally { - jetty = null; - enableProxy = false; - } - } + super.after(); // closes the backend (JettySolrRunner) } /** Resets the state. DEPRECATED; please don't call! */ @@ -82,14 +71,15 @@ public void startSolr(Path solrHome) { * @see JettySolrRunner#getProxy() */ public void enableProxy() { - assert jetty == null; + assert backend == null; this.enableProxy = true; } public void startSolr(Path solrHome, Properties nodeProperties, JettyConfig jettyConfig) { - if (jetty != null) throw new IllegalStateException("Jetty is already running"); + if (backend != null) throw new IllegalStateException("Jetty is already running"); - jetty = new JettySolrRunner(solrHome.toString(), nodeProperties, jettyConfig, enableProxy); + var jetty = new JettySolrRunner(solrHome.toString(), nodeProperties, jettyConfig, enableProxy); + backend = jetty; try { jetty.start(); } catch (RuntimeException e) { @@ -102,19 +92,25 @@ public void startSolr(Path solrHome, Properties nodeProperties, JettyConfig jett } public JettySolrRunner getJetty() { - if (jetty == null) throw new IllegalStateException("Jetty has not started"); - return jetty; + if (backend == null) throw new IllegalStateException("Jetty has not started"); + return (JettySolrRunner) backend; } @Override - public SolrClient getSolrClient(String collection) { + public HttpSolrClient getSolrClient(String collection) { if (collection == null) { collection = ""; } return clients.computeIfAbsent(collection, this::newSolrClient); } - protected SolrClient newSolrClient(String collection) { + @Override + public HttpSolrClient getAdminClient() { + // Use an HTTP client so requests route through Jetty, not the embedded shortcut. + return getSolrClient(null); + } + + protected HttpSolrClient newSolrClient(String collection) { return newSolrClientBuilder() .withDefaultCollection(collection) // Properly handles when collection is 'null' .build(); @@ -126,7 +122,7 @@ protected SolrClient newSolrClient(String collection) { */ public HttpJettySolrClient.Builder newSolrClientBuilder() { return new HttpJettySolrClient.Builder(getBaseUrl()) - .withHttpClient(jetty.getSolrClient()) + .withHttpClient((HttpJettySolrClient) backend.getSolrClient()) .withDefaultCollection(DEFAULT_TEST_COLLECTION_NAME); } @@ -135,7 +131,8 @@ public String getBaseUrl() { return getJetty().getBaseUrl().toString(); } - public CoreContainer getCoreContainer() { - return getJetty().getCoreContainer(); + @Override + protected void createColl(NewCollectionBuilder b) throws SolrServerException, IOException { + createCollStandalone(b); } }