geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-2565) gfsh destroy gateway-sender should be idempotent
Date Tue, 14 Nov 2017 16:18:00 GMT

    [ https://issues.apache.org/jira/browse/GEODE-2565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16251634#comment-16251634
] 

ASF GitHub Bot commented on GEODE-2565:
---------------------------------------

jinmeiliao closed pull request #1048: GEODE-2565: add if-exists option to DestoryGatewaySenderCommand
URL: https://github.com/apache/geode/pull/1048
 
 
   

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-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommand.java
index 97b4a95d1a..83dc1a9405 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommand.java
@@ -18,16 +18,16 @@
 import java.util.List;
 import java.util.Set;
 
+import org.apache.logging.log4j.Logger;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CliUtil;
-import org.apache.geode.management.internal.cli.LogWrapper;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.functions.GatewaySenderDestroyFunction;
 import org.apache.geode.management.internal.cli.functions.GatewaySenderDestroyFunctionArgs;
@@ -38,6 +38,8 @@
 import org.apache.geode.security.ResourcePermission;
 
 public class DestroyGatewaySenderCommand implements GfshCommand {
+  private static final Logger logger = LogService.getLogger();
+
   @CliCommand(value = CliStrings.DESTROY_GATEWAYSENDER,
       help = CliStrings.DESTROY_GATEWAYSENDER__HELP)
   @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_WAN)
@@ -52,39 +54,34 @@ public Result destroyGatewaySender(
           help = CliStrings.DESTROY_GATEWAYSENDER__MEMBER__HELP) String[] onMember,
       @CliOption(key = CliStrings.DESTROY_GATEWAYSENDER__ID, mandatory = true,
           optionContext = ConverterHint.GATEWAY_SENDER_ID,
-          help = CliStrings.DESTROY_GATEWAYSENDER__ID__HELP) String id) {
-    Result result;
-    try {
-      GatewaySenderDestroyFunctionArgs gatewaySenderDestroyFunctionArgs =
-          new GatewaySenderDestroyFunctionArgs(id);
+          help = CliStrings.DESTROY_GATEWAYSENDER__ID__HELP) String id,
+      @CliOption(key = CliStrings.IFEXISTS, help = CliStrings.IFEXISTS_HELP,
+          specifiedDefaultValue = "true", unspecifiedDefaultValue = "false") boolean ifExist)
{
 
-      Set<DistributedMember> membersToDestroyGatewaySenderOn =
-          CliUtil.findMembers(onGroups, onMember);
+    GatewaySenderDestroyFunctionArgs gatewaySenderDestroyFunctionArgs =
+        new GatewaySenderDestroyFunctionArgs(id, ifExist);
 
-      if (membersToDestroyGatewaySenderOn.isEmpty()) {
-        return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
-      }
+    Set<DistributedMember> members = getMembers(onGroups, onMember);
+
+    ResultCollector<?, ?> resultCollector = executeFunction(GatewaySenderDestroyFunction.INSTANCE,
+        gatewaySenderDestroyFunctionArgs, members);
 
-      ResultCollector<?, ?> resultCollector =
-          CliUtil.executeFunction(GatewaySenderDestroyFunction.INSTANCE,
-              gatewaySenderDestroyFunctionArgs, membersToDestroyGatewaySenderOn);
-      @SuppressWarnings("unchecked")
-      List<CliFunctionResult> gatewaySenderDestroyResults =
-          (List<CliFunctionResult>) resultCollector.getResult();
+    List<CliFunctionResult> functionResults = (List<CliFunctionResult>) resultCollector.getResult();
 
-      TabularResultData tabularResultData = ResultBuilder.createTabularResultData();
-      final String errorPrefix = "ERROR: ";
-      for (CliFunctionResult gatewaySenderDestroyResult : gatewaySenderDestroyResults) {
-        boolean success = gatewaySenderDestroyResult.isSuccessful();
-        tabularResultData.accumulate("Member", gatewaySenderDestroyResult.getMemberIdOrName());
-        tabularResultData.accumulate("Status",
-            (success ? "" : errorPrefix) + gatewaySenderDestroyResult.getMessage());
+    TabularResultData tabularResultData = ResultBuilder.createTabularResultData();
+    boolean errorOccurred = false;
+    for (CliFunctionResult functionResult : functionResults) {
+      tabularResultData.accumulate("Member", functionResult.getMemberIdOrName());
+      if (functionResult.isSuccessful()) {
+        tabularResultData.accumulate("Status", functionResult.getMessage());
+      } else {
+        // if result has exception, it will be logged by the server before throwing it.
+        // so we don't need to log it here anymore.
+        tabularResultData.accumulate("Status", "ERROR: " + functionResult.getErrorMessage());
+        errorOccurred = true;
       }
-      result = ResultBuilder.buildResult(tabularResultData);
-    } catch (IllegalArgumentException e) {
-      LogWrapper.getInstance().info(e.getMessage());
-      result = ResultBuilder.createUserErrorResult(e.getMessage());
     }
-    return result;
+    tabularResultData.setStatus(errorOccurred ? Result.Status.ERROR : Result.Status.OK);
+    return ResultBuilder.buildResult(tabularResultData);
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
index 904001f34e..134a15300c 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
@@ -29,6 +29,7 @@
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.management.ManagementService;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.exceptions.EntityNotFoundException;
@@ -161,6 +162,10 @@ default Execution getMembersFunctionExecutor(final Set<DistributedMember>
member
     return matchingMembers;
   }
 
+  default ManagementService getManagementService() {
+    return ManagementService.getExistingManagementService(getCache());
+  }
+
   default Set<DistributedMember> findMembersForRegion(InternalCache cache, String regionPath)
{
     return CliUtil.getRegionAssociatedMembers(regionPath, cache, true);
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
index 57589ef6e3..4bbf389811 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CliFunctionResult.java
@@ -117,6 +117,20 @@ public String getMessage() {
     return (String) this.serializables[0];
   }
 
+  public String getErrorMessage() {
+    // if message is not null, use that
+    if (getMessage() != null) {
+      return getMessage();
+    }
+
+    // otherwise use exception's message
+    if (throwable != null) {
+      return throwable.getMessage();
+    }
+
+    return null;
+  }
+
   public Serializable[] getSerializables() {
     return this.serializables;
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction.java
index fc3ec6ba23..f99592eaf4 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction.java
@@ -14,23 +14,16 @@
  */
 package org.apache.geode.management.internal.cli.functions;
 
-import org.apache.logging.log4j.Logger;
-
 import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.execute.FunctionAdapter;
+import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
 import org.apache.geode.cache.wan.GatewaySender;
 import org.apache.geode.internal.InternalEntity;
-import org.apache.geode.internal.cache.wan.GatewaySenderException;
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.internal.cli.CliUtil;
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
 
-public class GatewaySenderDestroyFunction extends FunctionAdapter implements InternalEntity
{
+public class GatewaySenderDestroyFunction implements Function, InternalEntity {
   private static final long serialVersionUID = 1L;
-
-  private static final Logger logger = LogService.getLogger();
   private static final String ID = GatewaySenderDestroyFunction.class.getName();
   public static GatewaySenderDestroyFunction INSTANCE = new GatewaySenderDestroyFunction();
 
@@ -45,41 +38,28 @@ public void execute(FunctionContext context) {
     GatewaySenderDestroyFunctionArgs gatewaySenderDestroyFunctionArgs =
         (GatewaySenderDestroyFunctionArgs) context.getArguments();
 
-    try {
-      GatewaySender gatewaySender =
-          cache.getGatewaySender(gatewaySenderDestroyFunctionArgs.getId());
-      if (gatewaySender != null) {
-        gatewaySender.stop();
-        gatewaySender.destroy();
+    String senderId = gatewaySenderDestroyFunctionArgs.getId();
+    boolean ifExists = gatewaySenderDestroyFunctionArgs.isIfExists();
+    GatewaySender gatewaySender = cache.getGatewaySender(senderId);
+    if (gatewaySender == null) {
+      String message = "Gateway sender " + senderId + " not found.";
+      if (ifExists) {
+        resultSender
+            .lastResult(new CliFunctionResult(memberNameOrId, true, "Skipping: " + message));
       } else {
-        throw new GatewaySenderException(
-            "GateWaySender with Id  " + gatewaySenderDestroyFunctionArgs.getId() + " not
found");
-      }
-      resultSender.lastResult(new CliFunctionResult(memberNameOrId, true,
-          CliStrings.format(CliStrings.DESTROY_GATEWAYSENDER__MSG__GATEWAYSENDER_0_DESTROYED_ON_1,
-              new Object[] {gatewaySenderDestroyFunctionArgs.getId(), memberNameOrId})));
-
-    } catch (GatewaySenderException gse) {
-      resultSender.lastResult(handleException(memberNameOrId, gse.getMessage(), gse));
-    } catch (Exception e) {
-      String exceptionMsg = e.getMessage();
-      if (exceptionMsg == null) {
-        exceptionMsg = CliUtil.stackTraceAsString(e);
+        resultSender.lastResult(new CliFunctionResult(memberNameOrId, false, message));
       }
-      resultSender.lastResult(handleException(memberNameOrId, exceptionMsg, e));
+      return;
     }
-  }
 
-  private CliFunctionResult handleException(final String memberNameOrId, final String exceptionMsg,
-      final Exception e) {
-    if (e != null && logger.isDebugEnabled()) {
-      logger.debug(e.getMessage(), e);
-    }
-    if (exceptionMsg != null) {
-      return new CliFunctionResult(memberNameOrId, false, exceptionMsg);
+    try {
+      gatewaySender.stop();
+      gatewaySender.destroy();
+      resultSender.lastResult(new CliFunctionResult(memberNameOrId, true,
+          String.format("GatewaySender \"%s\" destroyed on \"%s\"", senderId, memberNameOrId)));
+    } catch (Exception e) {
+      resultSender.lastResult(new CliFunctionResult(memberNameOrId, e, ""));
     }
-
-    return new CliFunctionResult(memberNameOrId);
   }
 
   @Override
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs.java
index 361cf63429..648e8bf59c 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs.java
@@ -20,13 +20,19 @@
 
   private static final long serialVersionUID = 3848480256348119530L;
   private String id;
+  private boolean ifExists;
 
 
-  public GatewaySenderDestroyFunctionArgs(String id) {
+  public GatewaySenderDestroyFunctionArgs(String id, boolean ifExists) {
     this.id = id;
+    this.ifExists = ifExists;
   }
 
   public String getId() {
     return id;
   }
+
+  public boolean isIfExists() {
+    return ifExists;
+  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommandTest.java
new file mode 100644
index 0000000000..78726ee49f
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyGatewaySenderCommandTest.java
@@ -0,0 +1,108 @@
+/*
+ * 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.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+
+@Category(UnitTest.class)
+public class DestroyGatewaySenderCommandTest {
+
+  @ClassRule
+  public static GfshParserRule parser = new GfshParserRule();
+
+  private DestroyGatewaySenderCommand command;
+  private ResultCollector collector;
+  private InternalCache cache;
+  private List<CliFunctionResult> functionResults;
+  private CliFunctionResult result1, result2;
+  private CommandResult result;
+
+  @Before
+  public void before() throws Exception {
+    command = spy(DestroyGatewaySenderCommand.class);
+    cache = mock(InternalCache.class);
+    doReturn(cache).when(command).getCache();
+    collector = mock(ResultCollector.class);
+    doReturn(collector).when(command).executeFunction(any(), any(), any(Set.class));
+    functionResults = new ArrayList<>();
+    doReturn(functionResults).when(collector).getResult();
+  }
+
+  @Test
+  public void mandatoryOptions() throws Exception {
+    assertThat(parser.parse("destroy gateway-sender --member=A")).isNull();
+  }
+
+  @Test
+  public void allFunctionReturnsOK() throws Exception {
+    result1 = new CliFunctionResult("member", true, "result1");
+    result2 = new CliFunctionResult("member", true, "result2");
+    functionResults.add(result1);
+    functionResults.add(result2);
+
+    doReturn(mock(Set.class)).when(command).getMembers(any(), any());
+    parser.executeAndAssertThat(command, "destroy gateway-sender --id=1").statusIsSuccess()
+        .tableHasColumnWithValuesContaining("Status", "result1", "result2");
+
+  }
+
+  @Test
+  public void oneFunctionReturnsError() throws Exception {
+    result1 = new CliFunctionResult("member", true, "result1");
+    result2 = new CliFunctionResult("member", false, "result2");
+    functionResults.add(result1);
+    functionResults.add(result2);
+
+    doReturn(mock(Set.class)).when(command).getMembers(any(), any());
+    parser.executeAndAssertThat(command, "destroy gateway-sender --id=1").statusIsError()
+        .tableHasColumnWithValuesContaining("Status", "result1", "ERROR: result2");
+
+  }
+
+  @Test
+  public void oneFunctionThrowsGeneralException() throws Exception {
+    result1 = new CliFunctionResult("member", true, "result1");
+    result2 = new CliFunctionResult("member", new Exception("something happened"), null);
+    functionResults.add(result1);
+    functionResults.add(result2);
+
+    doReturn(mock(Set.class)).when(command).getMembers(any(), any());
+    parser.executeAndAssertThat(command, "destroy gateway-sender --id=1").statusIsError()
+        .tableHasColumnWithValuesContaining("Status", "result1", "ERROR: something happened");
+
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CliFunctionResultTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CliFunctionResultTest.java
new file mode 100644
index 0000000000..2f0946b2b3
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CliFunctionResultTest.java
@@ -0,0 +1,42 @@
+/*
+ * 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.internal.cli.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+
+
+@Category(UnitTest.class)
+public class CliFunctionResultTest {
+
+  private CliFunctionResult result;
+
+  @Test
+  public void getErrorMessage() throws Exception {
+    result = new CliFunctionResult("memberName", false, "message");
+    assertThat(result.getErrorMessage()).isEqualTo("message");
+
+    result = new CliFunctionResult("memberName", new Exception("exception message"), "message");
+    assertThat(result.getErrorMessage()).isEqualTo("message");
+
+    result = new CliFunctionResult("memberName", new Exception("exception message"), null);
+    assertThat(result.getErrorMessage()).isEqualTo("exception message");
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionTest.java
new file mode 100644
index 0000000000..28f331157c
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionTest.java
@@ -0,0 +1,87 @@
+/*
+ * 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.internal.cli.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.distributed.DistributedSystem;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+
+@Category(UnitTest.class)
+public class GatewaySenderDestroyFunctionTest {
+
+  private GatewaySenderDestroyFunction function;
+  private FunctionContext context;
+  private InternalCache cache;
+  private ResultSender resultSender;
+  private ArgumentCaptor<CliFunctionResult> resultCaptor;
+  private GatewaySenderDestroyFunctionArgs args;
+
+  @Before
+  public void before() {
+    function = spy(GatewaySenderDestroyFunction.class);
+    context = mock(FunctionContext.class);
+    cache = mock(InternalCache.class);
+    args = mock(GatewaySenderDestroyFunctionArgs.class);
+    resultSender = mock(ResultSender.class);
+    when(context.getCache()).thenReturn(cache);
+    when(context.getResultSender()).thenReturn(resultSender);
+    when(context.getArguments()).thenReturn(args);
+    when(args.getId()).thenReturn("id");
+    resultCaptor = ArgumentCaptor.forClass(CliFunctionResult.class);
+    when(cache.getDistributedSystem()).thenReturn(mock(DistributedSystem.class));
+  }
+
+  @Test
+  public void gateWaySenderNotFound_ifExists_false() throws Exception {
+    when(cache.getGatewaySender(any())).thenReturn(null);
+    when(args.isIfExists()).thenReturn(false);
+    function.execute(context);
+
+    verify(resultSender).lastResult(resultCaptor.capture());
+    CliFunctionResult result = resultCaptor.getValue();
+    assertThat(result.isSuccessful()).isFalse();
+    assertThat(result.getThrowable()).isNull();
+    assertThat(result.getMessage()).isEqualTo("Gateway sender id not found.");
+  }
+
+  @Test
+  public void gateWaySenderNotFound_ifExists_true() throws Exception {
+    when(cache.getGatewaySender(any())).thenReturn(null);
+    when(args.isIfExists()).thenReturn(true);
+    function.execute(context);
+
+    verify(resultSender).lastResult(resultCaptor.capture());
+    CliFunctionResult result = resultCaptor.getValue();
+    assertThat(result.isSuccessful()).isTrue();
+    assertThat(result.getThrowable()).isNull();
+    assertThat(result.getMessage()).isEqualTo("Skipping: Gateway sender id not found.");
+  }
+}
diff --git a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
index f6b4302cdb..aa4669e466 100755
--- a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
+++ b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
@@ -519,7 +519,7 @@ org/apache/geode/management/internal/cli/functions/GatewayReceiverCreateFunction
 org/apache/geode/management/internal/cli/functions/GatewayReceiverFunctionArgs,true,-5158224572470173267,bindAddress:java/lang/String,endPort:java/lang/Integer,gatewayTransportFilters:java/lang/String[],hostnameForSenders:java/lang/String,manualStart:java/lang/Boolean,maximumTimeBetweenPings:java/lang/Integer,socketBufferSize:java/lang/Integer,startPort:java/lang/Integer
 org/apache/geode/management/internal/cli/functions/GatewaySenderCreateFunction,true,8746830191680509335
 org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction,true,1
-org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs,true,3848480256348119530,id:java/lang/String
+org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunctionArgs,true,3848480256348119530,id:java/lang/String,ifExists:boolean
 org/apache/geode/management/internal/cli/functions/GatewaySenderFunctionArgs,true,-5158224572470173267,alertThreshold:java/lang/Integer,batchSize:java/lang/Integer,batchTimeInterval:java/lang/Integer,diskStoreName:java/lang/String,diskSynchronous:java/lang/Boolean,dispatcherThreads:java/lang/Integer,enableBatchConflation:java/lang/Boolean,enablePersistence:java/lang/Boolean,gatewayEventFilters:java/lang/String[],gatewayTransportFilters:java/lang/String[],id:java/lang/String,manualStart:java/lang/Boolean,maxQueueMemory:java/lang/Integer,orderPolicy:java/lang/String,parallel:java/lang/Boolean,remoteDSId:java/lang/Integer,socketBufferSize:java/lang/Integer,socketReadTimeout:java/lang/Integer
 org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction,true,1
 org/apache/geode/management/internal/cli/functions/GetMemberInformationFunction,true,1
diff --git a/geode-wan/src/test/java/org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java
b/geode-wan/src/test/java/org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java
index 5166bc7ddc..7c3b98b167 100644
--- a/geode-wan/src/test/java/org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java
+++ b/geode-wan/src/test/java/org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java
@@ -16,13 +16,7 @@
 package org.apache.geode.internal.cache.wan.wancommand;
 
 import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
-import static org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
-import static org.apache.geode.test.dunit.LogWriterUtils.getLogWriter;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
-import java.util.List;
 import java.util.Properties;
 
 import org.junit.Before;
@@ -30,11 +24,6 @@
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.result.CommandResult;
-import org.apache.geode.management.internal.cli.result.TabularResultData;
-import org.apache.geode.test.dunit.IgnoredException;
 import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.categories.DistributedTest;
@@ -49,61 +38,23 @@
   @Rule
   public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
 
-  private MemberVM locatorSite1;
-  private MemberVM locatorSite2;
-  private MemberVM server1;
-  private MemberVM server2;
-  private MemberVM server3;
+  private MemberVM locator;
 
   @Before
   public void before() throws Exception {
     Properties props = new Properties();
     props.setProperty(DISTRIBUTED_SYSTEM_ID, "" + 1);
-    locatorSite1 = locatorServerStartupRule.startLocatorVM(1, props);
-
-    props.setProperty(DISTRIBUTED_SYSTEM_ID, "" + 2);
-    props.setProperty(REMOTE_LOCATORS, "localhost[" + locatorSite1.getPort() + "]");
-    locatorSite2 = locatorServerStartupRule.startLocatorVM(2, props);
-
-    // Connect Gfsh to locator.
-    gfsh.connectAndVerify(locatorSite1);
+    locator = locatorServerStartupRule.startLocatorVM(0, props);
+    gfsh.connectAndVerify(locator);
   }
 
   @Test
   public void testDestroyGatewaySender_NotCreatedSender() throws Exception {
-    Integer locator1Port = locatorSite1.getPort();
-
-    // setup servers in Site #1
-    server1 = locatorServerStartupRule.startServerVM(3, locator1Port);
-    server2 = locatorServerStartupRule.startServerVM(4, locator1Port);
-    server3 = locatorServerStartupRule.startServerVM(5, locator1Port);
-
-    // Test Destroy Command
-    String command =
-        CliStrings.DESTROY_GATEWAYSENDER + " --" + CliStrings.DESTROY_GATEWAYSENDER__ID +
"=ln";
-    CommandResult cmdResult = executeCommandWithIgnoredExceptions(command);
-    if (cmdResult != null) {
-      String strCmdResult = cmdResult.toString();
-      getLogWriter().info(
-          "testDestroyGatewaySender_NotCreatedSender stringResult : " + strCmdResult + ">>>>");
-      assertEquals(Result.Status.OK, cmdResult.getStatus());
-      TabularResultData resultData = (TabularResultData) cmdResult.getResultData();
-      List<String> status = resultData.retrieveAllValues("Status");
-      assertEquals(3, status.size());
-      for (String stat : status) {
-        assertTrue("GatewaySender destroy should fail", stat.contains("ERROR:"));
-      }
-    } else {
-      fail("testCreateDestroyParallelGatewaySender failed as did not get CommandResult");
-    }
-  }
+    locatorServerStartupRule.startServerVM(1, locator.getPort());
+    locatorServerStartupRule.startServerVM(2, locator.getPort());
 
-  private CommandResult executeCommandWithIgnoredExceptions(String command) {
-    final IgnoredException ignored = IgnoredException.addIgnoredException("Could not connect");
-    try {
-      return gfsh.executeCommand(command);
-    } finally {
-      ignored.remove();
-    }
+    // destroy a sender that does not exist
+    gfsh.executeAndAssertThat("destroy gateway-sender --id=ln").statusIsError()
+        .tableHasColumnWithValuesContaining("Status", "ERROR", "ERROR");
   }
 }


 

----------------------------------------------------------------
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:
users@infra.apache.org


> gfsh destroy gateway-sender should be idempotent
> ------------------------------------------------
>
>                 Key: GEODE-2565
>                 URL: https://issues.apache.org/jira/browse/GEODE-2565
>             Project: Geode
>          Issue Type: Sub-task
>          Components: gfsh
>            Reporter: Swapnil Bawaskar
>
> Currently, running destroy gateway-sender multiple times return the following error:
> {{{
> destroy gateway-sender --id=1
> Member | Status
> ------ | -----------------------------------------
> serv2  | ERROR: GateWaySender with Id  1 not found
> serv1  | ERROR: GateWaySender with Id  1 not found
> }}}
> We should add a {{--if-exists}} option to destroy gateway-sender command.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message