This is an automated email from the ASF dual-hosted git repository.
jjramos 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 9fa4dc2 GEODE-6293: Fix fire & forget functions in gfsh (#3138)
9fa4dc2 is described below
commit 9fa4dc25c3f1dd9e470729eb707b3fe81fd7fc7e
Author: Juan José Ramos <[email protected]>
AuthorDate: Thu Jan 31 08:35:02 2019 +0000
GEODE-6293: Fix fire & forget functions in gfsh (#3138)
* GEODE-6293: Fix fire & forget functions in gfsh
- Fixed minor warnings.
- Refactored class `UserFunctionExecution`.
- Added unit tests for class `UserFunctionExecution`.
- Class `UserFunctionExecution` now supports the execution of functions
that don't return any results.
---
.../commands/ExecuteFunctionCommandDUnitTest.java | 66 ++++-
.../ExecuteFunctionCommandSecurityTest.java | 11 +-
.../cache/execute/CoreFunctionSecurityTest.java | 6 +-
.../cli/commands/ExecuteFunctionCommand.java | 7 +-
.../cli/functions/UserFunctionExecution.java | 171 +++++++----
.../cli/functions/UserFunctionExecutionTest.java | 328 +++++++++++++++++++++
...xecuteFunctionCommandWithSecurityDUnitTest.java | 4 +-
7 files changed, 502 insertions(+), 91 deletions(-)
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandDUnitTest.java
index 2296fd4..276c852 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandDUnitTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandDUnitTest.java
@@ -12,7 +12,6 @@
* 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;
@@ -32,6 +31,7 @@ import org.apache.geode.cache.execute.Function;
import org.apache.geode.cache.execute.FunctionContext;
import org.apache.geode.cache.execute.FunctionService;
import org.apache.geode.cache.execute.RegionFunctionContext;
+import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.assertions.CommandResultAssert;
@@ -39,7 +39,7 @@ import
org.apache.geode.test.junit.assertions.TabularResultModelAssert;
import org.apache.geode.test.junit.categories.GfshTest;
import org.apache.geode.test.junit.rules.GfshCommandRule;
-@Category({GfshTest.class})
+@Category(GfshTest.class)
public class ExecuteFunctionCommandDUnitTest {
@ClassRule
public static ClusterStartupRule cluster = new ClusterStartupRule();
@@ -47,22 +47,23 @@ public class ExecuteFunctionCommandDUnitTest {
@ClassRule
public static GfshCommandRule gfsh = new GfshCommandRule();
- private static MemberVM locator, server1, server2, server3;
private static final String functionId = "genericFunctionId";
-
private static String command = "execute function --id=" + functionId + " ";
@BeforeClass
public static void setUpClass() throws Exception {
- locator = cluster.startLocatorVM(0);
+ MemberVM locator = cluster.startLocatorVM(0);
gfsh.connectAndVerify(locator);
- server1 = cluster.startServerVM(1, "group1", locator.getPort());
- server2 = cluster.startServerVM(2, "group1", locator.getPort());
- server3 = cluster.startServerVM(3, "group2", locator.getPort());
- MemberVM.invokeInEveryMember(() -> {
- FunctionService.registerFunction(new GenericFunctionOp(functionId));
- }, server1, server2, server3);
+ MemberVM server1 = cluster.startServerVM(1, "group1", locator.getPort());
+ MemberVM server2 = cluster.startServerVM(2, "group1", locator.getPort());
+ MemberVM server3 = cluster.startServerVM(3, "group2", locator.getPort());
+ MemberVM.invokeInEveryMember(
+ () -> FunctionService.registerFunction(new
GenericFunctionOp(functionId)), server1, server2,
+ server3);
+ MemberVM.invokeInEveryMember(
+ () -> FunctionService.registerFunction(new FireAndForgetFunction()),
server1, server2,
+ server3);
// create a partitioned region on only group1
gfsh.executeAndAssertThat(
@@ -73,7 +74,9 @@ public class ExecuteFunctionCommandDUnitTest {
locator.waitUntilRegionIsReadyOnExactlyThisManyServers("/regionA", 2);
server1.invoke(() -> {
- Region region = ClusterStartupRule.getCache().getRegion("/regionA");
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+ Region<String, String> region = cache.getRegion("/regionA");
region.put("a", "a");
region.put("b", "b");
});
@@ -267,7 +270,18 @@ public class ExecuteFunctionCommandDUnitTest {
"[GENERICFUNCTIONID-ARGUMENTS]", "[GENERICFUNCTIONID-ARGUMENTS]");
}
+ @Test
+ public void functionWithNoResults() {
+ TabularResultModelAssert tableAssert =
+ gfsh.executeAndAssertThat("execute function
--id=FireAndForget").statusIsSuccess()
+ .hasTableSection().hasRowSize(3).hasColumnSize(3);
+
+ tableAssert.hasColumn("Member").containsExactlyInAnyOrder("server-1",
"server-2", "server-3");
+ tableAssert.hasColumn("Status").containsExactlyInAnyOrder("OK", "OK",
"OK");
+ tableAssert.hasColumn("Message").containsExactlyInAnyOrder("[]", "[]",
"[]");
+ }
+ @SuppressWarnings("unused")
public static class MyPartitionResolver implements FixedPartitionResolver {
@Override
public String getPartitionName(final EntryOperation opDetails,
@@ -291,7 +305,6 @@ public class ExecuteFunctionCommandDUnitTest {
}
}
-
public static class GenericFunctionOp implements Function {
private String functionId;
@@ -300,6 +313,7 @@ public class ExecuteFunctionCommandDUnitTest {
}
@Override
+ @SuppressWarnings("unchecked")
public void execute(FunctionContext context) {
String filter = null;
if (context instanceof RegionFunctionContext) {
@@ -330,4 +344,30 @@ public class ExecuteFunctionCommandDUnitTest {
return functionId;
}
}
+
+
+ public static class FireAndForgetFunction implements Function {
+
+ FireAndForgetFunction() {}
+
+ @Override
+ public String getId() {
+ return "FireAndForget";
+ }
+
+ @Override
+ public boolean isHA() {
+ return false;
+ }
+
+ @Override
+ public boolean hasResult() {
+ return false;
+ }
+
+ @Override
+ public void execute(FunctionContext context) {
+ // Do Nothing.
+ }
+ }
}
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandSecurityTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandSecurityTest.java
index f31e052..124dcc2 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandSecurityTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandSecurityTest.java
@@ -45,7 +45,7 @@ import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.categories.SecurityTest;
import org.apache.geode.test.junit.rules.GfshCommandRule;
-@Category({SecurityTest.class})
+@Category(SecurityTest.class)
public class ExecuteFunctionCommandSecurityTest implements Serializable {
@ClassRule
@@ -54,13 +54,13 @@ public class ExecuteFunctionCommandSecurityTest implements
Serializable {
@Rule
public GfshCommandRule gfsh = new GfshCommandRule();
- private static MemberVM locator, server1, server2;
+ private static MemberVM locator;
private static String REPLICATED_REGION = "replicatedRegion";
private static String PARTITIONED_REGION = "partitionedRegion";
@BeforeClass
- public static void beforeClass() throws Exception {
+ public static void beforeClass() {
Properties locatorProps = new Properties();
locatorProps.setProperty(SECURITY_MANAGER,
SimpleTestSecurityManager.class.getName());
locator = lsRule.startLocatorVM(0, locatorProps);
@@ -68,14 +68,15 @@ public class ExecuteFunctionCommandSecurityTest implements
Serializable {
Properties serverProps = new Properties();
serverProps.setProperty(ResourceConstants.USER_NAME, "clusterManage");
serverProps.setProperty(ResourceConstants.PASSWORD, "clusterManage");
- server1 = lsRule.startServerVM(1, serverProps, locator.getPort());
- server2 = lsRule.startServerVM(2, serverProps, locator.getPort());
+ MemberVM server1 = lsRule.startServerVM(1, serverProps, locator.getPort());
+ MemberVM server2 = lsRule.startServerVM(2, serverProps, locator.getPort());
Stream.of(server1, server2).forEach(server -> server.invoke(() -> {
FunctionService.registerFunction(new ReadFunction());
FunctionService.registerFunction(new WriteFunction());
InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
cache.createRegionFactory(RegionShortcut.REPLICATE).create(REPLICATED_REGION);
cache.createRegionFactory(RegionShortcut.PARTITION).create(PARTITIONED_REGION);
}));
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/CoreFunctionSecurityTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/CoreFunctionSecurityTest.java
index dab4fbe..98e6669 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/CoreFunctionSecurityTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/cache/execute/CoreFunctionSecurityTest.java
@@ -157,10 +157,8 @@ public class CoreFunctionSecurityTest {
@Test
@ConnectionConfiguration(user = "user", password = "user")
- public void functionRequireExpectedPermission() throws Exception {
- functionStringMap.entrySet().stream().forEach(entry -> {
- Function function = entry.getKey();
- String permission = entry.getValue();
+ public void functionRequireExpectedPermission() {
+ functionStringMap.forEach((function, permission) -> {
System.out.println("function: " + function.getId() + ", permission: " +
permission);
gfsh.executeAndAssertThat("execute function --id=" + function.getId())
.tableHasRowCount(RESULT_HEADER, 1)
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
index 98dc99a..2111c44 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommand.java
@@ -76,7 +76,7 @@ public class ExecuteFunctionCommand extends
InternalGfshCommand {
}
if (dsMembers.size() == 0) {
- return new ResultModel().createError("No members found.");
+ return ResultModel.createError("No members found.");
}
// Build up our argument list
@@ -113,6 +113,7 @@ public class ExecuteFunctionCommand extends
InternalGfshCommand {
return ResultModel.createMemberStatusResult(results, false, false);
}
+ @SuppressWarnings("unused")
public static class ExecuteFunctionCommandInterceptor implements
CliAroundInterceptor {
@Override
public ResultModel preExecution(GfshParseResult parseResult) {
@@ -126,11 +127,11 @@ public class ExecuteFunctionCommand extends
InternalGfshCommand {
ResultModel result = new ResultModel();
if (moreThanOne) {
- return result.createError(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
+ return
ResultModel.createError(CliStrings.EXECUTE_FUNCTION__MSG__OPTIONS);
}
if (onRegion == null && filter != null) {
- return result.createError(
+ return ResultModel.createError(
CliStrings.EXECUTE_FUNCTION__MSG__MEMBER_SHOULD_NOT_HAVE_FILTER_FOR_EXECUTION);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
index 7503502..2836663 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecution.java
@@ -15,6 +15,7 @@
package org.apache.geode.management.internal.cli.functions;
import static
org.apache.geode.management.internal.cli.functions.CliFunctionResult.StatusState.ERROR;
+import static
org.apache.geode.management.internal.cli.functions.CliFunctionResult.StatusState.OK;
import java.util.ArrayList;
import java.util.Arrays;
@@ -36,6 +37,7 @@ import org.apache.geode.cache.execute.Function;
import org.apache.geode.cache.execute.FunctionContext;
import org.apache.geode.cache.execute.FunctionService;
import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.cache.query.RegionNotFoundException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.internal.cache.InternalCache;
@@ -50,17 +52,90 @@ import org.apache.geode.security.ResourcePermission;
* @since GemFire 7.0
*/
public class UserFunctionExecution implements InternalFunction<Object[]> {
+ private static final long serialVersionUID = 1L;
+ private static Logger logger = LogService.getLogger();
public static final String ID = UserFunctionExecution.class.getName();
- private static Logger logger = LogService.getLogger();
- private static final long serialVersionUID = 1L;
+ @Override
+ public boolean isHA() {
+ return false;
+ }
+
+ @Override
+ public String getId() {
+ return UserFunctionExecution.ID;
+ }
+
+ @Override
+ public Collection<ResourcePermission> getRequiredPermissions(String
regionName) {
+ return Collections.emptySet();
+ }
+
+ boolean loginRequired(SecurityService securityService) {
+ try {
+ // if the function is executed on a server with jmx-manager that user is
already logged into
+ // then we do not need to do login/logout here.
+ Subject subject = securityService.getSubject();
+ return subject == null || !subject.isAuthenticated();
+ } catch (AuthenticationRequiredException e) {
+ return true;
+ }
+ }
+
+ Function<?> loadFunction(String functionId) {
+ return FunctionService.getFunction(functionId);
+ }
+
+ String[] parseArguments(String argumentsString) {
+ if (argumentsString != null && argumentsString.length() > 0) {
+ return argumentsString.split(",");
+ } else {
+ return null;
+ }
+ }
+
+ Set<String> parseFilters(String filterString) {
+ if (filterString != null && filterString.length() > 0) {
+ return
Arrays.stream(filterString.split(",")).collect(Collectors.toSet());
+ } else {
+ return new HashSet<>();
+ }
+ }
+
+ ResultCollector parseResultCollector(String resultCollectorName)
+ throws ClassNotFoundException, IllegalAccessException,
InstantiationException {
+ if (resultCollectorName != null && resultCollectorName.length() > 0) {
+ return (ResultCollector)
ClassPathLoader.getLatest().forName(resultCollectorName)
+ .newInstance();
+ } else {
+ return null;
+ }
+ }
+
+ Execution buildExecution(Cache cache, String onRegion) throws
RegionNotFoundException {
+ Execution execution;
+ DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
+
+ if (onRegion != null && onRegion.length() > 0) {
+ Region region = cache.getRegion(onRegion);
+
+ if (region == null) {
+ throw new RegionNotFoundException(onRegion);
+ }
+
+ execution = FunctionService.onRegion(region);
+ } else {
+ execution = FunctionService.onMember(member);
+ }
+
+ return execution;
+ }
@Override
public void execute(FunctionContext<Object[]> context) {
Cache cache = ((InternalCache)
context.getCache()).getCacheForProcessingClientRequests();
DistributedMember member =
cache.getDistributedSystem().getDistributedMember();
- String[] functionArgs = null;
Object[] args = context.getArguments();
if (args == null) {
context.getResultSender().lastResult(new
CliFunctionResult(context.getMemberName(), ERROR,
@@ -74,39 +149,19 @@ public class UserFunctionExecution implements
InternalFunction<Object[]> {
String argumentsString = ((String) args[3]);
String onRegion = ((String) args[4]);
Properties credentials = (Properties) args[5];
-
SecurityService securityService = ((InternalCache)
context.getCache()).getSecurityService();
- boolean loginNeeded = false;
- try {
- // if the function is executed on a server with jmx-manager that user is
already logged into
- // then we do not need to do login/logout here.
- Subject subject = securityService.getSubject();
- loginNeeded = subject == null || !subject.isAuthenticated();
- } catch (AuthenticationRequiredException e) {
- loginNeeded = true;
- }
boolean loginSuccessful = false;
try {
- if (loginNeeded) {
+
+ // Authenticate If Needed
+ if (loginRequired(securityService)) {
securityService.login(credentials);
loginSuccessful = true;
}
- if (argumentsString != null && argumentsString.length() > 0) {
- functionArgs = argumentsString.split(",");
- }
- Set<String> filters = new HashSet<>();
- ResultCollector resultCollectorInstance = null;
- if (resultCollectorName != null && resultCollectorName.length() > 0) {
- resultCollectorInstance = (ResultCollector) ClassPathLoader.getLatest()
- .forName(resultCollectorName).newInstance();
- }
- if (filterString != null && filterString.length() > 0) {
- filters =
Arrays.stream(filterString.split(",")).collect(Collectors.toSet());
- }
-
- Function<?> function = FunctionService.getFunction(functionId);
+ // Load User Function
+ Function<?> function = loadFunction(functionId);
if (function == null) {
context.getResultSender()
.lastResult(new CliFunctionResult(context.getMemberName(), ERROR,
@@ -116,22 +171,16 @@ public class UserFunctionExecution implements
InternalFunction<Object[]> {
return;
}
- // security check
- function.getRequiredPermissions(onRegion,
functionArgs).forEach(securityService::authorize);
+ // Parse Arguments
+ Set<String> filters = parseFilters(filterString);
+ String[] functionArgs = parseArguments(argumentsString);
+ ResultCollector resultCollectorInstance =
parseResultCollector(resultCollectorName);
- Execution execution = null;
- if (onRegion != null && onRegion.length() > 0) {
- Region region = cache.getRegion(onRegion);
- if (region == null) {
- context.getResultSender().lastResult(
- new CliFunctionResult(context.getMemberName(), ERROR, onRegion +
" does not exist"));
- return;
- }
- execution = FunctionService.onRegion(region);
- } else {
- execution = FunctionService.onMember(member);
- }
+ // Security check
+ function.getRequiredPermissions(onRegion,
functionArgs).forEach(securityService::authorize);
+ // Build & Configure Execution Context
+ Execution execution = buildExecution(cache, onRegion);
if (execution == null) {
context.getResultSender()
.lastResult(new CliFunctionResult(context.getMemberName(), ERROR,
@@ -149,13 +198,20 @@ public class UserFunctionExecution implements
InternalFunction<Object[]> {
if (functionArgs != null && functionArgs.length > 0) {
execution = execution.setArguments(functionArgs);
}
+
if (filters.size() > 0) {
execution = execution.withFilter(filters);
}
- List<Object> results = (List<Object>)
execution.execute(function.getId()).getResult();
- List<String> resultMessage = new ArrayList<>();
+ // Execute Function and gather results
+ List results = null;
boolean functionSuccess = true;
+ List<String> resultMessage = new ArrayList<>();
+
+ ResultCollector rc = execution.execute(function.getId());
+ if (function.hasResult()) {
+ results = (List) rc.getResult();
+ }
if (results != null) {
for (Object resultObj : results) {
@@ -169,39 +225,26 @@ public class UserFunctionExecution implements
InternalFunction<Object[]> {
}
}
}
- context.getResultSender().lastResult(new
CliFunctionResult(context.getMemberName(),
- functionSuccess, resultMessage.toString()));
+ context.getResultSender().lastResult(new
CliFunctionResult(context.getMemberName(),
+ functionSuccess ? OK : ERROR, resultMessage.toString()));
+ } catch (RegionNotFoundException regionNotFoundException) {
+ context.getResultSender().lastResult(
+ new CliFunctionResult(context.getMemberName(), ERROR, onRegion + "
does not exist"));
} catch (ClassNotFoundException | IllegalAccessException |
InstantiationException e) {
context.getResultSender()
- .lastResult(new CliFunctionResult(context.getMemberName(), false,
+ .lastResult(new CliFunctionResult(context.getMemberName(), ERROR,
CliStrings.format(
CliStrings.EXECUTE_FUNCTION__MSG__RESULT_COLLECTOR_0_NOT_FOUND_ERROR_1,
resultCollectorName, e.getMessage())));
} catch (Exception e) {
logger.error("error executing function " + functionId, e);
context.getResultSender().lastResult(
- new CliFunctionResult(context.getMemberName(), false, "Exception: "
+ e.getMessage()));
+ new CliFunctionResult(context.getMemberName(), ERROR, "Exception: "
+ e.getMessage()));
} finally {
if (loginSuccessful) {
securityService.logout();
}
}
}
-
- @Override
- public Collection<ResourcePermission> getRequiredPermissions(String
regionName) {
- return Collections.emptySet();
- }
-
- @Override
- public String getId() {
- return UserFunctionExecution.ID;
- }
-
- @Override
- public boolean isHA() {
- return false;
- }
-
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecutionTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecutionTest.java
new file mode 100644
index 0000000..b192ffc
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecutionTest.java
@@ -0,0 +1,328 @@
+/*
+ * 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.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
+
+import org.apache.shiro.subject.Subject;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.execute.Execution;
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.cache.query.RegionNotFoundException;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.DistributedSystem;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.InternalCacheForClientAccess;
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.security.AuthenticationRequiredException;
+
+public class UserFunctionExecutionTest {
+ private Object[] arguments;
+ private Execution execution;
+ private Function userFunction;
+ private UserFunctionExecution function;
+ private SecurityService securityService;
+ private ResultCollector resultCollector;
+ private FunctionContext<Object[]> context;
+ private ResultSender<Object> resultSender;
+ private InternalCacheForClientAccess filterCache;
+ private ArgumentCaptor<CliFunctionResult> resultCaptor;
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Before
+ @SuppressWarnings("unchecked")
+ public void setUp() throws Exception {
+ execution = mock(Execution.class);
+ userFunction = mock(Function.class);
+ context = mock(FunctionContext.class);
+ resultSender = mock(ResultSender.class);
+ function = spy(UserFunctionExecution.class);
+ securityService = mock(SecurityService.class);
+ resultCollector = mock(ResultCollector.class);
+ filterCache = mock(InternalCacheForClientAccess.class);
+ arguments = new Object[] {"TestFunction", "key1,key2",
"TestResultCollector", "arg1,arg2",
+ "/TestRegion", new Properties()};
+
+ when(userFunction.getId()).thenReturn("TestFunction");
+
+ InternalCache cache = mock(InternalCache.class);
+ DistributedSystem distributedSystem =
mock(InternalDistributedSystem.class);
+ DistributedMember distributedMember =
mock(InternalDistributedMember.class);
+ when(distributedMember.getId()).thenReturn("MockMemberId");
+
when(distributedSystem.getDistributedMember()).thenReturn(distributedMember);
+ when(filterCache.getDistributedSystem()).thenReturn(distributedSystem);
+
+ when(cache.getSecurityService()).thenReturn(securityService);
+ when(cache.getCacheForProcessingClientRequests()).thenReturn(filterCache);
+ when(context.getCache()).thenReturn(cache);
+ when(context.getArguments()).thenReturn(arguments);
+ when(context.getResultSender()).thenReturn(resultSender);
+
+ when(execution.withFilter(any())).thenReturn(execution);
+ when(execution.setArguments(any())).thenReturn(execution);
+ when(execution.withCollector(any())).thenReturn(execution);
+ when(execution.execute(anyString())).thenReturn(resultCollector);
+
+ doReturn(false).when(function).loginRequired(securityService);
+ doReturn(userFunction).when(function).loadFunction("TestFunction");
+
doReturn(resultCollector).when(function).parseResultCollector("TestResultCollector");
+ doReturn(execution).when(function).buildExecution(any(), any());
+
+ resultCaptor = ArgumentCaptor.forClass(CliFunctionResult.class);
+ }
+
+ @Test
+ public void testDefaultAttributes() {
+ assertThat(function.isHA()).isFalse();
+ assertThat(function.getId()).isEqualTo(UserFunctionExecution.ID);
+ assertThat(function.getRequiredPermissions(anyString())).isEmpty();
+ }
+
+ @Test
+ public void parseArgumentsTest() {
+ assertThat(function.parseArguments(null)).isNull();
+ assertThat(function.parseArguments("")).isNull();
+ assertThat(function.parseArguments("arg1,arg2")).isNotNull()
+ .isEqualTo(new String[] {"arg1", "arg2"});
+ }
+
+ @Test
+ public void parseFiltersTest() {
+ assertThat(function.parseFilters(null)).isNotNull().isEmpty();
+ assertThat(function.parseFilters("")).isNotNull().isEmpty();
+
assertThat(function.parseFilters("key1,key2")).isNotNull().containsOnly("key1",
"key2");
+ }
+
+ @Test
+ public void
buildExecutionShouldThrowExceptionWhenRegionIsRequiredButItDoesNotExist()
+ throws Exception {
+ when(filterCache.getRegion("region")).thenReturn(null);
+ when(function.buildExecution(any(), any())).thenCallRealMethod();
+
+ assertThatThrownBy(() -> function.buildExecution(filterCache, "region"))
+ .isInstanceOf(RegionNotFoundException.class);
+ }
+
+ @Test
+ public void loginRequiredShouldReturnTrueWhenSubjectIsNull() {
+ when(securityService.getSubject()).thenReturn(null);
+ when(function.loginRequired(securityService)).thenCallRealMethod();
+
+ assertThat(function.loginRequired(securityService)).isTrue();
+ }
+
+ @Test
+ public void loginRequiredShouldReturnTrueWhenSubjectIsNotAuthenticated() {
+ Subject subject = mock(Subject.class);
+ when(subject.isAuthenticated()).thenReturn(false);
+ when(securityService.getSubject()).thenReturn(subject);
+ when(function.loginRequired(securityService)).thenCallRealMethod();
+
+ assertThat(function.loginRequired(securityService)).isTrue();
+ }
+
+ @Test
+ public void
loginRequiredShouldReturnTrueWhenSecurityServiceFailsToLoadSubject() {
+ when(function.loginRequired(securityService)).thenCallRealMethod();
+ doThrow(new AuthenticationRequiredException("Dummy
Exception")).when(securityService)
+ .getSubject();
+
+ assertThat(function.loginRequired(securityService)).isTrue();
+ }
+
+ @Test
+ public void loginRequiredShouldReturnFalseWhenSubjectIsAuthenticated() {
+ Subject subject = mock(Subject.class);
+ when(subject.isAuthenticated()).thenReturn(true);
+ when(securityService.getSubject()).thenReturn(subject);
+ when(function.loginRequired(securityService)).thenCallRealMethod();
+
+ assertThat(function.loginRequired(securityService)).isFalse();
+ }
+
+ @Test
+ public void executeShouldFailWhenNoArgumentsAreProvided() {
+ when(context.getArguments()).thenReturn(null);
+
+ function.execute(context);
+
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo("Could not retrieve
arguments");
+ }
+
+ @Test
+ public void executeShouldFailWhenTargetFunctionCanNotBeLoaded() {
+ doReturn(null).when(function).loadFunction(anyString());
+
+ function.execute(context);
+
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage())
+ .isEqualTo("Function : TestFunction is not registered on member.");
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void executeShouldFailWhenResultCollectorCanNotBeInstantiated()
throws Exception {
+ CliFunctionResult result;
+
+ doThrow(new
ClassNotFoundException("ClassNotFoundException")).when(function)
+ .parseResultCollector(anyString());
+ function.execute(context);
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo(
+ "ResultCollector : TestResultCollector not found. Error :
ClassNotFoundException");
+ reset(resultSender);
+
+ doThrow(new
IllegalAccessException("IllegalAccessException")).when(function)
+ .parseResultCollector(anyString());
+ function.execute(context);
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo(
+ "ResultCollector : TestResultCollector not found. Error :
IllegalAccessException");
+ reset(resultSender);
+
+ doThrow(new
InstantiationException("InstantiationException")).when(function)
+ .parseResultCollector(anyString());
+ function.execute(context);
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo(
+ "ResultCollector : TestResultCollector not found. Error :
InstantiationException");
+ reset(resultSender);
+ }
+
+ @Test
+ public void executeShouldFailWhenRegionIsSetAsArgumentButItDoesNotExist()
throws Exception {
+ when(function.buildExecution(any(), any())).thenCallRealMethod();
+ function.execute(context);
+
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo("/TestRegion does not
exist");
+ }
+
+ @Test
+ public void executeShouldFailWhenExecutorCanNotBeLoaded() throws Exception {
+ doReturn(null).when(function).buildExecution(any(), any());
+
+ function.execute(context);
+
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isFalse();
+ assertThat(result.getStatusMessage()).isEqualTo(
+ "While executing function : TestFunction on member : MockMemberId one
region : /TestRegion error occurred : Could not retrieve executor");
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void executeShouldProperlyConfigureExecutionContext() {
+ Set<String> filter = new HashSet<>();
+ filter.add("key1");
+ filter.add("key2");
+
+ arguments = new Object[] {"TestFunction", "key1,key2",
"TestResultCollector", "arg1,arg2",
+ "/TestRegion", new Properties()};
+ when(context.getArguments()).thenReturn(arguments);
+ function.execute(context);
+ verify(execution, times(1)).withFilter(filter);
+ verify(execution, times(1)).withCollector(resultCollector);
+ verify(execution, times(1)).setArguments(new String[] {"arg1", "arg2"});
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult resultFullArguments = resultCaptor.getValue();
+ assertThat(resultFullArguments.isSuccessful()).isTrue();
+
+ reset(resultSender);
+ reset(execution);
+ arguments = new Object[] {"TestFunction", "", "", "", "", new
Properties()};
+ when(context.getArguments()).thenReturn(arguments);
+ function.execute(context);
+ verify(execution, never()).withFilter(any());
+ verify(execution, never()).setArguments(any());
+ verify(execution, never()).withCollector(any());
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult resultNoArguments = resultCaptor.getValue();
+ assertThat(resultNoArguments.isSuccessful()).isTrue();
+ }
+
+ @Test
+ public void executeShouldWorkProperlyForFunctionsWithResults() {
+ when(userFunction.hasResult()).thenReturn(true);
+ doReturn(true).when(function).loginRequired(any());
+ when(resultCollector.getResult()).thenReturn(Arrays.asList("result1",
"result2"));
+
+ function.execute(context);
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getStatusMessage()).isEqualTo("[result1, result2]");
+ verify(securityService, times(1)).login(any());
+ verify(securityService, times(1)).logout();
+ }
+
+ @Test
+ public void executeShouldWorkProperlyForFunctionsWithoutResults() {
+ when(userFunction.hasResult()).thenReturn(false);
+ doReturn(true).when(function).loginRequired(any());
+
+ function.execute(context);
+ verify(resultSender, times(1)).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getStatusMessage()).isEqualTo("[]");
+ verify(securityService, times(1)).login(any());
+ verify(securityService, times(1)).logout();
+ }
+}
diff --git
a/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandWithSecurityDUnitTest.java
b/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandWithSecurityDUnitTest.java
index 45fb534..6cc148b 100644
---
a/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandWithSecurityDUnitTest.java
+++
b/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExecuteFunctionCommandWithSecurityDUnitTest.java
@@ -37,7 +37,7 @@ import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.categories.GfshTest;
import org.apache.geode.test.junit.rules.GfshCommandRule;
-@Category({GfshTest.class})
+@Category(GfshTest.class)
public class ExecuteFunctionCommandWithSecurityDUnitTest {
@ClassRule
public static ClusterStartupRule lsRule = new ClusterStartupRule();
@@ -48,7 +48,7 @@ public class ExecuteFunctionCommandWithSecurityDUnitTest {
private static MemberVM locator;
@BeforeClass
- public static void beforeClass() throws Exception {
+ public static void beforeClass() {
locator = lsRule.startLocatorVM(0,
l ->
l.withHttpService().withSecurityManager(SimpleTestSecurityManager.class));
