From e9d85914d78197314bee585500d94a6a90733789 Mon Sep 17 00:00:00 2001 From: Wenjun Ruan Date: Mon, 22 Apr 2024 17:41:17 +0800 Subject: [PATCH] Fix queryByTypeAndJobId might error due to multiple result (#15883) --- .../dao/mapper/TriggerRelationMapper.java | 2 +- .../dao/mapper/TriggerRelationMapperTest.java | 26 ++++----- dolphinscheduler-dist/release-docs/LICENSE | 2 +- .../process/TriggerRelationService.java | 4 +- .../process/TriggerRelationServiceImpl.java | 41 ++++++++++---- .../process/TriggerRelationServiceTest.java | 56 ++++++++----------- pom.xml | 7 +++ tools/dependencies/known-dependencies.txt | 4 +- 8 files changed, 79 insertions(+), 63 deletions(-) diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapper.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapper.java index 10a0acf47f..912ef28101 100644 --- a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapper.java +++ b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapper.java @@ -36,7 +36,7 @@ public interface TriggerRelationMapper extends BaseMapper { * @param jobId * @return */ - TriggerRelation queryByTypeAndJobId(@Param("triggerType") Integer triggerType, @Param("jobId") int jobId); + List queryByTypeAndJobId(@Param("triggerType") Integer triggerType, @Param("jobId") int jobId); /** * query triggerRelation by code diff --git a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapperTest.java b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapperTest.java index d3f4fcc666..7e31b64a23 100644 --- a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TriggerRelationMapperTest.java +++ b/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 class TriggerRelationMapperTest extends BaseDaoTest { @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 class TriggerRelationMapperTest extends BaseDaoTest { @Test public void testQueryByTriggerRelationCode() { TriggerRelation expectRelation = createTriggerRelation(); - List actualRelations = triggerRelationMapper.queryByTriggerRelationCode( - expectRelation.getTriggerCode()); - Assertions.assertEquals(actualRelations.size(), 1); + assertThat(triggerRelationMapper.queryByTriggerRelationCode(expectRelation.getTriggerCode())) + .containsExactly(expectRelation); } /** @@ -93,17 +92,15 @@ public class TriggerRelationMapperTest extends BaseDaoTest { @Test public void testQueryByTriggerRelationCodeAndType() { TriggerRelation expectRelation = createTriggerRelation(); - List 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 class TriggerRelationMapperTest extends BaseDaoTest { 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(); } /** diff --git a/dolphinscheduler-dist/release-docs/LICENSE b/dolphinscheduler-dist/release-docs/LICENSE index 8856138aa4..84832b2b32 100644 --- a/dolphinscheduler-dist/release-docs/LICENSE +++ b/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 diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationService.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationService.java index b78f477931..6de2506afd 100644 --- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationService.java +++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationService.java @@ -20,6 +20,8 @@ package org.apache.dolphinscheduler.service.process; 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 queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId); int saveCommandTrigger(Integer commandId, Integer processInstanceId); diff --git a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceImpl.java b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceImpl.java index df41c41eef..0344ea87ea 100644 --- a/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceImpl.java +++ b/dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceImpl.java @@ -21,14 +21,20 @@ import org.apache.dolphinscheduler.common.enums.ApiTriggerType; 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 class TriggerRelationServiceImpl implements TriggerRelationService { triggerRelation.setUpdateTime(new Date()); triggerRelationMapper.upsert(triggerRelation); } + @Override - public TriggerRelation queryByTypeAndJobId(ApiTriggerType apiTriggerType, int jobId) { + public List 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 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 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(); } } diff --git a/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceTest.java b/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceTest.java index 8f4790111c..4f3bbb0bc7 100644 --- a/dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/process/TriggerRelationServiceTest.java +++ b/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,8 +45,6 @@ import org.slf4j.LoggerFactory; @MockitoSettings(strictness = Strictness.LENIENT) public class TriggerRelationServiceTest { - private static final Logger logger = LoggerFactory.getLogger(CronUtilsTest.class); - @InjectMocks private TriggerRelationServiceImpl triggerRelationService; @Mock @@ -52,47 +52,37 @@ public class TriggerRelationServiceTest { @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() { diff --git a/pom.xml b/pom.xml index 4a71741f8e..dd4cb1adf1 100755 --- a/pom.xml +++ b/pom.xml @@ -89,6 +89,7 @@ 7.1.2 1.18.20 4.2.0 + 1.4.2 apache ${project.name} ${project.version} @@ -372,6 +373,12 @@ ${awaitility.version} test + + com.google.truth + truth + ${truth.version} + test + diff --git a/tools/dependencies/known-dependencies.txt b/tools/dependencies/known-dependencies.txt index 6c2e89fb37..33987119db 100644 --- a/tools/dependencies/known-dependencies.txt +++ b/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