This is an automated email from the ASF dual-hosted git repository.
dragonyliu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git
The following commit(s) were added to refs/heads/master by this push:
new aa7ec7b01 RATIS-1941. Do not print reflections log message in shell.
(#972)
aa7ec7b01 is described below
commit aa7ec7b01016252c58278bca848ddf3598c7baf6
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Mon Nov 27 22:41:36 2023 -0800
RATIS-1941. Do not print reflections log message in shell. (#972)
---
ratis-shell/pom.xml | 5 -
.../java/org/apache/ratis/shell/cli/Command.java | 7 +-
.../org/apache/ratis/shell/cli/sh/RatisShell.java | 54 ++++----
ratis-test/pom.xml | 7 +
.../apache/ratis/shell/cli/sh/TestRatisShell.java | 145 +++++++++++++++++++++
5 files changed, 180 insertions(+), 38 deletions(-)
diff --git a/ratis-shell/pom.xml b/ratis-shell/pom.xml
index 967f24980..78ab6679f 100644
--- a/ratis-shell/pom.xml
+++ b/ratis-shell/pom.xml
@@ -48,11 +48,6 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
</dependency>
- <dependency>
- <groupId>org.reflections</groupId>
- <artifactId>reflections</artifactId>
- <version>0.10.2</version>
- </dependency>
</dependencies>
<build>
<plugins>
diff --git a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/Command.java
b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/Command.java
index 606e55c35..ae4e70107 100644
--- a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/Command.java
+++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/Command.java
@@ -32,7 +32,7 @@ import java.util.Optional;
/**
* An interface for all the commands that can be run from a shell.
*/
-public interface Command extends Closeable {
+public interface Command extends Comparable<Command>, Closeable {
/**
* Gets the command name as input from the shell.
@@ -41,6 +41,11 @@ public interface Command extends Closeable {
*/
String getCommandName();
+ @Override
+ default int compareTo(Command that) {
+ return this.getCommandName().compareTo(that.getCommandName());
+ }
+
/**
* @return the supported {@link Options} of the command
*/
diff --git
a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
index 54f613210..2e53e3191 100644
--- a/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
+++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
@@ -19,19 +19,35 @@ package org.apache.ratis.shell.cli.sh;
import org.apache.ratis.shell.cli.AbstractShell;
import org.apache.ratis.shell.cli.Command;
+import org.apache.ratis.shell.cli.sh.command.AbstractParentCommand;
import org.apache.ratis.shell.cli.sh.command.Context;
-import org.apache.ratis.util.ReflectionUtils;
-import org.reflections.Reflections;
+import org.apache.ratis.shell.cli.sh.command.ElectionCommand;
+import org.apache.ratis.shell.cli.sh.command.GroupCommand;
+import org.apache.ratis.shell.cli.sh.command.LocalCommand;
+import org.apache.ratis.shell.cli.sh.command.PeerCommand;
+import org.apache.ratis.shell.cli.sh.command.SnapshotCommand;
import java.io.PrintStream;
-import java.lang.reflect.Modifier;
-import java.util.HashMap;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
/**
* Shell for manage ratis group.
*/
public class RatisShell extends AbstractShell {
+ static final List<Function<Context, AbstractParentCommand>>
PARENT_COMMAND_CONSTRUCTORS
+ = Collections.unmodifiableList(Arrays.asList(
+ PeerCommand::new, GroupCommand::new, ElectionCommand::new,
SnapshotCommand::new, LocalCommand::new));
+
+ static List<AbstractParentCommand> allParentCommands(Context context) {
+ return PARENT_COMMAND_CONSTRUCTORS.stream()
+ .map(constructor -> constructor.apply(context))
+ .collect(Collectors.toList());
+ }
/**
* Manage ratis shell command.
@@ -54,33 +70,7 @@ public class RatisShell extends AbstractShell {
@Override
protected Map<String, Command> loadCommands(Context context) {
- return loadCommands(RatisShell.class.getPackage().getName(),
- new Class[] {Context.class},
- new Object[] {getCloser().register(context)});
- }
-
- /**
- * Get instances of all subclasses of {@link Command} in a sub-package
called "command" the given
- * package.
- *
- * @param pkgName package prefix to look in
- * @param classArgs type of args to instantiate the class
- * @param objectArgs args to instantiate the class
- * @return a mapping from command name to command instance
- */
- private Map<String, Command> loadCommands(String pkgName, Class[] classArgs,
- Object[] objectArgs) {
- Map<String, Command> commandsMap = new HashMap<>();
- Reflections reflections = new Reflections(pkgName);
- for (Class<? extends Command> cls :
reflections.getSubTypesOf(Command.class)) {
- // Add commands from <pkgName>.command.*
- if (cls.getPackage().getName().equals(pkgName + ".command")
- && !Modifier.isAbstract(cls.getModifiers())) {
- // Only instantiate a concrete class
- final Command cmd = ReflectionUtils.newInstance(cls, classArgs,
objectArgs);
- commandsMap.put(cmd.getCommandName(), cmd);
- }
- }
- return commandsMap;
+ return allParentCommands(context).stream()
+ .collect(Collectors.toMap(Command::getCommandName,
Function.identity()));
}
}
diff --git a/ratis-test/pom.xml b/ratis-test/pom.xml
index f18b984d0..40b6b2ec2 100644
--- a/ratis-test/pom.xml
+++ b/ratis-test/pom.xml
@@ -100,6 +100,13 @@
</exclusions>
</dependency>
+ <dependency>
+ <groupId>org.reflections</groupId>
+ <artifactId>reflections</artifactId>
+ <version>0.10.2</version>
+ <scope>test</scope>
+ </dependency>
+
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk18on</artifactId>
diff --git
a/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/TestRatisShell.java
b/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/TestRatisShell.java
new file mode 100644
index 000000000..6e2227647
--- /dev/null
+++ b/ratis-test/src/test/java/org/apache/ratis/shell/cli/sh/TestRatisShell.java
@@ -0,0 +1,145 @@
+/*
+ * 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.ratis.shell.cli.sh;
+
+import org.apache.ratis.BaseTest;
+import org.apache.ratis.shell.cli.Command;
+import org.apache.ratis.shell.cli.sh.command.AbstractParentCommand;
+import org.apache.ratis.shell.cli.sh.command.Context;
+import org.apache.ratis.shell.cli.sh.command.ElectionCommand;
+import org.apache.ratis.shell.cli.sh.command.GroupCommand;
+import org.apache.ratis.shell.cli.sh.command.LocalCommand;
+import org.apache.ratis.shell.cli.sh.command.PeerCommand;
+import org.apache.ratis.shell.cli.sh.command.SnapshotCommand;
+import org.apache.ratis.util.ReflectionUtils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.reflections.Reflections;
+
+import java.io.PrintStream;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * Test {@link RatisShell}
+ */
+public class TestRatisShell extends BaseTest {
+ static final PrintStream OUT = System.out;
+ static final Class<?>[] ARG_CLASSES = new Class[] {Context.class};
+
+ static void assertCommand(String message, Command expected, Command
computed) {
+ Assert.assertEquals(message, expected.getClass(), computed.getClass());
+ Assert.assertEquals(message, expected.getCommandName(),
computed.getCommandName());
+ }
+
+ static void assertCommands(List<Command> expected, List<Command> computed) {
+ Assert.assertEquals(expected.size(), computed.size());
+ for(int i = 0; i < expected.size(); i++) {
+ assertCommand("Command " + i, expected.get(i), computed.get(i));
+ }
+ }
+
+ @Test
+ public void testFullParentCommandList() throws Exception {
+ final List<Command> expected = new
ArrayList<>(loadCommands(RatisShell.class.getPackage().getName() + ".command"));
+ Collections.sort(expected);
+
+ try(RatisShell shell = new RatisShell(OUT)) {
+ final List<Command> computed = new ArrayList<>(shell.getCommands());
+ Collections.sort(computed);
+
+ assertCommands(expected, computed);
+ }
+ }
+
+ @Test
+ public void testFullPeerCommandList() {
+ runTestFullCommandList(PeerCommand::new);
+ }
+
+ @Test
+ public void testFullGroupCommandList() {
+ runTestFullCommandList(GroupCommand::new);
+ }
+
+ @Test
+ public void testFullElectionCommandList() {
+ runTestFullCommandList(ElectionCommand::new);
+ }
+
+ @Test
+ public void testFullSnapshotCommandList() {
+ runTestFullCommandList(SnapshotCommand::new);
+ }
+
+ @Test
+ public void testFullLocalCommandList() {
+ runTestFullCommandList(LocalCommand::new);
+ }
+
+ static void runTestFullCommandList(Function<Context, AbstractParentCommand>
parentCommandConstructor) {
+ final AbstractParentCommand parent = parentCommandConstructor.apply(new
Context(OUT));
+ final List<Command> computed = new
ArrayList<>(parent.getSubCommands().values());
+ Collections.sort(computed);
+
+ Assert.assertFalse(computed.isEmpty());
+
+ final Package pkg = computed.iterator().next().getClass().getPackage();
+ final List<Command> expected = new ArrayList<>(loadCommands(pkg));
+ Collections.sort(expected);
+
+ assertCommands(expected, computed);
+ }
+
+ static Collection<Command> loadCommands(Package pkg) {
+ return loadCommands(pkg.getName());
+ }
+
+ static Collection<Command> loadCommands(String pkgName) {
+ return oldLoadCommands(pkgName, ARG_CLASSES, new Object[]{new
Context(OUT)}).values();
+ }
+
+ /**
+ * Get instances of all subclasses of {@link Command} in the given package.
+ *
+ * @param pkgName package prefix to look in
+ * @param classArgs type of args to instantiate the class
+ * @param objectArgs args to instantiate the class
+ * @return a mapping from command name to command instance
+ */
+ static Map<String, Command> oldLoadCommands(String pkgName, Class<?>[]
classArgs, Object[] objectArgs) {
+ Map<String, Command> commandsMap = new HashMap<>();
+ Reflections reflections = new Reflections(pkgName);
+ for (Class<? extends Command> cls :
reflections.getSubTypesOf(Command.class)) {
+ // Add commands from <pkgName>.*
+ if (cls.getPackage().getName().equals(pkgName)
+ && !Modifier.isAbstract(cls.getModifiers())) {
+ // Only instantiate a concrete class
+ final Command cmd = ReflectionUtils.newInstance(cls, classArgs,
objectArgs);
+ commandsMap.put(cmd.getCommandName(), cmd);
+ }
+ }
+ return commandsMap;
+ }
+}
