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 eb1b0cb7a7552a810c9e0db8318e0e83bf23cb24 Author: Serhiy Bzhezytskyy <[email protected]> AuthorDate: Thu Oct 8 18:44:13 2026 +0300 SOLR-18512: Port ConfigTool to picocli (#5039) Co-authored-by: Jan Høydahl <[email protected]> (cherry picked from commit adbbb8f49a1b612638af886ecd3e9dbcf93fee7e) --- .../SOLR-17697-picocli-experimental-cli.yml | 2 + .../src/java/org/apache/solr/cli/ConfigTool.java | 129 ++++++++++++++++++++- .../core/src/java/org/apache/solr/cli/SolrCLI.java | 3 +- .../org/apache/solr/cli/ConfigToolPicocliTest.java | 39 +++++++ .../test/org/apache/solr/cli/ConfigToolTest.java | 120 +++++++++++++++++++ .../modules/deployment-guide/deployment-nav.adoc | 1 + .../modules/deployment-guide/pages/cli/index.adoc | 3 + .../deployment-guide/pages/cli/solr-config.adoc | 91 +++++++++++++++ 8 files changed, 386 insertions(+), 2 deletions(-) diff --git a/changelog/unreleased/SOLR-17697-picocli-experimental-cli.yml b/changelog/unreleased/SOLR-17697-picocli-experimental-cli.yml index b554c9600fb..c008021477d 100644 --- a/changelog/unreleased/SOLR-17697-picocli-experimental-cli.yml +++ b/changelog/unreleased/SOLR-17697-picocli-experimental-cli.yml @@ -17,3 +17,5 @@ links: url: https://issues.apache.org/jira/browse/SOLR-18509 - name: SOLR-18511 url: https://issues.apache.org/jira/browse/SOLR-18511 + - name: SOLR-18512 + url: https://issues.apache.org/jira/browse/SOLR-18512 diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigTool.java index 4c0ac64935d..d9284e8fe4d 100644 --- a/solr/core/src/java/org/apache/solr/cli/ConfigTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ConfigTool.java @@ -25,6 +25,8 @@ import org.apache.commons.cli.MissingArgumentException; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.common.util.EnvUtils; import org.apache.solr.common.util.NamedList; import org.noggit.CharArr; import org.noggit.JSONWriter; @@ -34,8 +36,23 @@ import org.noggit.JSONWriter; * * <p>Sends a POST to the Config API to perform a specified action. */ +@SuppressWarnings("UnnecessarilyFullyQualified") [email protected]( + name = "config", + description = "Sends a POST to the Config API to perform a specified action.", + footerHeading = "%nExamples:%n", + footer = { + " # Set a config property", + " bin/solr config -c mycollection --property updateHandler.autoSoftCommit.maxTime --value" + + " 10000" + }) public class ConfigTool extends ToolBase { + /** + * @deprecated Only used by the commons-cli parser; the picocli path declares this as an annotated + * field. + */ + @Deprecated private static final Option COLLECTION_NAME_OPTION = Option.builder("c") .longOpt("name") @@ -45,6 +62,11 @@ public class ConfigTool extends ToolBase { .desc("Name of the collection.") .get(); + /** + * @deprecated Only used by the commons-cli parser; the picocli path declares this as an annotated + * field. + */ + @Deprecated private static final Option ACTION_OPTION = Option.builder("a") .longOpt("action") @@ -54,6 +76,11 @@ public class ConfigTool extends ToolBase { "Config API action, one of: set-property, unset-property, set-user-property, unset-user-property; default is 'set-property'.") .get(); + /** + * @deprecated Only used by the commons-cli parser; the picocli path declares this as an annotated + * field. + */ + @Deprecated private static final Option PROPERTY_OPTION = Option.builder() .longOpt("property") @@ -64,6 +91,11 @@ public class ConfigTool extends ToolBase { "Name of the Config API property to apply the action to, such as: 'updateHandler.autoSoftCommit.maxTime'.") .get(); + /** + * @deprecated Only used by the commons-cli parser; the picocli path declares this as an annotated + * field. + */ + @Deprecated private static final Option VALUE_OPTION = Option.builder("v") .longOpt("value") @@ -81,6 +113,68 @@ public class ConfigTool extends ToolBase { String value, String credentials) {} + // --- picocli fields --- + + @picocli.CommandLine.ArgGroup(exclusive = true, multiplicity = "0..1") + private ConnectionOptions connectionOptions; + + @picocli.CommandLine.Mixin private CredentialsOptions credentialsOptions; + + @picocli.CommandLine.Option( + names = {"-c", "--name"}, + required = true, + paramLabel = "NAME", + description = "Name of the collection.") + private String nameOpt; + + /** The values of {@code --action}, spelled on the command line as {@link #toString()} says. */ + enum Action { + SET_PROPERTY("set-property"), + UNSET_PROPERTY("unset-property"), + SET_USER_PROPERTY("set-user-property"), + UNSET_USER_PROPERTY("unset-user-property"); + + private final String id; + + Action(String id) { + this.id = id; + } + + @Override + public String toString() { + return id; + } + } + + @picocli.CommandLine.Option( + names = {"-a", "--action"}, + defaultValue = "set-property", + paramLabel = "ACTION", + description = + "Config API action, one of: ${COMPLETION-CANDIDATES}; default is '${DEFAULT-VALUE}'.") + private Action actionOpt; + + @picocli.CommandLine.Option( + names = "--property", + required = true, + paramLabel = "PROP", + description = + "Name of the Config API property to apply the action to, such as:" + + " 'updateHandler.autoSoftCommit.maxTime'.") + private String propertyOpt; + + // Long-only: "-v" is ToolBase's --verbose, and picocli rejects a duplicate short name. Under + // commons-cli the later-added VALUE_OPTION wins, so there "-v" still means --value. + @picocli.CommandLine.Option( + names = "--value", + paramLabel = "VALUE", + description = "Set the property to this value; accepts JSON objects and strings.") + private String valueOpt; + + public ConfigTool() { + this(new DefaultToolRuntime()); + } + public ConfigTool(ToolRuntime runtime) { super(runtime); } @@ -165,6 +259,39 @@ public class ConfigTool extends ToolBase { @Override public int callTool() throws Exception { - throw new UnsupportedOperationException("This tool does not yet support PicoCli"); + String solrUrl = resolveSolrUrl(credentialsOptions.credentials); + + // value is required unless the property is one of the "unset-" type. + String action = actionOpt.toString(); + if (!action.contains("unset-") && valueOpt == null) { + throw new MissingArgumentException("'value' is a required option."); + } + + ConfigParams params = + new ConfigParams( + solrUrl, action, nameOpt, propertyOpt, valueOpt, credentialsOptions.credentials); + updateConfig(params); + return 0; + } + + private String resolveSolrUrl(String credentials) throws Exception { + String solrUrlArg = (connectionOptions != null) ? connectionOptions.effectiveSolrUrl() : null; + if (solrUrlArg != null) { + return CLIUtils.normalizeSolrUrl(solrUrlArg); + } + String zkHostArg = + (connectionOptions != null) + ? connectionOptions.effectiveZkHost() + : EnvUtils.getProperty("zkHost"); + if (zkHostArg != null) { + return CLIUtils.solrUrlFromConnection( + CloudSolrClient.CloudSolrClientConnection.parse(zkHostArg), credentials); + } + String defaultSolrUrl = CLIUtils.getDefaultSolrUrl(); + CLIO.err( + "Neither --zk-host or --solr-url parameters, nor ZK_HOST env var provided, so assuming solr url is " + + defaultSolrUrl + + "."); + return defaultSolrUrl; } } diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java index 978d3681cf6..a1da71e04ae 100755 --- a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java +++ b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java @@ -84,7 +84,8 @@ import org.slf4j.LoggerFactory; DeleteTool.class, AssertTool.class, ApiTool.class, - ClusterTool.class + ClusterTool.class, + ConfigTool.class }) public class SolrCLI implements CLIO, java.util.concurrent.Callable<Integer> { diff --git a/solr/core/src/test/org/apache/solr/cli/ConfigToolPicocliTest.java b/solr/core/src/test/org/apache/solr/cli/ConfigToolPicocliTest.java new file mode 100644 index 00000000000..6ba1d85d968 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cli/ConfigToolPicocliTest.java @@ -0,0 +1,39 @@ +/* + * 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 java.util.Arrays; +import picocli.CommandLine; + +/** + * Runs all {@link ConfigToolTest} tests through the picocli invocation path. + * + * <p>All {@code @Test} methods are inherited; only the invocation strategy is overridden. + */ +public class ConfigToolPicocliTest extends ConfigToolTest { + + @Override + protected int runTool(String[] args, Class<? extends ToolBase> clazz) throws Exception { + // args[0] is the tool name used by commons-cli dispatch; strip it for picocli. + String[] toolArgs = Arrays.copyOfRange(args, 1, args.length); + ToolRuntime runtime = new CLITestHelper.TestingRuntime(false); + ToolBase tool = clazz.getDeclaredConstructor(ToolRuntime.class).newInstance(runtime); + return new CommandLine(tool) + .setDefaultValueProvider(new CliDefaultValueProvider()) + .execute(toolArgs); + } +} diff --git a/solr/core/src/test/org/apache/solr/cli/ConfigToolTest.java b/solr/core/src/test/org/apache/solr/cli/ConfigToolTest.java new file mode 100644 index 00000000000..162144c8c75 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cli/ConfigToolTest.java @@ -0,0 +1,120 @@ +/* + * 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.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.GenericSolrRequest; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.NamedList; +import org.junit.BeforeClass; +import org.junit.Test; + +public class ConfigToolTest extends SolrCloudTestCase { + private static final String COLLECTION = "configToolColl"; + + /** Runs the tool. Overridden by the picocli variant of this test. */ + protected int runTool(String[] args, Class<? extends ToolBase> clazz) throws Exception { + return CLITestHelper.runTool(args, clazz); + } + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(1) + .addConfig( + "config", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf")) + .configure(); + CollectionAdminRequest.createCollection(COLLECTION, "config", 1, 1) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION, 1, 1); + } + + private String overlay() throws Exception { + try (SolrClient client = + CLIUtils.getSolrClient(cluster.getJettySolrRunner(0).getBaseUrl().toString(), null)) { + NamedList<Object> rsp = + client.request( + new GenericSolrRequest( + SolrRequest.METHOD.GET, + "/" + COLLECTION + "/config/overlay", + new ModifiableSolrParams())); + return String.valueOf(rsp.get("overlay")); + } + } + + private String[] args(String... rest) { + String[] common = { + "config", "-c", COLLECTION, "-s", cluster.getJettySolrRunner(0).getBaseUrl().toString() + }; + String[] all = new String[common.length + rest.length]; + System.arraycopy(common, 0, all, 0, common.length); + System.arraycopy(rest, 0, all, common.length, rest.length); + return all; + } + + @Test + public void testSetAndUnsetProperty() throws Exception { + assertEquals( + 0, + runTool( + args("--property", "updateHandler.autoCommit.maxDocs", "--value", "100"), + ConfigTool.class)); + assertTrue(overlay(), overlay().contains("maxDocs=100")); + + assertEquals( + 0, + runTool( + args("--action", "unset-property", "--property", "updateHandler.autoCommit.maxDocs"), + ConfigTool.class)); + assertFalse(overlay(), overlay().contains("maxDocs")); + } + + @Test + public void testSetPropertyRequiresValue() throws Exception { + assertEquals( + 1, runTool(args("--property", "updateHandler.autoCommit.maxDocs"), ConfigTool.class)); + } + + @Test + public void testSetAndUnsetUserProperty() throws Exception { + assertEquals( + 0, + runTool( + args("--action", "set-user-property", "--property", "my.prop", "--value", "abc"), + ConfigTool.class)); + assertTrue(overlay(), overlay().contains("my.prop=abc")); + + assertEquals( + 0, + runTool( + args("--action", "unset-user-property", "--property", "my.prop"), ConfigTool.class)); + assertFalse(overlay(), overlay().contains("my.prop")); + } + + @Test + public void testUnknownActionFails() throws Exception { + // commons-cli passes it on and the Config API refuses it (1); picocli rejects it first (2) + assertNotEquals( + 0, + runTool( + args("--action", "no-such-action", "--property", "my.prop", "--value", "x"), + ConfigTool.class)); + assertFalse(overlay(), overlay().contains("my.prop")); + } +} diff --git a/solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc b/solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc index 0d6de3d1693..47f6a502c08 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc @@ -24,6 +24,7 @@ *** xref:cli/solr-assert.adoc[assert] *** xref:cli/solr-auth.adoc[auth] *** xref:cli/solr-cluster.adoc[cluster] +*** xref:cli/solr-config.adoc[config] *** xref:cli/solr-create.adoc[create] *** xref:cli/solr-delete.adoc[delete] *** xref:cli/solr-start.adoc[start] diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/index.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/index.adoc index cc5d9215739..600c2d842c5 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/index.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/index.adoc @@ -63,6 +63,7 @@ Any command not listed here is unavailable when the picocli implementation is en |xref:cli/solr-assert.adoc[bin/solr assert] |xref:cli/solr-auth.adoc[bin/solr auth] |xref:cli/solr-cluster.adoc[bin/solr cluster] +|xref:cli/solr-config.adoc[bin/solr config] |xref:cli/solr-create.adoc[bin/solr create] |xref:cli/solr-delete.adoc[bin/solr delete] |xref:cli/solr-start.adoc[bin/solr start] @@ -86,6 +87,8 @@ Any command not listed here is unavailable when the picocli implementation is en In addition to the commands that have not been migrated yet: * On a sub-command, `-v` means `--verbose`; only the top-level `bin/solr` accepts `-v` as a request for the version. +* `bin/solr config` has no `-v` short form for `--value`, because `-v` means `--verbose`; use `--value`. + The default implementation accepts `-v` for it. * `--fail-on-exists` on `bin/solr zk mkroot` treats the option as set whenever it is present, whatever value follows it. This matches the default implementation, but means `--fail-on-exists false` does not disable the check. * Error messages for invalid or mutually exclusive options are worded differently from the default implementation. diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-config.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-config.adoc new file mode 100644 index 00000000000..d598219ae76 --- /dev/null +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-config.adoc @@ -0,0 +1,91 @@ +// 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. +// +// DO NOT EDIT -- this page is auto-generated from picocli annotations. +// To update: modify the @Command/@Option annotations in the Java source, then run: +// ./gradlew :solr:solr-ref-guide:generateCliDocs + += bin/solr config +:page-toclevels: 2 + +[WARNING] +==== +This page documents the *experimental* picocli implementation of `bin/solr`, which is not enabled by default. +For what `bin/solr` does by default, see xref:solr-control-script-reference.adoc[]. +See xref:cli/index.adoc[] to enable this implementation and for its known limitations. +==== + +// tag::picocli-generated-man-section-synopsis[] +== Synopsis + +.... +bin/solr config [-hv] [-a=ACTION] -c=NAME --property=PROP [-u=<credentials>] + [--value=VALUE] [-s=<solrConnection> | --solr-url=<solrUrl> | + -z=<zkHost>] +.... + +// end::picocli-generated-man-section-synopsis[] + +// tag::picocli-generated-man-section-description[] +== Description + +Sends a POST to the Config API to perform a specified action. + +// end::picocli-generated-man-section-description[] + +// tag::picocli-generated-man-section-options[] +== Options + +*-a*, *--action*=_ACTION_:: + Config API action, one of: set-property, unset-property, set-user-property, unset-user-property; default is 'set-property'. + +*-c*, *--name*=_NAME_:: + *(required)* Name of the collection. + +*-h*, *--help*:: + Print this help message and exit. + +*--property*=_PROP_:: + *(required)* Name of the Config API property to apply the action to, such as: 'updateHandler.autoSoftCommit.maxTime'. + +*-s*, *--solr-connection*=_<solrConnection>_:: + Zookeeper or HTTP(s) connection string; unnecessary if SOLR_CONNECTION is defined in solr.in.sh; otherwise, defaults to localhost:9983. + +*--solr-url*=_<solrUrl>_:: + Base Solr URL, which can be used to determine the zk-host if that's not known. + +*-u*, *--credentials*=_<credentials>_:: + Credentials in the format username:password. Example: --credentials solr:SolrRocks + +*-v*, *--verbose*:: + Enable verbose mode. + +*--value*=_VALUE_:: + Set the property to this value; accepts JSON objects and strings. + +*-z*, *--zk-host*=_<zkHost>_:: + Zookeeper connection string; unnecessary if ZK_HOST is defined in solr.in.sh; otherwise, defaults to localhost:9983. + +// end::picocli-generated-man-section-options[] + +// tag::picocli-generated-man-section-footer[] +== Examples +[%hardbreaks] + # Set a config property + bin/solr config -c mycollection --property updateHandler.autoSoftCommit.maxTime --value 10000 + +// end::picocli-generated-man-section-footer[]
