From 200d23fc3ebc67058fb49167b41b95c1eb62be8d Mon Sep 17 00:00:00 2001 From: Evan Sun Date: Sun, 7 Apr 2024 09:58:28 +0800 Subject: [PATCH] [TEST] increase coverage of datasource service (#15801) Co-authored-by: abzymeinsjtu --- .../service/impl/DataSourceServiceImpl.java | 6 +- .../api/service/DataSourceServiceTest.java | 144 ++++++++++++++---- .../dao/mapper/DataSourceMapper.java | 3 +- 3 files changed, 121 insertions(+), 32 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java index a030762457..210900e54c 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java @@ -230,7 +230,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource @Override public PageInfo queryDataSourceListPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize) { - IPage dataSourceList = null; + IPage dataSourceList; Page dataSourcePage = new Page<>(pageNo, pageSize); PageInfo pageInfo = new PageInfo<>(pageNo, pageSize); if (loginUser.getUserType().equals(UserType.ADMIN_USER)) { @@ -282,7 +282,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource @Override public List queryDataSourceList(User loginUser, Integer type) { - List datasourceList = null; + List datasourceList; if (loginUser.getUserType().equals(UserType.ADMIN_USER)) { datasourceList = dataSourceMapper.queryDataSourceByType(0, type); } else { @@ -420,7 +420,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource public List getTables(Integer datasourceId, String database) { DataSource dataSource = dataSourceMapper.selectById(datasourceId); - List tableList = null; + List tableList; BaseConnectionParam connectionParam = (BaseConnectionParam) DataSourceUtils.buildConnectionParams( dataSource.getType(), diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java index 68d4db0ff2..b4e7aae4b8 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java @@ -52,15 +52,15 @@ import org.apache.dolphinscheduler.spi.enums.DbType; import org.apache.commons.collections4.CollectionUtils; +import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; +import java.util.Random; import java.util.concurrent.ExecutionException; import org.junit.jupiter.api.Assertions; @@ -73,6 +73,9 @@ import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.dao.DuplicateKeyException; + +import com.baomidou.mybatisplus.core.metadata.IPage; /** * data source service test @@ -96,6 +99,15 @@ public class DataSourceServiceTest { @Mock private ResourcePermissionCheckService resourcePermissionCheckService; + @Mock + private IPage dataSourceList; + + private String randomStringWithLengthN(int n) { + byte[] bitArray = new byte[n]; + new Random().nextBytes(bitArray); + return new String(bitArray, StandardCharsets.UTF_8); + } + private void passResourcePermissionCheckService() { when(resourcePermissionCheckService.operationPermissionCheck(Mockito.any(), Mockito.anyInt(), Mockito.anyString(), Mockito.any())).thenReturn(true); @@ -131,6 +143,7 @@ public class DataSourceServiceTest { when(dataSourceMapper.queryDataSourceByName(dataSourceName.trim())).thenReturn(dataSourceList); passResourcePermissionCheckService(); + // DATASOURCE_EXIST assertThrowsServiceException(Status.DATASOURCE_EXIST, () -> dataSourceService.createDataSource(loginUser, postgreSqlDatasourceParam)); @@ -140,13 +153,24 @@ public class DataSourceServiceTest { when(dataSourceMapper.queryDataSourceByName(dataSourceName.trim())).thenReturn(null); + // DESCRIPTION TOO LONG + postgreSqlDatasourceParam.setNote(randomStringWithLengthN(512)); + assertThrowsServiceException(Status.DESCRIPTION_TOO_LONG_ERROR, + () -> dataSourceService.createDataSource(loginUser, postgreSqlDatasourceParam)); + postgreSqlDatasourceParam.setNote(dataSourceDesc); + // SUCCESS assertDoesNotThrow(() -> dataSourceService.createDataSource(loginUser, postgreSqlDatasourceParam)); + + // Duplicated Key Exception + when(dataSourceMapper.insert(Mockito.any(DataSource.class))).thenThrow(DuplicateKeyException.class); + assertThrowsServiceException(Status.DATASOURCE_EXIST, + () -> dataSourceService.createDataSource(loginUser, postgreSqlDatasourceParam)); } } @Test - public void updateDataSourceTest() throws ExecutionException { + public void updateDataSourceTest() { User loginUser = getAdminUser(); int dataSourceId = 12; @@ -200,32 +224,74 @@ public class DataSourceServiceTest { // DATASOURCE_CONNECT_FAILED when(dataSourceMapper.queryDataSourceByName(postgreSqlDatasourceParam.getName())).thenReturn(null); + // DESCRIPTION TOO LONG + postgreSqlDatasourceParam.setNote(randomStringWithLengthN(512)); + assertThrowsServiceException(Status.DESCRIPTION_TOO_LONG_ERROR, + () -> dataSourceService.updateDataSource(loginUser, postgreSqlDatasourceParam)); + postgreSqlDatasourceParam.setNote(dataSourceDesc); + // SUCCESS assertDoesNotThrow(() -> dataSourceService.updateDataSource(loginUser, postgreSqlDatasourceParam)); + + // Duplicated Key Exception + when(dataSourceMapper.updateById(Mockito.any(DataSource.class))).thenThrow(DuplicateKeyException.class); + assertThrowsServiceException(Status.DATASOURCE_EXIST, + () -> dataSourceService.updateDataSource(loginUser, postgreSqlDatasourceParam)); } } @Test - public void queryDataSourceListPagingTest() { - User loginUser = getAdminUser(); + public void testQueryDataSourceListPaging() { + + User adminUser = getAdminUser(); + User generalUser = getGeneralUser(); String searchVal = ""; int pageNo = 1; int pageSize = 10; PageInfo pageInfo = - dataSourceService.queryDataSourceListPaging(loginUser, searchVal, pageNo, pageSize); + dataSourceService.queryDataSourceListPaging(adminUser, searchVal, pageNo, pageSize); Assertions.assertNotNull(pageInfo); + + // test query datasource as general user with no datasource authed + when(dataSourceList.getRecords()).thenReturn(getSingleDataSourceList()); + when(dataSourceMapper.selectPagingByIds(Mockito.any(), Mockito.any(), Mockito.any())) + .thenReturn(dataSourceList); + assertDoesNotThrow(() -> dataSourceService.queryDataSourceListPaging(generalUser, searchVal, pageNo, pageSize)); + + // test query datasource as general user with datasource authed + when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.DATASOURCE, + generalUser.getId(), dataSourceServiceLogger)).thenReturn(Collections.singleton(1)); + + assertDoesNotThrow(() -> dataSourceService.queryDataSourceListPaging(generalUser, searchVal, pageNo, pageSize)); } @Test - public void connectionTest() { + public void testConnectionTest() { int dataSourceId = -1; when(dataSourceMapper.selectById(dataSourceId)).thenReturn(null); assertThrowsServiceException(Status.RESOURCE_NOT_EXIST, () -> dataSourceService.connectionTest(dataSourceId)); + + try ( + MockedStatic ignored = + Mockito.mockStatic(DataSourceUtils.class)) { + DataSource dataSource = getOracleDataSource(999); + when(dataSourceMapper.selectById(dataSource.getId())).thenReturn(dataSource); + DataSourceProcessor dataSourceProcessor = Mockito.mock(DataSourceProcessor.class); + + when(DataSourceUtils.getDatasourceProcessor(Mockito.any())).thenReturn(dataSourceProcessor); + when(dataSourceProcessor.checkDataSourceConnectivity(Mockito.any())).thenReturn(true); + assertDoesNotThrow(() -> dataSourceService.connectionTest(dataSource.getId())); + + when(dataSourceProcessor.checkDataSourceConnectivity(Mockito.any())).thenReturn(false); + assertThrowsServiceException(Status.CONNECTION_TEST_FAILURE, + () -> dataSourceService.connectionTest(dataSource.getId())); + } + } @Test - public void deleteTest() { + public void testDelete() { User loginUser = getAdminUser(); int dataSourceId = 1; // resource not exist @@ -252,7 +318,7 @@ public class DataSourceServiceTest { } @Test - public void unauthDatasourceTest() { + public void testUnAuthDatasource() { User loginUser = getAdminUser(); loginUser.setId(1); loginUser.setUserType(UserType.ADMIN_USER); @@ -279,7 +345,7 @@ public class DataSourceServiceTest { } @Test - public void authedDatasourceTest() { + public void testAuthedDatasource() { User loginUser = getAdminUser(); loginUser.setId(1); loginUser.setUserType(UserType.ADMIN_USER); @@ -300,19 +366,28 @@ public class DataSourceServiceTest { } @Test - public void queryDataSourceListTest() { - User loginUser = new User(); - loginUser.setUserType(UserType.GENERAL_USER); - Set dataSourceIds = new HashSet<>(); - dataSourceIds.add(1); + public void testQueryDataSourceList() { + User adminUser = getAdminUser(); + assertDoesNotThrow(() -> dataSourceService.queryDataSourceList(adminUser, DbType.MYSQL.ordinal())); + + User generalUser = getGeneralUser(); + when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.DATASOURCE, - loginUser.getId(), dataSourceServiceLogger)).thenReturn(dataSourceIds); + generalUser.getId(), dataSourceServiceLogger)).thenReturn(Collections.emptySet()); + List emptyList = dataSourceService.queryDataSourceList(generalUser, DbType.MYSQL.ordinal()); + Assertions.assertEquals(emptyList.size(), 0); + + when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.DATASOURCE, + generalUser.getId(), dataSourceServiceLogger)).thenReturn(Collections.singleton(1)); DataSource dataSource = new DataSource(); + dataSource.setId(1); dataSource.setType(DbType.MYSQL); - when(dataSourceMapper.selectBatchIds(dataSourceIds)).thenReturn(Collections.singletonList(dataSource)); + when(dataSourceMapper.selectBatchIds(Collections.singleton(1))) + .thenReturn(Collections.singletonList(dataSource)); + List list = - dataSourceService.queryDataSourceList(loginUser, DbType.MYSQL.ordinal()); + dataSourceService.queryDataSourceList(generalUser, DbType.MYSQL.ordinal()); Assertions.assertNotNull(list); } @@ -327,21 +402,28 @@ public class DataSourceServiceTest { } @Test - public void queryDataSourceTest() { - when(dataSourceMapper.selectById(Mockito.anyInt())).thenReturn(null); + public void testQueryDataSource() { + // datasource not exists + when(dataSourceMapper.selectById(999)).thenReturn(null); User loginUser = new User(); loginUser.setUserType(UserType.GENERAL_USER); loginUser.setId(2); - try { - dataSourceService.queryDataSource(Mockito.anyInt(), loginUser); - } catch (Exception e) { - Assertions.assertTrue(e.getMessage().contains(Status.RESOURCE_NOT_EXIST.getMsg())); - } + + assertThrowsServiceException(Status.RESOURCE_NOT_EXIST, + () -> dataSourceService.queryDataSource(999, loginUser)); DataSource dataSource = getOracleDataSource(1); - when(dataSourceMapper.selectById(Mockito.anyInt())).thenReturn(dataSource); + when(dataSourceMapper.selectById(dataSource.getId())).thenReturn(dataSource); when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.DATASOURCE, loginUser.getId(), DATASOURCE, baseServiceLogger)).thenReturn(true); + + // no perm + when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.DATASOURCE, + new Object[]{dataSource.getId()}, loginUser.getId(), baseServiceLogger)).thenReturn(false); + assertThrowsServiceException(Status.USER_NO_OPERATION_PERM, + () -> dataSourceService.queryDataSource(dataSource.getId(), loginUser)); + + // success when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.DATASOURCE, new Object[]{dataSource.getId()}, loginUser.getId(), baseServiceLogger)).thenReturn(true); BaseDataSourceParamDTO paramDTO = dataSourceService.queryDataSource(dataSource.getId(), loginUser); @@ -472,8 +554,16 @@ public class DataSourceServiceTest { */ private User getAdminUser() { User loginUser = new User(); - loginUser.setId(-1); + loginUser.setId(1); loginUser.setUserName("admin"); + loginUser.setUserType(UserType.ADMIN_USER); + return loginUser; + } + + private User getGeneralUser() { + User loginUser = new User(); + loginUser.setId(2); + loginUser.setUserName("user"); loginUser.setUserType(UserType.GENERAL_USER); return loginUser; } diff --git a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/DataSourceMapper.java b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/DataSourceMapper.java index b5dcc31627..e26fe77ae4 100644 --- a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/DataSourceMapper.java +++ b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/DataSourceMapper.java @@ -102,8 +102,7 @@ public interface DataSourceMapper extends BaseMapper { /** * selectPagingByIds * @param dataSourcePage - * @param ids - * @param searchVal + * @param dataSourceIds * @return */ IPage selectPagingByIds(Page dataSourcePage,