ruanwenjun commented on a change in pull request #5422:
URL: https://github.com/apache/dolphinscheduler/pull/5422#discussion_r628079662
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
##########
@@ -222,8 +209,8 @@ public Result verifyGroupName(@ApiIgnore
@RequestAttribute(value = Constants.SES
@GetMapping(value = "/list-paging")
@ResponseStatus(HttpStatus.OK)
@ApiException(LIST_PAGING_ALERT_PLUGIN_INSTANCE_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result listPaging(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
+ @AccessLogAnnotation()
+ public Result listPaging(@ApiIgnore
Review comment:
Remove `@ApiIgnore`
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java
##########
@@ -535,23 +503,22 @@ public Result getNodeListByDefinitionId(
/**
* get tasks list by process definition id
*
- * @param loginUser login user
- * @param projectName project name
+ * @param projectName project name
* @param processDefinitionIdList process definition id list
* @return node list data
*/
@ApiOperation(value = "getNodeListByDefinitionIdList", notes =
"GET_NODE_LIST_BY_DEFINITION_ID_NOTES")
@ApiImplicitParams({
- @ApiImplicitParam(name = "processDefinitionIdList", value =
"PROCESS_DEFINITION_ID_LIST", required = true, type = "String")
+ @ApiImplicitParam(name = "processDefinitionIdList", value =
"PROCESS_DEFINITION_ID_LIST", required = true, type = "String")
})
@GetMapping(value = "get-task-list")
@ResponseStatus(HttpStatus.OK)
@ApiException(GET_TASKS_LIST_BY_PROCESS_DEFINITION_ID_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+ @AccessLogAnnotation()
public Result getNodeListByDefinitionIdList(
- @ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User
loginUser,
- @ApiParam(name = "projectName", value = "PROJECT_NAME", required =
true) @PathVariable String projectName,
- @RequestParam("processDefinitionIdList") String
processDefinitionIdList) {
+ @ApiIgnore
Review comment:
Remove
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataAnalysisController.java
##########
@@ -155,37 +144,28 @@ public Result countDefinitionByUser(@ApiIgnore
@RequestAttribute(value = Constan
@GetMapping(value = "/command-state-count")
@ResponseStatus(HttpStatus.OK)
@ApiException(COMMAND_STATE_COUNT_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result countCommandState(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
- @RequestParam(value = "startDate",
required = false) String startDate,
+ @AccessLogAnnotation()
+ public Result countCommandState(@RequestParam(value = "startDate",
required = false) String startDate,
@RequestParam(value = "endDate", required
= false) String endDate,
@RequestParam(value = "projectId",
required = false, defaultValue = "0") int projectId) {
- Map<String, Object> result =
dataAnalysisService.countCommandState(loginUser, projectId, startDate, endDate);
+ Map<String, Object> result =
dataAnalysisService.countCommandState(AuthUtils.getAuthUser(), projectId,
startDate, endDate);
return returnDataList(result);
}
/**
* queue count
*
- * @param loginUser login user
* @param projectId project id
* @return queue state count
*/
- @ApiOperation(value = "countQueueState", notes = "COUNT_QUEUE_STATE_NOTES")
Review comment:
Why remove this annotation?
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java
##########
@@ -245,42 +234,40 @@ public Result updateProcessDefinition(@ApiIgnore
@RequestAttribute(value = Const
@RequestParam(value = "description",
required = false) String description,
@RequestParam(value =
"releaseState", required = false, defaultValue = "OFFLINE") ReleaseState
releaseState) {
- Map<String, Object> result =
processDefinitionService.updateProcessDefinition(loginUser, projectName, id,
name,
- processDefinitionJson, description, locations, connects);
+ Map<String, Object> result =
processDefinitionService.updateProcessDefinition(AuthUtils.getAuthUser(),
projectName, id, name,
+ processDefinitionJson, description, locations, connects);
// If the update fails, the result will be returned directly
if (result.get(Constants.STATUS) != Status.SUCCESS) {
return returnDataList(result);
}
// Judge whether to go online after editing,0 means offline, 1 means
online
if (releaseState == ReleaseState.ONLINE) {
- result =
processDefinitionService.releaseProcessDefinition(loginUser, projectName, id,
releaseState);
+ result =
processDefinitionService.releaseProcessDefinition(AuthUtils.getAuthUser(),
projectName, id, releaseState);
}
return returnDataList(result);
}
/**
* query process definition version paging list info
*
- * @param loginUser login user info
- * @param projectName the process definition project name
- * @param pageNo the process definition version list current
page number
- * @param pageSize the process definition version list page size
+ * @param projectName the process definition project name
Review comment:
It is best not to modify the existing format, you need to take a check
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
##########
@@ -376,14 +348,7 @@ public Result viewResource(@ApiIgnore
@RequestAttribute(value = Constants.SESSIO
/**
* create resource file online
- * @param loginUser
- * @param type
- * @param fileName
- * @param fileSuffix
- * @param description
- * @param content
- * @param pid
- * @param currentDir
+ *
Review comment:
Why remove?
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataSourceController.java
##########
@@ -244,17 +228,16 @@ public Result connectionTest(@ApiIgnore
@RequestAttribute(value = Constants.SESS
@GetMapping(value = "/delete")
@ResponseStatus(HttpStatus.OK)
@ApiException(DELETE_DATA_SOURCE_FAILURE)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result delete(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
+ @AccessLogAnnotation()
+ public Result delete(@ApiIgnore
Review comment:
Remove `@ApiIgnore`
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
##########
@@ -115,25 +111,18 @@
})
@PostMapping(value = "/directory/create")
@ApiException(CREATE_RESOURCE_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result createDirectory(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
- @RequestParam(value = "type") ResourceType
type,
+ @AccessLogAnnotation()
+ public Result createDirectory(@RequestParam(value = "type") ResourceType
type,
@RequestParam(value = "name") String alias,
@RequestParam(value = "description",
required = false) String description,
@RequestParam(value = "pid") int pid,
@RequestParam(value = "currentDir") String
currentDir) {
- return resourceService.createDirectory(loginUser, alias, description,
type, pid, currentDir);
+ return resourceService.createDirectory(AuthUtils.getAuthUser(), alias,
description, type, pid, currentDir);
}
/**
* create resource
- * @param loginUser
- * @param type
- * @param alias
- * @param description
- * @param file
- * @param pid
- * @param currentDir
+ *
Review comment:
Why remove?
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java
##########
@@ -96,147 +95,137 @@
/**
* create process definition
*
- * @param loginUser login user
* @param projectName project name
- * @param name process definition name
- * @param json process definition json
+ * @param name process definition name
+ * @param json process definition json
* @param description description
- * @param locations locations for nodes
- * @param connects connects for nodes
+ * @param locations locations for nodes
+ * @param connects connects for nodes
* @return create result code
*/
@ApiOperation(value = "save", notes = "CREATE_PROCESS_DEFINITION_NOTES")
@ApiImplicitParams({
- @ApiImplicitParam(name = "name", value = "PROCESS_DEFINITION_NAME",
required = true, type = "String"),
- @ApiImplicitParam(name = "processDefinitionJson", value =
"PROCESS_DEFINITION_JSON", required = true, type = "String"),
- @ApiImplicitParam(name = "locations", value =
"PROCESS_DEFINITION_LOCATIONS", required = true, type = "String"),
- @ApiImplicitParam(name = "connects", value =
"PROCESS_DEFINITION_CONNECTS", required = true, type = "String"),
- @ApiImplicitParam(name = "description", value =
"PROCESS_DEFINITION_DESC", required = false, type = "String"),
+ @ApiImplicitParam(name = "name", value =
"PROCESS_DEFINITION_NAME", required = true, type = "String"),
+ @ApiImplicitParam(name = "processDefinitionJson", value =
"PROCESS_DEFINITION_JSON", required = true, type = "String"),
+ @ApiImplicitParam(name = "locations", value =
"PROCESS_DEFINITION_LOCATIONS", required = true, type = "String"),
+ @ApiImplicitParam(name = "connects", value =
"PROCESS_DEFINITION_CONNECTS", required = true, type = "String"),
+ @ApiImplicitParam(name = "description", value =
"PROCESS_DEFINITION_DESC", required = false, type = "String"),
})
@PostMapping(value = "/save")
@ResponseStatus(HttpStatus.CREATED)
@ApiException(CREATE_PROCESS_DEFINITION)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result createProcessDefinition(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
- @ApiParam(name = "projectName",
value = "PROJECT_NAME", required = true) @PathVariable String projectName,
+ @AccessLogAnnotation()
+ public Result createProcessDefinition(@ApiParam(name = "projectName",
value = "PROJECT_NAME", required = true) @PathVariable String projectName,
@RequestParam(value = "name",
required = true) String name,
@RequestParam(value =
"processDefinitionJson", required = true) String json,
@RequestParam(value = "locations",
required = true) String locations,
@RequestParam(value = "connects",
required = true) String connects,
@RequestParam(value = "description",
required = false) String description) throws JsonProcessingException {
- Map<String, Object> result =
processDefinitionService.createProcessDefinition(loginUser, projectName, name,
json,
- description, locations, connects);
+ Map<String, Object> result =
processDefinitionService.createProcessDefinition(AuthUtils.getAuthUser(),
projectName, name, json,
+ description, locations, connects);
return returnDataList(result);
}
/**
* copy process definition
*
- * @param loginUser login user
- * @param projectName project name
+ * @param projectName project name
* @param processDefinitionIds process definition ids
- * @param targetProjectId target project id
+ * @param targetProjectId target project id
* @return copy result code
*/
@ApiOperation(value = "copyProcessDefinition", notes =
"COPY_PROCESS_DEFINITION_NOTES")
@ApiImplicitParams({
- @ApiImplicitParam(name = "processDefinitionIds", value =
"PROCESS_DEFINITION_IDS", required = true, dataType = "String", example =
"3,4"),
- @ApiImplicitParam(name = "targetProjectId", value =
"TARGET_PROJECT_ID", required = true, dataType = "Int", example = "10")
+ @ApiImplicitParam(name = "processDefinitionIds", value =
"PROCESS_DEFINITION_IDS", required = true, dataType = "String", example =
"3,4"),
Review comment:
It is best not to modify the existing format, the same goes for below.
##########
File path:
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionControllerTest.java
##########
@@ -17,415 +17,488 @@
package org.apache.dolphinscheduler.api.controller;
+import static
org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static
org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static
org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static
org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
import org.apache.dolphinscheduler.api.enums.Status;
-import org.apache.dolphinscheduler.api.service.ProcessDefinitionVersionService;
-import
org.apache.dolphinscheduler.api.service.impl.ProcessDefinitionServiceImpl;
-import org.apache.dolphinscheduler.api.utils.PageInfo;
import org.apache.dolphinscheduler.api.utils.Result;
-import org.apache.dolphinscheduler.common.Constants;
import org.apache.dolphinscheduler.common.enums.ReleaseState;
-import org.apache.dolphinscheduler.common.enums.UserType;
+import org.apache.dolphinscheduler.common.utils.JSONUtils;
import org.apache.dolphinscheduler.dao.entity.ProcessDefinition;
import org.apache.dolphinscheduler.dao.entity.ProcessDefinitionVersion;
-import org.apache.dolphinscheduler.dao.entity.Resource;
-import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.ProcessDefinitionMapper;
+import org.apache.dolphinscheduler.dao.mapper.ProcessDefinitionVersionMapper;
-import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
-
-import javax.servlet.http.HttpServletResponse;
import org.junit.Assert;
-import org.junit.Before;
+import org.junit.FixMethodOrder;
import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.junit.runners.MethodSorters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.MediaType;
+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+
+import com.baomidou.mybatisplus.core.conditions.query.QueryWrapper;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import com.fasterxml.jackson.core.type.TypeReference;
+
/**
* process definition controller test
*/
-@RunWith(MockitoJUnitRunner.Silent.class)
-public class ProcessDefinitionControllerTest {
-
- private static Logger logger =
LoggerFactory.getLogger(ProcessDefinitionControllerTest.class);
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class ProcessDefinitionControllerTest extends AbstractControllerTest {
+ private static final Logger logger =
LoggerFactory.getLogger(ProcessDefinitionControllerTest.class);
- @InjectMocks
- private ProcessDefinitionController processDefinitionController;
+ public static String projectName;
+ public static String projectId;
+ public static String definitionId;
- @Mock
- private ProcessDefinitionServiceImpl processDefinitionService;
+ @Autowired
+ private ProcessDefinitionVersionMapper processDefinitionVersionMapper;
Review comment:
If you remove @Mock, this will affect the unit test when there is no
database, I am not sure. It does not seem to be using DB2 at the moment
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ExecutorController.java
##########
@@ -151,20 +147,19 @@ public Result startProcessInstance(@ApiIgnore
@RequestAttribute(value = Constant
@PostMapping(value = "/execute")
@ResponseStatus(HttpStatus.OK)
@ApiException(EXECUTE_PROCESS_INSTANCE_ERROR)
- @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
- public Result execute(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
+ @AccessLogAnnotation()
+ public Result execute(@ApiIgnore
Review comment:
Remove `@ApiIgnore`
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/AuthUtils.java
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.api.utils;
+
+import org.apache.dolphinscheduler.common.Constants;
+import org.apache.dolphinscheduler.dao.entity.User;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.util.ObjectUtils;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;
+
+public class AuthUtils {
+
+ private AuthUtils() {
+ throw new IllegalStateException("AuthUtil class");
+ }
+
+ /**
+ * get LoginUser info from request
+ * @return User
+ */
+ public static User getAuthUser() {
Review comment:
It is recommended to change the method name, the user here is only the
login user, the method name will make confusion. Permission verification will
be added in the future
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
