Skip to content

Commit

Permalink
Change long form of -r is --recursive
Browse files Browse the repository at this point in the history
Deprecate --recurse
  • Loading branch information
epugh committed Oct 7, 2024
1 parent 08a17da commit 487dc52
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 44 deletions.
12 changes: 6 additions & 6 deletions solr/bin/solr.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,7 @@ IF "%1"=="-V" (
) ELSE IF "%1"=="-n" (
goto set_config_name
) ELSE IF "%1"=="-r" (
goto set_zk_recurse
goto set_zk_recursive
) ELSE IF "%1"=="-configname" (
goto set_config_name
) ELSE IF "%1"=="-d" (
Expand Down Expand Up @@ -1369,8 +1369,8 @@ set ZK_DST=%~1
SHIFT
goto parse_zk_args

:set_zk_recurse
set ZK_RECURSE="true"
:set_zk_recursive
set ZK_RECURSIVE="true"
SHIFT
goto parse_zk_args

Expand Down Expand Up @@ -1455,7 +1455,7 @@ IF "!ZK_OP!"=="upconfig" (
"%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% %SOLR_TOOL_OPTS% -Dsolr.install.dir="%SOLR_TIP%" ^
-Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
-classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --source !ZK_SRC! --destination !ZK_DST! --recurse !ZK_RECURSE! %ZK_VERBOSE%
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --source !ZK_SRC! --destination !ZK_DST! --recursive !ZK_RECURSIVE! %ZK_VERBOSE%
) ELSE IF "!ZK_OP!"=="mv" (
IF "%ZK_SRC%"=="" (
set ERROR_MSG="<src> must be specified for 'mv' command"
Expand All @@ -1477,7 +1477,7 @@ IF "!ZK_OP!"=="upconfig" (
"%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% %SOLR_TOOL_OPTS% -Dsolr.install.dir="%SOLR_TIP%" ^
-Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
-classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --path !ZK_SRC! --recurse !ZK_RECURSE! %ZK_VERBOSE%
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --path !ZK_SRC! --recursive !ZK_RECURSIVE! %ZK_VERBOSE%
) ELSE IF "!ZK_OP!"=="ls" (
IF "%ZK_SRC"=="" (
set ERROR_MSG="Zookeeper path to remove must be specified when using the 'ls' command"
Expand All @@ -1486,7 +1486,7 @@ IF "!ZK_OP!"=="upconfig" (
"%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% %SOLR_TOOL_OPTS% -Dsolr.install.dir="%SOLR_TIP%" ^
-Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
-classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --path !ZK_SRC! --recurse !ZK_RECURSE! %ZK_VERBOSE%
org.apache.solr.cli.SolrCLI !ZK_OP! -z !ZK_HOST! --path !ZK_SRC! --recursive !ZK_RECURSIVE! %ZK_VERBOSE%
) ELSE IF "!ZK_OP!"=="mkroot" (
IF "%ZK_SRC"=="" (
set ERROR_MSG="Zookeeper path to create must be specified when using the 'mkroot' command"
Expand Down
17 changes: 15 additions & 2 deletions solr/core/src/java/org/apache/solr/cli/SolrCLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,22 @@ public class SolrCLI implements CLIO {
public static final Option OPTION_HELP =
Option.builder("h").longOpt("help").required(false).desc("Print this message.").build();

public static final Option OPTION_RECURSE =
Option.builder("r")
public static final Option OPTION_RECURSE_DEPRECATED =
Option.builder()
.longOpt("recurse")
.deprecated(
DeprecatedAttributes.builder()
.setForRemoval(true)
.setSince("9.8")
.setDescription("Use --recursive instead")
.get())
.required(false)
.desc("Apply the command recursively.")
.build();

public static final Option OPTION_RECURSIVE =
Option.builder("r")
.longOpt("recursive")
.required(false)
.desc("Apply the command recursively.")
.build();
Expand Down
7 changes: 4 additions & 3 deletions solr/core/src/java/org/apache/solr/cli/ZkCpTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public List<Option> getOptions() {
.required(false)
.desc("Required to look up configuration for compressing state.json.")
.build(),
SolrCLI.OPTION_RECURSE,
SolrCLI.OPTION_RECURSE_DEPRECATED,
SolrCLI.OPTION_RECURSIVE,
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_SOLRURL_DEPRECATED,
SolrCLI.OPTION_ZKHOST,
Expand Down Expand Up @@ -131,7 +132,7 @@ public void runImpl(CommandLine cli) throws Exception {
echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
String src = cli.getArgs()[0];
String dst = cli.getArgs()[1];
boolean recurse = cli.hasOption("recurse");
boolean recursive = cli.hasOption("recursive") || cli.hasOption("recurse");
echo("Copying from '" + src + "' to '" + dst + "'. ZooKeeper at " + zkHost);

boolean srcIsZk = src.toLowerCase(Locale.ROOT).startsWith("zk:");
Expand Down Expand Up @@ -208,7 +209,7 @@ public void runImpl(CommandLine cli) throws Exception {
.withStateFileCompression(minStateByteLenForCompression, compressor)
.build()) {

zkClient.zkTransfer(srcName, srcIsZk, dstName, dstIsZk, recurse);
zkClient.zkTransfer(srcName, srcIsZk, dstName, dstIsZk, recursive);

} catch (Exception e) {
log.error("Could not complete the zk operation for reason: ", e);
Expand Down
11 changes: 6 additions & 5 deletions solr/core/src/java/org/apache/solr/cli/ZkLsTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public ZkLsTool(PrintStream stdout) {
@Override
public List<Option> getOptions() {
return List.of(
SolrCLI.OPTION_RECURSE,
SolrCLI.OPTION_RECURSE_DEPRECATED,
SolrCLI.OPTION_RECURSIVE,
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_SOLRURL_DEPRECATED,
SolrCLI.OPTION_ZKHOST,
Expand Down Expand Up @@ -68,15 +69,15 @@ public void runImpl(CommandLine cli) throws Exception {
try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");

boolean recurse = cli.hasOption("recurse");
boolean recursive = cli.hasOption("recursive") || cli.hasOption("recurse");
echoIfVerbose(
"Getting listing for ZooKeeper node "
+ znode
+ " from ZooKeeper at "
+ zkHost
+ " recurse: "
+ recurse);
stdout.print(zkClient.listZnode(znode, recurse));
+ " recursive: "
+ recursive);
stdout.print(zkClient.listZnode(znode, recursive));
} catch (Exception e) {
log.error("Could not complete ls operation for reason: ", e);
throw (e);
Expand Down
13 changes: 7 additions & 6 deletions solr/core/src/java/org/apache/solr/cli/ZkRmTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ public ZkRmTool(PrintStream stdout) {
@Override
public List<Option> getOptions() {
return List.of(
SolrCLI.OPTION_RECURSE,
SolrCLI.OPTION_RECURSE_DEPRECATED,
SolrCLI.OPTION_RECURSIVE,
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_SOLRURL_DEPRECATED,
SolrCLI.OPTION_ZKHOST,
Expand All @@ -66,7 +67,7 @@ public void runImpl(CommandLine cli) throws Exception {
String zkHost = SolrCLI.getZkHost(cli);

String target = cli.getArgs()[0];
boolean recurse = cli.hasOption("recurse");
boolean recursive = cli.hasOption("recursive") || cli.hasOption("recurse");

String znode = target;
if (target.toLowerCase(Locale.ROOT).startsWith("zk:")) {
Expand All @@ -77,17 +78,17 @@ public void runImpl(CommandLine cli) throws Exception {
}
echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
if (!recurse && zkClient.getChildren(znode, null, true).size() != 0) {
if (!recursive && zkClient.getChildren(znode, null, true).size() != 0) {
throw new SolrServerException(
"ZooKeeper node " + znode + " has children and recurse has NOT been specified.");
"ZooKeeper node " + znode + " has children and recursive has NOT been specified.");
}
echo(
"Removing ZooKeeper node "
+ znode
+ " from ZooKeeper at "
+ zkHost
+ " recurse: "
+ recurse);
+ " recursive: "
+ recursive);
zkClient.clean(znode);
} catch (Exception e) {
log.error("Could not complete rm operation for reason: ", e);
Expand Down
41 changes: 23 additions & 18 deletions solr/core/src/test/org/apache/solr/cli/SolrCLIZkToolsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public void testCp() throws Exception {
Path tmp = createTempDir("tmpNewPlace2");
args =
new String[] {
"--recurse", "--zk-host", zkAddr, "zk:/configs/cp1", "file:" + tmp.toAbsolutePath()
"--recursive", "--zk-host", zkAddr, "zk:/configs/cp1", "file:" + tmp.toAbsolutePath()
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
Expand All @@ -195,7 +195,7 @@ public void testCp() throws Exception {
tmp = createTempDir("tmpNewPlace3");
args =
new String[] {
"--recurse", "--zk-host", zkAddr, "zk:/configs/cp1", tmp.toAbsolutePath().toString()
"--recursive", "--zk-host", zkAddr, "zk:/configs/cp1", tmp.toAbsolutePath().toString()
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
Expand All @@ -205,7 +205,7 @@ public void testCp() throws Exception {
// try with local->zk
args =
new String[] {
"--recurse", "--zk-host", zkAddr, srcPathCheck.toAbsolutePath().toString(), "zk:/cp3"
"--recursive", "--zk-host", zkAddr, srcPathCheck.toAbsolutePath().toString(), "zk:/cp3"
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
Expand All @@ -215,7 +215,7 @@ public void testCp() throws Exception {
// try with local->zk, file: specified
args =
new String[] {
"--recurse", "--zk-host", zkAddr, "file:" + srcPathCheck.toAbsolutePath(), "zk:/cp4"
"--recursive", "--zk-host", zkAddr, "file:" + srcPathCheck.toAbsolutePath(), "zk:/cp4"
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
Expand Down Expand Up @@ -309,7 +309,7 @@ public void testCp() throws Exception {
// source path to the dst
args =
new String[] {
"--recurse", "--zk-host", zkAddr, "file:" + srcPathCheck.toAbsolutePath(), "zk:/cp7/"
"--recursive", "--zk-host", zkAddr, "file:" + srcPathCheck.toAbsolutePath(), "zk:/cp7/"
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
Expand Down Expand Up @@ -341,7 +341,9 @@ public void testCp() throws Exception {

tmp = createTempDir("cp8");
args =
new String[] {"--recurse", "--zk-host", zkAddr, "zk:/cp7", "file:" + tmp.toAbsolutePath()};
new String[] {
"--recursive", "--zk-host", zkAddr, "zk:/cp7", "file:" + tmp.toAbsolutePath()
};
res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
assertEquals("Copy should have succeeded.", 0, res);

Expand All @@ -351,7 +353,9 @@ public void testCp() throws Exception {

// Finally, copy up to cp8 and verify that the data is up there.
args =
new String[] {"--recurse", "--zk-host", zkAddr, "file:" + tmp.toAbsolutePath(), "zk:/cp9"};
new String[] {
"--recursive", "--zk-host", zkAddr, "file:" + tmp.toAbsolutePath(), "zk:/cp9"
};

res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
assertEquals("Copy should have succeeded.", 0, res);
Expand Down Expand Up @@ -394,7 +398,7 @@ public void testCp() throws Exception {

args =
new String[] {
"--recurse",
"--recursive",
"--zk-host",
zkAddr,
"file:" + emptyFile.getParent().getParent().toString(),
Expand All @@ -408,7 +412,7 @@ public void testCp() throws Exception {
tmp2 = createTempDir("cp10");
args =
new String[] {
"--recurse", "--zk-host", zkAddr, "zk:/cp10", "file:" + tmp2.toAbsolutePath()
"--recursive", "--zk-host", zkAddr, "zk:/cp10", "file:" + tmp2.toAbsolutePath()
};
res = cpTool.runTool(SolrCLI.processCommandLineArgs(cpTool, args));
assertEquals("Copy should have succeeded.", 0, res);
Expand Down Expand Up @@ -516,7 +520,7 @@ public void testLs() throws Exception {
assertFalse("Return should NOT contain a child node", content.contains("solrconfig.xml"));

// ls with recursion
args = new String[] {"--recurse", "--zk-host", zkAddr, "/configs"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "/configs"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
content = baos.toString(StandardCharsets.UTF_8);
Expand All @@ -526,7 +530,7 @@ public void testLs() throws Exception {
assertTrue("Return should contain a child node", content.contains("solrconfig.xml"));

// Saw a case where going from root didn't work, so test it.
args = new String[] {"--recurse", "--zk-host", zkAddr, "/"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "/"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
content = baos.toString(StandardCharsets.UTF_8);
Expand All @@ -543,7 +547,7 @@ public void testLs() throws Exception {
assertFalse("Return should not contain /zookeeper", content.contains("/zookeeper"));

// Saw a case where ending in slash didn't work, so test it.
args = new String[] {"--recurse", "--zk-host", zkAddr, "/configs/"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "/configs/"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
content = baos.toString(StandardCharsets.UTF_8);
Expand All @@ -569,36 +573,37 @@ public void testRm() throws Exception {

int res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));

assertTrue("Should have failed to remove node with children unless --recurse is set", res != 0);
assertTrue(
"Should have failed to remove node with children unless --recursive is set", res != 0);

// Are we sure all the znodes are still there?
verifyZkLocalPathsMatch(srcPathCheck, "/configs/rm1");

// run without recurse specified
// run without recursive specified
args = new String[] {"--zk-host", zkAddr, "zk:/configs/rm1"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));

assertTrue(
"Should have failed to remove node with children if --recurse is set to false", res != 0);
"Should have failed to remove node with children if --recursive is set to false", res != 0);

args = new String[] {"--recurse", "--zk-host", zkAddr, "/configs/rm1"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "/configs/rm1"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
assertEquals("Should have removed node /configs/rm1", res, 0);
assertFalse(
"Znode /configs/toremove really should be gone", zkClient.exists("/configs/rm1", true));

// Check that zk prefix also works.
args = new String[] {"--recurse", "--zk-host", zkAddr, "zk:/configs/rm2"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "zk:/configs/rm2"};

res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
assertEquals("Should have removed node /configs/rm2", res, 0);
assertFalse(
"Znode /configs/toremove2 really should be gone", zkClient.exists("/configs/rm2", true));

// This should silently just refuse to do anything to the / or /zookeeper
args = new String[] {"--recurse", "--zk-host", zkAddr, "zk:/"};
args = new String[] {"--recursive", "--zk-host", zkAddr, "zk:/"};

AbstractDistribZkTestBase.copyConfigUp(configSet, "cloud-subdirs", "rm3", zkAddr);
res = tool.runTool(SolrCLI.processCommandLineArgs(tool, args));
Expand Down
2 changes: 1 addition & 1 deletion solr/packaging/test/test_zk.bats
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ teardown() {

@test "listing out files" {
sleep 1
run solr zk ls / -z localhost:${ZK_PORT}
run solr zk ls / -z localhost:${ZK_PORT} --recursive
assert_output --partial "aliases.json"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,7 @@ This command will copy from the local drive to ZooKeeper, from ZooKeeper to the

==== ZK Copy Parameters

`-r`::
`-r` or `--recursive`::
+
[%autowidth,frame=none]
|===
Expand Down Expand Up @@ -1401,7 +1401,7 @@ Use the `zk rm` command to remove a znode (and optionally all child nodes) from

==== ZK Remove Parameters

`-r`::
`-r` or `--recursive`::
+
[%autowidth,frame=none]
|===
Expand Down Expand Up @@ -1522,7 +1522,7 @@ Use the `zk ls` command to see the children of a znode.

==== ZK List Parameters

`-r`::
`-r` or `--recursive`::
+
[%autowidth,frame=none]
|===
Expand Down

0 comments on commit 487dc52

Please sign in to comment.