Fix queryByTypeAndJobId might error due to multiple result (apache#15883

)

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapper.java

@@ -36,7 +36,7 @@ public interface TriggerRelationMapper extends BaseMapper<TriggerRelation> {
      * @param jobId
      * @return
      */
-    TriggerRelation queryByTypeAndJobId(@Param("triggerType") Integer triggerType, @Param("jobId") int jobId);
+    List<TriggerRelation> queryByTypeAndJobId(@Param("triggerType") Integer triggerType, @Param("jobId") int jobId);
 
     /**
      * query triggerRelation by code

dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapperTest.java

@@ -17,13 +17,13 @@
 
 package org.apache.dolphinscheduler.dao.mapper;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import org.apache.dolphinscheduler.common.enums.ApiTriggerType;
 import org.apache.dolphinscheduler.common.utils.DateUtils;
 import org.apache.dolphinscheduler.dao.BaseDaoTest;
 import org.apache.dolphinscheduler.dao.entity.TriggerRelation;
 
-import java.util.List;
-
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -67,9 +67,9 @@ public void testSelectById() {
     @Test
     public void testQueryByTypeAndJobId() {
         TriggerRelation expectRelation = createTriggerRelation();
-        TriggerRelation actualRelation = triggerRelationMapper.queryByTypeAndJobId(
-                expectRelation.getTriggerType(), expectRelation.getJobId());
-        Assertions.assertEquals(expectRelation, actualRelation);
+        assertThat(
+                triggerRelationMapper.queryByTypeAndJobId(expectRelation.getTriggerType(), expectRelation.getJobId()))
+                        .containsExactly(expectRelation);
     }
 
     /**
@@ -80,9 +80,8 @@ public void testQueryByTypeAndJobId() {
     @Test
     public void testQueryByTriggerRelationCode() {
         TriggerRelation expectRelation = createTriggerRelation();
-        List<TriggerRelation> actualRelations = triggerRelationMapper.queryByTriggerRelationCode(
-                expectRelation.getTriggerCode());
-        Assertions.assertEquals(actualRelations.size(), 1);
+        assertThat(triggerRelationMapper.queryByTriggerRelationCode(expectRelation.getTriggerCode()))
+                .containsExactly(expectRelation);
     }
 
     /**
@@ -93,17 +92,15 @@ public void testQueryByTriggerRelationCode() {
     @Test
     public void testQueryByTriggerRelationCodeAndType() {
         TriggerRelation expectRelation = createTriggerRelation();
-        List<TriggerRelation> actualRelations = triggerRelationMapper.queryByTriggerRelationCodeAndType(
-                expectRelation.getTriggerCode(), expectRelation.getTriggerType());
-        Assertions.assertEquals(actualRelations.size(), 1);
+        assertThat(triggerRelationMapper.queryByTriggerRelationCodeAndType(expectRelation.getTriggerCode(),
+                expectRelation.getTriggerType())).containsExactly(expectRelation);
     }
 
     @Test
     public void testUpsert() {
         TriggerRelation expectRelation = createTriggerRelation();
         triggerRelationMapper.upsert(expectRelation);
-        TriggerRelation actualRelation = triggerRelationMapper.selectById(expectRelation.getId());
-        Assertions.assertEquals(expectRelation, actualRelation);
+        assertThat(triggerRelationMapper.selectById(expectRelation.getId())).isEqualTo(expectRelation);
     }
 
     /**
@@ -113,8 +110,7 @@ public void testUpsert() {
     public void testDelete() {
         TriggerRelation expectRelation = createTriggerRelation();
         triggerRelationMapper.deleteById(expectRelation.getId());
-        TriggerRelation actualRelation = triggerRelationMapper.selectById(expectRelation.getId());
-        Assertions.assertNull(actualRelation);
+        assertThat(triggerRelationMapper.selectById(expectRelation.getId())).isNull();
     }
 
     /**

dolphinscheduler-dist/release-docs/LICENSE

@@ -528,7 +528,7 @@ The text of each license is also included at licenses/LICENSE-[project].txt.
     nimbus-jose-jwt 9.22: https://mvnrepository.com/artifact/com.nimbusds/nimbus-jose-jwt/9.22, Apache 2.0
     woodstox-core 6.4.0: https://mvnrepository.com/artifact/com.fasterxml.woodstox/woodstox-core/6.4.0, Apache 2.0
     auto-value 1.10.1: https://mvnrepository.com/artifact/com.google.auto.value/auto-value/1.10.1, Apache 2.0
-    auto-value-annotations 1.10.1: https://mvnrepository.com/artifact/com.google.auto.value/auto-value-annotations/1.10.1, Apache 2.0
+    auto-value-annotations 1.10.4: https://mvnrepository.com/artifact/com.google.auto.value/auto-value-annotations/1.10.4, Apache 2.0
     conscrypt-openjdk-uber 2.5.2: https://mvnrepository.com/artifact/org.conscrypt/conscrypt-openjdk-uber/2.5.2, Apache 2.0
     gapic-google-cloud-storage-v2 2.18.0-alpha: https://mvnrepository.com/artifact/com.google.api.grpc/gapic-google-cloud-storage-v2/2.18.0-alpha, Apache 2.0
     google-api-client 2.2.0: https://mvnrepository.com/artifact/com.google.api-client/google-api-client/2.2.0, Apache 2.0

dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationService.java

@@ -20,6 +20,8 @@
 import org.apache.dolphinscheduler.common.enums.ApiTriggerType;
 import org.apache.dolphinscheduler.dao.entity.TriggerRelation;
 
+import java.util.List;
+
 import org.springframework.stereotype.Component;
 
 /**
@@ -30,7 +32,7 @@ public interface TriggerRelationService {
 
     void saveTriggerToDb(ApiTriggerType type, Long triggerCode, Integer jobId);
 
-    TriggerRelation queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId);
+    List<TriggerRelation> queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId);
 
     int saveCommandTrigger(Integer commandId, Integer processInstanceId);
 

dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceImpl.java

@@ -21,14 +21,20 @@
 import org.apache.dolphinscheduler.dao.entity.TriggerRelation;
 import org.apache.dolphinscheduler.dao.mapper.TriggerRelationMapper;
 
+import org.apache.commons.collections4.CollectionUtils;
+
 import java.util.Date;
+import java.util.List;
+
+import lombok.extern.slf4j.Slf4j;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 /**
- *  Trigger relation operator to db
+ * Trigger relation operator to db
  */
+@Slf4j
 @Component
 public class TriggerRelationServiceImpl implements TriggerRelationService {
 
@@ -45,29 +51,44 @@ public void saveTriggerToDb(ApiTriggerType type, Long triggerCode, Integer jobId
         triggerRelation.setUpdateTime(new Date());
         triggerRelationMapper.upsert(triggerRelation);
     }
+
     @Override
-    public TriggerRelation queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId) {
+    public List<TriggerRelation> queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId) {
         return triggerRelationMapper.queryByTypeAndJobId(apiTriggerType.getCode(), jobId);
     }
 
     @Override
     public int saveCommandTrigger(Integer commandId, Integer processInstanceId) {
-        TriggerRelation exist = queryByTypeAndJobId(ApiTriggerType.PROCESS, processInstanceId);
-        if (exist == null) {
+        List<TriggerRelation> existTriggers = queryByTypeAndJobId(ApiTriggerType.PROCESS, processInstanceId);
+        if (CollectionUtils.isEmpty(existTriggers)) {
             return 0;
         }
-        saveTriggerToDb(ApiTriggerType.COMMAND, exist.getTriggerCode(), commandId);
-        return 1;
+        existTriggers.forEach(triggerRelation -> saveTriggerToDb(ApiTriggerType.COMMAND,
+                triggerRelation.getTriggerCode(), commandId));
+        int triggerRelationSize = existTriggers.size();
+        if (triggerRelationSize > 1) {
+            // Fix https://github.com/apache/dolphinscheduler/issues/15864
+            // This case shouldn't happen
+            log.error("The PROCESS TriggerRelation of command: {} is more than one", commandId);
+        }
+        return existTriggers.size();
     }
 
     @Override
     public int saveProcessInstanceTrigger(Integer commandId, Integer processInstanceId) {
-        TriggerRelation exist = queryByTypeAndJobId(ApiTriggerType.COMMAND, commandId);
-        if (exist == null) {
+        List<TriggerRelation> existTriggers = queryByTypeAndJobId(ApiTriggerType.COMMAND, commandId);
+        if (CollectionUtils.isEmpty(existTriggers)) {
             return 0;
         }
-        saveTriggerToDb(ApiTriggerType.PROCESS, exist.getTriggerCode(), processInstanceId);
-        return 1;
+        existTriggers.forEach(triggerRelation -> saveTriggerToDb(ApiTriggerType.PROCESS,
+                triggerRelation.getTriggerCode(), processInstanceId));
+        int triggerRelationSize = existTriggers.size();
+        if (triggerRelationSize > 1) {
+            // Fix https://github.com/apache/dolphinscheduler/issues/15864
+            // This case shouldn't happen
+            log.error("The COMMAND TriggerRelation of command: {} is more than one", commandId);
+        }
+        return existTriggers.size();
     }
 
 }

dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceTest.java

@@ -17,24 +17,26 @@
 
 package org.apache.dolphinscheduler.service.process;
 
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.when;
+
 import org.apache.dolphinscheduler.common.enums.ApiTriggerType;
 import org.apache.dolphinscheduler.dao.entity.TriggerRelation;
 import org.apache.dolphinscheduler.dao.mapper.TriggerRelationMapper;
-import org.apache.dolphinscheduler.service.cron.CronUtilsTest;
 
 import java.util.Date;
 
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.Lists;
 
 /**
  * Trigger Relation Service Test
@@ -43,56 +45,44 @@
 @MockitoSettings(strictness = Strictness.LENIENT)
 public class TriggerRelationServiceTest {
 
-    private static final Logger logger = LoggerFactory.getLogger(CronUtilsTest.class);
-
     @InjectMocks
     private TriggerRelationServiceImpl triggerRelationService;
     @Mock
     private TriggerRelationMapper triggerRelationMapper;
 
     @Test
     public void saveTriggerToDb() {
-        Mockito.doNothing().when(triggerRelationMapper).upsert(Mockito.any());
+        doNothing().when(triggerRelationMapper).upsert(any());
         triggerRelationService.saveTriggerToDb(ApiTriggerType.COMMAND, 1234567890L, 100);
     }
 
     @Test
     public void queryByTypeAndJobId() {
-        Mockito.doNothing().when(triggerRelationMapper).upsert(Mockito.any());
-        Mockito.when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.PROCESS.getCode(), 100))
-                .thenReturn(getTriggerTdoDb());
+        doNothing().when(triggerRelationMapper).upsert(any());
+        when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.PROCESS.getCode(), 100))
+                .thenReturn(Lists.newArrayList(getTriggerTdoDb()));
 
-        TriggerRelation triggerRelation1 = triggerRelationService.queryByTypeAndJobId(
-                ApiTriggerType.PROCESS, 100);
-        Assertions.assertNotNull(triggerRelation1);
-        TriggerRelation triggerRelation2 = triggerRelationService.queryByTypeAndJobId(
-                ApiTriggerType.PROCESS, 200);
-        Assertions.assertNull(triggerRelation2);
+        assertThat(triggerRelationService.queryByTypeAndJobId(ApiTriggerType.PROCESS, 100)).hasSize(1);
+        assertThat(triggerRelationService.queryByTypeAndJobId(ApiTriggerType.PROCESS, 200)).isEmpty();
     }
 
     @Test
     public void saveCommandTrigger() {
-        Mockito.doNothing().when(triggerRelationMapper).upsert(Mockito.any());
-        Mockito.when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.PROCESS.getCode(), 100))
-                .thenReturn(getTriggerTdoDb());
-        int result = -1;
-        result = triggerRelationService.saveCommandTrigger(1234567890, 100);
-        Assertions.assertTrue(result > 0);
-        result = triggerRelationService.saveCommandTrigger(1234567890, 200);
-        Assertions.assertTrue(result == 0);
+        doNothing().when(triggerRelationMapper).upsert(any());
+        when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.PROCESS.getCode(), 100))
+                .thenReturn(Lists.newArrayList(getTriggerTdoDb()));
+        assertThat(triggerRelationService.saveCommandTrigger(1234567890, 100)).isAtLeast(1);
+        assertThat(triggerRelationService.saveCommandTrigger(1234567890, 200)).isEqualTo(0);
 
     }
 
     @Test
     public void saveProcessInstanceTrigger() {
-        Mockito.doNothing().when(triggerRelationMapper).upsert(Mockito.any());
-        Mockito.when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.COMMAND.getCode(), 100))
-                .thenReturn(getTriggerTdoDb());
-        int result = -1;
-        result = triggerRelationService.saveProcessInstanceTrigger(100, 1234567890);
-        Assertions.assertTrue(result > 0);
-        result = triggerRelationService.saveProcessInstanceTrigger(200, 1234567890);
-        Assertions.assertTrue(result == 0);
+        doNothing().when(triggerRelationMapper).upsert(any());
+        when(triggerRelationMapper.queryByTypeAndJobId(ApiTriggerType.COMMAND.getCode(), 100))
+                .thenReturn(Lists.newArrayList(getTriggerTdoDb()));
+        assertThat(triggerRelationService.saveProcessInstanceTrigger(100, 1234567890)).isAtLeast(1);
+        assertThat(triggerRelationService.saveProcessInstanceTrigger(200, 1234567890)).isEqualTo(0);
     }
 
     private TriggerRelation getTriggerTdoDb() {

pom.xml

@@ -89,6 +89,7 @@
         <owasp-dependency-check-maven.version>7.1.2</owasp-dependency-check-maven.version>
         <lombok.version>1.18.20</lombok.version>
         <awaitility.version>4.2.0</awaitility.version>
+        <truth.version>1.4.2</truth.version>
         <docker.hub>apache</docker.hub>
         <docker.repo>${project.name}</docker.repo>
         <docker.tag>${project.version}</docker.tag>
@@ -372,6 +373,12 @@
             <version>${awaitility.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.google.truth</groupId>
+            <artifactId>truth</artifactId>
+            <version>${truth.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

tools/dependencies/known-dependencies.txt

@@ -9,7 +9,7 @@ animal-sniffer-annotations-1.19.jar
 annotations-2.17.282.jar
 annotations-13.0.jar
 apache-client-2.17.282.jar
-asm-9.1.jar
+asm-9.6.jar
 aspectjweaver-1.9.7.jar
 aspectjrt-1.9.7.jar
 auth-2.17.282.jar
@@ -434,7 +434,7 @@ woodstox-core-6.4.0.jar
 azure-core-management-1.10.1.jar
 api-common-2.6.0.jar
 auto-value-1.10.1.jar
-auto-value-annotations-1.10.1.jar
+auto-value-annotations-1.10.4.jar
 bcpkix-jdk15on-1.67.jar
 bcprov-jdk15on-1.67.jar
 conscrypt-openjdk-uber-2.5.2.jar