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

This commit is contained in:
Wenjun Ruan 2024-04-22 17:41:17 +08:00 committed by GitHub
parent e2c8b080f9
commit e9d85914d7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 79 additions and 63 deletions

View File

@ -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

View File

@ -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();
}
/**

View File

@ -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

View File

@ -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);

View File

@ -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();
}
}

View File

@ -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() {

View File

@ -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>

View File

@ -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