Generalize workflow request payload and call external mediator #70
Conversation
|
|
...dentity.workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/BPELDeployer.java
Show resolved
Hide resolved
...tity.workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/RequestExecutor.java
Outdated
Show resolved
Hide resolved
| @@ -87,10 +120,19 @@ public void execute(WorkflowRequest workFlowRequest) throws WorkflowException { | |||
| validateExecutionParams(); | |||
| OMElement requestBody = WorkflowRequestBuilder.buildXMLRequest(workFlowRequest, this.parameterList); | |||
| try { | |||
| callService(requestBody); | |||
| String templateId = getTemplateIdByWorkflowId(parameterList.get(0).getWorkflowId()); | |||
There was a problem hiding this comment.
Check the parameterList is not empty by using CollectionUtils.isNotEmpty(this.parameterList) and then invoke the getTemplateIdByWorkflowId method
There was a problem hiding this comment.
addressed the comment ref:641aaf1
...tity.workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/RequestExecutor.java
Outdated
Show resolved
Hide resolved
| @@ -108,6 +150,97 @@ private void validateExecutionParams() throws InternalWorkflowException { | |||
| throw new InternalWorkflowException("Init params for the BPELExecutor is null."); | |||
| } | |||
| } | |||
| private String getExternalWorkflowId(String workflowId) throws WorkflowException { | |||
There was a problem hiding this comment.
Please check new lines between methods, new line in the 1st line of the method body and remove unnecessary new lines. (Check all the places)
|
|
||
| while (workflowDetails.hasNext()) { | ||
| OMElementImpl workflowDetail = (OMElementImpl) workflowDetails.next(); | ||
| if (PROCESS_UUID.equals(workflowDetail.getLocalName())) { |
There was a problem hiding this comment.
We can improve the code to use switch case statements instead of multiple if conditions.
There was a problem hiding this comment.
addressed the comment ref:641aaf1
...tity.workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/RequestExecutor.java
Outdated
Show resolved
Hide resolved
...tity.workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/RequestExecutor.java
Outdated
Show resolved
Hide resolved
...low.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/constant/WorkflowConstant.java
Show resolved
Hide resolved
| package org.wso2.carbon.identity.workflow.impl.util.model; | ||
|
|
||
| public class Variable { | ||
| private String name; |
There was a problem hiding this comment.
Add new lines. check all the places in this class
There was a problem hiding this comment.
addressed the comment ref:641aaf1
....workflow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/util/model/Variable.java
Outdated
Show resolved
Hide resolved
...ow.impl/src/main/java/org/wso2/carbon/identity/workflow/impl/util/model/WorkFlowRequest.java
Outdated
Show resolved
Hide resolved
pom.xml
Outdated
| <dependency> | ||
| <groupId>com.google.code.gson</groupId> | ||
| <artifactId>gson</artifactId> | ||
| <version>2.9.0</version> |
There was a problem hiding this comment.
Let's define the gson version under and refer here. Check how other dependencies do that.
| @@ -0,0 +1,52 @@ | |||
|
|
|||
|
|
|||
malithiperera
force-pushed
the
call-external-mediator
branch
from
7a805e8 to
ec92736
Compare
| @@ -251,7 +256,7 @@ | |||
| <commons-io.wso2.version>2.4.0.wso2v1</commons-io.wso2.version> | |||
| <commons-lang.wso2.version>2.6.0.wso2v1</commons-lang.wso2.version> | |||
|
|
|||
| <carbon.identity.framework.version>5.25.36</carbon.identity.framework.version> | |||
| <carbon.identity.framework.version>5.25.145-SNAPSHOT</carbon.identity.framework.version> | |||
There was a problem hiding this comment.
need to update this once framework PR is merged : ref
| generateAndDeployArtifacts(); | ||
|
|
| @@ -0,0 +1,207 @@ | |||
| /* | |||
There was a problem hiding this comment.
update the license header
| import org.wso2.carbon.identity.workflow.mgt.util.WorkflowManagementUtil; | ||
| import org.wso2.carbon.identity.workflow.mgt.workflow.WorkFlowExecutor; | ||
|
|
||
| import java.io.*; |
| import static javax.ws.rs.HttpMethod.POST; | ||
| import static javax.ws.rs.core.MediaType.APPLICATION_JSON; | ||
|
|
||
| public class WorkflowMediatorRequestExecutor implements WorkFlowExecutor { |
There was a problem hiding this comment.
this class has multiple formatting issues. please make sure these are addressed throughout the PR
|
|
||
| public class WorkflowMediatorRequestExecutor implements WorkFlowExecutor { | ||
|
|
||
| private static final Log log = LogFactory.getLog(WorkflowMediatorRequestExecutor.class); |
There was a problem hiding this comment.
refactor the class name MediatorRequestExecutor
|
|
||
| public class WorkflowMediatorRequestExecutor implements WorkFlowExecutor { | ||
|
|
||
| private static final Log log = LogFactory.getLog(WorkflowMediatorRequestExecutor.class); |
| private static final String EXECUTOR_NAME = "Workflow Mediator Request Executor"; | ||
|
|
||
| private static final Gson gson = new Gson(); | ||
|
|
There was a problem hiding this comment.
remove unwanted new lines
| bpsProfile = WorkflowImplServiceDataHolder.getInstance().getWorkflowImplService().getBPSProfile | ||
| (bpsProfileName, tenantId); | ||
| } catch (WorkflowImplException e) { | ||
| String errorMsg = "Error occurred while reading bps profile, " + e.getMessage(); |
There was a problem hiding this comment.
renamed all bps references
|
|
||
|
|
There was a problem hiding this comment.
unwanted new lines. check all applicable places
| this.parameterList = parameterList; | ||
|
|
||
| Parameter bpsProfileParameter = WorkflowManagementUtil | ||
| .getParameter(parameterList, WFImplConstant.ParameterName.BPS_PROFILE, WFConstant.ParameterHolder |
There was a problem hiding this comment.
what is the reason to use WFImplConstant.ParameterName.BPS_PROFILE here?
|
|
||
| con.setRequestMethod(POST); | ||
| con.setRequestProperty(CONTENT_TYPE, APPLICATION_JSON); | ||
| con.setRequestProperty(MEDIATOR_API_KEY, bpsProfile.getApiKey()); |
There was a problem hiding this comment.
where is getApiKey() defined?
| private String request_id; | ||
| private String workflow_iD; | ||
| private List<WorkflowVariable> workflow_Workflow_variables; |
There was a problem hiding this comment.
| private String request_id; | |
| private String workflow_iD; | |
| private List<WorkflowVariable> workflow_Workflow_variables; | |
| private String requestId; | |
| private String workflowId; | |
| private List<WorkflowVariable> workflowParameters; |
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
please format the file properly
| public static final String APPROVAL_TEMPLATE_NAME = "External Workflow Template"; | ||
|
|
||
| public static final String TEMPLATE_ID = "ExternalWorkflowTemplate"; | ||
|
|
||
| public static final String BPS_PROFILE = "BPSProfile"; | ||
|
|
||
| public static final String HT_SUBJECT = "HTSubject"; |
There was a problem hiding this comment.
where are these constants used?
| @@ -0,0 +1,68 @@ | |||
|
|
|||
| @@ -0,0 +1,46 @@ | |||
|
|
|||
| package org.wso2.carbon.identity.workflow.impl.util.model; | ||
|
|
||
| public class WorkFlowResponse { | ||
| private int statusCode; |
|
|
||
| package org.wso2.carbon.identity.workflow.impl.util.model; | ||
|
|
||
| public class WorkFlowResponse { |
| public void setStatusMessage(String statusMessage) { | ||
| this.statusMessage = statusMessage; | ||
| } | ||
| } |
|
|
||
|
|
There was a problem hiding this comment.
remove these new lines. fix this in all places
| * Define the Variable of the workflow request | ||
| */ | ||
|
|
||
| public class WorkflowVariable { |
There was a problem hiding this comment.
| public class WorkflowVariable { | |
| public class WorkflowParameter { |
Proposed changes in this pull request
Generalize the workflow request payload which we send to the external workflow mediator and implement the call mediator service.