[GEODE-10668] Consolidate CliMetaData handling in OnlineCommandProcessor

geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java

@@ -112,6 +112,13 @@ public ResultModel executeCommand(String command, Map<String, String> env,
     }
 
     Method method = parseResult.getMethod();
+    CliMetaData metaData = method.getAnnotation(CliMetaData.class);
+
+    // shell-only commands run in the gfsh client, not on a member
+    if (metaData != null && metaData.shellOnly()) {
+      return ResultModel.createError(parseResult.getCommandName()
+          + " can only be run from gfsh and is not available on a member.");
+    }
 
     // do general authorization check here
     ResourceOperation resourceOperation = method.getAnnotation(ResourceOperation.class);
@@ -121,7 +128,6 @@ public ResultModel executeCommand(String command, Map<String, String> env,
     }
 
     // this command processor does not execute commands that need fileData passed from client
-    CliMetaData metaData = method.getAnnotation(CliMetaData.class);
     if (metaData != null && metaData.isFileUploaded() && stagedFilePaths == null) {
       return ResultModel
           .createError(command + " can not be executed only from server side");

geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java

@@ -19,6 +19,8 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.Properties;
@@ -29,6 +31,7 @@
 import org.junit.rules.TemporaryFolder;
 
 import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.CommandManager;
 import org.apache.geode.management.internal.cli.GfshParser;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
@@ -74,14 +77,14 @@ public void executeStripsComments() {
 
   @Test
   public void executeReturnsExecutorResult() {
-    ResultModel commandResult = onlineCommandProcessor.executeCommand("start locator");
+    ResultModel commandResult = onlineCommandProcessor.executeCommand("list members");
     assertThat(commandResult).isSameAs(result);
   }
 
   @Test
   public void handlesNotAuthorizedException() {
     when(executor.execute(any())).thenThrow(new NotAuthorizedException("not authorized"));
-    assertThatThrownBy(() -> onlineCommandProcessor.executeCommand("start locator"))
+    assertThatThrownBy(() -> onlineCommandProcessor.executeCommand("list members"))
         .isInstanceOf(NotAuthorizedException.class);
   }
 
@@ -94,4 +97,16 @@ public void handlesParsingError() {
         .contains(
             "The command or some options in this command may not be supported by this locator");
   }
+
+  @Test
+  public void shellOnlyCommandReturnsError() {
+    ResultModel commandResult = onlineCommandProcessor.executeCommand("echo --string=hello");
+    assertThat(commandResult.getStatus()).isEqualTo(Result.Status.ERROR);
+  }
+
+  @Test
+  public void shellOnlyCommandIsNotPassedToExecutor() {
+    onlineCommandProcessor.executeCommand("start locator");
+    verify(executor, never()).execute(any());
+  }
 }