You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2018/03/27 16:32:18 UTC
[geode] branch develop updated: GEODE-4945: InternalGfshCommand should not have to implement CommandM… (#1684)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 7b9ac98 GEODE-4945: InternalGfshCommand should not have to implement CommandM… (#1684)
7b9ac98 is described below
commit 7b9ac98824322bc14e29258643dce6ac2ddf743e
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Tue Mar 27 09:32:15 2018 -0700
GEODE-4945: InternalGfshCommand should not have to implement CommandM… (#1684)
* Correct class scanning for user gfsh commands
---
.../management/internal/cli/CommandManager.java | 41 ++++++++++------------
.../internal/cli/commands/InternalGfshCommand.java | 4 +--
.../internal/cli/util/ClasspathScanLoadHelper.java | 6 ++--
.../test/java/com/examples/UserGfshCommand.java | 25 +++++++++++++
.../extension/mock/MockExtensionCommands.java | 3 +-
.../cli/ClasspathScanLoadHelperJUnitTest.java | 15 ++++----
.../internal/cli/CommandManagerJUnitTest.java | 13 +++++++
7 files changed, 70 insertions(+), 37 deletions(-)
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 387ff03..785c78b 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
@@ -18,12 +18,12 @@ import static org.apache.geode.distributed.ConfigurationProperties.USER_COMMAND_
import java.lang.reflect.Method;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Properties;
import java.util.ServiceLoader;
import java.util.Set;
-import java.util.StringTokenizer;
import org.springframework.shell.converters.EnumConverter;
import org.springframework.shell.converters.SimpleFileConverter;
@@ -95,22 +95,15 @@ public class CommandManager {
private void loadUserCommands() {
final Set<String> userCommandPackages = new HashSet<String>();
+ List<String> userCommandSources = new ArrayList<>();
// 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());
- }
+ userCommandSources.add(System.getProperty(USER_CMD_PACKAGES_PROPERTY));
}
// 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());
- }
+ userCommandSources.add(System.getenv().get(USER_CMD_PACKAGES_ENV_VARIABLE));
}
// Find by packages specified in the distribution config
@@ -118,18 +111,19 @@ public class CommandManager {
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());
- }
+ userCommandSources.add(cacheUserCmdPackages);
}
}
+ for (String source : userCommandSources) {
+ Arrays.stream(source.split(",")).forEach(userCommandPackages::add);
+ }
+
// Load commands found in all of the packages
for (String userCommandPackage : userCommandPackages) {
try {
- Set<Class<?>> foundClasses = ClasspathScanLoadHelper
- .scanPackageForClassesImplementing(userCommandPackage, CommandMarker.class);
+ Set<Class<?>> foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(
+ CommandMarker.class, userCommandPackage, GfshCommand.class.getPackage().getName());
for (Class<?> klass : foundClasses) {
try {
add((CommandMarker) klass.newInstance());
@@ -181,8 +175,8 @@ public class CommandManager {
Set<Class<?>> foundClasses;
// Converters
try {
- foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
- "org.apache.geode.management.internal.cli.converters", Converter.class);
+ foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(Converter.class,
+ "org.apache.geode.management.internal.cli.converters");
for (Class<?> klass : foundClasses) {
try {
Converter<?> object = (Converter<?>) klass.newInstance();
@@ -196,8 +190,8 @@ public class CommandManager {
raiseExceptionIfEmpty(foundClasses, "Converters");
// Spring shell's converters
- foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
- "org.springframework.shell.converters", Converter.class);
+ foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(Converter.class,
+ "org.springframework.shell.converters");
for (Class<?> klass : foundClasses) {
if (!SHL_CONVERTERS_TOSKIP.contains(klass)) {
try {
@@ -220,8 +214,9 @@ public class CommandManager {
Set<Class<?>> foundClasses;
try {
// geode's commands
- foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
- InternalGfshCommand.class.getPackage().getName(), CommandMarker.class);
+ foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(CommandMarker.class,
+ GfshCommand.class.getPackage().getName(),
+ InternalGfshCommand.class.getPackage().getName());
for (Class<?> klass : foundClasses) {
try {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/InternalGfshCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/InternalGfshCommand.java
index e32c730..e32df99 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/InternalGfshCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/InternalGfshCommand.java
@@ -17,8 +17,6 @@ package org.apache.geode.management.internal.cli.commands;
import java.util.List;
import java.util.Objects;
-import org.springframework.shell.core.CommandMarker;
-
import org.apache.geode.management.ManagementService;
import org.apache.geode.management.cli.GfshCommand;
import org.apache.geode.management.cli.Result;
@@ -34,7 +32,7 @@ import org.apache.geode.management.internal.configuration.domain.XmlEntity;
* GfshCommandJUnitTest
*/
@SuppressWarnings("unused")
-public abstract class InternalGfshCommand extends GfshCommand implements CommandMarker {
+public abstract class InternalGfshCommand extends GfshCommand {
public static final String EXPERIMENTAL = "(Experimental) ";
public void persistClusterConfiguration(Result result, Runnable runnable) {
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 046d890..35b0422 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
@@ -28,10 +28,10 @@ import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
* @since GemFire 7.0
*/
public class ClasspathScanLoadHelper {
- public static Set<Class<?>> scanPackageForClassesImplementing(String packageToScan,
- Class<?> implementedInterface) {
+ public static Set<Class<?>> scanPackagesForClassesImplementing(Class<?> implementedInterface,
+ String... packagesToScan) {
Set<Class<?>> classesImplementing = new HashSet<>();
- new FastClasspathScanner(packageToScan)
+ new FastClasspathScanner(packagesToScan)
.matchClassesImplementing(implementedInterface, classesImplementing::add).scan();
return classesImplementing.stream().filter(ClasspathScanLoadHelper::isInstantiable)
diff --git a/geode-core/src/test/java/com/examples/UserGfshCommand.java b/geode-core/src/test/java/com/examples/UserGfshCommand.java
new file mode 100644
index 0000000..86e770e
--- /dev/null
+++ b/geode-core/src/test/java/com/examples/UserGfshCommand.java
@@ -0,0 +1,25 @@
+/*
+ * 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 com.examples;
+
+import org.apache.geode.management.cli.GfshCommand;
+
+/**
+ * Used for testing in CommandManagerJUnitTest. Ensure that this class exists in a package
+ * outside of org.apache.geode so we know package scanning works correctly.
+ */
+public class UserGfshCommand extends GfshCommand {
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java b/geode-core/src/test/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
index cfd1e20..2c8485d 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
@@ -31,6 +31,7 @@ import org.apache.geode.distributed.internal.InternalClusterConfigurationService
import org.apache.geode.distributed.internal.InternalLocator;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.cli.GfshCommand;
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.cli.Result.Status;
import org.apache.geode.management.internal.cli.CliUtil;
@@ -47,7 +48,7 @@ import org.apache.geode.security.ResourcePermission.Resource;
*
* @since GemFire 8.1
*/
-public class MockExtensionCommands implements CommandMarker {
+public class MockExtensionCommands extends GfshCommand {
public static final String OPTION_VALUE = "value";
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 1dfd61d..21fbc0d 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,7 +14,8 @@
*/
package org.apache.geode.management.internal.cli;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import java.util.Set;
@@ -45,26 +46,26 @@ public class ClasspathScanLoadHelperJUnitTest {
@Test
public void testLoadAndGet() throws Exception {
Set<Class<?>> classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE1);
+ ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE1, PACKAGE_NAME);
assertEquals(2, classLoaded.size());
assertTrue(classLoaded.contains(IMPL1));
assertTrue(classLoaded.contains(IMPL2));
classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE2);
+ ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE2, PACKAGE_NAME);
assertEquals(1, classLoaded.size());
assertTrue(classLoaded.contains(IMPL2));
classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME, INTERFACE2);
+ ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE2, WRONG_PACKAGE_NAME);
assertEquals(0, classLoaded.size());
classLoaded =
- ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, NO_IMPL_INTERFACE);
+ ClasspathScanLoadHelper.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE, PACKAGE_NAME);
assertEquals(0, classLoaded.size());
- classLoaded = ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME,
- NO_IMPL_INTERFACE);
+ classLoaded = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE,
+ WRONG_PACKAGE_NAME);
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 ff2be92..597c8ce 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
@@ -19,7 +19,9 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.util.List;
+import java.util.Properties;
+import com.examples.UserGfshCommand;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -31,6 +33,7 @@ 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.distributed.ConfigurationProperties;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.security.ResourceOperation;
@@ -126,6 +129,16 @@ public class CommandManagerJUnitTest {
!commandManager.getHelper().getCommands().contains("mock plugin command unlisted"));
}
+ @Test
+ public void testCommandManagerLoadsUserCommand() throws Exception {
+ Properties props = new Properties();
+ props.setProperty(ConfigurationProperties.USER_COMMAND_PACKAGES, "com.examples");
+ CommandManager commandManager = new CommandManager(props, null);
+
+ assertThat(
+ commandManager.getCommandMarkers().stream().anyMatch(c -> c instanceof UserGfshCommand));
+ }
+
/**
* class that represents dummy commands
*/
--
To stop receiving notification emails like this one, please contact
jensdeppe@apache.org.