Skip to content
This repository has been archived by the owner on Jul 23, 2024. It is now read-only.

[FLPATH-424] Return error message with status #411

Merged
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
8 changes: 8 additions & 0 deletions workflow-service-sdk/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1332,17 +1332,22 @@ components:
- null
- null
name: name
message: message
type: TASK
status: FAILED
- works:
- null
- null
name: name
message: message
type: TASK
status: FAILED
workFlowExecutionId: 046b6c7f-0b8a-43b9-b35d-6489e6daee91
message: message
status: FAILED
properties:
message:
type: string
status:
enum:
- FAILED
Expand Down Expand Up @@ -1424,9 +1429,12 @@ components:
- null
- null
name: name
message: message
type: TASK
status: FAILED
properties:
message:
type: string
name:
type: string
status:
Expand Down
1 change: 1 addition & 0 deletions workflow-service-sdk/docs/WorkFlowStatusResponseDTO.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

| Name | Type | Description | Notes |
|------------ | ------------- | ------------- | -------------|
|**message** | **String** | | [optional] |
|**status** | [**StatusEnum**](#StatusEnum) | | [optional] |
|**workFlowExecutionId** | **UUID** | | [optional] |
|**workFlowName** | **String** | | [optional] |
Expand Down
1 change: 1 addition & 0 deletions workflow-service-sdk/docs/WorkStatusResponseDTO.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

| Name | Type | Description | Notes |
|------------ | ------------- | ------------- | -------------|
|**message** | **String** | | [optional] |
|**name** | **String** | | [optional] |
|**status** | [**StatusEnum**](#StatusEnum) | | [optional] |
|**type** | [**TypeEnum**](#TypeEnum) | | [optional] |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
public class WorkFlowStatusResponseDTO {

public static final String SERIALIZED_NAME_MESSAGE = "message";

@SerializedName(SERIALIZED_NAME_MESSAGE)
private String message;

/**
* Gets or Sets status
*/
Expand Down Expand Up @@ -120,6 +125,26 @@ public StatusEnum read(final JsonReader jsonReader) throws IOException {
public WorkFlowStatusResponseDTO() {
}

public WorkFlowStatusResponseDTO message(String message) {

this.message = message;
return this;
}

/**
* Get message
* @return message
**/
@javax.annotation.Nullable

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

public WorkFlowStatusResponseDTO status(StatusEnum status) {

this.status = status;
Expand Down Expand Up @@ -217,21 +242,23 @@ public boolean equals(Object o) {
return false;
}
WorkFlowStatusResponseDTO workFlowStatusResponseDTO = (WorkFlowStatusResponseDTO) o;
return Objects.equals(this.status, workFlowStatusResponseDTO.status)
return Objects.equals(this.message, workFlowStatusResponseDTO.message)
&& Objects.equals(this.status, workFlowStatusResponseDTO.status)
&& Objects.equals(this.workFlowExecutionId, workFlowStatusResponseDTO.workFlowExecutionId)
&& Objects.equals(this.workFlowName, workFlowStatusResponseDTO.workFlowName)
&& Objects.equals(this.works, workFlowStatusResponseDTO.works);
}

@Override
public int hashCode() {
return Objects.hash(status, workFlowExecutionId, workFlowName, works);
return Objects.hash(message, status, workFlowExecutionId, workFlowName, works);
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("class WorkFlowStatusResponseDTO {\n");
sb.append(" message: ").append(toIndentedString(message)).append("\n");
sb.append(" status: ").append(toIndentedString(status)).append("\n");
sb.append(" workFlowExecutionId: ").append(toIndentedString(workFlowExecutionId)).append("\n");
sb.append(" workFlowName: ").append(toIndentedString(workFlowName)).append("\n");
Expand All @@ -258,6 +285,7 @@ private String toIndentedString(Object o) {
static {
// a set of all properties/fields (JSON key names)
openapiFields = new HashSet<String>();
openapiFields.add("message");
openapiFields.add("status");
openapiFields.add("workFlowExecutionId");
openapiFields.add("workFlowName");
Expand Down Expand Up @@ -297,6 +325,12 @@ public static void validateJsonObject(JsonObject jsonObj) throws IOException {
entry.getKey(), jsonObj.toString()));
}
}
if ((jsonObj.get("message") != null && !jsonObj.get("message").isJsonNull())
&& !jsonObj.get("message").isJsonPrimitive()) {
throw new IllegalArgumentException(
String.format("Expected the field `message` to be a primitive type in the JSON string but got `%s`",
jsonObj.get("message").toString()));
}
if ((jsonObj.get("status") != null && !jsonObj.get("status").isJsonNull())
&& !jsonObj.get("status").isJsonPrimitive()) {
throw new IllegalArgumentException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
public class WorkStatusResponseDTO {

public static final String SERIALIZED_NAME_MESSAGE = "message";

@SerializedName(SERIALIZED_NAME_MESSAGE)
private String message;

public static final String SERIALIZED_NAME_NAME = "name";

@SerializedName(SERIALIZED_NAME_NAME)
Expand Down Expand Up @@ -170,6 +175,26 @@ public TypeEnum read(final JsonReader jsonReader) throws IOException {
public WorkStatusResponseDTO() {
}

public WorkStatusResponseDTO message(String message) {

this.message = message;
return this;
}

/**
* Get message
* @return message
**/
@javax.annotation.Nullable

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

public WorkStatusResponseDTO name(String name) {

this.name = name;
Expand Down Expand Up @@ -267,21 +292,23 @@ public boolean equals(Object o) {
return false;
}
WorkStatusResponseDTO workStatusResponseDTO = (WorkStatusResponseDTO) o;
return Objects.equals(this.name, workStatusResponseDTO.name)
return Objects.equals(this.message, workStatusResponseDTO.message)
&& Objects.equals(this.name, workStatusResponseDTO.name)
&& Objects.equals(this.status, workStatusResponseDTO.status)
&& Objects.equals(this.type, workStatusResponseDTO.type)
&& Objects.equals(this.works, workStatusResponseDTO.works);
}

@Override
public int hashCode() {
return Objects.hash(name, status, type, works);
return Objects.hash(message, name, status, type, works);
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("class WorkStatusResponseDTO {\n");
sb.append(" message: ").append(toIndentedString(message)).append("\n");
sb.append(" name: ").append(toIndentedString(name)).append("\n");
sb.append(" status: ").append(toIndentedString(status)).append("\n");
sb.append(" type: ").append(toIndentedString(type)).append("\n");
Expand All @@ -308,6 +335,7 @@ private String toIndentedString(Object o) {
static {
// a set of all properties/fields (JSON key names)
openapiFields = new HashSet<String>();
openapiFields.add("message");
openapiFields.add("name");
openapiFields.add("status");
openapiFields.add("type");
Expand Down Expand Up @@ -344,6 +372,12 @@ public static void validateJsonObject(JsonObject jsonObj) throws IOException {
entry.getKey(), jsonObj.toString()));
}
}
if ((jsonObj.get("message") != null && !jsonObj.get("message").isJsonNull())
&& !jsonObj.get("message").isJsonPrimitive()) {
throw new IllegalArgumentException(
String.format("Expected the field `message` to be a primitive type in the JSON string but got `%s`",
jsonObj.get("message").toString()));
}
if ((jsonObj.get("name") != null && !jsonObj.get("name").isJsonNull())
&& !jsonObj.get("name").isJsonPrimitive()) {
throw new IllegalArgumentException(
Expand Down
6 changes: 6 additions & 0 deletions workflow-service/generated/openapi/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,9 @@
"WorkFlowStatusResponseDTO" : {
"type" : "object",
"properties" : {
"message" : {
"type" : "string"
},
"status" : {
"type" : "string",
"enum" : [ "FAILED", "COMPLETED", "IN_PROGRESS", "REJECTED", "PENDING" ]
Expand Down Expand Up @@ -1457,6 +1460,9 @@
"WorkStatusResponseDTO" : {
"type" : "object",
"properties" : {
"message" : {
"type" : "string"
},
"name" : {
"type" : "string"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public WorkReport executeAroundAdvice(ProceedingJoinPoint proceedingJoinPoint, W
}
catch (Throwable e) {
log.error("Workflow {} has failed! with error: {}", workflowName, e.getMessage());
report = new DefaultWorkReport(WorkStatus.FAILED, workContext);
report = new DefaultWorkReport(WorkStatus.FAILED, workContext, e);
}

return executionHandler.handlePostWorkFlowExecution(report, (WorkFlow) proceedingJoinPoint.getTarget());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ public WorkReport handlePostWorkFlowExecution(WorkReport report, WorkFlow workFl
// update workflow execution entity
workFlowExecution.setStatus(report.getStatus());
workFlowExecution.setEndDate(new Date());
if (report.getError() != null) {
workFlowExecution.setMessage(report.getError().getMessage());
}

WorkFlowPostInterceptor postExecutor = createPostExecutor(workFlow, report.getStatus());
WorkReport workReport = postExecutor.handlePostWorkFlowExecution();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ else if (WorkStatus.FAILED != workFlowTaskExecution.getStatus()) {
}
catch (Throwable e) {
log.error("Workflow task execution {} has failed! error message: {}", workFlowTaskName, e.getMessage());
report = new DefaultWorkReport(WorkStatus.FAILED, workContext);
report = new DefaultWorkReport(WorkStatus.FAILED, workContext, e);
}

WorkContextDelegate.write(workContext, WorkContextDelegate.ProcessType.WORKFLOW_TASK_EXECUTION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class WorkFlowStatusResponseDTO {

private WorkStatus status;

private String message;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
private List<WorkStatusResponseDTO> works;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public class WorkStatusResponseDTO {

private WorkStatus status;

private String message;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JsonInclude(JsonInclude.Include.NON_EMPTY) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API contract/spec defines that the response contains a message field, this field shall be present for parodos to fulfil the contract.
This way, the API consumer will not have to worry about non-existing key when processing the response

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it can be optional. we can address it later


@JsonProperty("works")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
private List<WorkStatusResponseDTO> works;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public class WorkFlowExecution extends AbstractEntity {

private WorkStatus status;

private String message;

@Column(updatable = false)
private Date startDate;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ private void buildWorkFlowStatusDTO(WorkFlowExecution workFlowExecution, WorkFlo
CopyOnWriteArrayList<WorkStatusResponseDTO> workStatusResponseDTOList,
Map<String, Integer> workFlowWorkStartIndexMap) {
// build workflow status DTO
workStatusResponseDTOList.add(WorkStatusResponseDTO.builder().name(workFlowDefinition.getName())
.type(WorkType.WORKFLOW)
.status(WorkStatus.IN_PROGRESS.equals(workFlowExecution.getStatus()) ? WorkStatus.PENDING
: WorkStatus.valueOf(workFlowExecution.getStatus().name()))
.workExecution(workFlowExecution).numberOfWorks(workFlowDefinition.getNumberOfWorks())
.works(new ArrayList<>()).build());
workStatusResponseDTOList
.add(WorkStatusResponseDTO.builder().name(workFlowDefinition.getName()).type(WorkType.WORKFLOW)
.status(WorkStatus.IN_PROGRESS.equals(workFlowExecution.getStatus()) ? WorkStatus.PENDING
: WorkStatus.valueOf(workFlowExecution.getStatus().name()))
.message(workFlowExecution.getMessage()).workExecution(workFlowExecution)
.numberOfWorks(workFlowDefinition.getNumberOfWorks()).works(new ArrayList<>()).build());

// save the start index of the workflow's works
workFlowWorkStartIndexMap.put(workFlowDefinition.getName(), workStatusResponseDTOList.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public WorkFlowStatusResponseDTO getWorkFlowStatus(UUID workFlowExecutionId) {

return WorkFlowStatusResponseDTO.builder().workFlowExecutionId(workFlowExecution.getId())
.workFlowName(workFlowDefinition.getName()).status(workFlowExecution.getStatus())
.works(workFlowWorksStatusResponseDTOs).build();
.message(workFlowExecution.getMessage()).works(workFlowWorksStatusResponseDTOs).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
<constraints primaryKey="true" nullable="false"/>
</column>
<column name="status" type="varchar(255)"/>
<column name="message" type="varchar(4000)"/>
<column name="start_date" type="timestamp" defaultValueComputed="CURRENT_TIMESTAMP"/>
<column name="end_date" type="timestamp" defaultValueComputed="CURRENT_TIMESTAMP"/>
<column name="arguments" type="varchar(4000)"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
Expand Down Expand Up @@ -140,11 +141,43 @@ public void testHandleCompletePostWorkFlowExecution() {
// then
verify(workFlowService, times(0)).saveWorkFlow(any(UUID.class), any(UUID.class), any(),
eq(WorkStatus.IN_PROGRESS), any(), anyString());
verify(workFlowService, times(1)).updateWorkFlow(argThat(we -> we.getMessage() == null));
verify(workFlowSchedulerService, times(1)).stop(any(), any(), any(WorkFlow.class));
verify(workFlowContinuationServiceImpl, times(1)).continueWorkFlow(any(ExecutionContext.class));
assertEquals(result.getStatus(), report.getStatus());
}

@Test
public void testHandlePostWorkFlowExecutionWithFailedStatus() {
// given
WorkReport report = mock(WorkReport.class);

// when
when(report.getStatus()).thenReturn(WorkStatus.FAILED);
when(report.getError()).thenReturn(new Throwable("Error while executing"));
when(workContext.get(WorkContextDelegate.buildKey(WorkContextDelegate.ProcessType.WORKFLOW_DEFINITION,
WorkContextDelegate.Resource.NAME))).thenReturn(TEST_WORKFLOW_NAME);
WorkFlow workFlow = mock(WorkFlow.class);
when(workFlow.getName()).thenReturn(TEST_WORKFLOW_NAME);
when(workFlowDefinition.getType()).thenReturn(WorkFlowType.CHECKER);
when(workFlowDefinition.getCheckerWorkFlowDefinition())
.thenReturn(mock(WorkFlowCheckerMappingDefinition.class));
when(mainWorkFlowExecution.getId()).thenReturn(UUID.randomUUID());
when(mainWorkFlowExecution.getWorkFlowDefinition()).thenReturn(workFlowDefinition);

WorkFlowExecution workFlowExecution = interceptor.handlePreWorkFlowExecution();
assertNotNull(workFlowExecution);
WorkReport result = interceptor.handlePostWorkFlowExecution(report, workFlow);

// then
verify(workFlowService, times(0)).saveWorkFlow(any(UUID.class), any(UUID.class), any(),
eq(WorkStatus.IN_PROGRESS), any(), anyString());
verify(workFlowService, times(1))
.updateWorkFlow(argThat(we -> we.getMessage().equals(report.getError().getMessage())));
assertEquals(result.getStatus(), report.getStatus());
assertEquals(result.getError(), report.getError());
}

private User createUser() {
User user = User.builder().username(UUID.randomUUID().toString()).build();
user.setId(UUID.randomUUID());
Expand Down
Loading