You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by we...@apache.org on 2024/04/22 09:41:23 UTC
(dolphinscheduler) branch dev updated: Fix queryByTypeAndJobId might error due to multiple result (#15883)
This is an automated email from the ASF dual-hosted git repository.
wenjun pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new e9d85914d7 Fix queryByTypeAndJobId might error due to multiple result (#15883)
e9d85914d7 is described below
commit e9d85914d78197314bee585500d94a6a90733789
Author: Wenjun Ruan <we...@apache.org>
AuthorDate: Mon Apr 22 17:41:17 2024 +0800
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 +-
.../service/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<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
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<TriggerRelation> 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<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 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<TriggerRelation> 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<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();
}
}
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 @@
<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>
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