fix(integration): external approval is always created when initiating…

… a ticket (#140)

* fix: external approval

* response to CR

* refine

* response to CR

* fix: cancel external approval

* test

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowApprovalInstanceTest.java

@@ -204,7 +204,7 @@ public void approve_approveWithComment_approveSucceed() {
 
     private FlowApprovalInstance createApprovalInstance(boolean startEndPoint, boolean endEndPoint, String name,
             String activityId, Long flowInstanceId) {
-        FlowApprovalInstance instance = new FlowApprovalInstance(1L, flowInstanceId, null, null, 12, startEndPoint,
+        FlowApprovalInstance instance = new FlowApprovalInstance(1L, flowInstanceId, null, 12, startEndPoint,
                 endEndPoint, false, flowableAdaptor, taskService, formService, new LocalEventPublisher(),
                 authenticationFacade, nodeRepository, sequenceRepository, userTaskInstanceRepository);
         instance.setName(name);

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowFactoryTest.java

@@ -123,7 +123,7 @@ public void generateFlowGatewayInstance_createNewOne_returnFlowGatewayInstance()
     public void generateFlowApprovalInstance_createNewOne_returnFlowApprovalInstance() {
         FlowInstance flowInstance = flowFactory.generateFlowInstance("Test FlowInstance", "Test");
         FlowApprovalInstance approvalInstance =
-                flowFactory.generateFlowApprovalInstance(flowInstance.getId(), true, false, false, 15, null, null);
+                flowFactory.generateFlowApprovalInstance(flowInstance.getId(), true, false, false, 15, null);
 
         Assert.assertEquals((long) flowInstance.getId(), approvalInstance.getFlowInstanceId());
     }
@@ -190,7 +190,7 @@ private FlowTaskInstance createTaskInstance(Long flowInstanceId, ExecutionStrate
 
     private FlowApprovalInstance createApprovalInstance(Long flowInstanceId) {
         return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId, null,
-                null, 10, false, false, false, flowAdaptor, taskService, formService, new LocalEventPublisher(),
+                10, false, false, false, flowAdaptor, taskService, formService, new LocalEventPublisher(),
                 authenticationFacade, nodeRepository, sequenceRepository, userTaskInstanceRepository);
     }
 

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceServiceTest.java

@@ -551,7 +551,7 @@ public TestFlowApprovalInstance(@NonNull Long organizationId, @NonNull Long flow
             @NonNull NodeInstanceEntityRepository nodeRepository,
             @NonNull SequenceInstanceRepository sequenceRepository,
             @NonNull UserTaskInstanceRepository userTaskInstanceRepository) {
-        super(organizationId, flowInstanceId, null, null, expireIntervalSeconds, startEndpoint, endEndPoint, false,
+        super(organizationId, flowInstanceId, null, expireIntervalSeconds, startEndpoint, endEndPoint, false,
                 flowableAdaptor, taskService, formService, eventPublisher, authenticationFacade, nodeRepository,
                 sequenceRepository, userTaskInstanceRepository);
     }

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceTest.java

@@ -296,7 +296,7 @@ private FlowTaskInstance createTaskInstance(Long flowInstanceId, TaskType taskTy
     }
 
     private FlowApprovalInstance createApprovalInstance(Long flowInstanceId, Integer expireIntervalSeconds) {
-        return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId, null, null,
+        return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId, null,
                 expireIntervalSeconds, false, false, false, flowableAdaptor, taskService, formService,
                 new LocalEventPublisher(), authenticationFacade, nodeRepository, sequenceRepository,
                 userTaskInstanceRepository);

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceUtilTest.java

@@ -164,7 +164,7 @@ private FlowTaskInstance createTaskInstance(Long flowInstanceId, ExecutionStrate
 
     private FlowApprovalInstance createApprovalInstance(Long flowInstanceId) {
         return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId, null,
-                null, 10, false, false, false, flowAdaptor, taskService, formService, new LocalEventPublisher(),
+                10, false, false, false, flowAdaptor, taskService, formService, new LocalEventPublisher(),
                 authenticationFacade, nodeRepository, sequenceRepository, userTaskInstanceRepository);
     }
 

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowResponseMapperFactoryTest.java

@@ -259,7 +259,7 @@ private Database getDatabase() {
     private FlowApprovalInstance createApprovalInstance(Long flowInstanceId, Integer expireIntervalSeconds,
             boolean startEndPoint, boolean endEndPoint) {
         return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId, null,
-                null, expireIntervalSeconds, startEndPoint, endEndPoint, false, flowAdaptor, taskService, formService,
+                expireIntervalSeconds, startEndPoint, endEndPoint, false, flowAdaptor, taskService, formService,
                 eventPublisher, authenticationFacade, nodeRepository, sequenceRepository, userTaskInstanceRepository);
     }
 

server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowableAdaptorTest.java

@@ -207,7 +207,7 @@ private FlowGatewayInstance createGatewayInstance(Long flowInstanceId) {
     }
 
     private FlowApprovalInstance createApprovalInstance(Long flowInstanceId) {
-        return new FlowApprovalInstance(1L, flowInstanceId, null, null, 12, true, true, false, flowableAdaptor,
+        return new FlowApprovalInstance(1L, flowInstanceId, null, 12, true, true, false, flowableAdaptor,
                 taskService, formService, new LocalEventPublisher(), authenticationFacade,
                 nodeRepository, sequenceRepository, userTaskInstanceRepository);
     }

server/integration-test/src/test/java/com/oceanbase/odc/service/integration/IntegrationTestUtil.java

@@ -18,6 +18,7 @@
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -70,7 +71,7 @@ public static IntegrationConfig createApprovalConfig() {
     }
 
     public static TemplateVariables createTemplateVariables() {
-        Map<String, Object> variables = new HashMap<>();
+        Map<String, Serializable> variables = new HashMap<>();
         variables.put(Variable.USER_NAME.key(), "Jack");
         variables.put(Variable.USER_ACCOUNT.key(), "Jack");
         variables.put(Variable.USER_ID.key(), 10001L);

server/odc-core/src/main/java/com/oceanbase/odc/core/flow/model/FlowableElement.java

@@ -16,6 +16,7 @@
 package com.oceanbase.odc.core.flow.model;
 
 import com.oceanbase.odc.core.flow.builder.ErrorBoundaryEventBuilder;
+import com.oceanbase.odc.core.flow.builder.ServiceTaskBuilder;
 import com.oceanbase.odc.core.flow.builder.SignalCatchEventBuilder;
 import com.oceanbase.odc.core.flow.builder.TimerBoundaryEventBuilder;
 import com.oceanbase.odc.core.flow.builder.UserTaskBuilder;
@@ -64,4 +65,8 @@ public FlowableElement(@NonNull UserTaskBuilder userTaskBuilder) {
         this(userTaskBuilder.getGraphId(), userTaskBuilder.getName(), FlowableElementType.USER_TASK);
     }
 
+    public FlowableElement(@NonNull ServiceTaskBuilder serviceTaskBuilder) {
+        this(serviceTaskBuilder.getGraphId(), serviceTaskBuilder.getName(), FlowableElementType.SERVICE_TASK);
+    }
+
 }

server/odc-service/src/main/java/com/oceanbase/odc/metadb/flow/UserTaskInstanceRepository.java

@@ -72,7 +72,8 @@ List<UserTaskInstanceEntity> findByResourceRoleIdentifierIn(
     @Transactional
     @Query("update UserTaskInstanceEntity as ut set ut.userTaskId=:#{#param.userTaskId},ut.status=:#{#param.status},"
             + "ut.approved=:#{#param.approved},ut.operatorId=:#{#param.operatorId},ut.comment=:#{#param.comment},"
-            + "ut.expireIntervalSeconds=:#{#param.expireIntervalSeconds} where ut.id=:#{#param.id}")
+            + "ut.expireIntervalSeconds=:#{#param.expireIntervalSeconds},ut.externalFlowInstanceId=:#{#param.externalFlowInstanceId}"
+            + " where ut.id=:#{#param.id}")
     @Modifying
     int update(@Param("param") UserTaskInstanceEntity entity);
 

server/odc-service/src/main/java/com/oceanbase/odc/service/flow/FlowInstanceService.java

@@ -212,7 +212,6 @@ public class FlowInstanceService {
     private final List<Consumer<ShadowTableComparingUpdateEvent>> shadowTableComparingTaskHooks = new ArrayList<>();
     private static final long MAX_EXPORT_OBJECT_COUNT = 10000;
     private static final String ODC_SITE_URL = "odc.site.url";
-    private static final String INVALID_EXTERNAL_INSTANCE_ID = "N/A";
 
     @PostConstruct
     public void init() {
@@ -273,11 +272,6 @@ public List<FlowInstanceDetailResp> create(@NotNull @Valid CreateFlowInstanceReq
             PreConditions.maxLength(taskParameters.getSqlContent(), "sql content",
                     flowTaskProperties.getSqlContentMaxLength());
         }
-
-        Long connId = createReq.getConnectionId();
-        ConnectionConfig conn = connectionService.getForConnectionSkipPermissionCheck(connId);
-
-        // acquire export masking policy
         if (createReq.getTaskType() == TaskType.EXPORT) {
             DataTransferConfig dataTransferConfig = (DataTransferConfig) createReq.getParameters();
             if (dataTransferConfig.getExportDbObjects().size() > MAX_EXPORT_OBJECT_COUNT) {
@@ -288,6 +282,7 @@ public List<FlowInstanceDetailResp> create(@NotNull @Valid CreateFlowInstanceReq
         }
         List<RiskLevel> riskLevels = riskLevelService.list();
         Verify.notEmpty(riskLevels, "riskLevels");
+        ConnectionConfig conn = connectionService.getForConnectionSkipPermissionCheck(createReq.getConnectionId());
         return Collections.singletonList(buildFlowInstance(riskLevels, createReq, conn));
     }
 
@@ -488,11 +483,11 @@ private FlowInstanceDetailResp cancel(@NotNull FlowInstance flowInstance, Boolea
             Verify.singleton(approvalInstances, "FlowApprovalInstance");
             FlowApprovalInstance instance = approvalInstances.get(0);
             Verify.verify(instance.isPresentOnThisMachine(), "Approval instance is not on this machine");
+            // Cancel external process instance when related ODC flow instance is cancelled
+            cancelAllRelatedExternalInstance(flowInstance);
             instance.disApprove(null, !skipAuth);
             flowInstanceRepository.updateStatusById(instance.getFlowInstanceId(), FlowStatus.CANCELLED);
             userTaskInstanceRepository.updateStatusById(instance.getId(), FlowNodeStatus.CANCELLED);
-            // Cancel external process instance when related ODC approval node is cancelled
-            cancelAllRelatedExternalInstance(flowInstance);
             return FlowInstanceDetailResp.withIdAndType(id, taskTypeHolder.getValue());
         }
 
@@ -691,8 +686,8 @@ private FlowInstanceDetailResp buildFlowInstance(List<RiskLevel> riskLevels,
                     flowInstance.newFlowInstance().next(riskDetectInstance).next(riskLevelGateway);
             for (int i = 0; i < riskLevels.size(); i++) {
                 FlowInstanceConfigurer targetConfigurer = buildConfigurer(riskLevels.get(i).getApprovalFlowConfig(),
-                        flowInstance, flowInstanceReq.getTaskType(), connectionConfig,
-                        taskEntity.getId(), flowInstanceReq.getParameters(), flowInstanceReq);
+                        flowInstance, flowInstanceReq.getTaskType(), taskEntity.getId(),
+                        flowInstanceReq.getParameters(), flowInstanceReq);
                 startConfigurer.route(
                         String.format("${%s == %d}", RuntimeTaskConstants.RISKLEVEL, riskLevels.get(i).getLevel()),
                         targetConfigurer);
@@ -733,7 +728,6 @@ private FlowInstanceConfigurer buildConfigurer(
             @NonNull ApprovalFlowConfig approvalFlowConfig,
             @NonNull FlowInstance flowInstance,
             @NonNull TaskType taskType,
-            @NonNull ConnectionConfig connectionConfig,
             @NonNull Long targetTaskId,
             @NonNull TaskParameters parameters,
             @NonNull CreateFlowInstanceReq flowInstanceReq) {
@@ -744,23 +738,10 @@ private FlowInstanceConfigurer buildConfigurer(
             FlowInstanceConfigurer configurer;
             ApprovalNodeConfig nodeConfig = nodeConfigs.get(nodeSequence);
             Long resourceRoleId = nodeConfig.getResourceRoleId();
-            String externalFlowInstanceId = null;
-            Long externalApprovalId = nodeConfig.getExternalApprovalId();
-            if (Objects.nonNull(externalApprovalId)) {
-                IntegrationConfig config = integrationService.detailWithoutPermissionCheck(externalApprovalId);
-                ApprovalProperties properties = ApprovalProperties.from(config);
-                TemplateVariables variables = buildTemplateVariables(flowInstanceReq, connectionConfig);
-                try {
-                    externalFlowInstanceId = approvalClient.start(properties, variables);
-                } catch (Exception e) {
-                    externalFlowInstanceId = INVALID_EXTERNAL_INSTANCE_ID;
-                    log.warn("Create external approval instance failed, the instance will be force closed!");
-                }
-            }
             FlowApprovalInstance approvalInstance = flowFactory.generateFlowApprovalInstance(flowInstance.getId(),
                     false, false,
                     nodeConfig.getAutoApproval(), approvalFlowConfig.getApprovalExpirationIntervalSeconds(),
-                    externalApprovalId, externalFlowInstanceId);
+                    nodeConfig.getExternalApprovalId());
             if (Objects.nonNull(resourceRoleId)) {
                 approvalPermissionService.setCandidateResourceRole(approvalInstance.getId(),
                         StringUtils.join(flowInstanceReq.getProjectId(), ":", resourceRoleId));
@@ -861,11 +842,10 @@ private TemplateVariables buildTemplateVariables(CreateFlowInstanceReq flowInsta
         variables.setAttribute(Variable.TASK_TYPE, taskType.getLocalizedMessage());
         variables.setAttribute(Variable.TASK_DETAILS, JsonUtils.toJson(flowInstanceReq.getParameters()));
         // set connection related variables
-        ConnectionConfig connection = connectionService.getWithoutPermissionCheck(flowInstanceReq.getConnectionId());
-        if (Objects.nonNull(connection)) {
-            variables.setAttribute(Variable.CONNECTION_NAME, connection.getName());
-            variables.setAttribute(Variable.CONNECTION_TENANT, connection.getTenantName());
-            for (Entry<String, String> entry : connection.getProperties().entrySet()) {
+        if (Objects.nonNull(config)) {
+            variables.setAttribute(Variable.CONNECTION_NAME, config.getName());
+            variables.setAttribute(Variable.CONNECTION_TENANT, config.getTenantName());
+            for (Entry<String, String> entry : config.getProperties().entrySet()) {
                 variables.setAttribute(Variable.CONNECTION_PROPERTIES, entry.getKey(), entry.getValue());
             }
         }

server/odc-service/src/main/java/com/oceanbase/odc/service/flow/factory/FlowFactory.java

@@ -126,11 +126,10 @@ public FlowGatewayInstance generateFlowGatewayInstance(@NonNull Long flowInstanc
     }
 
     public FlowApprovalInstance generateFlowApprovalInstance(@NonNull Long flowInstanceId, boolean isStartEndPoint,
-            boolean isEndEndPoint, boolean autoApprove, int expireIntervalSeconds, Long externalApprovalId,
-            String externalFlowInstanceId) {
+            boolean isEndEndPoint, boolean autoApprove, int expireIntervalSeconds, Long externalApprovalId) {
         Verify.verify(expireIntervalSeconds > 0, "ApprovalExpirationInterval can not be negative");
         return new FlowApprovalInstance(authenticationFacade.currentOrganizationId(), flowInstanceId,
-                externalApprovalId, externalFlowInstanceId, expireIntervalSeconds, isStartEndPoint, isEndEndPoint,
+                externalApprovalId, expireIntervalSeconds, isStartEndPoint, isEndEndPoint,
                 autoApprove, flowableAdaptor, flowableTaskService, formService, eventPublisher, authenticationFacade,
                 nodeRepository, sequenceRepository, userTaskInstanceRepository);
     }

server/odc-service/src/main/java/com/oceanbase/odc/service/flow/factory/FlowResponseMapperFactory.java

@@ -57,6 +57,7 @@
 import com.oceanbase.odc.metadb.iam.UserRoleRepository;
 import com.oceanbase.odc.metadb.iam.resourcerole.UserResourceRoleEntity;
 import com.oceanbase.odc.metadb.iam.resourcerole.UserResourceRoleRepository;
+import com.oceanbase.odc.metadb.integration.IntegrationEntity;
 import com.oceanbase.odc.metadb.regulation.risklevel.RiskLevelRepository;
 import com.oceanbase.odc.metadb.task.TaskEntity;
 import com.oceanbase.odc.metadb.task.TaskRepository;
@@ -235,6 +236,10 @@ private FlowNodeInstanceMapper generateNodeMapper(@NonNull Collection<Long> flow
                 .withGetTaskById(taskId2TaskEntity::get)
                 .withGetUserById(userId2User::get)
                 .withGetRolesByUserId(userId2Roles::get)
+                .withGetExternalApprovalNameById(externalApprovalId -> {
+                    IntegrationEntity config = integrationService.nullSafeGet(externalApprovalId);
+                    return config.getName();
+                })
                 .withGetExternalUrlByExternalId(externalApproval -> {
                     IntegrationConfig config =
                             integrationService.detailWithoutPermissionCheck(externalApproval.getApprovalId());

server/odc-service/src/main/java/com/oceanbase/odc/service/flow/instance/FlowApprovalInstance.java

@@ -40,6 +40,7 @@
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NonNull;
+import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -56,12 +57,14 @@
 public class FlowApprovalInstance extends BaseFlowUserTaskInstance {
 
     public static final String APPROVAL_VARIABLE_NAME = "approved";
+    @Setter
     private String comment;
     private Long operatorId;
     private boolean approved;
     private final Integer expireIntervalSeconds;
     private final boolean autoApprove;
-    private final String externalFlowInstanceId;
+    @Setter
+    private String externalFlowInstanceId;
     private final Long externalApprovalId;
     private boolean waitForConfirm;
 
@@ -94,7 +97,7 @@ public FlowApprovalInstance(@NonNull UserTaskInstanceEntity entity, @NonNull Flo
     }
 
     public FlowApprovalInstance(@NonNull Long organizationId, @NonNull Long flowInstanceId,
-            Long externalApprovalId, String externalFlowInstanceId,
+            Long externalApprovalId,
             @NonNull Integer expireIntervalSeconds, boolean startEndpoint, boolean endEndPoint, boolean autoApprove,
             @NonNull FlowableAdaptor flowableAdaptor, @NonNull TaskService taskService,
             @NonNull FormService formService,
@@ -109,7 +112,6 @@ public FlowApprovalInstance(@NonNull Long organizationId, @NonNull Long flowInst
         this.authenticationFacade = authenticationFacade;
         this.expireIntervalSeconds = expireIntervalSeconds;
         this.autoApprove = autoApprove;
-        this.externalFlowInstanceId = externalFlowInstanceId;
         this.externalApprovalId = externalApprovalId;
         alloc();
         create();
@@ -135,7 +137,6 @@ public FlowApprovalInstance(@NonNull Long organizationId, @NonNull Long flowInst
         this.expireIntervalSeconds = expireIntervalSeconds;
         this.autoApprove = autoApprove;
         this.waitForConfirm = waitForConfirm;
-        this.externalFlowInstanceId = null;
         this.externalApprovalId = null;
         alloc();
         create();
@@ -188,6 +189,7 @@ public void update() {
         entity.setOperatorId(getOperatorId());
         entity.setComment(getComment());
         entity.setExpireIntervalSeconds(getExpireIntervalSeconds());
+        entity.setExternalFlowInstanceId(getExternalFlowInstanceId());
         int affectRows = userTaskInstanceRepository.update(entity);
         log.info("Update approval task instance successfully, affectRows={}, approvalTask={}", affectRows, entity);
     }