From 6a309eebbabbfe21bc3ded844b6b4740691374b6 Mon Sep 17 00:00:00 2001 From: Thisara-Welmilla Date: Fri, 24 Jan 2025 13:08:59 +0530 Subject: [PATCH] Addressed comments. --- ...ActionInvocationResponseClassProvider.java | 6 +++++- .../impl/ActionExecutorServiceImpl.java | 3 ++- .../ActionInvocationResponseClassFactory.java | 3 +-- ...ActionInvocationResponseClassProvider.java | 19 ++++-------------- .../{model => impl}/DefaultResponseData.java | 4 +++- .../ResponseDataDeserializer.java | 5 +++-- .../ActionInvocationSuccessResponse.java | 1 + .../action/execution/util/APIClient.java | 2 +- .../impl/ActionExecutorServiceImplTest.java | 20 ++++++++++++++++--- ...tionSuccessResponseContextFactoryTest.java | 1 - .../action/execution/util/APIClientTest.java | 9 +++------ ...ActionInvocationResponseClassProvider.java | 2 +- 12 files changed, 41 insertions(+), 34 deletions(-) rename components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/{ => impl}/DefaultActionInvocationResponseClassProvider.java (69%) rename components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/{model => impl}/DefaultResponseData.java (87%) rename components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/{model => impl}/ResponseDataDeserializer.java (89%) diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/ActionInvocationResponseClassProvider.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/ActionInvocationResponseClassProvider.java index 5be0ae5b4b7e..53e1e86d9d7e 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/ActionInvocationResponseClassProvider.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/ActionInvocationResponseClassProvider.java @@ -18,6 +18,7 @@ package org.wso2.carbon.identity.action.execution; +import org.wso2.carbon.identity.action.execution.impl.DefaultResponseData; import org.wso2.carbon.identity.action.execution.model.ActionType; import org.wso2.carbon.identity.action.execution.model.ResponseData; @@ -39,5 +40,8 @@ public interface ActionInvocationResponseClassProvider { * * @return The extended ResponseData class. */ - Class getSuccessResponseDataClass(); + default Class getSuccessResponseDataClass() { + + return DefaultResponseData.class; + } } diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImpl.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImpl.java index 276c1b9630c9..f466ed6f809d 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImpl.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImpl.java @@ -373,7 +373,8 @@ private ActionExecutionStatus processSuccessResponse(Action action, validatePerformableOperations(actionRequest, successResponse.getOperations(), action); ActionInvocationSuccessResponse.Builder successResponseBuilder = new ActionInvocationSuccessResponse.Builder().actionStatus(ActionInvocationResponse.Status.SUCCESS) - .operations(allowedPerformableOperations); + .operations(allowedPerformableOperations) + .responseData(successResponse.getData()); return actionExecutionResponseProcessor.processSuccessResponse(eventContext, actionRequest.getEvent(), successResponseBuilder.build()); } diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionInvocationResponseClassFactory.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionInvocationResponseClassFactory.java index 6270a0b96d24..15dad3d5d9e5 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionInvocationResponseClassFactory.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ActionInvocationResponseClassFactory.java @@ -19,7 +19,6 @@ package org.wso2.carbon.identity.action.execution.impl; import org.wso2.carbon.identity.action.execution.ActionInvocationResponseClassProvider; -import org.wso2.carbon.identity.action.execution.DefaultActionInvocationResponseClassProvider; import org.wso2.carbon.identity.action.execution.model.ActionType; import org.wso2.carbon.identity.action.execution.model.ResponseData; @@ -27,7 +26,7 @@ import java.util.Map; /** - * This class defines the classes for extended implementations of action invocation responses for + * This class manages ActionInvocationResponseClassProvider implementations that extends action invocation responses for * different action types. * The ActionInvocationResponseClassFactory is the component that is responsible for providing the classes * defined by the downstream component based on the action type. diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/DefaultActionInvocationResponseClassProvider.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultActionInvocationResponseClassProvider.java similarity index 69% rename from components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/DefaultActionInvocationResponseClassProvider.java rename to components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultActionInvocationResponseClassProvider.java index 60bc20438d13..c95e7b4b96de 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/DefaultActionInvocationResponseClassProvider.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultActionInvocationResponseClassProvider.java @@ -16,11 +16,10 @@ * under the License. */ -package org.wso2.carbon.identity.action.execution; +package org.wso2.carbon.identity.action.execution.impl; +import org.wso2.carbon.identity.action.execution.ActionInvocationResponseClassProvider; import org.wso2.carbon.identity.action.execution.model.ActionType; -import org.wso2.carbon.identity.action.execution.model.DefaultResponseData; -import org.wso2.carbon.identity.action.execution.model.ResponseData; /** * Default implementation of the ActionInvocationResponseClassProvider. The downStream components need to extend @@ -37,17 +36,7 @@ public static DefaultActionInvocationResponseClassProvider getInstance() { @Override public ActionType getSupportedActionType() { - throw new UnsupportedOperationException("This method should not called for default implementation."); - } - - /** - * Get the default ResponseData class for action invocation success response. - * - * @return The default ResponseData class. - */ - @Override - public Class getSuccessResponseDataClass() { - - return DefaultResponseData.class; + throw new UnsupportedOperationException( + "This method is not allowed for DefaultActionInvocationResponseClassProvider."); } } diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/DefaultResponseData.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultResponseData.java similarity index 87% rename from components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/DefaultResponseData.java rename to components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultResponseData.java index 6ef7ffb9d502..0952f852de4d 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/DefaultResponseData.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/DefaultResponseData.java @@ -16,7 +16,9 @@ * under the License. */ -package org.wso2.carbon.identity.action.execution.model; +package org.wso2.carbon.identity.action.execution.impl; + +import org.wso2.carbon.identity.action.execution.model.ResponseData; /** * Default ResponseData implementation, which can be used when there are no extended ResponseData class for diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ResponseDataDeserializer.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ResponseDataDeserializer.java similarity index 89% rename from components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ResponseDataDeserializer.java rename to components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ResponseDataDeserializer.java index 0fcda80261c6..5f52a024e65d 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ResponseDataDeserializer.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/impl/ResponseDataDeserializer.java @@ -16,14 +16,15 @@ * under the License. */ -package org.wso2.carbon.identity.action.execution.model; +package org.wso2.carbon.identity.action.execution.impl; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import org.wso2.carbon.identity.action.execution.impl.ActionInvocationResponseClassFactory; +import org.wso2.carbon.identity.action.execution.model.ActionType; +import org.wso2.carbon.identity.action.execution.model.ResponseData; import java.io.IOException; diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ActionInvocationSuccessResponse.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ActionInvocationSuccessResponse.java index f58b0d71e49c..e359b00b00b3 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ActionInvocationSuccessResponse.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/model/ActionInvocationSuccessResponse.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; +import org.wso2.carbon.identity.action.execution.impl.ResponseDataDeserializer; import java.util.ArrayList; import java.util.List; diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java index 815a834892fe..715989a767ba 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java @@ -36,6 +36,7 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.util.EntityUtils; import org.wso2.carbon.identity.action.execution.exception.ActionInvocationException; +import org.wso2.carbon.identity.action.execution.impl.ResponseDataDeserializer; import org.wso2.carbon.identity.action.execution.model.ActionExecutionStatus; import org.wso2.carbon.identity.action.execution.model.ActionInvocationErrorResponse; import org.wso2.carbon.identity.action.execution.model.ActionInvocationFailureResponse; @@ -44,7 +45,6 @@ import org.wso2.carbon.identity.action.execution.model.ActionInvocationSuccessResponse; import org.wso2.carbon.identity.action.execution.model.ActionType; import org.wso2.carbon.identity.action.execution.model.ResponseData; -import org.wso2.carbon.identity.action.execution.model.ResponseDataDeserializer; import java.io.IOException; import java.net.SocketTimeoutException; diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImplTest.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImplTest.java index 140a3912c5e2..232560e4cac2 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImplTest.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/ActionExecutorServiceImplTest.java @@ -53,6 +53,7 @@ import org.wso2.carbon.identity.action.execution.model.Operation; import org.wso2.carbon.identity.action.execution.model.Organization; import org.wso2.carbon.identity.action.execution.model.Param; +import org.wso2.carbon.identity.action.execution.model.PerformableOperation; import org.wso2.carbon.identity.action.execution.model.Request; import org.wso2.carbon.identity.action.execution.model.SuccessStatus; import org.wso2.carbon.identity.action.execution.model.Tenant; @@ -447,9 +448,13 @@ public void testActionExecuteSuccessWhenNoRuleConfiguredInAction() throws Except .getActionExecutionResponseProcessor(any())) .thenReturn(actionExecutionResponseProcessor); + requestFilter.when(() -> RequestFilter.getFilteredHeaders(any(), any())).thenReturn(new ArrayList
()); + requestFilter.when(() -> RequestFilter.getFilteredParams(any(), any())).thenReturn(new ArrayList()); + + ActionExecutionRequest actionExecutionRequest = createActionExecutionRequest(actionType); when(actionExecutionRequestBuilder.getSupportedActionType()).thenReturn(actionType); when(actionExecutionRequestBuilder.buildActionExecutionRequest(eventContext)).thenReturn( - mock(ActionExecutionRequest.class)); + actionExecutionRequest); ActionInvocationResponse actionInvocationResponse = createSuccessActionInvocationResponse(); @@ -491,9 +496,13 @@ public void testActionExecuteSuccessWhenRuleConfiguredInActionIsSatisfied() thro .getActionExecutionResponseProcessor(any())) .thenReturn(actionExecutionResponseProcessor); + requestFilter.when(() -> RequestFilter.getFilteredHeaders(any(), any())).thenReturn(new ArrayList
()); + requestFilter.when(() -> RequestFilter.getFilteredParams(any(), any())).thenReturn(new ArrayList()); + + ActionExecutionRequest actionExecutionRequest = createActionExecutionRequest(actionType); when(actionExecutionRequestBuilder.getSupportedActionType()).thenReturn(actionType); when(actionExecutionRequestBuilder.buildActionExecutionRequest(eventContext)).thenReturn( - mock(ActionExecutionRequest.class)); + actionExecutionRequest); ActionInvocationResponse actionInvocationResponse = createSuccessActionInvocationResponse(); when(apiClient.callAPI(any(), any(), any(), any())).thenReturn(actionInvocationResponse); @@ -675,9 +684,14 @@ private String getJSONRequestPayload(ActionExecutionRequest actionExecutionReque private ActionInvocationResponse createSuccessActionInvocationResponse() throws Exception { + PerformableOperation performableOp = new PerformableOperation(); + performableOp.setOp(Operation.ADD); + performableOp.setPath("/accessToken/claims/-"); + performableOp.setValue("testValue"); + ActionInvocationSuccessResponse successResponse = mock(ActionInvocationSuccessResponse.class); when(successResponse.getActionStatus()).thenReturn(ActionInvocationResponse.Status.SUCCESS); - when(successResponse.getOperations()).thenReturn(Collections.emptyList()); + when(successResponse.getOperations()).thenReturn(new ArrayList<>(Collections.singletonList(performableOp))); ActionInvocationResponse actionInvocationResponse = mock(ActionInvocationResponse.class); setField(actionInvocationResponse, "actionStatus", ActionInvocationResponse.Status.SUCCESS); diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/InvocationSuccessResponseContextFactoryTest.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/InvocationSuccessResponseContextFactoryTest.java index 436fd3119c2d..14c4fad3d2e7 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/InvocationSuccessResponseContextFactoryTest.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/impl/InvocationSuccessResponseContextFactoryTest.java @@ -20,7 +20,6 @@ import org.testng.annotations.Test; import org.wso2.carbon.identity.action.execution.model.ActionType; -import org.wso2.carbon.identity.action.execution.model.DefaultResponseData; import org.wso2.carbon.identity.action.execution.model.ResponseData; import org.wso2.carbon.identity.action.execution.util.TestActionInvocationResponseClassProvider; import org.wso2.carbon.identity.action.execution.util.UserData; diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/APIClientTest.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/APIClientTest.java index ca8bfbf212b8..2aa66daa132a 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/APIClientTest.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/APIClientTest.java @@ -132,8 +132,7 @@ public void testCallAPIUnacceptableContentTypeForSuccessResponse() @DataProvider(name = "unacceptableSuccessResponsePayloads") public String[] unacceptableSuccessResponsePayloads() { - return new String[]{"{}", "", "success", "{\"actionStatus\":\"ERROR\"}, " + - "{\"actionStatus\": \"FAILED\"}"}; + return new String[]{"{}", "", "success", "{\"actionStatus\":\"ERROR\"}, " + "{\"actionStatus\": \"FAILED\"}"}; } @Test(dataProvider = "unacceptableSuccessResponsePayloads") @@ -472,10 +471,9 @@ public void testCallAPIRetryOnTimeoutAndReceiveSuccessResponse() throws Exceptio } @Test - public void testCallAPIRetryOnTimeoutAndReceiveSuccessResponseWithExtendedResponseData() throws Exception { + public void testReceiveSuccessResponseWithExtendedResponseData() throws Exception { - when(httpClient.execute(any(HttpPost.class))).thenThrow(new ConnectTimeoutException("Timeout")) - .thenReturn(httpResponse); + when(httpClient.execute(any(HttpPost.class))).thenReturn(httpResponse); when(httpResponse.getStatusLine()).thenReturn(statusLine); when(statusLine.getStatusCode()).thenReturn(200); @@ -494,7 +492,6 @@ public void testCallAPIRetryOnTimeoutAndReceiveSuccessResponseWithExtendedRespon assertNotNull(response); assertTrue(response.isSuccess()); - verify(httpClient, times(2)).execute(any(HttpPost.class)); } @Test diff --git a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/TestActionInvocationResponseClassProvider.java b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/TestActionInvocationResponseClassProvider.java index a9c28fded36f..dfef3ce71fa3 100644 --- a/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/TestActionInvocationResponseClassProvider.java +++ b/components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/TestActionInvocationResponseClassProvider.java @@ -18,7 +18,7 @@ package org.wso2.carbon.identity.action.execution.util; -import org.wso2.carbon.identity.action.execution.DefaultActionInvocationResponseClassProvider; +import org.wso2.carbon.identity.action.execution.impl.DefaultActionInvocationResponseClassProvider; import org.wso2.carbon.identity.action.execution.model.ActionType; import org.wso2.carbon.identity.action.execution.model.ResponseData;