This is an automated email from the ASF dual-hosted git repository.
SbloodyS 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 16044f72af [Improvement-18224][API] Migrate EnvironmentService
Map<String,Object> returns to typed returns (#18225)
16044f72af is described below
commit 16044f72afc1c7e1518c8a331f9de82990e0a2b5
Author: Wenjun Ruan <[email protected]>
AuthorDate: Sat May 9 15:16:10 2026 +0800
[Improvement-18224][API] Migrate EnvironmentService Map<String,Object>
returns to typed returns (#18225)
---
.gitignore | 3 +
dolphinscheduler-api/CLAUDE.md | 46 +++++++-
.../api/controller/EnvironmentController.java | 39 +++----
.../dolphinscheduler/api/python/PythonGateway.java | 9 +-
.../api/service/EnvironmentService.java | 15 ++-
.../api/service/impl/EnvironmentServiceImpl.java | 127 ++++++++-------------
.../api/service/impl/EnvironmentServiceTest.java | 78 +++++--------
dolphinscheduler-master/pom.xml | 2 +-
8 files changed, 151 insertions(+), 168 deletions(-)
diff --git a/.gitignore b/.gitignore
index 47e516d7c7..482e117f35 100644
--- a/.gitignore
+++ b/.gitignore
@@ -55,4 +55,7 @@ dolphinscheduler-master/logs
dolphinscheduler-api/logs
__pycache__
ds_schema_check_test
+
+# Agent
docs/superpowers/
+.claude/worktrees
diff --git a/dolphinscheduler-api/CLAUDE.md b/dolphinscheduler-api/CLAUDE.md
index 1f9db04fc8..b306edfa3d 100644
--- a/dolphinscheduler-api/CLAUDE.md
+++ b/dolphinscheduler-api/CLAUDE.md
@@ -49,8 +49,50 @@ REST API server. Entry point for the UI and external clients
(curl, Python SDK).
## Tests
-- Unit tests in `src/test/java`.
-- **Integration tests live in `dolphinscheduler-api-test`** (separate module,
Docker Compose + Testcontainers).
+Unit tests live in `src/test/java`. **Integration tests live in
`dolphinscheduler-api-test`** (separate module, Docker Compose +
Testcontainers).
+
+### Running unit tests from the command line
+
+The CI flow (`.github/workflows/unit-test.yml`) runs in two phases. Mirror it
locally:
+
+**One-time setup** — install BOM + upstream deps to local m2:
+
+```bash
+export MAVEN_OPTS="-Xmx4g -XX:MetaspaceSize=256m -XX:MaxMetaspaceSize=1024m"
+./mvnw install -B \
+ -pl "dolphinscheduler-bom,dolphinscheduler-api" \
+ -am -DskipTests=true -Dspotless.skip=true -DskipUT=true \
+ -Djacoco.skip=true -Danalyze.skip=true
+```
+
+**Run tests** — use `verify` (NOT `test`), no `-am`, no `clean`:
+
+```bash
+./mvnw verify -B -pl "dolphinscheduler-api" \
+ -Dmaven.test.skip=false -Dspotless.skip=true -DskipUT=false
-Danalyze.skip=true \
+ -Dsurefire.printSummary=true -Dsurefire.useFile=false \
+ -Dsurefire.reportFormat=plain -Dsurefire.redirectTestOutputToFile=false \
+ -Dsurefire.failIfNoSpecifiedTests=false \
+ -Dtest='EnvironmentServiceTest#testVerifyEnvironment'
+```
+
+Drop the `-Dtest=...` filter to run the whole module.
+
+### Why the unusual flags
+
+- **`verify` not `test`** — root pom uses `jacoco-maven-plugin` in
offline-instrument + restore-instrumented-classes mode. Only the `verify`
lifecycle runs the `restore` goal, so `test` alone leaves `target/classes`
instrumented and the next build fails with "Cannot process instrumented class".
+- **`dolphinscheduler-bom` must be in the install `-pl`** — without it, the
installed pom lacks effective dependencyManagement and downstream compile/test
classpaths drop transitive deps like `commons-collections4` and `oshi-core`.
+- **`-DskipTests=true` (not `-Dmaven.test.skip=true`) during install** —
`maven.test.skip` skips test-source attach which breaks downstream modules that
depend on the test-jar.
+- **No `-am` on the test run** — `-am` re-instruments upstream modules with
jacoco and triggers the same "Cannot process instrumented class" failure.
+- **No `clean` between `install` and `verify`** — clean wipes `target/classes`
and the next compile re-resolves transitive deps from the (still installed)
pom; some resolutions fail with `[WARNING] The POM for ... is invalid` and the
build can't see `commons-collections4` etc.
+
+### If things go sideways
+
+- `Cannot process instrumented class` — re-run with `-Djacoco.skip=true`, or
wipe `target/` for the affected module and re-run.
+- `NoClassDefFoundError: oshi/SystemInfo` or
`commons-collections4/CollectionUtils` at test runtime — the bom didn't get
installed; redo the setup step including `dolphinscheduler-bom` in `-pl`.
+- `class file contains wrong class` — local
`~/.m2/.../dolphinscheduler-*-dev-SNAPSHOT.jar` is stale/corrupt. Delete the
offending `dev-SNAPSHOT` directory under
`~/.m2/repository/org/apache/dolphinscheduler/` and re-run install.
+
+Surefire XML reports land at `target/surefire-reports/TEST-<class>.xml` for
the per-test counts.
## Related modules
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/EnvironmentController.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/EnvironmentController.java
index 810f511d8a..25d73d3f44 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/EnvironmentController.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/EnvironmentController.java
@@ -26,6 +26,7 @@ import static
org.apache.dolphinscheduler.api.enums.Status.VERIFY_ENVIRONMENT_ER
import org.apache.dolphinscheduler.api.audit.OperatorLog;
import org.apache.dolphinscheduler.api.audit.enums.AuditType;
+import org.apache.dolphinscheduler.api.dto.EnvironmentDto;
import org.apache.dolphinscheduler.api.exceptions.ApiException;
import org.apache.dolphinscheduler.api.service.EnvironmentService;
import org.apache.dolphinscheduler.api.utils.Result;
@@ -34,7 +35,7 @@ import org.apache.dolphinscheduler.dao.entity.Environment;
import org.apache.dolphinscheduler.dao.entity.User;
import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils;
-import java.util.Map;
+import java.util.List;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
@@ -127,7 +128,6 @@ public class EnvironmentController extends BaseController {
* query environment details by code
*
* @param environmentCode environment code
- * @return environment detail information
*/
@Operation(summary = "queryEnvironmentByCode", description =
"QUERY_ENVIRONMENT_BY_CODE_NOTES")
@Parameters({
@@ -136,11 +136,10 @@ public class EnvironmentController extends BaseController
{
@GetMapping(value = "/query-by-code")
@ResponseStatus(HttpStatus.OK)
@ApiException(QUERY_ENVIRONMENT_BY_CODE_ERROR)
- public Result queryEnvironmentByCode(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
- @RequestParam("environmentCode") Long
environmentCode) {
-
- Map<String, Object> result =
environmentService.queryEnvironmentByCode(environmentCode);
- return returnDataList(result);
+ public Result<EnvironmentDto> queryEnvironmentByCode(@Parameter(hidden =
true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
+
@RequestParam("environmentCode") Long environmentCode) {
+ EnvironmentDto dto =
environmentService.queryEnvironmentByCode(environmentCode);
+ return Result.success(dto);
}
/**
@@ -175,7 +174,6 @@ public class EnvironmentController extends BaseController {
*
* @param loginUser login user
* @param environmentCode environment code
- * @return delete result code
*/
@Operation(summary = "deleteEnvironmentByCode", description =
"DELETE_ENVIRONMENT_BY_CODE_NOTES")
@Parameters({
@@ -185,26 +183,24 @@ public class EnvironmentController extends BaseController
{
@ResponseStatus(HttpStatus.OK)
@ApiException(DELETE_ENVIRONMENT_ERROR)
@OperatorLog(auditType = AuditType.ENVIRONMENT_DELETE)
- public Result deleteEnvironment(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
- @RequestParam("environmentCode") Long
environmentCode) {
-
- Map<String, Object> result =
environmentService.deleteEnvironmentByCode(loginUser, environmentCode);
- return returnDataList(result);
+ public Result<Void> deleteEnvironment(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
+ @RequestParam("environmentCode")
Long environmentCode) {
+ environmentService.deleteEnvironmentByCode(loginUser, environmentCode);
+ return Result.success();
}
/**
* query all environment list
*
* @param loginUser login user
- * @return all environment list
*/
@Operation(summary = "queryAllEnvironmentList", description =
"QUERY_ALL_ENVIRONMENT_LIST_NOTES")
@GetMapping(value = "/query-environment-list")
@ResponseStatus(HttpStatus.OK)
@ApiException(QUERY_ENVIRONMENT_ERROR)
- public Result queryAllEnvironmentList(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
- Map<String, Object> result =
environmentService.queryAllEnvironmentList(loginUser);
- return returnDataList(result);
+ public Result<List<EnvironmentDto>>
queryAllEnvironmentList(@Parameter(hidden = true) @RequestAttribute(value =
Constants.SESSION_USER) User loginUser) {
+ List<EnvironmentDto> dtos =
environmentService.queryAllEnvironmentList(loginUser);
+ return Result.success(dtos);
}
/**
@@ -212,7 +208,6 @@ public class EnvironmentController extends BaseController {
*
* @param loginUser login user
* @param environmentName environment name
- * @return true if the environment name not exists, otherwise return false
*/
@Operation(summary = "verifyEnvironment", description =
"VERIFY_ENVIRONMENT_NOTES")
@Parameters({
@@ -221,9 +216,9 @@ public class EnvironmentController extends BaseController {
@PostMapping(value = "/verify-environment")
@ResponseStatus(HttpStatus.OK)
@ApiException(VERIFY_ENVIRONMENT_ERROR)
- public Result verifyEnvironment(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
- @RequestParam(value = "environmentName")
String environmentName) {
- Map<String, Object> result =
environmentService.verifyEnvironment(environmentName);
- return returnDataList(result);
+ public Result<Void> verifyEnvironment(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
+ @RequestParam(value =
"environmentName") String environmentName) {
+ environmentService.verifyEnvironment(environmentName);
+ return Result.success();
}
}
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java
index 51291d0cc4..6635d97356 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/python/PythonGateway.java
@@ -18,7 +18,6 @@
package org.apache.dolphinscheduler.api.python;
import org.apache.dolphinscheduler.api.configuration.ApiConfig;
-import org.apache.dolphinscheduler.api.dto.EnvironmentDto;
import org.apache.dolphinscheduler.api.dto.resources.ResourceComponent;
import org.apache.dolphinscheduler.api.dto.workflow.WorkflowTriggerRequest;
import org.apache.dolphinscheduler.api.enums.Status;
@@ -610,15 +609,13 @@ public class PythonGateway {
* @param environmentName name of the environment
*/
public Long getEnvironmentInfo(String environmentName) {
- Map<String, Object> result =
environmentService.queryEnvironmentByName(environmentName);
-
- if (result.get("data") == null) {
+ try {
+ return
environmentService.queryEnvironmentByName(environmentName).getCode();
+ } catch (ServiceException e) {
String msg = String.format("Can not find valid environment by name
%s", environmentName);
log.error(msg);
throw new IllegalArgumentException(msg);
}
- EnvironmentDto environmentDto =
EnvironmentDto.class.cast(result.get("data"));
- return environmentDto.getCode();
}
/**
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/EnvironmentService.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/EnvironmentService.java
index de694f7bd0..20ccbd66ef 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/EnvironmentService.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/EnvironmentService.java
@@ -17,11 +17,12 @@
package org.apache.dolphinscheduler.api.service;
+import org.apache.dolphinscheduler.api.dto.EnvironmentDto;
import org.apache.dolphinscheduler.api.utils.Result;
import org.apache.dolphinscheduler.dao.entity.Environment;
import org.apache.dolphinscheduler.dao.entity.User;
-import java.util.Map;
+import java.util.List;
public interface EnvironmentService {
@@ -41,14 +42,14 @@ public interface EnvironmentService {
*
* @param name environment name
*/
- Map<String, Object> queryEnvironmentByName(String name);
+ EnvironmentDto queryEnvironmentByName(String name);
/**
* query environment
*
* @param code environment code
*/
- Map<String, Object> queryEnvironmentByCode(Long code);
+ EnvironmentDto queryEnvironmentByCode(Long code);
/**
* delete environment
@@ -56,7 +57,7 @@ public interface EnvironmentService {
* @param loginUser login user
* @param code environment code
*/
- Map<String, Object> deleteEnvironmentByCode(User loginUser, Long code);
+ void deleteEnvironmentByCode(User loginUser, Long code);
/**
* update environment
@@ -85,16 +86,14 @@ public interface EnvironmentService {
* query all environment
*
* @param loginUser
- * @return all environment list
*/
- Map<String, Object> queryAllEnvironmentList(User loginUser);
+ List<EnvironmentDto> queryAllEnvironmentList(User loginUser);
/**
* verify environment name
*
* @param environmentName environment name
- * @return true if the environment name not exists, otherwise return false
*/
- Map<String, Object> verifyEnvironment(String environmentName);
+ void verifyEnvironment(String environmentName);
}
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java
index 591ab3ecaf..1d96750714 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceImpl.java
@@ -27,7 +27,6 @@ import
org.apache.dolphinscheduler.api.exceptions.ServiceException;
import org.apache.dolphinscheduler.api.service.EnvironmentService;
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.enums.AuthorizationType;
import org.apache.dolphinscheduler.common.enums.UserType;
import org.apache.dolphinscheduler.common.utils.CodeGenerateUtils;
@@ -47,7 +46,6 @@ import org.apache.commons.lang3.StringUtils;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -205,38 +203,28 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
* query all environment
*
* @param loginUser
- * @return all environment list
*/
@Override
- public Map<String, Object> queryAllEnvironmentList(User loginUser) {
- Map<String, Object> result = new HashMap<>();
+ public List<EnvironmentDto> queryAllEnvironmentList(User loginUser) {
Set<Integer> ids =
resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ENVIRONMENT,
loginUser.getId(), log);
if (ids.isEmpty()) {
- result.put(Constants.DATA_LIST, Collections.emptyList());
- putMsg(result, Status.SUCCESS);
- return result;
+ return Collections.emptyList();
}
List<Environment> environmentList =
environmentMapper.selectBatchIds(ids);
- if (CollectionUtils.isNotEmpty(environmentList)) {
- Map<Long, List<String>> relationMap =
relationMapper.selectList(null).stream()
-
.collect(Collectors.groupingBy(EnvironmentWorkerGroupRelation::getEnvironmentCode,
-
Collectors.mapping(EnvironmentWorkerGroupRelation::getWorkerGroup,
Collectors.toList())));
-
- List<EnvironmentDto> dtoList =
environmentList.stream().map(environment -> {
- EnvironmentDto dto = new EnvironmentDto();
- BeanUtils.copyProperties(environment, dto);
- List<String> workerGroups =
relationMap.getOrDefault(environment.getCode(), new ArrayList<String>());
- dto.setWorkerGroups(workerGroups);
- return dto;
- }).collect(Collectors.toList());
- result.put(Constants.DATA_LIST, dtoList);
- } else {
- result.put(Constants.DATA_LIST, new ArrayList<>());
+ if (CollectionUtils.isEmpty(environmentList)) {
+ return Collections.emptyList();
}
+ Map<Long, List<String>> relationMap =
relationMapper.selectList(null).stream()
+
.collect(Collectors.groupingBy(EnvironmentWorkerGroupRelation::getEnvironmentCode,
+
Collectors.mapping(EnvironmentWorkerGroupRelation::getWorkerGroup,
Collectors.toList())));
- putMsg(result, Status.SUCCESS);
- return result;
+ return environmentList.stream().map(environment -> {
+ EnvironmentDto dto = new EnvironmentDto();
+ BeanUtils.copyProperties(environment, dto);
+
dto.setWorkerGroups(relationMap.getOrDefault(environment.getCode(), new
ArrayList<>()));
+ return dto;
+ }).collect(Collectors.toList());
}
/**
@@ -245,25 +233,19 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
* @param code environment code
*/
@Override
- public Map<String, Object> queryEnvironmentByCode(Long code) {
- Map<String, Object> result = new HashMap<>();
-
+ public EnvironmentDto queryEnvironmentByCode(Long code) {
Environment env = environmentMapper.queryByEnvironmentCode(code);
-
if (env == null) {
- putMsg(result, Status.QUERY_ENVIRONMENT_BY_CODE_ERROR, code);
- } else {
- List<String> workerGroups =
relationMapper.queryByEnvironmentCode(env.getCode()).stream()
- .map(item -> item.getWorkerGroup())
- .collect(Collectors.toList());
-
- EnvironmentDto dto = new EnvironmentDto();
- BeanUtils.copyProperties(env, dto);
- dto.setWorkerGroups(workerGroups);
- result.put(Constants.DATA_LIST, dto);
- putMsg(result, Status.SUCCESS);
+ throw new ServiceException(Status.QUERY_ENVIRONMENT_BY_CODE_ERROR,
code);
}
- return result;
+ List<String> workerGroups =
relationMapper.queryByEnvironmentCode(env.getCode()).stream()
+ .map(EnvironmentWorkerGroupRelation::getWorkerGroup)
+ .collect(Collectors.toList());
+
+ EnvironmentDto dto = new EnvironmentDto();
+ BeanUtils.copyProperties(env, dto);
+ dto.setWorkerGroups(workerGroups);
+ return dto;
}
/**
@@ -272,24 +254,19 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
* @param name environment name
*/
@Override
- public Map<String, Object> queryEnvironmentByName(String name) {
- Map<String, Object> result = new HashMap<>();
-
+ public EnvironmentDto queryEnvironmentByName(String name) {
Environment env = environmentMapper.queryByEnvironmentName(name);
if (env == null) {
- putMsg(result, Status.QUERY_ENVIRONMENT_BY_NAME_ERROR, name);
- } else {
- List<String> workerGroups =
relationMapper.queryByEnvironmentCode(env.getCode()).stream()
- .map(item -> item.getWorkerGroup())
- .collect(Collectors.toList());
-
- EnvironmentDto dto = new EnvironmentDto();
- BeanUtils.copyProperties(env, dto);
- dto.setWorkerGroups(workerGroups);
- result.put(Constants.DATA_LIST, dto);
- putMsg(result, Status.SUCCESS);
+ throw new ServiceException(Status.QUERY_ENVIRONMENT_BY_NAME_ERROR,
name);
}
- return result;
+ List<String> workerGroups =
relationMapper.queryByEnvironmentCode(env.getCode()).stream()
+ .map(EnvironmentWorkerGroupRelation::getWorkerGroup)
+ .collect(Collectors.toList());
+
+ EnvironmentDto dto = new EnvironmentDto();
+ BeanUtils.copyProperties(env, dto);
+ dto.setWorkerGroups(workerGroups);
+ return dto;
}
/**
@@ -300,11 +277,9 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
*/
@Transactional
@Override
- public Map<String, Object> deleteEnvironmentByCode(User loginUser, Long
code) {
- Map<String, Object> result = new HashMap<>();
+ public void deleteEnvironmentByCode(User loginUser, Long code) {
if (!canOperatorPermissions(loginUser, null,
AuthorizationType.ENVIRONMENT, ENVIRONMENT_DELETE)) {
- putMsg(result, Status.USER_NO_OPERATION_PERM);
- return result;
+ throw new ServiceException(Status.USER_NO_OPERATION_PERM);
}
Long relatedTaskNumber = taskDefinitionMapper
@@ -313,22 +288,18 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
if (relatedTaskNumber > 0) {
log.warn("Delete environment failed because {} tasks is using it,
environmentCode:{}.",
relatedTaskNumber, code);
- putMsg(result, Status.DELETE_ENVIRONMENT_RELATED_TASK_EXISTS);
- return result;
+ throw new
ServiceException(Status.DELETE_ENVIRONMENT_RELATED_TASK_EXISTS);
}
int delete = environmentMapper.deleteByCode(code);
- if (delete > 0) {
- relationMapper.delete(new
QueryWrapper<EnvironmentWorkerGroupRelation>()
- .lambda()
- .eq(EnvironmentWorkerGroupRelation::getEnvironmentCode,
code));
- log.info("Environment and relations delete complete,
environmentCode:{}.", code);
- putMsg(result, Status.SUCCESS);
- } else {
+ if (delete <= 0) {
log.error("Environment delete error, environmentCode:{}.", code);
- putMsg(result, Status.DELETE_ENVIRONMENT_ERROR);
+ throw new ServiceException(Status.DELETE_ENVIRONMENT_ERROR);
}
- return result;
+ relationMapper.delete(new
QueryWrapper<EnvironmentWorkerGroupRelation>()
+ .lambda()
+ .eq(EnvironmentWorkerGroupRelation::getEnvironmentCode, code));
+ log.info("Environment and relations delete complete,
environmentCode:{}.", code);
}
/**
@@ -418,27 +389,19 @@ public class EnvironmentServiceImpl extends
BaseServiceImpl implements Environme
* verify environment name
*
* @param environmentName environment name
- * @return true if the environment name not exists, otherwise return false
*/
@Override
- public Map<String, Object> verifyEnvironment(String environmentName) {
- Map<String, Object> result = new HashMap<>();
-
+ public void verifyEnvironment(String environmentName) {
if (StringUtils.isEmpty(environmentName)) {
log.warn("parameter environment name is empty.");
- putMsg(result, Status.ENVIRONMENT_NAME_IS_NULL);
- return result;
+ throw new ServiceException(Status.ENVIRONMENT_NAME_IS_NULL);
}
Environment environment =
environmentMapper.queryByEnvironmentName(environmentName);
if (environment != null) {
log.warn("Environment with the same name already exist, name:{}.",
environment.getName());
- putMsg(result, Status.ENVIRONMENT_NAME_EXISTS, environmentName);
- return result;
+ throw new ServiceException(Status.ENVIRONMENT_NAME_EXISTS,
environmentName);
}
-
- result.put(Constants.STATUS, Status.SUCCESS);
- return result;
}
private void checkUsedEnvironmentWorkerGroupRelation(Set<String>
deleteKeySet,
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceTest.java
index 2f375052a5..4304ecf874 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/impl/EnvironmentServiceTest.java
@@ -27,12 +27,12 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
+import org.apache.dolphinscheduler.api.dto.EnvironmentDto;
import org.apache.dolphinscheduler.api.enums.Status;
import
org.apache.dolphinscheduler.api.permission.ResourcePermissionCheckService;
import org.apache.dolphinscheduler.api.utils.PageInfo;
import org.apache.dolphinscheduler.api.utils.Result;
import org.apache.dolphinscheduler.api.utils.ServiceTestUtil;
-import org.apache.dolphinscheduler.common.constants.Constants;
import org.apache.dolphinscheduler.common.enums.AuthorizationType;
import org.apache.dolphinscheduler.common.enums.UserType;
import org.apache.dolphinscheduler.common.utils.CodeGenerateUtils;
@@ -49,7 +49,6 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import org.assertj.core.util.Lists;
@@ -75,7 +74,6 @@ import
com.baomidou.mybatisplus.extension.plugins.pagination.Page;
@MockitoSettings(strictness = Strictness.LENIENT)
public class EnvironmentServiceTest {
- public static final Logger logger =
LoggerFactory.getLogger(EnvironmentServiceTest.class);
private static final Logger baseServiceLogger =
LoggerFactory.getLogger(BaseServiceImpl.class);
private static final Logger environmentServiceLogger =
LoggerFactory.getLogger(EnvironmentServiceImpl.class);
@@ -200,8 +198,8 @@ public class EnvironmentServiceTest {
public void testQueryAllEnvironmentList() {
when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ENVIRONMENT,
1,
environmentServiceLogger)).thenReturn(Collections.emptySet());
- Map<String, Object> result =
environmentService.queryAllEnvironmentList(getAdminUser());
- assertEquals(0, ((List<Environment>)
result.get(Constants.DATA_LIST)).size());
+ List<EnvironmentDto> emptyResult =
environmentService.queryAllEnvironmentList(getAdminUser());
+ assertEquals(0, emptyResult.size());
Set<Integer> ids = new HashSet<>();
ids.add(1);
@@ -209,18 +207,12 @@ public class EnvironmentServiceTest {
1, environmentServiceLogger)).thenReturn(ids);
when(environmentMapper.selectBatchIds(ids)).thenReturn(Lists.newArrayList(getEnvironment()));
- result = environmentService.queryAllEnvironmentList(getAdminUser());
- logger.info(result.toString());
- Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
-
- List<Environment> list = (List<Environment>)
(result.get(Constants.DATA_LIST));
- Assertions.assertEquals(1, list.size());
+ List<EnvironmentDto> oneResult =
environmentService.queryAllEnvironmentList(getAdminUser());
+ assertEquals(1, oneResult.size());
when(environmentMapper.selectBatchIds(ids)).thenReturn(Collections.emptyList());
- result = environmentService.queryAllEnvironmentList(getAdminUser());
- Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
- list = (List<Environment>) (result.get(Constants.DATA_LIST));
- Assertions.assertEquals(0, list.size());
+ List<EnvironmentDto> noneResult =
environmentService.queryAllEnvironmentList(getAdminUser());
+ assertEquals(0, noneResult.size());
}
@Test
@@ -258,67 +250,59 @@ public class EnvironmentServiceTest {
@Test
public void testQueryEnvironmentByName() {
when(environmentMapper.queryByEnvironmentName(environmentName)).thenReturn(null);
- Map<String, Object> result =
environmentService.queryEnvironmentByName(environmentName);
- logger.info(result.toString());
- Assertions.assertEquals(Status.QUERY_ENVIRONMENT_BY_NAME_ERROR,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.QUERY_ENVIRONMENT_BY_NAME_ERROR,
+ () ->
environmentService.queryEnvironmentByName(environmentName));
when(environmentMapper.queryByEnvironmentName(environmentName)).thenReturn(getEnvironment());
- result = environmentService.queryEnvironmentByName(environmentName);
- logger.info(result.toString());
- Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+ EnvironmentDto dto =
environmentService.queryEnvironmentByName(environmentName);
+ Assertions.assertEquals(environmentName, dto.getName());
+ Assertions.assertNotNull(dto.getWorkerGroups());
}
@Test
public void testQueryEnvironmentByCode() {
when(environmentMapper.queryByEnvironmentCode(1L)).thenReturn(null);
- Map<String, Object> result =
environmentService.queryEnvironmentByCode(1L);
- logger.info(result.toString());
- Assertions.assertEquals(Status.QUERY_ENVIRONMENT_BY_CODE_ERROR,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.QUERY_ENVIRONMENT_BY_CODE_ERROR,
+ () -> environmentService.queryEnvironmentByCode(1L));
when(environmentMapper.queryByEnvironmentCode(1L)).thenReturn(getEnvironment());
- result = environmentService.queryEnvironmentByCode(1L);
- logger.info(result.toString());
- Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+ EnvironmentDto dto = environmentService.queryEnvironmentByCode(1L);
+ Assertions.assertEquals(1L, dto.getCode());
+ Assertions.assertNotNull(dto.getWorkerGroups());
}
@Test
public void testDeleteEnvironmentByCode() {
- User loginUser = getGeneralUser();
+ User generalUser = getGeneralUser();
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ENVIRONMENT,
- loginUser.getId(), ENVIRONMENT_DELETE,
baseServiceLogger)).thenReturn(true);
+ generalUser.getId(), ENVIRONMENT_DELETE,
baseServiceLogger)).thenReturn(true);
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ENVIRONMENT,
null,
0, baseServiceLogger)).thenReturn(true);
- Map<String, Object> result =
environmentService.deleteEnvironmentByCode(loginUser, 1L);
- logger.info(result.toString());
- Assertions.assertEquals(Status.USER_NO_OPERATION_PERM,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () -> environmentService.deleteEnvironmentByCode(generalUser,
1L));
- loginUser = getAdminUser();
+ User adminUser = getAdminUser();
when(taskDefinitionMapper.selectCount(any(LambdaQueryWrapper.class))).thenReturn(1L);
- result = environmentService.deleteEnvironmentByCode(loginUser, 1L);
- logger.info(result.toString());
- Assertions.assertEquals(Status.DELETE_ENVIRONMENT_RELATED_TASK_EXISTS,
result.get(Constants.STATUS));
+
assertThrowsServiceException(Status.DELETE_ENVIRONMENT_RELATED_TASK_EXISTS,
+ () -> environmentService.deleteEnvironmentByCode(adminUser,
1L));
when(taskDefinitionMapper.selectCount(any(LambdaQueryWrapper.class))).thenReturn(0L);
when(environmentMapper.deleteByCode(1L)).thenReturn(1);
- result = environmentService.deleteEnvironmentByCode(loginUser, 1L);
- logger.info(result.toString());
- Assertions.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+ assertDoesNotThrow(() ->
environmentService.deleteEnvironmentByCode(adminUser, 1L));
when(environmentMapper.deleteByCode(1L)).thenReturn(-1);
- result = environmentService.deleteEnvironmentByCode(loginUser, 1L);
- Assertions.assertEquals(Status.DELETE_ENVIRONMENT_ERROR,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.DELETE_ENVIRONMENT_ERROR,
+ () -> environmentService.deleteEnvironmentByCode(adminUser,
1L));
}
@Test
public void testVerifyEnvironment() {
- Map<String, Object> result = environmentService.verifyEnvironment("");
- logger.info(result.toString());
- Assertions.assertEquals(Status.ENVIRONMENT_NAME_IS_NULL,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.ENVIRONMENT_NAME_IS_NULL,
+ () -> environmentService.verifyEnvironment(""));
when(environmentMapper.queryByEnvironmentName(environmentName)).thenReturn(getEnvironment());
- result = environmentService.verifyEnvironment(environmentName);
- logger.info(result.toString());
- Assertions.assertEquals(Status.ENVIRONMENT_NAME_EXISTS,
result.get(Constants.STATUS));
+ assertThrowsServiceException(Status.ENVIRONMENT_NAME_EXISTS,
+ () -> environmentService.verifyEnvironment(environmentName));
when(environmentMapper.queryByEnvironmentName(environmentName)).thenReturn(null);
assertDoesNotThrow(() ->
environmentService.verifyEnvironment(environmentName));
diff --git a/dolphinscheduler-master/pom.xml b/dolphinscheduler-master/pom.xml
index 2914a70e49..43b62ce38b 100644
--- a/dolphinscheduler-master/pom.xml
+++ b/dolphinscheduler-master/pom.xml
@@ -397,7 +397,7 @@
<includes>
<include>**/integration/cases/*TestCase.java</include>
</includes>
- <forkCount>4</forkCount>
+ <forkCount>2</forkCount>
<reuseForks>true</reuseForks>
<properties>
<configurationParameters>junit.jupiter.execution.parallel.enabled =
false</configurationParameters>
