You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by pr...@apache.org on 2018/03/30 17:29:33 UTC

[geode] branch develop updated: GEODE-4811: Add @Disabled "feature flag" annotation for gfsh commands.

This is an automated email from the ASF dual-hosted git repository.

prhomberg 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 c42905c  GEODE-4811: Add @Disabled "feature flag" annotation for gfsh commands.
c42905c is described below

commit c42905c5ad9853e16be096f21d32e0c321c047ea
Author: Patrick Rhomberg <pr...@pivotal.io>
AuthorDate: Fri Mar 30 10:29:29 2018 -0700

    GEODE-4811: Add @Disabled "feature flag" annotation for gfsh commands.
---
 .../org/apache/geode/management/cli/Disabled.java  |  41 ++++++++
 .../management/internal/cli/CommandManager.java    |   8 ++
 .../internal/cli/CommandManagerJUnitTest.java      | 103 +++++++++++++--------
 3 files changed, 114 insertions(+), 38 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/cli/Disabled.java b/geode-core/src/main/java/org/apache/geode/management/cli/Disabled.java
new file mode 100644
index 0000000..6eb0d35
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/management/cli/Disabled.java
@@ -0,0 +1,41 @@
+/*
+ * 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.geode.management.cli;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * This annotation disables a {@link org.apache.geode.management.cli.GfshCommand}
+ * class from being loaded by the
+ * {@link org.apache.geode.management.internal.cli.CommandManager}
+ * unless the provided flag value exists in the VM's environment.
+ *
+ * Because the default value is an empty string, and because {@code System.getProperty("")}
+ * invalid, a class annotated with {@code @Disabled} cannot be reached other than explicit
+ * instantiation of the class.
+ */
+@Inherited
+@Documented
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface Disabled {
+  String unlessPropertyIsSet() default "";
+}
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 785c78b..690353a 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
@@ -37,6 +37,7 @@ import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.cli.Disabled;
 import org.apache.geode.management.cli.GfshCommand;
 import org.apache.geode.management.internal.cli.commands.InternalGfshCommand;
 import org.apache.geode.management.internal.cli.help.Helper;
@@ -50,6 +51,7 @@ import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
  * @since GemFire 7.0
  */
 public class CommandManager {
+
   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";
@@ -267,6 +269,12 @@ public class CommandManager {
    * Method to add new Commands to the parser
    */
   void add(CommandMarker commandMarker) {
+    Disabled classDisabled = commandMarker.getClass().getAnnotation(Disabled.class);
+    if (classDisabled != null && (classDisabled.unlessPropertyIsSet().isEmpty()
+        || System.getProperty(classDisabled.unlessPropertyIsSet()) == null)) {
+      return;
+    }
+
     // inject the cache into the commands
     if (GfshCommand.class.isAssignableFrom(commandMarker.getClass())) {
       ((GfshCommand) commandMarker).setCache(cache);
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 597c8ce..cae675b 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,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Properties;
 
@@ -35,7 +36,9 @@ 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.Disabled;
 import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission.Operation;
 import org.apache.geode.security.ResourcePermission.Resource;
@@ -100,7 +103,7 @@ public class CommandManagerJUnitTest {
    * tests loadCommands()
    */
   @Test
-  public void testCommandManagerLoadCommands() throws Exception {
+  public void testCommandManagerLoadCommands() {
     assertNotNull(commandManager);
     assertThat(commandManager.getCommandMarkers().size()).isGreaterThan(0);
     assertThat(commandManager.getConverters().size()).isGreaterThan(0);
@@ -110,7 +113,7 @@ public class CommandManagerJUnitTest {
    * tests commandManagerInstance method
    */
   @Test
-  public void testCommandManagerInstance() throws Exception {
+  public void testCommandManagerInstance() {
     assertNotNull(commandManager);
   }
 
@@ -120,7 +123,7 @@ public class CommandManagerJUnitTest {
    * @since GemFire 8.1
    */
   @Test
-  public void testCommandManagerLoadPluginCommands() throws Exception {
+  public void testCommandManagerLoadPluginCommands() {
     assertNotNull(commandManager);
 
     assertTrue("Should find listed plugin.",
@@ -139,6 +142,28 @@ public class CommandManagerJUnitTest {
         commandManager.getCommandMarkers().stream().anyMatch(c -> c instanceof UserGfshCommand));
   }
 
+  @Test
+  public void commandManagerDoesNotAddUnsatisfiedFeatureFlaggedCommands() {
+    System.setProperty("enabled.flag", "true");
+    try {
+      CommandMarker accessibleCommand = new AccessibleCommand();
+      CommandMarker enabledCommand = new FeatureFlaggedAndEnabledCommand();
+      CommandMarker reachableButDisabledCommand = new FeatureFlaggedReachableCommand();
+      CommandMarker unreachableCommand = new FeatureFlaggedUnreachableCommand();
+
+      commandManager.add(accessibleCommand);
+      commandManager.add(enabledCommand);
+      commandManager.add(reachableButDisabledCommand);
+      commandManager.add(unreachableCommand);
+
+      assertThat(commandManager.getCommandMarkers()).contains(accessibleCommand, enabledCommand);
+      assertThat(commandManager.getCommandMarkers()).doesNotContain(reachableButDisabledCommand,
+          unreachableCommand);
+    } finally {
+      System.clearProperty("enabled.flag");
+    }
+  }
+
   /**
    * class that represents dummy commands
    */
@@ -192,41 +217,6 @@ public class CommandManagerJUnitTest {
     }
   }
 
-  /**
-   * 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")
     @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
@@ -243,4 +233,41 @@ public class CommandManagerJUnitTest {
     }
   }
 
+
+  class AccessibleCommand implements CommandMarker {
+    @CliCommand(value = "test-command")
+    public Result ping() {
+      return ResultBuilder.createInfoResult("pong");
+    }
+
+    @CliAvailabilityIndicator("test-command")
+    public boolean always() {
+      return true;
+    }
+  }
+
+  @Disabled
+  class FeatureFlaggedUnreachableCommand implements CommandMarker {
+    @CliCommand(value = "unreachable")
+    public Result nothing() {
+      throw new RuntimeException("You reached the body of a feature-flagged command.");
+    }
+  }
+
+  @Disabled(unlessPropertyIsSet = "reachable.flag")
+  class FeatureFlaggedReachableCommand implements CommandMarker {
+    @CliCommand(value = "reachable")
+    public Result nothing() {
+      throw new RuntimeException("You reached the body of a feature-flagged command.");
+    }
+  }
+
+  @Disabled(unlessPropertyIsSet = "enabled.flag")
+  class FeatureFlaggedAndEnabledCommand implements CommandMarker {
+    @CliCommand(value = "reachable")
+    public Result nothing() {
+      throw new RuntimeException("You reached the body of a feature-flagged command.");
+    }
+  }
+
 }

-- 
To stop receiving notification emails like this one, please contact
prhomberg@apache.org.