This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit eecc76bebfa645bfa55677af03ba8cde77572cf6 Author: Jan Høydahl <[email protected]> AuthorDate: Wed Oct 7 16:09:09 2026 +0200 CLI: Fix SOLR_CONNECTION fallback and other small improvements (#5054) (cherry picked from commit 5e044a331fd5ff2e4fa1330f0709775f675ad6dc) --- AGENTS.md | 1 + .../unreleased/PR#5054-picocli-cli-small-fixes.yml | 10 +++++ .../src/java/org/apache/solr/cli/CLIUtils.java | 2 +- .../apache/solr/cli/CliDefaultValueProvider.java | 2 +- .../src/java/org/apache/solr/cli/CreateTool.java | 2 +- .../src/java/org/apache/solr/cli/StartCommand.java | 6 +-- .../src/test/org/apache/solr/cli/CLIUtilsTest.java | 30 +++++++++++++ .../solr/cli/CliDefaultValueProviderTest.java | 50 ++++++++++++++++++++++ .../deployment-guide/pages/cli/solr-create.adoc | 2 +- .../deployment-guide/pages/cli/solr-start.adoc | 8 ++-- 10 files changed, 102 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 47cb9fb4ba6..2232abc0b4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,6 +39,7 @@ While README.md and CONTRIBUTING.md are mainly written for humans, this file is - Subclass SolrTestCase, or if SolrCloud is needed then SolrCloudTestCase - If SolrTestCase and need to embed Solr, use either EmbeddedSolrServerTestRule (doesn't use HTTP) or SolrJettyTestRule if HTTP/Jetty is relevant to what is being tested. - Avoid SolrTestCaseJ4 for new tests +- System properties set in a test are restored after each test by the base classes (`SystemPropertiesRestoreRule`); do not add `try/finally` + `System.clearProperty` cleanup - For BATS shell integration tests in `solr/packaging/test/`: - Always use `run <command>` followed by `assert_output --partial "..."` or `refute_output --partial "..."` instead of capturing output into local variables and using `[[ ]]` comparisons - Avoid patterns like `local var=$(cmd | grep ...); [[ "$var" == *"..."* ]]` — use `run cmd` + `assert_output`/`refute_output` instead diff --git a/changelog/unreleased/PR#5054-picocli-cli-small-fixes.yml b/changelog/unreleased/PR#5054-picocli-cli-small-fixes.yml new file mode 100644 index 00000000000..7e873f5c312 --- /dev/null +++ b/changelog/unreleased/PR#5054-picocli-cli-small-fixes.yml @@ -0,0 +1,10 @@ +title: > + The SOLR_CONNECTION environment variable (solr.connection system property) is now honored by the + bin/solr commands as documented. +type: fixed +authors: + - name: Jan Høydahl + nick: janhoy +links: + - name: PR#5054 + url: https://github.com/apache/solr/pull/5054 diff --git a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java index 68d4eedc470..f60d03c3574 100644 --- a/solr/core/src/java/org/apache/solr/cli/CLIUtils.java +++ b/solr/core/src/java/org/apache/solr/cli/CLIUtils.java @@ -67,7 +67,7 @@ public final class CLIUtils { CommandLine cli) throws IOException { String solrConnection = getCliOptionOrPropValue( - cli, CommonCLIOptions.SOLR_CONNECTION_OPTION, "solr-connection", null); + cli, CommonCLIOptions.SOLR_CONNECTION_OPTION, "solr.connection", null); if (solrConnection != null && !solrConnection.isBlank()) { return CloudSolrClient.CloudSolrClientConnection.parse(solrConnection); } diff --git a/solr/core/src/java/org/apache/solr/cli/CliDefaultValueProvider.java b/solr/core/src/java/org/apache/solr/cli/CliDefaultValueProvider.java index bb0521c94d8..b449d23ee4e 100644 --- a/solr/core/src/java/org/apache/solr/cli/CliDefaultValueProvider.java +++ b/solr/core/src/java/org/apache/solr/cli/CliDefaultValueProvider.java @@ -25,7 +25,7 @@ public class CliDefaultValueProvider implements CommandLine.IDefaultValueProvide public String defaultValue(CommandLine.Model.ArgSpec argSpec) throws Exception { return switch (argSpec.paramLabel()) { case "<zkHost>" -> EnvUtils.getProperty("zkHost"); - case "<solrConnection>" -> EnvUtils.getProperty("solr-connection"); + case "<solrConnection>" -> EnvUtils.getProperty("solr.connection"); case "<solrUrl>" -> EnvUtils.getProperty("solr.url"); // Must match CLIUtils.getDefaultSolrUrl(), which reads solr.port.listen case "<port>" -> EnvUtils.getProperty("solr.port.listen", "8983"); diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java index 25bf09db871..58149b97475 100644 --- a/solr/core/src/java/org/apache/solr/cli/CreateTool.java +++ b/solr/core/src/java/org/apache/solr/cli/CreateTool.java @@ -59,7 +59,7 @@ import org.apache.solr.core.ConfigSetService; " bin/solr create -c myCollection", "", " # Create a collection with 2 shards and 2 replicas", - " bin/solr create -c myCollection -s 2 -rf 2", + " bin/solr create -c myCollection -sh 2 -rf 2", "", " # Create a collection using an existing configset", " bin/solr create -c myCollection -d myconfig" diff --git a/solr/core/src/java/org/apache/solr/cli/StartCommand.java b/solr/core/src/java/org/apache/solr/cli/StartCommand.java index 06a747b361f..5af9b497aa8 100644 --- a/solr/core/src/java/org/apache/solr/cli/StartCommand.java +++ b/solr/core/src/java/org/apache/solr/cli/StartCommand.java @@ -113,11 +113,11 @@ public class StartCommand implements Callable<Integer> { boolean noPrompt; @CommandLine.Option( - names = "--prompt-inputs", + names = "--script-inputs", description = "Don't prompt for input; comma-delimited list of inputs to use when running examples that accept user input", paramLabel = "<values>") - String promptInputs; + String scriptInputs; @CommandLine.Option( names = "--example-dir", @@ -227,7 +227,7 @@ public class StartCommand implements Callable<Integer> { -y, --no-prompt Don't prompt for input; accept all defaults when running examples. - --prompt-inputs <values> + --script-inputs <values> Don't prompt for input; comma-delimited list of inputs for examples that accept user input. diff --git a/solr/core/src/test/org/apache/solr/cli/CLIUtilsTest.java b/solr/core/src/test/org/apache/solr/cli/CLIUtilsTest.java index 5ec2ddfa5fa..872bcf661c4 100644 --- a/solr/core/src/test/org/apache/solr/cli/CLIUtilsTest.java +++ b/solr/core/src/test/org/apache/solr/cli/CLIUtilsTest.java @@ -18,6 +18,9 @@ package org.apache.solr.cli; import java.net.SocketException; import java.net.URISyntaxException; +import java.util.List; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.Options; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrException; @@ -141,4 +144,31 @@ public class CLIUtilsTest extends SolrCloudTestCase { // "//example.com" are not throwing URISyntaxException. This however is an issue of // java.lang.URI, which is very lenient. } + + private static Options connectionOptions() { + return new Options() + .addOption(CommonCLIOptions.SOLR_CONNECTION_OPTION) + .addOption(CommonCLIOptions.ZK_HOST_OPTION) + .addOption(CommonCLIOptions.SOLR_URL_OPTION); + } + + @Test + public void testSolrConnectionFromSystemProperty() throws Exception { + var cli = new DefaultParser().parse(connectionOptions(), new String[0]); + System.setProperty("solr.connection", "zk1:2181,zk2:2181/solr"); + var connection = CLIUtils.getSolrConnection(cli); + assertNotNull("SOLR_CONNECTION / solr.connection should be picked up", connection); + assertTrue(connection.isZookeeper()); + assertEquals(List.of("zk1:2181", "zk2:2181"), connection.quorumItems()); + assertEquals("/solr", connection.zkChroot()); + } + + @Test + public void testZkHostFromSystemProperty() throws Exception { + var cli = new DefaultParser().parse(connectionOptions(), new String[0]); + System.setProperty("zkHost", "zk1:2181/solr"); + var connection = CLIUtils.getSolrConnection(cli); + assertNotNull(connection); + assertTrue(connection.isZookeeper()); + } } diff --git a/solr/core/src/test/org/apache/solr/cli/CliDefaultValueProviderTest.java b/solr/core/src/test/org/apache/solr/cli/CliDefaultValueProviderTest.java new file mode 100644 index 00000000000..80ccf0feb9f --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cli/CliDefaultValueProviderTest.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.solr.cli; + +import org.apache.solr.SolrTestCase; +import org.junit.Test; +import picocli.CommandLine; + +public class CliDefaultValueProviderTest extends SolrTestCase { + + private static String defaultFor(String optionName, String paramLabel) throws Exception { + var spec = CommandLine.Model.OptionSpec.builder(optionName).paramLabel(paramLabel).build(); + return new CliDefaultValueProvider().defaultValue(spec); + } + + @Test + public void testConnectionDefaultsFromSystemProperties() throws Exception { + System.setProperty("solr.connection", "zk1:2181/solr"); + System.setProperty("zkHost", "zk2:2181/solr"); + System.setProperty("solr.url", "http://solr.local:8983";); + assertEquals("zk1:2181/solr", defaultFor("--solr-connection", "<solrConnection>")); + assertEquals("zk2:2181/solr", defaultFor("--zk-host", "<zkHost>")); + assertEquals("http://solr.local:8983";, defaultFor("--solr-url", "<solrUrl>")); + } + + @Test + public void testNoDefaultWhenPropertiesUnset() throws Exception { + System.clearProperty("solr.connection"); + System.clearProperty("zkHost"); + System.clearProperty("solr.url"); + assertNull(defaultFor("--solr-connection", "<solrConnection>")); + assertNull(defaultFor("--zk-host", "<zkHost>")); + assertNull(defaultFor("--solr-url", "<solrUrl>")); + assertNull(defaultFor("--other", "<other>")); + } +} diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-create.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-create.adoc index a359ae8fd2f..a584a9c5565 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-create.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-create.adoc @@ -103,7 +103,7 @@ Creates a core or collection depending on whether Solr is running in standalone [%hardbreaks] # Create a collection with 2 shards and 2 replicas - bin/solr create -c myCollection -s 2 -rf 2 + bin/solr create -c myCollection -sh 2 -rf 2 [%hardbreaks] # Create a collection using an existing configset diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-start.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-start.adoc index 28970862336..a6ba0f61c2b 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-start.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-start.adoc @@ -36,7 +36,7 @@ See xref:cli/index.adoc[] to enable this implementation and for its known limita bin/solr start [-fhqy] [--force] [--user-managed] [--verbose] [--data-home=<dataHome>] [-e=<example>] [--example-dir=<exampleDir>] [--host=<host>] [-j=<jettyParams>] [--jvm-opts=<jvmOpts>] - [-m=<memory>] [-p=<port>] [--prompt-inputs=<values>] + [-m=<memory>] [-p=<port>] [--script-inputs=<values>] [--server-dir=<serverDir>] [--solr-home=<solrHome>] [-z=<zkHost>] .... @@ -85,12 +85,12 @@ Starts Solr *-p*, *--port*=_<port>_:: Specify the Solr HTTP port; default is 8983. STOP_PORT=($SOLR_PORT-1000), RMI_PORT=($SOLR_PORT+10000) -*--prompt-inputs*=_<values>_:: - Don't prompt for input; comma-delimited list of inputs to use when running examples that accept user input - *-q*, *--quiet*:: Set log level to WARN (quiet); default is INFO +*--script-inputs*=_<values>_:: + Don't prompt for input; comma-delimited list of inputs to use when running examples that accept user input + *--server-dir*=_<serverDir>_:: Specify the Solr server directory; default is 'server'
