From 3d033da55d44a9c5f6a6eaae86602f58843288d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=97=BA=E9=98=B3?= Date: Thu, 20 Jul 2023 20:02:51 +0800 Subject: [PATCH] [Improvement] Dataquality code style enhance (#14592) * code style enhance * update --------- Co-authored-by: xiangzihao <460888207@qq.com> --- .../api/controller/DataSourceController.java | 105 ++++++++-------- .../api/service/DataSourceService.java | 21 ++-- .../service/impl/DataSourceServiceImpl.java | 114 ++++++------------ .../api/service/DataSourceServiceTest.java | 69 ++++++----- 4 files changed, 142 insertions(+), 167 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataSourceController.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataSourceController.java index 8794f4301d..71b753e711 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataSourceController.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataSourceController.java @@ -35,8 +35,10 @@ import org.apache.dolphinscheduler.api.aspect.AccessLogAnnotation; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.exceptions.ApiException; import org.apache.dolphinscheduler.api.service.DataSourceService; +import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.constants.Constants; +import org.apache.dolphinscheduler.dao.entity.DataSource; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.plugin.datasource.api.datasource.BaseDataSourceParamDTO; import org.apache.dolphinscheduler.plugin.datasource.api.utils.CommonUtils; @@ -44,8 +46,9 @@ import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils; import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils; import org.apache.dolphinscheduler.spi.datasource.ConnectionParam; import org.apache.dolphinscheduler.spi.enums.DbType; +import org.apache.dolphinscheduler.spi.params.base.ParamsOptions; -import java.util.Map; +import java.util.List; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; @@ -91,8 +94,8 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.CREATED) @ApiException(CREATE_DATASOURCE_ERROR) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result createDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @Parameter(name = "dataSourceParam", description = "DATA_SOURCE_PARAM", required = true) @RequestBody String jsonStr) { + public Result createDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @Parameter(name = "dataSourceParam", description = "DATA_SOURCE_PARAM", required = true) @RequestBody String jsonStr) { BaseDataSourceParamDTO dataSourceParam = DataSourceUtils.buildDatasourceParam(jsonStr); return dataSourceService.createDataSource(loginUser, dataSourceParam); } @@ -115,9 +118,9 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(UPDATE_DATASOURCE_ERROR) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result updateDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @PathVariable(value = "id") Integer id, - @RequestBody String jsonStr) { + public Result updateDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @PathVariable(value = "id") Integer id, + @RequestBody String jsonStr) { BaseDataSourceParamDTO dataSourceParam = DataSourceUtils.buildDatasourceParam(jsonStr); dataSourceParam.setId(id); return dataSourceService.updateDataSource(dataSourceParam.getId(), loginUser, dataSourceParam); @@ -139,11 +142,10 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_DATASOURCE_ERROR) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result queryDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @PathVariable("id") int id) { - - Map result = dataSourceService.queryDataSource(id, loginUser); - return returnDataList(result); + public Result queryDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @PathVariable("id") int id) { + BaseDataSourceParamDTO dataSource = dataSourceService.queryDataSource(id, loginUser); + return Result.success(dataSource); } /** @@ -161,10 +163,10 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_DATASOURCE_ERROR) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result queryDataSourceList(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("type") DbType type) { - Map result = dataSourceService.queryDataSourceList(loginUser, type.ordinal()); - return returnDataList(result); + public Result queryDataSourceList(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam("type") DbType type) { + List datasourceList = dataSourceService.queryDataSourceList(loginUser, type.ordinal()); + return Result.success(datasourceList); } /** @@ -186,16 +188,18 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_DATASOURCE_ERROR) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result queryDataSourceListPaging(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "searchVal", required = false) String searchVal, - @RequestParam("pageNo") Integer pageNo, - @RequestParam("pageSize") Integer pageSize) { - Result result = checkPageParams(pageNo, pageSize); + public Result queryDataSourceListPaging(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam(value = "searchVal", required = false) String searchVal, + @RequestParam("pageNo") Integer pageNo, + @RequestParam("pageSize") Integer pageSize) { + Result result = checkPageParams(pageNo, pageSize); if (!result.checkResult()) { return result; } searchVal = ParameterUtils.handleEscapes(searchVal); - return dataSourceService.queryDataSourceListPaging(loginUser, searchVal, pageNo, pageSize); + PageInfo pageInfo = + dataSourceService.queryDataSourceListPaging(loginUser, searchVal, pageNo, pageSize); + return Result.success(pageInfo); } /** @@ -211,8 +215,8 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(CONNECT_DATASOURCE_FAILURE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result connectDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @io.swagger.v3.oas.annotations.parameters.RequestBody(description = "dataSourceParam") @RequestBody String jsonStr) { + public Result connectDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @io.swagger.v3.oas.annotations.parameters.RequestBody(description = "dataSourceParam") @RequestBody String jsonStr) { BaseDataSourceParamDTO dataSourceParam = DataSourceUtils.buildDatasourceParam(jsonStr); DataSourceUtils.checkDatasourceParam(dataSourceParam); ConnectionParam connectionParams = DataSourceUtils.buildConnectionParams(dataSourceParam); @@ -234,8 +238,8 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(CONNECTION_TEST_FAILURE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result connectionTest(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @PathVariable("id") int id) { + public Result connectionTest(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @PathVariable("id") int id) { return dataSourceService.connectionTest(id); } @@ -254,8 +258,8 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(DELETE_DATA_SOURCE_FAILURE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result deleteDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @PathVariable("id") int id) { + public Result deleteDataSource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @PathVariable("id") int id) { return dataSourceService.delete(loginUser, id); } @@ -274,8 +278,8 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(VERIFY_DATASOURCE_NAME_FAILURE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result verifyDataSourceName(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "name") String name) { + public Result verifyDataSourceName(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam(value = "name") String name) { return dataSourceService.verifyDataSourceName(name); } @@ -294,11 +298,11 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(UNAUTHORIZED_DATASOURCE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result unauthDatasource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("userId") Integer userId) { + public Result unAuthDatasource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam("userId") Integer userId) { - Map result = dataSourceService.unauthDatasource(loginUser, userId); - return returnDataList(result); + List unAuthDatasourceList = dataSourceService.unAuthDatasource(loginUser, userId); + return Result.success(unAuthDatasourceList); } /** @@ -316,11 +320,10 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(AUTHORIZED_DATA_SOURCE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result authedDatasource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("userId") Integer userId) { - - Map result = dataSourceService.authedDatasource(loginUser, userId); - return returnDataList(result); + public Result authedDatasource(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser, + @RequestParam("userId") Integer userId) { + List authedDatasourceList = dataSourceService.authedDatasource(loginUser, userId); + return Result.success(authedDatasourceList); } /** @@ -334,7 +337,7 @@ public class DataSourceController extends BaseController { @ResponseStatus(HttpStatus.OK) @ApiException(KERBEROS_STARTUP_STATE) @AccessLogAnnotation(ignoreRequestArgs = "loginUser") - public Result getKerberosStartupState(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser) { + public Result getKerberosStartupState(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser) { // if upload resource is HDFS and kerberos startup is true , else false return success(Status.SUCCESS.getMsg(), CommonUtils.getKerberosStartupState()); } @@ -347,10 +350,10 @@ public class DataSourceController extends BaseController { @GetMapping(value = "/tables") @ResponseStatus(HttpStatus.OK) @ApiException(GET_DATASOURCE_TABLES_ERROR) - public Result getTables(@RequestParam("datasourceId") Integer datasourceId, - @RequestParam(value = "database") String database) { - Map result = dataSourceService.getTables(datasourceId, database); - return returnDataList(result); + public Result getTables(@RequestParam("datasourceId") Integer datasourceId, + @RequestParam(value = "database") String database) { + List options = dataSourceService.getTables(datasourceId, database); + return Result.success(options); } @Operation(summary = "tableColumns", description = "GET_DATASOURCE_TABLE_COLUMNS_NOTES") @@ -362,11 +365,11 @@ public class DataSourceController extends BaseController { @GetMapping(value = "/tableColumns") @ResponseStatus(HttpStatus.OK) @ApiException(GET_DATASOURCE_TABLE_COLUMNS_ERROR) - public Result getTableColumns(@RequestParam("datasourceId") Integer datasourceId, - @RequestParam("tableName") String tableName, - @RequestParam(value = "database") String database) { - Map result = dataSourceService.getTableColumns(datasourceId, database, tableName); - return returnDataList(result); + public Result getTableColumns(@RequestParam("datasourceId") Integer datasourceId, + @RequestParam("tableName") String tableName, + @RequestParam(value = "database") String database) { + List options = dataSourceService.getTableColumns(datasourceId, database, tableName); + return Result.success(options); } @Operation(summary = "databases", description = "GET_DATASOURCE_DATABASE_NOTES") @@ -376,8 +379,8 @@ public class DataSourceController extends BaseController { @GetMapping(value = "/databases") @ResponseStatus(HttpStatus.OK) @ApiException(GET_DATASOURCE_DATABASES_ERROR) - public Result getDatabases(@RequestParam("datasourceId") Integer datasourceId) { - Map result = dataSourceService.getDatabases(datasourceId); - return returnDataList(result); + public Result getDatabases(@RequestParam("datasourceId") Integer datasourceId) { + List options = dataSourceService.getDatabases(datasourceId); + return Result.success(options); } } diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataSourceService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataSourceService.java index 4cf8b88347..42ab666aa1 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataSourceService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataSourceService.java @@ -17,13 +17,16 @@ package org.apache.dolphinscheduler.api.service; +import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.dao.entity.DataSource; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.plugin.datasource.api.datasource.BaseDataSourceParamDTO; import org.apache.dolphinscheduler.spi.datasource.ConnectionParam; import org.apache.dolphinscheduler.spi.enums.DbType; +import org.apache.dolphinscheduler.spi.params.base.ParamsOptions; -import java.util.Map; +import java.util.List; /** * data source service @@ -55,7 +58,7 @@ public interface DataSourceService { * @param id datasource id * @return data source detail */ - Map queryDataSource(int id, User loginUser); + BaseDataSourceParamDTO queryDataSource(int id, User loginUser); /** * query datasource list by keyword @@ -66,7 +69,7 @@ public interface DataSourceService { * @param pageSize page size * @return data source list page */ - Result queryDataSourceListPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize); + PageInfo queryDataSourceListPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize); /** * query data resource list @@ -75,7 +78,7 @@ public interface DataSourceService { * @param type data source type * @return data source list page */ - Map queryDataSourceList(User loginUser, Integer type); + List queryDataSourceList(User loginUser, Integer type); /** * verify datasource exists @@ -118,7 +121,7 @@ public interface DataSourceService { * @param userId user id * @return unauthed data source result code */ - Map unauthDatasource(User loginUser, Integer userId); + List unAuthDatasource(User loginUser, Integer userId); /** * authorized datasource @@ -127,7 +130,7 @@ public interface DataSourceService { * @param userId user id * @return authorized result code */ - Map authedDatasource(User loginUser, Integer userId); + List authedDatasource(User loginUser, Integer userId); /** * get tables @@ -135,7 +138,7 @@ public interface DataSourceService { * @param database * @return */ - Map getTables(Integer datasourceId, String database); + List getTables(Integer datasourceId, String database); /** * get table columns @@ -144,12 +147,12 @@ public interface DataSourceService { * @param tableName * @return */ - Map getTableColumns(Integer datasourceId, String database, String tableName); + List getTableColumns(Integer datasourceId, String database, String tableName); /** * get databases * @param datasourceId * @return */ - Map getDatabases(Integer datasourceId); + List getDatabases(Integer datasourceId); } 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 844363c064..cc659f02ce 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 @@ -53,10 +53,8 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -228,20 +226,16 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return data source detail */ @Override - public Map queryDataSource(int id, User loginUser) { - - Map result = new HashMap<>(); + public BaseDataSourceParamDTO queryDataSource(int id, User loginUser) { DataSource dataSource = dataSourceMapper.selectById(id); if (dataSource == null) { log.error("Datasource does not exist, id:{}.", id); - putMsg(result, Status.RESOURCE_NOT_EXIST); - return result; + throw new ServiceException(Status.RESOURCE_NOT_EXIST); } if (!canOperatorPermissions(loginUser, new Object[]{dataSource.getId()}, AuthorizationType.DATASOURCE, ApiFuncIdentificationConstant.DATASOURCE)) { - putMsg(result, Status.USER_NO_OPERATION_PERM); - return result; + throw new ServiceException(Status.USER_NO_OPERATION_PERM); } // type @@ -251,9 +245,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource baseDataSourceParamDTO.setName(dataSource.getName()); baseDataSourceParamDTO.setNote(dataSource.getNote()); - result.put(Constants.DATA_LIST, baseDataSourceParamDTO); - putMsg(result, Status.SUCCESS); - return result; + return baseDataSourceParamDTO; } /** @@ -266,8 +258,8 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return data source list page */ @Override - public Result queryDataSourceListPaging(User loginUser, String searchVal, Integer pageNo, Integer pageSize) { - Result result = new Result(); + public PageInfo queryDataSourceListPaging(User loginUser, String searchVal, Integer pageNo, + Integer pageSize) { IPage dataSourceList = null; Page dataSourcePage = new Page<>(pageNo, pageSize); PageInfo pageInfo = new PageInfo<>(pageNo, pageSize); @@ -277,9 +269,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource Set ids = resourcePermissionCheckService .userOwnedResourceIdsAcquisition(AuthorizationType.DATASOURCE, loginUser.getId(), log); if (ids.isEmpty()) { - result.setData(pageInfo); - putMsg(result, Status.SUCCESS); - return result; + return pageInfo; } dataSourceList = dataSourceMapper.selectPagingByIds(dataSourcePage, new ArrayList<>(ids), searchVal); } @@ -288,9 +278,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource handlePasswd(dataSources); pageInfo.setTotal((int) (dataSourceList != null ? dataSourceList.getTotal() : 0L)); pageInfo.setTotalList(dataSources); - result.setData(pageInfo); - putMsg(result, Status.SUCCESS); - return result; + return pageInfo; } /** @@ -322,8 +310,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return data source list page */ @Override - public Map queryDataSourceList(User loginUser, Integer type) { - Map result = new HashMap<>(); + public List queryDataSourceList(User loginUser, Integer type) { List datasourceList = null; if (loginUser.getUserType().equals(UserType.ADMIN_USER)) { @@ -332,16 +319,13 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource Set ids = resourcePermissionCheckService .userOwnedResourceIdsAcquisition(AuthorizationType.DATASOURCE, loginUser.getId(), log); if (ids.isEmpty()) { - result.put(Constants.DATA_LIST, Collections.emptyList()); - putMsg(result, Status.SUCCESS); - return result; + return Collections.emptyList(); } datasourceList = dataSourceMapper.selectBatchIds(ids).stream() .filter(dataSource -> dataSource.getType().getCode() == type).collect(Collectors.toList()); } - result.put(Constants.DATA_LIST, datasourceList); - putMsg(result, Status.SUCCESS); - return result; + + return datasourceList; } /** @@ -468,8 +452,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return unauthed data source result code */ @Override - public Map unauthDatasource(User loginUser, Integer userId) { - Map result = new HashMap<>(); + public List unAuthDatasource(User loginUser, Integer userId) { List datasourceList; if (canOperatorPermissions(loginUser, null, AuthorizationType.DATASOURCE, null)) { // admin gets all data sources except userId @@ -492,9 +475,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource } resultList = new ArrayList<>(datasourceSet); } - result.put(Constants.DATA_LIST, resultList); - putMsg(result, Status.SUCCESS); - return result; + return resultList; } /** @@ -505,19 +486,13 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource * @return authorized result code */ @Override - public Map authedDatasource(User loginUser, Integer userId) { - Map result = new HashMap<>(); - + public List authedDatasource(User loginUser, Integer userId) { List authedDatasourceList = dataSourceMapper.queryAuthedDatasource(userId); - result.put(Constants.DATA_LIST, authedDatasourceList); - putMsg(result, Status.SUCCESS); - return result; + return authedDatasourceList; } @Override - public Map getTables(Integer datasourceId, String database) { - Map result = new HashMap<>(); - + public List getTables(Integer datasourceId, String database) { DataSource dataSource = dataSourceMapper.selectById(datasourceId); List tableList = null; @@ -527,8 +502,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource dataSource.getConnectionParams()); if (null == connectionParam) { - putMsg(result, Status.DATASOURCE_CONNECT_FAILED); - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } Connection connection = @@ -538,8 +512,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource try { if (null == connection) { - putMsg(result, Status.DATASOURCE_CONNECT_FAILED); - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } DatabaseMetaData metaData = connection.getMetaData(); @@ -548,6 +521,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource schema = metaData.getConnection().getSchema(); } catch (SQLException e) { log.error("Cant not get the schema, datasourceId:{}.", datasourceId, e); + throw new ServiceException(Status.GET_DATASOURCE_TABLES_ERROR); } tables = metaData.getTables( @@ -556,8 +530,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource "%", TABLE_TYPES); if (null == tables) { log.error("Get datasource tables error, datasourceId:{}.", datasourceId); - putMsg(result, Status.GET_DATASOURCE_TABLES_ERROR); - return result; + throw new ServiceException(Status.GET_DATASOURCE_TABLES_ERROR); } tableList = new ArrayList<>(); @@ -568,24 +541,18 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource } catch (Exception e) { log.error("Get datasource tables error, datasourceId:{}.", datasourceId, e); - putMsg(result, Status.GET_DATASOURCE_TABLES_ERROR); - return result; + throw new ServiceException(Status.GET_DATASOURCE_TABLES_ERROR); } finally { closeResult(tables); releaseConnection(connection); } List options = getParamsOptions(tableList); - - result.put(Constants.DATA_LIST, options); - putMsg(result, Status.SUCCESS); - return result; + return options; } @Override - public Map getTableColumns(Integer datasourceId, String database, String tableName) { - Map result = new HashMap<>(); - + public List getTableColumns(Integer datasourceId, String database, String tableName) { DataSource dataSource = dataSourceMapper.selectById(datasourceId); BaseConnectionParam connectionParam = (BaseConnectionParam) DataSourceUtils.buildConnectionParams( @@ -593,8 +560,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource dataSource.getConnectionParams()); if (null == connectionParam) { - putMsg(result, Status.DATASOURCE_CONNECT_FAILED); - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } Connection connection = @@ -604,7 +570,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource try { if (null == connection) { - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } DatabaseMetaData metaData = connection.getMetaData(); @@ -614,34 +580,30 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource } rs = metaData.getColumns(database, null, tableName, "%"); if (rs == null) { - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } while (rs.next()) { columnList.add(rs.getString(COLUMN_NAME)); } } catch (Exception e) { log.error("Get datasource table columns error, datasourceId:{}.", dataSource.getId(), e); + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } finally { closeResult(rs); releaseConnection(connection); } List options = getParamsOptions(columnList); - - result.put(Constants.DATA_LIST, options); - putMsg(result, Status.SUCCESS); - return result; + return options; } @Override - public Map getDatabases(Integer datasourceId) { - Map result = new HashMap<>(); + public List getDatabases(Integer datasourceId) { DataSource dataSource = dataSourceMapper.selectById(datasourceId); if (dataSource == null) { - putMsg(result, Status.QUERY_DATASOURCE_ERROR); - return result; + throw new ServiceException(Status.QUERY_DATASOURCE_ERROR); } List tableList; @@ -651,8 +613,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource dataSource.getConnectionParams()); if (null == connectionParam) { - putMsg(result, Status.DATASOURCE_CONNECT_FAILED); - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } Connection connection = @@ -661,8 +622,7 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource try { if (null == connection) { - putMsg(result, Status.DATASOURCE_CONNECT_FAILED); - return result; + throw new ServiceException(Status.DATASOURCE_CONNECT_FAILED); } if (dataSource.getType() == DbType.POSTGRESQL) { rs = connection.createStatement().executeQuery(Constants.DATABASES_QUERY_PG); @@ -676,18 +636,14 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource } } catch (Exception e) { log.error("Get databases error, datasourceId:{}.", datasourceId, e); - putMsg(result, Status.GET_DATASOURCE_TABLES_ERROR); - return result; + throw new ServiceException(Status.GET_DATASOURCE_TABLES_ERROR); } finally { closeResult(rs); releaseConnection(connection); } List options = getParamsOptions(tableList); - - result.put(Constants.DATA_LIST, options); - putMsg(result, Status.SUCCESS); - return result; + return options; } private List getParamsOptions(List columnList) { 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 ca20875191..c1117c27c3 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 @@ -23,8 +23,8 @@ import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.permission.ResourcePermissionCheckService; import org.apache.dolphinscheduler.api.service.impl.BaseServiceImpl; import org.apache.dolphinscheduler.api.service.impl.DataSourceServiceImpl; +import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; -import org.apache.dolphinscheduler.common.constants.Constants; import org.apache.dolphinscheduler.common.constants.DataSourceConstants; import org.apache.dolphinscheduler.common.enums.AuthorizationType; import org.apache.dolphinscheduler.common.enums.UserType; @@ -34,6 +34,7 @@ import org.apache.dolphinscheduler.dao.entity.DataSource; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.dao.mapper.DataSourceMapper; import org.apache.dolphinscheduler.dao.mapper.DataSourceUserMapper; +import org.apache.dolphinscheduler.plugin.datasource.api.datasource.BaseDataSourceParamDTO; import org.apache.dolphinscheduler.plugin.datasource.api.plugin.DataSourceClientProvider; import org.apache.dolphinscheduler.plugin.datasource.api.utils.CommonUtils; import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils; @@ -216,8 +217,9 @@ public class DataSourceServiceTest { int pageNo = 1; int pageSize = 10; - Result result = dataSourceService.queryDataSourceListPaging(loginUser, searchVal, pageNo, pageSize); - Assertions.assertEquals(Status.SUCCESS.getCode(), (int) result.getCode()); + PageInfo pageInfo = + dataSourceService.queryDataSourceListPaging(loginUser, searchVal, pageNo, pageSize); + Assertions.assertNotNull(pageInfo); } @Test @@ -270,9 +272,8 @@ public class DataSourceServiceTest { // test admin user Mockito.when(dataSourceMapper.queryAuthedDatasource(userId)).thenReturn(getSingleDataSourceList()); Mockito.when(dataSourceMapper.queryDatasourceExceptUserId(userId)).thenReturn(getDataSourceList()); - Map result = dataSourceService.unauthDatasource(loginUser, userId); - logger.info(result.toString()); - List dataSources = (List) result.get(Constants.DATA_LIST); + List dataSources = dataSourceService.unAuthDatasource(loginUser, userId); + logger.info(dataSources.toString()); Assertions.assertTrue(CollectionUtils.isNotEmpty(dataSources)); // test non-admin user @@ -280,9 +281,8 @@ public class DataSourceServiceTest { loginUser.setUserType(UserType.GENERAL_USER); Mockito.when(dataSourceMapper.selectByMap(Collections.singletonMap("user_id", loginUser.getId()))) .thenReturn(getDataSourceList()); - result = dataSourceService.unauthDatasource(loginUser, userId); - logger.info(result.toString()); - dataSources = (List) result.get(Constants.DATA_LIST); + dataSources = dataSourceService.unAuthDatasource(loginUser, userId); + logger.info(dataSources.toString()); Assertions.assertTrue(CollectionUtils.isNotEmpty(dataSources)); } @@ -295,17 +295,16 @@ public class DataSourceServiceTest { // test admin user Mockito.when(dataSourceMapper.queryAuthedDatasource(userId)).thenReturn(getSingleDataSourceList()); - Map result = dataSourceService.authedDatasource(loginUser, userId); - logger.info(result.toString()); - List dataSources = (List) result.get(Constants.DATA_LIST); + List dataSources = dataSourceService.authedDatasource(loginUser, userId); + logger.info(dataSources.toString()); Assertions.assertTrue(CollectionUtils.isNotEmpty(dataSources)); // test non-admin user loginUser.setId(2); loginUser.setUserType(UserType.GENERAL_USER); - Map success = dataSourceService.authedDatasource(loginUser, userId); - logger.info(result.toString()); - Assertions.assertEquals(Status.SUCCESS, success.get(Constants.STATUS)); + dataSources = dataSourceService.authedDatasource(loginUser, userId); + logger.info(dataSources.toString()); + Assertions.assertNotNull(dataSources); } @Test @@ -320,9 +319,9 @@ public class DataSourceServiceTest { DataSource dataSource = new DataSource(); dataSource.setType(DbType.MYSQL); Mockito.when(dataSourceMapper.selectBatchIds(dataSourceIds)).thenReturn(Collections.singletonList(dataSource)); - Map map = + List list = dataSourceService.queryDataSourceList(loginUser, DbType.MYSQL.ordinal()); - Assertions.assertEquals(Status.SUCCESS, map.get(Constants.STATUS)); + Assertions.assertNotNull(list); } @Test @@ -341,8 +340,11 @@ public class DataSourceServiceTest { User loginUser = new User(); loginUser.setUserType(UserType.GENERAL_USER); loginUser.setId(2); - Map result = dataSourceService.queryDataSource(Mockito.anyInt(), loginUser); - Assertions.assertEquals(((Status) result.get(Constants.STATUS)).getCode(), Status.RESOURCE_NOT_EXIST.getCode()); + try { + dataSourceService.queryDataSource(Mockito.anyInt(), loginUser); + } catch (Exception e) { + Assertions.assertTrue(e.getMessage().contains(Status.RESOURCE_NOT_EXIST.getMsg())); + } DataSource dataSource = getOracleDataSource(1); Mockito.when(dataSourceMapper.selectById(Mockito.anyInt())).thenReturn(dataSource); @@ -350,8 +352,8 @@ public class DataSourceServiceTest { loginUser.getId(), DATASOURCE, baseServiceLogger)).thenReturn(true); Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.DATASOURCE, new Object[]{dataSource.getId()}, loginUser.getId(), baseServiceLogger)).thenReturn(true); - result = dataSourceService.queryDataSource(dataSource.getId(), loginUser); - Assertions.assertEquals(((Status) result.get(Constants.STATUS)).getCode(), Status.SUCCESS.getCode()); + BaseDataSourceParamDTO paramDTO = dataSourceService.queryDataSource(dataSource.getId(), loginUser); + Assertions.assertNotNull(paramDTO); } private List getDataSourceList() { @@ -524,10 +526,13 @@ public class DataSourceServiceTest { DataSource dataSource = getOracleDataSource(); int datasourceId = 1; dataSource.setId(datasourceId); - Map result; Mockito.when(dataSourceMapper.selectById(datasourceId)).thenReturn(null); - result = dataSourceService.getDatabases(datasourceId); - Assertions.assertEquals(Status.QUERY_DATASOURCE_ERROR, result.get(Constants.STATUS)); + + try { + dataSourceService.getDatabases(datasourceId); + } catch (Exception e) { + Assertions.assertTrue(e.getMessage().contains(Status.QUERY_DATASOURCE_ERROR.getMsg())); + } Mockito.when(dataSourceMapper.selectById(datasourceId)).thenReturn(dataSource); MySQLConnectionParam connectionParam = new MySQLConnectionParam(); @@ -536,13 +541,21 @@ public class DataSourceServiceTest { dataSourceUtils.when(() -> DataSourceUtils.getConnection(Mockito.any(), Mockito.any())).thenReturn(connection); dataSourceUtils.when(() -> DataSourceUtils.buildConnectionParams(Mockito.any(), Mockito.any())) .thenReturn(connectionParam); - result = dataSourceService.getDatabases(datasourceId); - Assertions.assertEquals(Status.GET_DATASOURCE_TABLES_ERROR, result.get(Constants.STATUS)); + + try { + dataSourceService.getDatabases(datasourceId); + } catch (Exception e) { + Assertions.assertTrue(e.getMessage().contains(Status.GET_DATASOURCE_TABLES_ERROR.getMsg())); + } dataSourceUtils.when(() -> DataSourceUtils.buildConnectionParams(Mockito.any(), Mockito.any())) .thenReturn(null); - result = dataSourceService.getDatabases(datasourceId); - Assertions.assertEquals(Status.DATASOURCE_CONNECT_FAILED, result.get(Constants.STATUS)); + + try { + dataSourceService.getDatabases(datasourceId); + } catch (Exception e) { + Assertions.assertTrue(e.getMessage().contains(Status.DATASOURCE_CONNECT_FAILED.getMsg())); + } connection.close(); dataSourceUtils.close(); }