[
https://issues.apache.org/jira/browse/GEODE-3007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16257743#comment-16257743
]
ASF GitHub Bot commented on GEODE-3007:
---------------------------------------
jdeppe-pivotal closed pull request #1072: Revert "GEODE-3007: Simplify support
for custom GFSH commands (#1042)"
URL: https://github.com/apache/geode/pull/1072
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index 4615a6b08d..1f139a5ac9 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -198,12 +198,14 @@ def cp = {
it.contains('snappy') ||
it.contains('jgroups') ||
it.contains('netty') ||
+
// dependencies from geode-lucene
it.contains('lucene-analyzers-common') ||
it.contains('lucene-core') ||
it.contains('lucene-queries') ||
it.contains('lucene-queryparser') ||
it.contains('lucene-analyzers-phonetic') ||
+
// dependencies from geode-protobuf
it.contains('protobuf-java')
}
diff --git
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
index 94ddc8f322..7e7b2bdca8 100644
---
a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
+++
b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
@@ -30,6 +30,7 @@
@Test
public void statusLocatorSucceedsWhenConnected() throws Exception {
GfshScript.of("start locator --name=locator1").execute(gfshRule);
+
GfshScript.of("connect", "status locator
--name=locator1").execute(gfshRule);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
index 4944e984bd..649bfad407 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
@@ -1860,10 +1860,7 @@
* <U>Default</U>: <code>""</code>
* </p>
* <U>Since</U>: GemFire 8.0
- *
- * @deprecated Since Geode 1.4 (See GEODE-3007)
*/
- @Deprecated()
String USER_COMMAND_PACKAGES = "user-command-packages";
/**
* The static String definition of the <i>"off-heap-memory-size"</i>
property <a
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
index 7c5929c4a8..43c5cb48fe 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
@@ -1018,6 +1018,9 @@ public static Class _getAttributeType(String attName) {
m.put(GROUPS,
"A comma separated list of all the groups this member belongs to." + "
Defaults to \"\".");
+ m.put(USER_COMMAND_PACKAGES,
+ "A comma separated list of the names of the packages containing
classes that implement user commands.");
+
m.put(JMX_MANAGER,
"If true then this member is willing to be a jmx manager. Defaults to
false except on a locator.");
m.put(JMX_MANAGER_START,
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
index 3aeb19fad2..eabe6ccee6 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
@@ -386,6 +386,34 @@
*/
File DEFAULT_DEPLOY_WORKING_DIR = new File(System.getProperty("user.dir"));
+ /**
+ * Returns the value of the {@link
ConfigurationProperties#USER_COMMAND_PACKAGES} property
+ */
+ @ConfigAttributeGetter(name = USER_COMMAND_PACKAGES)
+ String getUserCommandPackages();
+
+ /**
+ * Sets the system's user command path.
+ *
+ * @throws IllegalArgumentException if the specified value is not acceptable.
+ * @throws org.apache.geode.UnmodifiableException if this attribute can not
be modified.
+ * @throws org.apache.geode.GemFireIOException if the set failure is caused
by an error when
+ * writing to the system's configuration file.
+ */
+ @ConfigAttributeSetter(name = USER_COMMAND_PACKAGES)
+ void setUserCommandPackages(String value);
+
+ /**
+ * The name of the {@link ConfigurationProperties#USER_COMMAND_PACKAGES}
property.
+ */
+ @ConfigAttribute(type = String.class)
+ String USER_COMMAND_PACKAGES_NAME = USER_COMMAND_PACKAGES;
+
+ /**
+ * The default value of the {@link
ConfigurationProperties#USER_COMMAND_PACKAGES} property
+ */
+ String DEFAULT_USER_COMMAND_PACKAGES = "";
+
/**
* Returns the value of the {@link ConfigurationProperties#LOG_FILE} property
*
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
index 0c13480702..cceeaf9eb0 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
@@ -625,6 +625,8 @@
private Map<String, ConfigSource> sourceMap =
Collections.synchronizedMap(new HashMap<String, ConfigSource>());
+ protected String userCommandPackages = DEFAULT_USER_COMMAND_PACKAGES;
+
/**
* "off-heap-memory-size" with value of "" or "<size>[g|m]"
*/
@@ -757,6 +759,7 @@ public DistributionConfigImpl(DistributionConfig other) {
this.redisPort = other.getRedisPort();
this.redisBindAddress = other.getRedisBindAddress();
this.redisPassword = other.getRedisPassword();
+ this.userCommandPackages = other.getUserCommandPackages();
// following added for 8.0
this.enableSharedConfiguration = other.getEnableClusterConfiguration();
@@ -1831,6 +1834,10 @@ public int getAsyncMaxQueueSize() {
return this.asyncMaxQueueSize;
}
+ public String getUserCommandPackages() {
+ return this.userCommandPackages;
+ }
+
public int getHttpServicePort() {
return this.httpServicePort;
}
@@ -1855,6 +1862,10 @@ public void setStartDevRestApi(boolean value) {
this.startDevRestApi = value;
}
+ public void setUserCommandPackages(String value) {
+ this.userCommandPackages = value;
+ }
+
public boolean getDeltaPropagation() {
return this.deltaPropagation;
}
@@ -3002,8 +3013,9 @@ public boolean equals(final Object o) {
.append(sslTrustStore, that.sslTrustStore)
.append(sslTrustStorePassword, that.sslTrustStorePassword)
.append(locatorSSLAlias, that.locatorSSLAlias).append(sslDefaultAlias,
that.sslDefaultAlias)
- .append(sourceMap, that.sourceMap).append(offHeapMemorySize,
that.offHeapMemorySize)
- .append(shiroInit, that.shiroInit).isEquals();
+ .append(sourceMap, that.sourceMap).append(userCommandPackages,
that.userCommandPackages)
+ .append(offHeapMemorySize, that.offHeapMemorySize).append(shiroInit,
that.shiroInit)
+ .isEquals();
}
/**
@@ -3071,8 +3083,9 @@ public int hashCode() {
.append(sslCiphers).append(sslRequireAuthentication).append(sslKeyStore)
.append(sslKeyStoreType).append(sslKeyStorePassword).append(sslTrustStore)
.append(sslTrustStorePassword).append(sslWebServiceRequireAuthentication)
-
.append(locatorSSLAlias).append(sslDefaultAlias).append(sourceMap).append(offHeapMemorySize)
- .append(lockMemory).append(shiroInit).append(modifiable).toHashCode();
+ .append(locatorSSLAlias).append(sslDefaultAlias).append(sourceMap)
+
.append(userCommandPackages).append(offHeapMemorySize).append(lockMemory).append(shiroInit)
+ .append(modifiable).toHashCode();
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
index f84799f40f..2a6565ace2 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
@@ -338,7 +338,8 @@ public MemberMBeanBridge(InternalCache cache,
SystemManagementService service) {
this.config = system.getConfig();
try {
- this.commandProcessor = new
OnlineCommandProcessor(cache.getSecurityService());
+ this.commandProcessor =
+ new OnlineCommandProcessor(system.getProperties(),
cache.getSecurityService());
} catch (Exception e) {
commandServiceInitError = e.getMessage();
logger.info(LogMarker.CONFIG, "Command processor could not be
initialized. {}",
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
index 0b13d62cf4..44617d3f18 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
@@ -14,93 +14,293 @@
*/
package org.apache.geode.management.internal.cli;
+import static
org.apache.geode.distributed.ConfigurationProperties.USER_COMMAND_PACKAGES;
-import static java.util.stream.Collectors.toSet;
-
-import java.util.Objects;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Properties;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
import java.util.Set;
-import java.util.stream.Stream;
+import java.util.StringTokenizer;
+import org.springframework.shell.converters.EnumConverter;
+import org.springframework.shell.converters.SimpleFileConverter;
import org.springframework.shell.core.CommandMarker;
import org.springframework.shell.core.Converter;
+import org.springframework.shell.core.MethodTarget;
+import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
+import org.springframework.shell.core.annotation.CliCommand;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.ClassPathLoader;
+import org.apache.geode.management.internal.cli.commands.GfshCommand;
import org.apache.geode.management.internal.cli.help.Helper;
import org.apache.geode.management.internal.cli.shell.Gfsh;
import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
/**
+ *
* this only takes care of loading all available command markers and
converters from the application
*
* @since GemFire 7.0
*/
public class CommandManager {
- // Skip some of the Converters from Spring Shell for our customization
- private static final Set<String> EXCLUDED_CLASSES =
- Stream.of("-org.springframework.shell.converters.SimpleFileConverter",
- "-org.springframework.shell.converters.FileConverter",
- "-org.springframework.shell.converters.EnumConverter",
- "-org.springframework.shell.commands.ExitCommands",
- "-org.springframework.shell.commands.HelpCommands",
- "-org.springframework.shell.commands.VersionCommands",
-
"-org.springframework.shell.commands.ConsoleCommands").collect(toSet());
-
- private final LogWrapper logWrapper = LogWrapper.getInstance();
- private final Set<CommandMarker> commandMarkers;
- private final Set<Converter> converters;
- private final Helper helper;
+ public static final String USER_CMD_PACKAGES_PROPERTY =
+ DistributionConfig.GEMFIRE_PREFIX + USER_COMMAND_PACKAGES;
+ public static final String USER_CMD_PACKAGES_ENV_VARIABLE =
"GEMFIRE_USER_COMMAND_PACKAGES";
+ private static final Object INSTANCE_LOCK = new Object();
+
+ private final Helper helper = new Helper();
+
+ private final List<Converter<?>> converters = new ArrayList<Converter<?>>();
+ private final List<CommandMarker> commandMarkers = new ArrayList<>();
+ private Properties cacheProperties;
+ private LogWrapper logWrapper;
+
+ /**
+ * this constructor is used from Gfsh VM. We are getting the
user-command-package from system
+ * environment. used by Gfsh.
+ */
public CommandManager() {
- helper = new Helper();
- converters = loadConverters();
- commandMarkers = loadCommandMarkers();
+ this(null);
}
- private Set<Converter> loadConverters() {
- Set<Converter> converters =
instantiateAllClassesImplementing(Converter.class);
- raiseExceptionIfEmpty(converters, "converters");
+ /**
+ * this is used when getting the instance in a cache server. We are getting
the
+ * user-command-package from distribution properties. used by
OnlineCommandProcessor.
+ */
+ public CommandManager(final Properties cacheProperties) {
+ if (cacheProperties != null) {
+ this.cacheProperties = cacheProperties;
+ }
+ logWrapper = LogWrapper.getInstance();
+ loadCommands();
+ }
- converters.forEach(this::setContextIfCommandManagerAware);
- return converters;
+ private static void raiseExceptionIfEmpty(Set<Class<?>> foundClasses, String
errorFor)
+ throws IllegalStateException {
+ if (foundClasses == null || foundClasses.isEmpty()) {
+ throw new IllegalStateException(
+ "Required " + errorFor + " classes were not loaded. Check logs for
errors.");
+ }
}
- private Set<CommandMarker> loadCommandMarkers() {
- Set<CommandMarker> commandMarkers =
instantiateAllClassesImplementing(CommandMarker.class);
- raiseExceptionIfEmpty(commandMarkers, "commandMarkers");
+ private void loadUserCommands() {
+ final Set<String> userCommandPackages = new HashSet<String>();
- commandMarkers.forEach(this::setContextIfCommandManagerAware);
- commandMarkers.forEach(helper::registerCommand);
- return commandMarkers;
+ // Find by packages specified by the system property
+ if (System.getProperty(USER_CMD_PACKAGES_PROPERTY) != null) {
+ StringTokenizer tokenizer =
+ new StringTokenizer(System.getProperty(USER_CMD_PACKAGES_PROPERTY),
",");
+ while (tokenizer.hasMoreTokens()) {
+ userCommandPackages.add(tokenizer.nextToken());
+ }
+ }
+
+ // Find by packages specified by the environment variable
+ if (System.getenv().containsKey(USER_CMD_PACKAGES_ENV_VARIABLE)) {
+ StringTokenizer tokenizer =
+ new
StringTokenizer(System.getenv().get(USER_CMD_PACKAGES_ENV_VARIABLE), ",");
+ while (tokenizer.hasMoreTokens()) {
+ userCommandPackages.add(tokenizer.nextToken());
+ }
+ }
+
+ // Find by packages specified in the distribution config
+ if (this.cacheProperties != null) {
+ String cacheUserCmdPackages =
+
this.cacheProperties.getProperty(ConfigurationProperties.USER_COMMAND_PACKAGES);
+ if (cacheUserCmdPackages != null && !cacheUserCmdPackages.isEmpty()) {
+ StringTokenizer tokenizer = new StringTokenizer(cacheUserCmdPackages,
",");
+ while (tokenizer.hasMoreTokens()) {
+ userCommandPackages.add(tokenizer.nextToken());
+ }
+ }
+ }
+
+ // Load commands found in all of the packages
+ for (String userCommandPackage : userCommandPackages) {
+ try {
+ Set<Class<?>> foundClasses = ClasspathScanLoadHelper
+ .scanPackageForClassesImplementing(userCommandPackage,
CommandMarker.class);
+ for (Class<?> klass : foundClasses) {
+ try {
+ add((CommandMarker) klass.newInstance());
+ } catch (Exception e) {
+ logWrapper.warning("Could not load User Commands from: " + klass +
" due to "
+ + e.getLocalizedMessage()); // continue
+ }
+ }
+ raiseExceptionIfEmpty(foundClasses, "User Command");
+ } catch (IllegalStateException e) {
+ logWrapper.warning(e.getMessage(), e);
+ throw e;
+ }
+ }
+ }
+
+ /**
+ * Loads commands via {@link ServiceLoader} from {@link ClassPathLoader}.
+ *
+ * @since GemFire 8.1
+ */
+ private void loadPluginCommands() {
+ final Iterator<CommandMarker> iterator = ServiceLoader
+ .load(CommandMarker.class,
ClassPathLoader.getLatest().asClassLoader()).iterator();
+ while (iterator.hasNext()) {
+ try {
+ final CommandMarker commandMarker = iterator.next();
+ try {
+ add(commandMarker);
+ } catch (Exception e) {
+ logWrapper.warning("Could not load Command from: " +
commandMarker.getClass() + " due to "
+ + e.getLocalizedMessage(), e); // continue
+ }
+ } catch (ServiceConfigurationError e) {
+ logWrapper.severe("Could not load Command: " +
e.getLocalizedMessage(), e); // continue
+ }
+ }
}
- private <T> Set<T> instantiateAllClassesImplementing(Class<T>
implementedInterface) {
- Set<Class<? extends T>> classes =
ClasspathScanLoadHelper.scanClasspathForClassesImplementing(
- implementedInterface, EXCLUDED_CLASSES.toArray(new String[0]));
- return
classes.stream().map(this::instantiateClass).filter(Objects::nonNull).collect(toSet());
+ private void loadCommands() {
+ loadUserCommands();
+
+ loadPluginCommands();
+ loadGeodeCommands();
+ loadConverters();
}
- private <T> T instantiateClass(Class<T> classToInstantiate) {
+ private void loadConverters() {
+ Set<Class<?>> foundClasses;
+ // Converters
try {
- return classToInstantiate.newInstance();
- } catch (Exception e) {
- logWrapper.warning("Could not load command or converter from: " +
classToInstantiate, e);
+ foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+ "org.apache.geode.management.internal.cli.converters",
Converter.class);
+ for (Class<?> klass : foundClasses) {
+ try {
+ Converter<?> object = (Converter<?>) klass.newInstance();
+ add(object);
+
+ } catch (Exception e) {
+ logWrapper.warning(
+ "Could not load Converter from: " + klass + " due to " +
e.getLocalizedMessage()); // continue
+ }
+ }
+ raiseExceptionIfEmpty(foundClasses, "Converters");
+
+ // Spring shell's converters
+ foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+ "org.springframework.shell.converters", Converter.class);
+ for (Class<?> klass : foundClasses) {
+ if (!SHL_CONVERTERS_TOSKIP.contains(klass)) {
+ try {
+ add((Converter<?>) klass.newInstance());
+ } catch (Exception e) {
+ logWrapper.warning(
+ "Could not load Converter from: " + klass + " due to " +
e.getLocalizedMessage()); // continue
+ }
+ }
+ }
+ raiseExceptionIfEmpty(foundClasses, "Basic Converters");
+ } catch (IllegalStateException e) {
+ logWrapper.warning(e.getMessage(), e);
+ throw e;
}
- return null;
}
- private void setContextIfCommandManagerAware(Object commandOrConverter) {
- if
(CommandManagerAware.class.isAssignableFrom(commandOrConverter.getClass())) {
- ((CommandManagerAware) commandOrConverter).setCommandManager(this);
+ private void loadGeodeCommands() {
+ // CommandMarkers
+ Set<Class<?>> foundClasses;
+ try {
+ // geode's commands
+ foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+ GfshCommand.class.getPackage().getName(), CommandMarker.class);
+
+ for (Class<?> klass : foundClasses) {
+ try {
+ add((CommandMarker) klass.newInstance());
+ } catch (Exception e) {
+ logWrapper.warning(
+ "Could not load Command from: " + klass + " due to " +
e.getLocalizedMessage()); // continue
+ }
+ }
+ raiseExceptionIfEmpty(foundClasses, "Commands");
+
+ // do not add Spring shell's commands for now. When we add it, we need
to tell the parser that
+ // these are offline commands.
+ } catch (IllegalStateException e) {
+ logWrapper.warning(e.getMessage(), e);
+ throw e;
}
}
- private static void raiseExceptionIfEmpty(Set<?> foundClasses, String
classType)
- throws IllegalStateException {
- if (foundClasses == null || foundClasses.isEmpty()) {
- throw new IllegalStateException("No " + classType + " were loaded. Check
logs for errors.");
+ /** Skip some of the Converters from Spring Shell for our customization */
+ private static List<Class> SHL_CONVERTERS_TOSKIP = new ArrayList();
+ static {
+ // skip springs SimpleFileConverter to use our own FilePathConverter
+ SHL_CONVERTERS_TOSKIP.add(SimpleFileConverter.class);
+ // skip spring's EnumConverter to use our own EnumConverter
+ SHL_CONVERTERS_TOSKIP.add(EnumConverter.class);
+ }
+
+ public List<Converter<?>> getConverters() {
+ return converters;
+ }
+
+ public List<CommandMarker> getCommandMarkers() {
+ return commandMarkers;
+ }
+
+ /**
+ * Method to add new Converter
+ *
+ * @param converter
+ */
+ void add(Converter<?> converter) {
+ if (CommandManagerAware.class.isAssignableFrom(converter.getClass())) {
+ ((CommandManagerAware) converter).setCommandManager(this);
+ }
+ converters.add(converter);
+ }
+
+ /**
+ * Method to add new Commands to the parser
+ *
+ * @param commandMarker
+ */
+ void add(CommandMarker commandMarker) {
+ if (CommandManagerAware.class.isAssignableFrom(commandMarker.getClass())) {
+ ((CommandManagerAware) commandMarker).setCommandManager(this);
+ }
+ commandMarkers.add(commandMarker);
+ for (Method method : commandMarker.getClass().getMethods()) {
+ CliCommand cliCommand = method.getAnnotation(CliCommand.class);
+ CliAvailabilityIndicator availability =
method.getAnnotation(CliAvailabilityIndicator.class);
+ if (cliCommand == null && availability == null) {
+ continue;
+ }
+
+ if (cliCommand != null) {
+ helper.addCommand(cliCommand, method);
+ }
+
+ if (availability != null) {
+ helper.addAvailabilityIndicator(availability, new MethodTarget(method,
commandMarker));
+ }
}
}
+ public Helper getHelper() {
+ return helper;
+ }
+
public String obtainHelp(String buffer) {
int terminalWidth = -1;
Gfsh gfsh = Gfsh.getCurrentInstance();
@@ -114,15 +314,4 @@ public String obtainHint(String topic) {
return helper.getHint(topic);
}
- public Set<Converter> getConverters() {
- return converters;
- }
-
- public Set<CommandMarker> getCommandMarkers() {
- return commandMarkers;
- }
-
- public Helper getHelper() {
- return helper;
- }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
index 4d3b546f1e..4fc7d4e8b0 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
@@ -57,14 +57,10 @@ public GfshParser(CommandManager commandManager) {
add(command);
}
- for (Converter converter : commandManager.getConverters()) {
+ for (Converter<?> converter : commandManager.getConverters()) {
if (converter.getClass().isAssignableFrom(ArrayConverter.class)) {
ArrayConverter arrayConverter = (ArrayConverter) converter;
- HashSet<Converter<?>> converters = new HashSet<>();
- for (Converter c : commandManager.getConverters()) {
- converters.add(c);
- }
- arrayConverter.setConverters(converters);
+ arrayConverter.setConverters(new
HashSet<>(commandManager.getConverters()));
}
add(converter);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
index ec6f4e3ae1..917f3e4961 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
@@ -14,10 +14,9 @@
*/
package org.apache.geode.management.internal.cli.help;
-import static java.util.stream.Collectors.joining;
-
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -28,7 +27,6 @@
import java.util.TreeMap;
import org.apache.commons.lang.StringUtils;
-import org.springframework.shell.core.CommandMarker;
import org.springframework.shell.core.MethodTarget;
import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
import org.springframework.shell.core.annotation.CliCommand;
@@ -67,7 +65,8 @@
private final Map<String, Topic> topics = new HashMap<>();
private final Map<String, Method> commands = new TreeMap<String, Method>();
- private final Map<String, MethodTarget> availabilityIndicators = new
HashMap<>();
+ private final Map<String, MethodTarget> availabilityIndicators =
+ new HashMap<String, MethodTarget>();
public Helper() {
initTopic(CliStrings.DEFAULT_TOPIC_GEODE,
CliStrings.DEFAULT_TOPIC_GEODE__DESC);
@@ -95,25 +94,7 @@ private void initTopic(String topic, String desc) {
topics.put(topic, new Topic(topic, desc));
}
- public void registerCommand(CommandMarker commandMarker) {
- for (Method method : commandMarker.getClass().getMethods()) {
- CliCommand cliCommand = method.getAnnotation(CliCommand.class);
- CliAvailabilityIndicator availability =
method.getAnnotation(CliAvailabilityIndicator.class);
- if (cliCommand == null && availability == null) {
- continue;
- }
-
- if (cliCommand != null) {
- addCommand(cliCommand, method);
- }
-
- if (availability != null) {
- addAvailabilityIndicator(availability, new MethodTarget(method,
commandMarker));
- }
- }
- }
-
- protected void addCommand(CliCommand command, Method commandMethod) {
+ public void addCommand(CliCommand command, Method commandMethod) {
// put all the command synonyms in the command map
Arrays.stream(command.value()).forEach(cmd -> {
commands.put(cmd, commandMethod);
@@ -139,8 +120,7 @@ protected void addCommand(CliCommand command, Method
commandMethod) {
});
}
- private void addAvailabilityIndicator(CliAvailabilityIndicator availability,
- MethodTarget target) {
+ public void addAvailabilityIndicator(CliAvailabilityIndicator availability,
MethodTarget target) {
Arrays.stream(availability.value()).forEach(command -> {
availabilityIndicators.put(command, target);
});
@@ -172,7 +152,9 @@ public String getHint(String buffer) {
builder.append(CliStrings.HINT__MSG__TOPICS_AVAILABLE).append(GfshParser.LINE_SEPARATOR)
.append(GfshParser.LINE_SEPARATOR);
- topics.keySet().stream().sorted()
+ List<String> sortedTopics = new ArrayList<>(topics.keySet());
+ Collections.sort(sortedTopics);
+ sortedTopics.stream()
.forEachOrdered(topic ->
builder.append(topic).append(GfshParser.LINE_SEPARATOR));
return builder.toString();
}
@@ -183,7 +165,8 @@ public String getHint(String buffer) {
}
builder.append(topic.desc).append(GfshParser.LINE_SEPARATOR).append(GfshParser.LINE_SEPARATOR);
- topic.relatedCommands.stream().sorted().forEach(command ->
builder.append(command.command)
+ Collections.sort(topic.relatedCommands);
+ topic.relatedCommands.stream().forEachOrdered(command ->
builder.append(command.command)
.append(": ").append(command.desc).append(GfshParser.LINE_SEPARATOR));
return builder.toString();
}
@@ -192,7 +175,7 @@ public String getHint(String buffer) {
return topics.keySet();
}
- private boolean isAvailable(String command) {
+ boolean isAvailable(String command) {
MethodTarget target = availabilityIndicators.get(command);
if (target == null) {
return true;
@@ -278,12 +261,16 @@ HelpBlock getOptionDetail(CliOption cliOption) {
HelpBlock optionNode = new HelpBlock(getPrimaryKey(cliOption));
String help = cliOption.help();
optionNode.addChild(new HelpBlock((StringUtils.isNotBlank(help) ? help :
"")));
-
- String synonyms = getSynonyms(cliOption).stream().collect(joining(","));
- if (StringUtils.isNotEmpty(synonyms)) {
- optionNode.addChild(new HelpBlock(SYNONYMS_SUB_NAME + synonyms));
+ if (getSynonyms(cliOption).size() > 0) {
+ StringBuilder builder = new StringBuilder();
+ for (String string : getSynonyms(cliOption)) {
+ if (builder.length() > 0) {
+ builder.append(",");
+ }
+ builder.append(string);
+ }
+ optionNode.addChild(new HelpBlock(SYNONYMS_SUB_NAME +
builder.toString()));
}
-
optionNode.addChild(
new HelpBlock(REQUIRED_SUB_NAME + ((cliOption.mandatory()) ?
TRUE_TOKEN : FALSE_TOKEN)));
if (!isNullOrBlank(cliOption.specifiedDefaultValue())) {
@@ -372,14 +359,18 @@ private static String getPrimaryKey(CliOption option) {
}
private static List<String> getSynonyms(CliOption option) {
+ List<String> synonyms = new ArrayList<>();
String[] keys = option.key();
if (keys.length < 2)
- return Collections.emptyList();
+ return synonyms;
// if the primary key is empty (like sh and help command), then there
should be no synonyms.
if ("".equals(keys[0]))
- return Collections.emptyList();
+ return synonyms;
- return Arrays.asList(keys).subList(1, keys.length);
+ for (int i = 1; i < keys.length; i++) {
+ synonyms.add(keys[i]);
+ }
+ return synonyms;
}
private static boolean isNullOrBlank(String value) {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
index a4c6f59802..6130117d2e 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
@@ -35,7 +35,8 @@
public MemberCommandService(InternalCache cache) throws
CommandServiceException {
this.cache = cache;
try {
- this.onlineCommandProcessor = new
OnlineCommandProcessor(cache.getSecurityService());
+ this.onlineCommandProcessor = new OnlineCommandProcessor(
+ cache.getDistributedSystem().getProperties(),
cache.getSecurityService());
} catch (Exception e) {
throw new CommandServiceException("Could not load commands.", e);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
index 38ab364392..7fa4acba51 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
@@ -18,6 +18,7 @@
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.Map;
+import java.util.Properties;
import org.springframework.shell.core.Parser;
import org.springframework.shell.event.ParseResult;
@@ -25,6 +26,7 @@
import org.apache.geode.annotations.TestingOnly;
import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.internal.security.SecurityServiceFactory;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.CommandProcessingException;
import org.apache.geode.management.cli.Result;
@@ -47,14 +49,20 @@
private final SecurityService securityService;
- public OnlineCommandProcessor(SecurityService securityService)
+ @TestingOnly
+ public OnlineCommandProcessor() throws ClassNotFoundException, IOException {
+ this(new Properties(), SecurityServiceFactory.create());
+ }
+
+ public OnlineCommandProcessor(Properties cacheProperties, SecurityService
securityService)
throws ClassNotFoundException, IOException {
- this(securityService, new CommandExecutor());
+ this(cacheProperties, securityService, new CommandExecutor());
}
@TestingOnly
- public OnlineCommandProcessor(SecurityService securityService,
CommandExecutor commandExecutor) {
- this.gfshParser = new GfshParser(new CommandManager());
+ public OnlineCommandProcessor(Properties cacheProperties, SecurityService
securityService,
+ CommandExecutor commandExecutor) {
+ this.gfshParser = new GfshParser(new CommandManager(cacheProperties));
this.executor = commandExecutor;
this.securityService = securityService;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
index 11da4b208a..046d890567 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
@@ -21,7 +21,6 @@
import java.util.Set;
import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
-import
io.github.lukehutch.fastclasspathscanner.matchprocessor.ImplementingClassMatchProcessor;
/**
* Utility class to scan class-path & load classes.
@@ -29,26 +28,18 @@
* @since GemFire 7.0
*/
public class ClasspathScanLoadHelper {
-
- public static <T> Set<Class<? extends T>>
scanClasspathForClassesImplementing(
- Class<T> implementedInterface, String... packageSpec) {
- return scanPackageForClassesImplementing(implementedInterface,
packageSpec);
- }
-
- public static <T> Set<Class<? extends T>> scanPackageForClassesImplementing(
- Class<T> implementedInterface, String... packageSpec) {
- Set<Class<? extends T>> classesImplementing = new HashSet<>();
- ImplementingClassMatchProcessor<T> matchProcessor =
classesImplementing::add;
-
- new FastClasspathScanner(packageSpec)
- .matchClassesImplementing(implementedInterface, matchProcessor).scan();
+ public static Set<Class<?>> scanPackageForClassesImplementing(String
packageToScan,
+ Class<?> implementedInterface) {
+ Set<Class<?>> classesImplementing = new HashSet<>();
+ new FastClasspathScanner(packageToScan)
+ .matchClassesImplementing(implementedInterface,
classesImplementing::add).scan();
return
classesImplementing.stream().filter(ClasspathScanLoadHelper::isInstantiable)
.collect(toSet());
}
- private static <T> boolean isInstantiable(Class<T> classToInstantiate) {
- int modifiers = classToInstantiate.getModifiers();
+ private static boolean isInstantiable(Class<?> klass) {
+ int modifiers = klass.getModifiers();
return !Modifier.isAbstract(modifiers) && !Modifier.isInterface(modifiers)
&& Modifier.isPublic(modifiers);
diff --git
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
index e874898d82..7e5f83f4f2 100644
---
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
@@ -99,7 +99,7 @@ public void before() {
@Test
public void testGetAttributeNames() {
String[] attNames = AbstractDistributionConfig._getAttNames();
- assertEquals(156, attNames.length);
+ assertEquals(attNames.length, 157);
List boolList = new ArrayList();
List intList = new ArrayList();
@@ -135,7 +135,7 @@ public void testGetAttributeNames() {
// are.
assertEquals(29, boolList.size());
assertEquals(33, intList.size());
- assertEquals(85, stringList.size());
+ assertEquals(86, stringList.size());
assertEquals(5, fileList.size());
assertEquals(4, otherList.size());
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
index e48c8d24a7..1dfd61d9a1 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
@@ -14,8 +14,7 @@
*/
package org.apache.geode.management.internal.cli;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
import java.util.Set;
@@ -36,36 +35,36 @@
private final String PACKAGE_NAME =
"org.apache.geode.management.internal.cli.domain";
private final String WRONG_PACKAGE_NAME =
"org.apache.geode.management.internal.cli.domain1";
- private final Class INTERFACE1 = Interface1.class;
- private final Class NO_IMPL_INTERFACE = Versionable.class;
- private final Class INTERFACE2 = Interface2.class;
- private final Class IMPL1 = Impl1.class;
- private final Class IMPL2 = Impl12.class;
- private final Class ABSTRACT_IMPL = AbstractImpl.class;
+ private final Class<?> INTERFACE1 = Interface1.class;
+ private final Class<?> NO_IMPL_INTERFACE = Versionable.class;
+ private final Class<?> INTERFACE2 = Interface2.class;
+ private final Class<?> IMPL1 = Impl1.class;
+ private final Class<?> IMPL2 = Impl12.class;
+ private final Class<?> ABSTRACT_IMPL = AbstractImpl.class;
@Test
public void testLoadAndGet() throws Exception {
Set<Class<?>> classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE1,
PACKAGE_NAME);
+
ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME,
INTERFACE1);
assertEquals(2, classLoaded.size());
assertTrue(classLoaded.contains(IMPL1));
assertTrue(classLoaded.contains(IMPL2));
classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE2,
PACKAGE_NAME);
+
ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME,
INTERFACE2);
assertEquals(1, classLoaded.size());
assertTrue(classLoaded.contains(IMPL2));
classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE2,
WRONG_PACKAGE_NAME);
+
ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME,
INTERFACE2);
assertEquals(0, classLoaded.size());
classLoaded =
-
ClasspathScanLoadHelper.scanPackageForClassesImplementing(NO_IMPL_INTERFACE,
PACKAGE_NAME);
+
ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME,
NO_IMPL_INTERFACE);
assertEquals(0, classLoaded.size());
- classLoaded =
ClasspathScanLoadHelper.scanPackageForClassesImplementing(NO_IMPL_INTERFACE,
- WRONG_PACKAGE_NAME);
+ classLoaded =
ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME,
+ NO_IMPL_INTERFACE);
assertEquals(0, classLoaded.size());
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
index c65ff04936..813347128c 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
@@ -18,12 +18,20 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import java.util.List;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.springframework.shell.core.CommandMarker;
+import org.springframework.shell.core.Completion;
+import org.springframework.shell.core.Converter;
+import org.springframework.shell.core.MethodTarget;
+import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.security.ResourceOperation;
import org.apache.geode.security.ResourcePermission.Operation;
@@ -35,6 +43,49 @@
*/
@Category(UnitTest.class)
public class CommandManagerJUnitTest {
+
+ private static final String COMMAND1_NAME = "command1";
+ private static final String COMMAND1_NAME_ALIAS = "command1_alias";
+ private static final String COMMAND2_NAME = "c2";
+
+ private static final String COMMAND1_HELP = "help for " + COMMAND1_NAME;
+ // ARGUMENTS
+ private static final String ARGUMENT1_NAME = "argument1";
+ private static final String ARGUMENT1_HELP = "help for argument1";
+ private static final String ARGUMENT1_CONTEXT = "context for argument 1";
+ private static final Completion[] ARGUMENT1_COMPLETIONS =
+ {new Completion("arg1"), new Completion("arg1alt")};
+ private static final String ARGUMENT2_NAME = "argument2";
+ private static final String ARGUMENT2_CONTEXT = "context for argument 2";
+ private static final String ARGUMENT2_HELP = "help for argument2";
+ private static final String ARGUMENT2_UNSPECIFIED_DEFAULT_VALUE =
+ "{unspecified default value for argument2}";
+ private static final Completion[] ARGUMENT2_COMPLETIONS =
+ {new Completion("arg2"), new Completion("arg2alt")};
+
+ // OPTIONS
+ private static final String OPTION1_NAME = "option1";
+ private static final String OPTION1_SYNONYM = "opt1";
+ private static final String OPTION1_HELP = "help for option1";
+ private static final String OPTION1_CONTEXT = "context for option1";
+ private static final String OPTION1_SPECIFIED_DEFAULT_VALUE =
+ "{specified default value for option1}";
+ private static final Completion[] OPTION1_COMPLETIONS =
+ {new Completion("option1"), new Completion("option1Alternate")};
+ private static final String OPTION2_NAME = "option2";
+ private static final String OPTION2_HELP = "help for option2";
+ private static final String OPTION2_CONTEXT = "context for option2";
+ private static final String OPTION2_SPECIFIED_DEFAULT_VALUE =
+ "{specified default value for option2}";
+ private static final String OPTION3_NAME = "option3";
+ private static final String OPTION3_SYNONYM = "opt3";
+ private static final String OPTION3_HELP = "help for option3";
+ private static final String OPTION3_CONTEXT = "context for option3";
+ private static final String OPTION3_SPECIFIED_DEFAULT_VALUE =
+ "{specified default value for option3}";
+ private static final String OPTION3_UNSPECIFIED_DEFAULT_VALUE =
+ "{unspecified default value for option3}";
+
private CommandManager commandManager;
@Before
@@ -60,6 +111,11 @@ public void testCommandManagerInstance() throws Exception {
assertNotNull(commandManager);
}
+ /**
+ * Tests {@link CommandManager#loadPluginCommands()}.
+ *
+ * @since GemFire 8.1
+ */
@Test
public void testCommandManagerLoadPluginCommands() throws Exception {
assertNotNull(commandManager);
@@ -67,10 +123,97 @@ public void testCommandManagerLoadPluginCommands() throws
Exception {
// see META-INF/services/org.springframework.shell.core.CommandMarker
service loader file.
assertTrue("Should find listed plugin.",
commandManager.getHelper().getCommands().contains("mock plugin
command"));
- assertThat(
- commandManager.getCommandMarkers().stream().anyMatch(c -> c instanceof
MockPluginCommand));
+ assertTrue("Should not find unlisted plugin.",
+ !commandManager.getHelper().getCommands().contains("mock plugin
command unlisted"));
+ }
+
+ /**
+ * class that represents dummy commands
+ */
+ public static class Commands implements CommandMarker {
+
+ @CliCommand(value = {COMMAND1_NAME, COMMAND1_NAME_ALIAS}, help =
COMMAND1_HELP)
+ @CliMetaData(shellOnly = true, relatedTopic = {"relatedTopicOfCommand1"})
+ @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+ public static String command1(
+ @CliOption(key = ARGUMENT1_NAME, optionContext = ARGUMENT1_CONTEXT,
help = ARGUMENT1_HELP,
+ mandatory = true) String argument1,
+ @CliOption(key = ARGUMENT2_NAME, optionContext = ARGUMENT2_CONTEXT,
help = ARGUMENT2_HELP,
+ unspecifiedDefaultValue = ARGUMENT2_UNSPECIFIED_DEFAULT_VALUE)
String argument2,
+ @CliOption(key = {OPTION1_NAME, OPTION1_SYNONYM}, help = OPTION1_HELP,
mandatory = true,
+ optionContext = OPTION1_CONTEXT,
+ specifiedDefaultValue = OPTION1_SPECIFIED_DEFAULT_VALUE) String
option1,
+ @CliOption(key = {OPTION2_NAME}, help = OPTION2_HELP, optionContext =
OPTION2_CONTEXT,
+ specifiedDefaultValue = OPTION2_SPECIFIED_DEFAULT_VALUE) String
option2,
+ @CliOption(key = {OPTION3_NAME, OPTION3_SYNONYM}, help = OPTION3_HELP,
+ optionContext = OPTION3_CONTEXT,
+ unspecifiedDefaultValue = OPTION3_UNSPECIFIED_DEFAULT_VALUE,
+ specifiedDefaultValue = OPTION3_SPECIFIED_DEFAULT_VALUE) String
option3) {
+ return null;
+ }
+
+ @CliCommand(value = {COMMAND2_NAME})
+ @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+ public static String command2() {
+ return null;
+ }
+
+ @CliCommand(value = {"testParamConcat"})
+ @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+ public static Result testParamConcat(@CliOption(key = {"string"}) String
string,
+ @CliOption(key = {"stringArray"}) String[] stringArray,
+ @CliOption(key = {"integer"}) Integer integer,
+ @CliOption(key = {"colonArray"}) String[] colonArray) {
+ return null;
+ }
+
+ @CliCommand(value = {"testMultiWordArg"})
+ @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+ public static Result testMultiWordArg(@CliOption(key = "arg1") String arg1,
+ @CliOption(key = "arg2") String arg2) {
+ return null;
+ }
+
+ @CliAvailabilityIndicator({COMMAND1_NAME})
+ public boolean isAvailable() {
+ return true; // always available on server
+ }
}
+ /**
+ * Used by testCommandManagerLoadPluginCommands
+ */
+ private static class SimpleConverter implements Converter<String> {
+
+ @Override
+ public boolean supports(Class<?> type, String optionContext) {
+ return type.isAssignableFrom(String.class);
+ }
+
+ @Override
+ public String convertFromText(String value, Class<?> targetType, String
optionContext) {
+ return value;
+ }
+
+ @Override
+ public boolean getAllPossibleValues(List<Completion> completions, Class<?>
targetType,
+ String existingData, String context, MethodTarget target) {
+ if (context.equals(ARGUMENT1_CONTEXT)) {
+ for (Completion completion : ARGUMENT1_COMPLETIONS) {
+ completions.add(completion);
+ }
+ } else if (context.equals(ARGUMENT2_CONTEXT)) {
+ for (Completion completion : ARGUMENT2_COMPLETIONS) {
+ completions.add(completion);
+ }
+ } else if (context.equals(OPTION1_CONTEXT)) {
+ for (Completion completion : OPTION1_COMPLETIONS) {
+ completions.add(completion);
+ }
+ }
+ return true;
+ }
+ }
public static class MockPluginCommand implements CommandMarker {
@CliCommand(value = "mock plugin command")
@@ -79,4 +222,13 @@ public Result mockPluginCommand() {
return null;
}
}
+
+ public static class MockPluginCommandUnlisted implements CommandMarker {
+ @CliCommand(value = "mock plugin command unlisted")
+ @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+ public Result mockPluginCommandUnlisted() {
+ return null;
+ }
+ }
+
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
index d79993201b..5714829cfa 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
@@ -27,9 +27,9 @@
import java.io.PrintStream;
import java.net.InetAddress;
import java.net.UnknownHostException;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
-import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -80,7 +80,7 @@
public static boolean checkIfCommandsAreLoadedOrNot() {
CommandManager manager = new CommandManager();
- Set<CommandMarker> commands = manager.getCommandMarkers();
+ List<CommandMarker> commands = manager.getCommandMarkers();
return commands.size() >= 1;
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
index db7176ba4f..df00cf17aa 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
@@ -21,6 +21,8 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.Properties;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -34,6 +36,7 @@
@Category(UnitTest.class)
public class OnlineCommandProcessorTest {
+ Properties properties;
SecurityService securityService;
CommandExecutor executor;
OnlineCommandProcessor onlineCommandProcessor;
@@ -41,12 +44,13 @@
@Before
public void before() {
+ properties = new Properties();
securityService = mock(SecurityService.class);
executor = mock(CommandExecutor.class);
result = mock(Result.class);
when(executor.execute(any())).thenReturn(result);
- onlineCommandProcessor = new OnlineCommandProcessor(securityService,
executor);
+ onlineCommandProcessor = new OnlineCommandProcessor(properties,
securityService, executor);
}
diff --git
a/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker
b/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker
new file mode 100644
index 0000000000..1bb7c6d141
--- /dev/null
+++
b/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker
@@ -0,0 +1,8 @@
+# Mock command for
org.apache.geode.management.internal.cli.CommandManagerJUnitTest.testLoadPluginCommands
+org.apache.geode.management.internal.cli.CommandManagerJUnitTest$MockPluginCommand
+# Should cause ServiceConfigurationException while iterating.
+#TODO jbarrett - causes tests to be marked as failed because of exception in
log
org.apache.geode.management.internal.cli.CommandManagerJUnitTest$MockPluginCommandNotFound
+
+
+# Mock Extension commands
+org.apache.geode.internal.cache.extension.mock.MockExtensionCommands
\ No newline at end of file
diff --git
a/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
b/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
new file mode 100644
index 0000000000..0879e15732
--- /dev/null
+++
b/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
@@ -0,0 +1,2 @@
+# Lucene Extensions command
+org.apache.geode.cache.lucene.internal.cli.LuceneIndexCommands
\ No newline at end of file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Simplify support for custom GFSH commands
> -----------------------------------------
>
> Key: GEODE-3007
> URL: https://issues.apache.org/jira/browse/GEODE-3007
> Project: Geode
> Issue Type: Improvement
> Components: docs, gfsh
> Reporter: Jared Stewart
> Assignee: Jared Stewart
>
> Geode currently supports three ways to load GFSH commands:
> 1. Scan the classpath for commands in
> "org.apache.geode.management.internal.cli.commands”
> 2. Scan the classpath for commands in a package specified by a user via the
> “user-command-packages” system property.
> 3. Scan the classpath for commands registered in files inside
> META-INF.services (e.g.
> "geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker”)
>
> After the improvements made by GEODE-2989, there is no reason to require a
> user to specify the location of their custom commands via one of these
> mechanisms. Instead, we should simply scan the entire classpath for any
> classes implementing CommandMarker (regardless of whatever packages they live
> in).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
