Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve action execution core #6262

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
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;
import org.wso2.carbon.identity.action.execution.model.ActionInvocationIncompleteResponse;
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.Error;
import org.wso2.carbon.identity.action.execution.model.ErrorStatus;
import org.wso2.carbon.identity.action.execution.model.Event;
import org.wso2.carbon.identity.action.execution.model.FailedStatus;
import org.wso2.carbon.identity.action.execution.model.Failure;
import org.wso2.carbon.identity.action.execution.model.Incomplete;
import org.wso2.carbon.identity.action.execution.model.Success;

import java.util.Map;
Expand All @@ -47,6 +49,23 @@ ActionExecutionStatus<Success> processSuccessResponse(Map<String, Object> eventC
ActionInvocationSuccessResponse successResponse) throws
ActionExecutionResponseProcessorException;

/**
* This method processes the incomplete response received from the action execution.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better mention the default implementation throws an unsupported exception so that actions that requires incomplete state can implement this method

*
* @param eventContext The event context.
* @param actionEvent The action event.
* @param incompleteResponse The incomplete response.
* @return The incomplete status.
* @throws ActionExecutionResponseProcessorException If an error occurs while processing the response.
*/
default ActionExecutionStatus<Incomplete> processIncompleteResponse(Map<String, Object> eventContext,
Event actionEvent, ActionInvocationIncompleteResponse incompleteResponse) throws
ActionExecutionResponseProcessorException {

throw new UnsupportedOperationException(
"The INCOMPLETE status is not supported for the action type: " + getSupportedActionType());
}

default ActionExecutionStatus<Error> processErrorResponse(Map<String, Object> eventContext,
Event actionEvent,
ActionInvocationErrorResponse errorResponse) throws
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,18 @@ public interface ActionExecutorService {
ActionExecutionStatus execute(ActionType actionType, Map<String, Object> eventContext, String tenantDomain) throws
ActionExecutionException;

/**
* Resolve the actions by given the action id list and execute them.
*
* @param actionType Action Type.
* @param actionIdList Lis of action Ids of the actions that need to be executed.
* @param eventContext The event context of the corresponding flow.
* @param tenantDomain Tenant domain.
* @return {@link ActionExecutionStatus} The status of the action execution and the response context.
* @throws ActionExecutionException If an error occurs while executing the action.
*/
ActionExecutionStatus execute(ActionType actionType, String[] actionIdList,
Map<String, Object> eventContext, String tenantDomain)
throws ActionExecutionException;

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@
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;
import org.wso2.carbon.identity.action.execution.model.ActionInvocationIncompleteResponse;
import org.wso2.carbon.identity.action.execution.model.ActionInvocationResponse;
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.AllowedOperation;
import org.wso2.carbon.identity.action.execution.model.Error;
import org.wso2.carbon.identity.action.execution.model.Failure;
import org.wso2.carbon.identity.action.execution.model.Incomplete;
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.Success;
Expand Down Expand Up @@ -106,6 +108,7 @@ public boolean isExecutionEnabled(ActionType actionType) {
* @param tenantDomain Tenant domain.
* @return Action execution status.
*/
@Override
public ActionExecutionStatus<?> execute(ActionType actionType, Map<String, Object> eventContext,
String tenantDomain) throws ActionExecutionException {

Expand All @@ -131,6 +134,7 @@ public ActionExecutionStatus<?> execute(ActionType actionType, Map<String, Objec
* @param tenantDomain Tenant domain.
* @return Action execution status.
*/
@Override
public ActionExecutionStatus<?> execute(ActionType actionType, String[] actionIdList,
Map<String, Object> eventContext, String tenantDomain)
throws ActionExecutionException {
Expand Down Expand Up @@ -179,7 +183,7 @@ private Action getActionByActionId(ActionType actionType, String actionId, Strin

try {
return ActionExecutionServiceComponentHolder.getInstance().getActionManagementService().getActionByActionId(
Action.ActionTypes.valueOf(actionType.name()).getActionType(), actionId, tenantDomain);
Action.ActionTypes.valueOf(actionType.name()).getPathParam(), actionId, tenantDomain);
} catch (ActionMgtException e) {
throw new ActionExecutionException("Error occurred while retrieving action by action Id.", e);
}
Expand Down Expand Up @@ -310,6 +314,10 @@ private ActionExecutionStatus<?> processActionResponse(Action action,
return processSuccessResponse(action,
(ActionInvocationSuccessResponse) actionInvocationResponse.getResponse(),
eventContext, actionRequest, actionExecutionResponseProcessor);
} else if (actionInvocationResponse.isIncomplete()) {
return processIncompleteResponse(action,
(ActionInvocationIncompleteResponse) actionInvocationResponse.getResponse(),
eventContext, actionRequest, actionExecutionResponseProcessor);
} else if (actionInvocationResponse.isFailure() && actionInvocationResponse.getResponse() != null) {
return processFailureResponse(action, (ActionInvocationFailureResponse) actionInvocationResponse
.getResponse(), eventContext, actionRequest, actionExecutionResponseProcessor);
Expand All @@ -333,14 +341,35 @@ private ActionExecutionStatus<Success> processSuccessResponse(Action action,
logSuccessResponse(action, successResponse);

List<PerformableOperation> allowedPerformableOperations =
validatePerformableOperations(actionRequest, successResponse, action);
validatePerformableOperations(actionRequest, successResponse.getOperations(), action);
ActionInvocationSuccessResponse.Builder successResponseBuilder =
new ActionInvocationSuccessResponse.Builder().actionStatus(ActionInvocationResponse.Status.SUCCESS)
.operations(allowedPerformableOperations);
.operations(allowedPerformableOperations)
.data(successResponse.getData());
return actionExecutionResponseProcessor.processSuccessResponse(eventContext,
actionRequest.getEvent(), successResponseBuilder.build());
}

private ActionExecutionStatus<Incomplete> processIncompleteResponse(
Action action,
ActionInvocationIncompleteResponse incompleteResponse,
Map<String, Object> eventContext,
ActionExecutionRequest actionRequest,
ActionExecutionResponseProcessor actionExecutionResponseProcessor)
throws ActionExecutionResponseProcessorException {

logIncompleteResponse(action, incompleteResponse);

List<PerformableOperation> allowedPerformableOperations =
validatePerformableOperations(actionRequest, incompleteResponse.getOperations(), action);
ActionInvocationIncompleteResponse.Builder incompleteResponseBuilder =
new ActionInvocationIncompleteResponse.Builder()
.actionStatus(ActionInvocationResponse.Status.INCOMPLETE)
.operations(allowedPerformableOperations);
return actionExecutionResponseProcessor.processIncompleteResponse(eventContext,
actionRequest.getEvent(), incompleteResponseBuilder.build());
}

private ActionExecutionStatus<Error> processErrorResponse(Action action,
ActionInvocationErrorResponse errorResponse,
Map<String, Object> eventContext,
Expand Down Expand Up @@ -388,6 +417,27 @@ private void logSuccessResponse(Action action, ActionInvocationSuccessResponse s
}
}

private void logIncompleteResponse(Action action, ActionInvocationIncompleteResponse incompleteResponse) {

DIAGNOSTIC_LOGGER.logSuccessResponse(action);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? If the same functionality can be reused better rename

if (LOG.isDebugEnabled()) {
try {
String responseBody = serializeIncompleteResponse(incompleteResponse);
LOG.debug(String.format(
"Received incomplete response from API: %s for action type: %s action id: %s with " +
"authentication: %s. Response: %s",
action.getEndpoint().getUri(),
action.getType().getActionType(),
action.getId(),
action.getEndpoint().getAuthentication().getType(),
responseBody));
} catch (JsonProcessingException e) {
LOG.error("Error occurred while deserializing the incomplete response for action: " +
action.getId() + " for action type: " + action.getType().getActionType(), e);
}
}
}

private void logErrorResponse(Action action, ActionInvocationErrorResponse errorResponse) {

DIAGNOSTIC_LOGGER.logErrorResponse(action);
Expand Down Expand Up @@ -459,6 +509,13 @@ private String serializeSuccessResponse(ActionInvocationSuccessResponse response
return objectMapper.writeValueAsString(response);
}

private String serializeIncompleteResponse(ActionInvocationIncompleteResponse response)
throws JsonProcessingException {

ObjectMapper objectMapper = new ObjectMapper();
return objectMapper.writeValueAsString(response);
}

private String serializeErrorResponse(ActionInvocationErrorResponse response) throws JsonProcessingException {

ObjectMapper objectMapper = new ObjectMapper();
Expand All @@ -472,11 +529,14 @@ private String serializeFailureResponse(ActionInvocationFailureResponse response
}

private List<PerformableOperation> validatePerformableOperations(
ActionExecutionRequest request, ActionInvocationSuccessResponse response, Action action) {
ActionExecutionRequest request, List<PerformableOperation> operations, Action action) {

List<AllowedOperation> allowedOperations = request.getAllowedOperations();

List<PerformableOperation> allowedPerformableOperations = response.getOperations().stream()
if (operations == null) {
return new ArrayList<>();
}
List<PerformableOperation> allowedPerformableOperations = operations.stream()
.filter(performableOperation -> allowedOperations.stream()
.anyMatch(allowedOperation -> OperationComparator.compare(allowedOperation,
performableOperation)))
Expand All @@ -486,7 +546,7 @@ private List<PerformableOperation> validatePerformableOperations(
List<String> allowedOps = new ArrayList<>();
List<String> notAllowedOps = new ArrayList<>();

response.getOperations().forEach(operation -> {
operations.forEach(operation -> {
String operationDetails = "Operation: " + operation.getOp() + " Path: " + operation.getPath();
if (allowedPerformableOperations.contains(operation)) {
allowedOps.add(operationDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
* Action Execution Status is the status object that is returned by the Action Executor Service after executing an
* action. It contains the status of the action execution and the response context.
*
* @param <T> Status type (i.e. SUCCESS {@link Success}, FAILED {@link Failure}, ERROR {@link Error})
* @param <T> Status type (i.e. SUCCESS {@link Success}, FAILED {@link Failure}, ERROR {@link Error},
* INCOMPLETE {@link Incomplete})
*/
public abstract class ActionExecutionStatus<T> {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com).
*
* WSO2 LLC. 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.wso2.carbon.identity.action.execution.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;

import java.util.List;

/**
* This class is used to represent the incomplete response of an action invocation.
* This response will contain the list of operations that need to be performed.
*/
@JsonDeserialize(builder = ActionInvocationIncompleteResponse.Builder.class)
public class ActionInvocationIncompleteResponse implements ActionInvocationResponse.APIResponse {

private final ActionInvocationResponse.Status actionStatus;

private final List<PerformableOperation> operations;

private ActionInvocationIncompleteResponse(Builder builder) {

this.actionStatus = builder.actionStatus;
this.operations = builder.operations;
}

@Override
public ActionInvocationResponse.Status getActionStatus() {

return actionStatus;
}

public List<PerformableOperation> getOperations() {

return operations;
}

/**
* This class is used to build the {@link ActionInvocationIncompleteResponse}.
*/
@JsonPOJOBuilder(withPrefix = "")
public static class Builder {

private ActionInvocationResponse.Status actionStatus;
private List<PerformableOperation> operations;

@JsonProperty("actionStatus")
public Builder actionStatus(ActionInvocationResponse.Status actionStatus) {

this.actionStatus = actionStatus;
return this;
}

@JsonProperty("operations")
public Builder operations(@JsonProperty("operations") List<PerformableOperation> operations) {

this.operations = operations;
return this;
}

public ActionInvocationIncompleteResponse build() {

if (this.actionStatus == null) {
throw new IllegalArgumentException("actionStatus must not be null.");
}

if (!ActionInvocationResponse.Status.INCOMPLETE.equals(actionStatus)) {
throw new IllegalArgumentException("actionStatus must be INCOMPLETE.");
}

if (this.operations == null) {
throw new IllegalArgumentException("operations must not be null.");
}

return new ActionInvocationIncompleteResponse(this);
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public boolean isSuccess() {
return Status.SUCCESS.equals(actionStatus);
}

public boolean isIncomplete() {

return Status.INCOMPLETE.equals(actionStatus);
}

public boolean isFailure() {

return Status.FAILED.equals(actionStatus);
Expand All @@ -70,6 +75,7 @@ public String getErrorLog() {
*/
public enum Status {
SUCCESS,
INCOMPLETE,
FAILED,
ERROR
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@
public class ActionInvocationSuccessResponse implements ActionInvocationResponse.APIResponse {

private final ActionInvocationResponse.Status actionStatus;

private final List<PerformableOperation> operations;
private final ExtendedData data;
Copy link
Member

@malithie malithie Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we rename this to some thing like ResponseData, ResponseContext etc.
I prefer ResponseData given the value is data.


private ActionInvocationSuccessResponse(Builder builder) {

this.actionStatus = builder.actionStatus;
this.operations = builder.operations;
this.data = builder.data;
}

@Override
Expand All @@ -52,6 +53,11 @@ public List<PerformableOperation> getOperations() {
return operations;
}

public ExtendedData getData() {

return data;
}

/**
* This class is used to build the {@link ActionInvocationSuccessResponse}.
*/
Expand All @@ -60,6 +66,7 @@ public static class Builder {

private ActionInvocationResponse.Status actionStatus;
private List<PerformableOperation> operations;
private ExtendedData data = null;

@JsonProperty("actionStatus")
public Builder actionStatus(ActionInvocationResponse.Status actionStatus) {
Expand All @@ -75,6 +82,13 @@ public Builder operations(@JsonProperty("operations") List<PerformableOperation>
return this;
}

@JsonProperty("data")
public Builder data(@JsonProperty("data") ExtendedData data) {

this.data = data;
return this;
}

public ActionInvocationSuccessResponse build() {
Comment on lines +86 to 92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the builder now expecting operations to be not null ?
With this it should be either operations or data that should be present


if (this.actionStatus == null) {
Expand Down
Loading
Loading