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

combine task status with its checker #247

Merged
merged 1 commit into from
Apr 19, 2023
Merged

combine task status with its checker #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions parodos-model-api/src/main/java/com/redhat/parodos/workflow/annotation/Parameter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@

boolean optional();

String[] selectOptions() default {};

}
2 changes: 1 addition & 1 deletion parodos-model-api/src/main/java/com/redhat/parodos/workflow/enums/ParodosWorkStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@
*/
public enum ParodosWorkStatus {

FAILED, COMPLETED, PENDING, IN_PROGRESS
FAILED, COMPLETED, PENDING, IN_PROGRESS, REJECTED

}
17 changes: 13 additions & 4 deletions workflow-service-sdk/api/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ components:
outputs:
- EXCEPTION
- EXCEPTION
processingType: processingType
processingType: SEQUENTIAL
works:
- null
- null
Expand Down Expand Up @@ -331,6 +331,10 @@ components:
type: object
type: object
processingType:
enum:
- SEQUENTIAL
- PARALLEL
- OTHER
type: string
workType:
type: string
Expand All @@ -354,12 +358,12 @@ components:
type: object
WorkFlowDefinitionResponseDTO:
example:
processingType: processingType
processingType: SEQUENTIAL
works:
- outputs:
- EXCEPTION
- EXCEPTION
processingType: processingType
processingType: SEQUENTIAL
works:
- null
- null
Expand All @@ -373,7 +377,7 @@ components:
- outputs:
- EXCEPTION
- EXCEPTION
processingType: processingType
processingType: SEQUENTIAL
works:
- null
- null
Expand Down Expand Up @@ -416,6 +420,10 @@ components:
type: object
type: object
processingType:
enum:
- SEQUENTIAL
- PARALLEL
- OTHER
type: string
properties:
$ref: '#/components/schemas/WorkFlowPropertiesDefinitionDTO'
Expand Down Expand Up @@ -766,6 +774,7 @@ components:
- COMPLETED
- PENDING
- IN_PROGRESS
- REJECTED
type: string
type:
enum:
Expand Down
12 changes: 11 additions & 1 deletion workflow-service-sdk/docs/WorkDefinitionResponseDTO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Name | Type | Description | Notes
**name** | **String** | | [optional]
**outputs** | [**List<OutputsEnum>**](#List<OutputsEnum>) | | [optional]
**parameters** | **Map<String, Map<String, Object>>** | | [optional]
**processingType** | **String** | | [optional]
**processingType** | [**ProcessingTypeEnum**](#ProcessingTypeEnum) | | [optional]
**workType** | **String** | | [optional]
**works** | [**List<WorkDefinitionResponseDTO>**](WorkDefinitionResponseDTO.md) | | [optional]

Expand All @@ -29,3 +29,13 @@ OTHER | "OTHER"



## Enum: ProcessingTypeEnum

Name | Value
---- | -----
SEQUENTIAL | "SEQUENTIAL"
PARALLEL | "PARALLEL"
OTHER | "OTHER"



12 changes: 11 additions & 1 deletion workflow-service-sdk/docs/WorkFlowDefinitionResponseDTO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,20 @@ Name | Type | Description | Notes
**modifyDate** | **Date** | | [optional]
**name** | **String** | | [optional]
**parameters** | **Map<String, Map<String, Object>>** | | [optional]
**processingType** | **String** | | [optional]
**processingType** | [**ProcessingTypeEnum**](#ProcessingTypeEnum) | | [optional]
**properties** | [**WorkFlowPropertiesDefinitionDTO**](WorkFlowPropertiesDefinitionDTO.md) | | [optional]
**type** | **String** | | [optional]
**works** | [**List<WorkDefinitionResponseDTO>**](WorkDefinitionResponseDTO.md) | | [optional]



## Enum: ProcessingTypeEnum

Name | Value
---- | -----
SEQUENTIAL | "SEQUENTIAL"
PARALLEL | "PARALLEL"
OTHER | "OTHER"



1 change: 1 addition & 0 deletions workflow-service-sdk/docs/WorkStatusResponseDTO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ FAILED | "FAILED"
COMPLETED | "COMPLETED"
PENDING | "PENDING"
IN_PROGRESS | "IN_PROGRESS"
REJECTED | "REJECTED"



Expand Down
61 changes: 57 additions & 4 deletions ...low-service-sdk/src/main/java/com/redhat/parodos/sdk/model/WorkDefinitionResponseDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,63 @@ public OutputsEnum read(final JsonReader jsonReader) throws IOException {
@SerializedName(SERIALIZED_NAME_PARAMETERS)
private Map<String, Map<String, Object>> parameters = null;

/**
* Gets or Sets processingType
*/
@JsonAdapter(ProcessingTypeEnum.Adapter.class)
public enum ProcessingTypeEnum {

SEQUENTIAL("SEQUENTIAL"),

PARALLEL("PARALLEL"),

OTHER("OTHER");

private String value;

ProcessingTypeEnum(String value) {
this.value = value;
}

public String getValue() {
return value;
}

@Override
public String toString() {
return String.valueOf(value);
}

public static ProcessingTypeEnum fromValue(String value) {
for (ProcessingTypeEnum b : ProcessingTypeEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");
}

public static class Adapter extends TypeAdapter<ProcessingTypeEnum> {

@Override
public void write(final JsonWriter jsonWriter, final ProcessingTypeEnum enumeration) throws IOException {
jsonWriter.value(enumeration.getValue());
}

@Override
public ProcessingTypeEnum read(final JsonReader jsonReader) throws IOException {
String value = jsonReader.nextString();
return ProcessingTypeEnum.fromValue(value);
}

}

}

public static final String SERIALIZED_NAME_PROCESSING_TYPE = "processingType";

@SerializedName(SERIALIZED_NAME_PROCESSING_TYPE)
private String processingType;
private ProcessingTypeEnum processingType;

public static final String SERIALIZED_NAME_WORK_TYPE = "workType";

Expand Down Expand Up @@ -255,7 +308,7 @@ public void setParameters(Map<String, Map<String, Object>> parameters) {
this.parameters = parameters;
}

public WorkDefinitionResponseDTO processingType(String processingType) {
public WorkDefinitionResponseDTO processingType(ProcessingTypeEnum processingType) {

this.processingType = processingType;
return this;
Expand All @@ -268,11 +321,11 @@ public WorkDefinitionResponseDTO processingType(String processingType) {
@javax.annotation.Nullable
@ApiModelProperty(value = "")

public String getProcessingType() {
public ProcessingTypeEnum getProcessingType() {
return processingType;
}

public void setProcessingType(String processingType) {
public void setProcessingType(ProcessingTypeEnum processingType) {
this.processingType = processingType;
}

Expand Down
61 changes: 57 additions & 4 deletions ...service-sdk/src/main/java/com/redhat/parodos/sdk/model/WorkFlowDefinitionResponseDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,63 @@ public class WorkFlowDefinitionResponseDTO {
@SerializedName(SERIALIZED_NAME_PARAMETERS)
private Map<String, Map<String, Object>> parameters = null;

/**
* Gets or Sets processingType
*/
@JsonAdapter(ProcessingTypeEnum.Adapter.class)
public enum ProcessingTypeEnum {

SEQUENTIAL("SEQUENTIAL"),

PARALLEL("PARALLEL"),

OTHER("OTHER");

private String value;

ProcessingTypeEnum(String value) {
this.value = value;
}

public String getValue() {
return value;
}

@Override
public String toString() {
return String.valueOf(value);
}

public static ProcessingTypeEnum fromValue(String value) {
for (ProcessingTypeEnum b : ProcessingTypeEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");
}

public static class Adapter extends TypeAdapter<ProcessingTypeEnum> {

@Override
public void write(final JsonWriter jsonWriter, final ProcessingTypeEnum enumeration) throws IOException {
jsonWriter.value(enumeration.getValue());
}

@Override
public ProcessingTypeEnum read(final JsonReader jsonReader) throws IOException {
String value = jsonReader.nextString();
return ProcessingTypeEnum.fromValue(value);
}

}

}

public static final String SERIALIZED_NAME_PROCESSING_TYPE = "processingType";

@SerializedName(SERIALIZED_NAME_PROCESSING_TYPE)
private String processingType;
private ProcessingTypeEnum processingType;

public static final String SERIALIZED_NAME_PROPERTIES = "properties";

Expand Down Expand Up @@ -227,7 +280,7 @@ public void setParameters(Map<String, Map<String, Object>> parameters) {
this.parameters = parameters;
}

public WorkFlowDefinitionResponseDTO processingType(String processingType) {
public WorkFlowDefinitionResponseDTO processingType(ProcessingTypeEnum processingType) {

this.processingType = processingType;
return this;
Expand All @@ -240,11 +293,11 @@ public WorkFlowDefinitionResponseDTO processingType(String processingType) {
@javax.annotation.Nullable
@ApiModelProperty(value = "")

public String getProcessingType() {
public ProcessingTypeEnum getProcessingType() {
return processingType;
}

public void setProcessingType(String processingType) {
public void setProcessingType(ProcessingTypeEnum processingType) {
this.processingType = processingType;
}

Expand Down
4 changes: 3 additions & 1 deletion workflow-service-sdk/src/main/java/com/redhat/parodos/sdk/model/WorkStatusResponseDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ public enum StatusEnum {

PENDING("PENDING"),

IN_PROGRESS("IN_PROGRESS");
IN_PROGRESS("IN_PROGRESS"),

REJECTED("REJECTED");

private String value;

Expand Down
2 changes: 1 addition & 1 deletion workflow-service/generated/openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@
},
"status" : {
"type" : "string",
"enum" : [ "FAILED", "COMPLETED", "PENDING", "IN_PROGRESS" ]
"enum" : [ "FAILED", "COMPLETED", "PENDING", "IN_PROGRESS", "REJECTED" ]
},
"type" : {
"type" : "string",
Expand Down
2 changes: 2 additions & 0 deletions ...in/java/com/redhat/parodos/workflow/definition/service/WorkFlowDefinitionServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ private void getWorksFromWorkDefinition(List<WorkFlowWorkDefinition> workFlowWor
return;
}
responseDTOs.add(WorkDefinitionResponseDTO.fromWorkFlowTaskDefinition(wdt.get()));
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this break is not needed at all, n?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need it to avoid the errors (workflow with id not found) when app starts

Copy link
Contributor Author

@RichardW98 RichardW98 Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

in java, if no break then it continues to the case after it regardless if matches

case WORKFLOW:
Optional<WorkFlowDefinition> wd = workFlowDefinitionRepository
.findById(workFlowWorkDefinition.getWorkDefinitionId());
Expand All @@ -260,6 +261,7 @@ private void getWorksFromWorkDefinition(List<WorkFlowWorkDefinition> workFlowWor

responseDTOs.add(WorkDefinitionResponseDTO.fromWorkFlowDefinitionEntity(wd.get(),
wdWorkFlowWorkDependencies));
break;
default:
return;
}
Expand Down
29 changes: 22 additions & 7 deletions .../src/main/java/com/redhat/parodos/workflow/execution/service/WorkFlowServiceDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.redhat.parodos.workflow.execution.entity.WorkFlowTaskExecution;
import com.redhat.parodos.workflow.execution.repository.WorkFlowRepository;
import com.redhat.parodos.workflow.execution.repository.WorkFlowTaskRepository;
import com.redhat.parodos.workflow.task.enums.WorkFlowTaskStatus;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
Expand Down Expand Up @@ -148,12 +147,18 @@ private WorkStatusResponseDTO getWorkStatusResponseDTOFromWorkFlow(WorkFlowWorkD

WorkFlowExecution workExecution = workFlowRepository.findFirstByMainWorkFlowExecutionAndWorkFlowDefinitionId(
workFlowExecution, workFlowWorkDefinition.getWorkDefinitionId());

/*
* the workflow execution might be null when there is pending checker before it
*/
ParodosWorkStatus workStatus = workExecution == null
|| WorkFlowStatus.IN_PROGRESS.equals(workExecution.getStatus()) ? ParodosWorkStatus.PENDING
: ParodosWorkStatus.valueOf(workExecution.getStatus().name());
ParodosWorkStatus workStatus;

if (workExecution == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

switch?

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 logic here has some redundency, I have fixed it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

workStatus = ParodosWorkStatus.PENDING;
}
else {
workStatus = ParodosWorkStatus.valueOf(workExecution.getStatus().name());
}

return WorkStatusResponseDTO.builder().name(workFlowDefinition.getName()).type(WorkType.WORKFLOW)
.status(workStatus).works(new ArrayList<>()).workExecution(workExecution)
Expand All @@ -175,9 +180,19 @@ private WorkStatusResponseDTO getWorkStatusResponseDTOFromWorkFlowTask(
ParodosWorkStatus workStatus = ParodosWorkStatus.PENDING;

if (workFlowTaskExecutionOptional.isPresent()) {
workStatus = WorkFlowTaskStatus.IN_PROGRESS.equals(workFlowTaskExecutionOptional.get().getStatus())
? ParodosWorkStatus.PENDING
: ParodosWorkStatus.valueOf(workFlowTaskExecutionOptional.get().getStatus().name());
workStatus = ParodosWorkStatus.valueOf(workFlowTaskExecutionOptional.get().getStatus().name());
if (workFlowTaskDefinition.getWorkFlowCheckerMappingDefinition() != null) {
workStatus = Optional
.ofNullable(workFlowRepository.findFirstByMainWorkFlowExecutionAndWorkFlowDefinitionId(
workFlowExecution.getMainWorkFlowExecution(),
workFlowTaskDefinition
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you use the service instead of the repository, definitions are in another package, and service should be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just found there will be cyclic dependency if I use workflowService here, as workflowDelegate is also dependent of workflowService: https://github.com/RichardW98/parodos/blob/combine-task-checker-status/workflow-service/src/main/java/com/redhat/parodos/workflow/execution/service/WorkFlowServiceImpl.java#L72

Copy link
Contributor Author

Choose a reason for hiding this comment

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

personally I think workflowService should depend on its delegate but not in reverse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created a ticket in our board to rearrange dependencies: https://issues.redhat.com/browse/FLPATH-290

.getWorkFlowCheckerMappingDefinition().getCheckWorkFlow().getId()))
.map(WorkFlowExecution::getStatus)
.map(checkerStatus -> WorkFlowStatus.FAILED.equals(checkerStatus)
? ParodosWorkStatus.IN_PROGRESS : ParodosWorkStatus.valueOf(checkerStatus.name()))
.orElse(ParodosWorkStatus.COMPLETED.equals(workStatus) ? ParodosWorkStatus.IN_PROGRESS
: workStatus);
}
}

return WorkStatusResponseDTO.builder().name(workFlowTaskDefinition.getName()).type(WorkType.TASK)
Expand Down
Loading