From 9fa4dc25c3f1dd9e470729eb707b3fe81fd7fc7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Ramos?= Date: Thu, 31 Jan 2019 08:35:02 +0000 Subject: [PATCH] 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. --- .../ExecuteFunctionCommandDUnitTest.java | 66 +++- .../ExecuteFunctionCommandSecurityTest.java | 11 +- .../execute/CoreFunctionSecurityTest.java | 6 +- .../cli/commands/ExecuteFunctionCommand.java | 7 +- .../cli/functions/UserFunctionExecution.java | 171 +++++---- .../functions/UserFunctionExecutionTest.java | 328 ++++++++++++++++++ ...eFunctionCommandWithSecurityDUnitTest.java | 4 +- 7 files changed, 502 insertions(+), 91 deletions(-) create mode 100644 geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/UserFunctionExecutionTest.java 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 2296fd48b691..276c852579b3 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.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.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 static void setUpClass() throws Exception { locator.waitUntilRegionIsReadyOnExactlyThisManyServers("/regionA", 2); server1.invoke(() -> { - Region region = ClusterStartupRule.getCache().getRegion("/regionA"); + InternalCache cache = ClusterStartupRule.getCache(); + assertThat(cache).isNotNull(); + Region region = cache.getRegion("/regionA"); region.put("a", "a"); region.put("b", "b"); }); @@ -267,7 +270,18 @@ public void withArgumentAndResultCollector() { "[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 void close() { } } - public static class GenericFunctionOp implements Function { private String functionId; @@ -300,6 +313,7 @@ public static class GenericFunctionOp implements Function { } @Override + @SuppressWarnings("unchecked") public void execute(FunctionContext context) { String filter = null; if (context instanceof RegionFunctionContext) { @@ -330,4 +344,30 @@ public String getId() { 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 f31e05244c83..124dcc2b8eed 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.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 static void beforeClass() throws Exception { 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 dab4fbe69a62..98e6669a90c1 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 static void setupClass() { @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 98dc99a77fc2..2111c44cacf2 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 ResultModel executeFunction( } 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 ResultModel executeFunction( 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 ResultModel preExecution(GfshParseResult parseResult) { 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 75035022f74e..28366630db05 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.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 @@ * @since GemFire 7.0 */ public class UserFunctionExecution implements InternalFunction { + 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 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 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 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 void execute(FunctionContext context) { 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 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 void execute(FunctionContext context) { return; } - // security check - function.getRequiredPermissions(onRegion, functionArgs).forEach(securityService::authorize); + // Parse Arguments + Set 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 void execute(FunctionContext context) { if (functionArgs != null && functionArgs.length > 0) { execution = execution.setArguments(functionArgs); } + if (filters.size() > 0) { execution = execution.withFilter(filters); } - List results = (List) execution.execute(function.getId()).getResult(); - List resultMessage = new ArrayList<>(); + // Execute Function and gather results + List results = null; boolean functionSuccess = true; + List 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 void execute(FunctionContext context) { } } } - 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 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 000000000000..b192ffc02beb --- /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 context; + private ResultSender resultSender; + private InternalCacheForClientAccess filterCache; + private ArgumentCaptor 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 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 45fb534d291c..6cc148b75c64 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.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));