zhongjiajie commented on code in PR #13031: URL: https://github.com/apache/dolphinscheduler/pull/13031#discussion_r1034237604
########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/project/StatisticsStateRequest.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.dto.project; + +import java.util.Date; + +import lombok.Data; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import io.swagger.v3.oas.annotations.media.Schema; + +@Data +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class StatisticsStateRequest { + + @Schema(name = "isAll", example = "isAll") Review Comment: same as start date or end date ```suggestion @Schema(name = "isAll", example = "true") ``` ########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/StatisticsV2Controller.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.controller; + +import static org.apache.dolphinscheduler.api.enums.Status.QUERY_ALL_WORKFLOW_COUNT_ERROR; + +import org.apache.dolphinscheduler.api.aspect.AccessLogAnnotation; +import org.apache.dolphinscheduler.api.dto.project.StatisticsStateRequest; +import org.apache.dolphinscheduler.api.exceptions.ApiException; +import org.apache.dolphinscheduler.api.service.DataAnalysisService; +import org.apache.dolphinscheduler.api.service.ProjectService; +import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.common.constants.Constants; +import org.apache.dolphinscheduler.dao.entity.User; + +import java.util.Map; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.tags.Tag; +/** + * workflow instance controller + */ +@Tag(name = "STATISTICS_V2") +@RestController +@RequestMapping("/v2/statistics") +public class StatisticsV2Controller extends BaseController { + + @Autowired + private ProjectService projectService; + + @Autowired + private DataAnalysisService dataAnalysisService; + /** + * query all workflow count + * @param loginUser login user + * @return workflow count + */ + @Operation(summary = "queryAllWorkflowCount", description = "QUERY_ALL_WORKFLOW_COUNT") + @GetMapping(value = "/workflows/count") + @ResponseStatus(HttpStatus.OK) + @ApiException(QUERY_ALL_WORKFLOW_COUNT_ERROR) Review Comment: `QUERY_ALL_WORKFLOW_COUNT_ERROR` should we use different Exception in different entry point? ########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/StatisticsV2Controller.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.controller; + +import static org.apache.dolphinscheduler.api.enums.Status.QUERY_ALL_WORKFLOW_COUNT_ERROR; + +import org.apache.dolphinscheduler.api.aspect.AccessLogAnnotation; +import org.apache.dolphinscheduler.api.dto.project.StatisticsStateRequest; +import org.apache.dolphinscheduler.api.exceptions.ApiException; +import org.apache.dolphinscheduler.api.service.DataAnalysisService; +import org.apache.dolphinscheduler.api.service.ProjectService; +import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.common.constants.Constants; +import org.apache.dolphinscheduler.dao.entity.User; + +import java.util.Map; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.tags.Tag; +/** + * workflow instance controller + */ +@Tag(name = "STATISTICS_V2") +@RestController +@RequestMapping("/v2/statistics") +public class StatisticsV2Controller extends BaseController { + + @Autowired + private ProjectService projectService; + + @Autowired + private DataAnalysisService dataAnalysisService; + /** + * query all workflow count + * @param loginUser login user + * @return workflow count + */ + @Operation(summary = "queryAllWorkflowCount", description = "QUERY_ALL_WORKFLOW_COUNT") + @GetMapping(value = "/workflows/count") + @ResponseStatus(HttpStatus.OK) + @ApiException(QUERY_ALL_WORKFLOW_COUNT_ERROR) Review Comment: And I find out the Operation. summary is also the same, do you set they same for some reason? ########## dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/DateUtils.java: ########## @@ -480,6 +480,21 @@ public static Date getLastDayOfMonth(Date date) { return cal.getTime(); } + /** + * get a month ago + * + * @param date date + * @return get a month ago + */ + public static Date getMonthAgo(Date date) { Review Comment: can we reuse the function name `addMonths` in this util? ########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/project/StatisticsStateRequest.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.dto.project; + +import java.util.Date; + +import lombok.Data; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import io.swagger.v3.oas.annotations.media.Schema; + +@Data +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class StatisticsStateRequest { + + @Schema(name = "isAll", example = "isAll") + boolean isAll; + + @Schema(name = "projectName", example = "PROJECT-NAME") + String projectName; + + @Schema(name = "workflowName", example = "WORKFLOW-NAME") + String workflowName; + + @Schema(name = "startDate", example = "START-TIME") Review Comment: this example should use a date string instead of a pure string because this is an example of the parameter. Same as end date ```suggestion @Schema(name = "startDate", example = "2022-01-01 10:01:02") ``` ########## dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml: ########## @@ -308,6 +308,30 @@ </if> order by instance.start_time desc,instance.id desc </select> + <select id="countInstanceStateV2" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount"> Review Comment: we should use `org.apache.dolphinscheduler.api.enums.ExecuteType` instead of `org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount`, ExecuteStatusCount if for task status not for workflow ########## dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml: ########## @@ -308,6 +308,30 @@ </if> order by instance.start_time desc,instance.id desc </select> + <select id="countInstanceStateV2" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount"> + select t.state, count(0) as count + from t_ds_process_instance t + join t_ds_process_definition d on d.code=t.process_definition_code + join t_ds_project p on p.code=d.project_code + where 1 = 1 + and t.is_sub_process = 0 + <if test="projectIds != null and projectIds.size() != 0"> + and p.id in + <foreach collection="projectIds" index="index" item="i" open="(" close=")" separator=","> + #{i} + </foreach> + </if> + <if test="startTime != null and endTime != null"> + and t.start_time <![CDATA[ >= ]]> #{startTime} and t.start_time <![CDATA[ <= ]]> #{endTime} + </if> + <if test="model >= 1"> + and p.name = #{projectName} + </if> + <if test="model >= 2"> + and d.name = #{workflowName} Review Comment: should we also support the workflow_code(process definition code) to query the stats? ########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java: ########## @@ -799,4 +809,85 @@ public Result queryAllProjectListForDependent() { return result; } + /** + * query all workflow count + * + * @param loginUser login user + * @return workflow count + */ + @Override + public Result queryAllWorkflowCounts(User loginUser) { + Result result = new Result(); + WorkflowCountResponse workflowCountResponse = new WorkflowCountResponse(); + Set<Integer> projectIds = resourcePermissionCheckService + .userOwnedResourceIdsAcquisition(AuthorizationType.PROJECTS, loginUser.getId(), logger); + if (projectIds.isEmpty()) { + result.setData(workflowCountResponse); + putMsg(result, Status.SUCCESS); + return result; + } + List<Project> projects = projectMapper.selectBatchIds(projectIds); + + List<Long> projectCodes = projects.stream().map(project -> project.getCode()).collect(Collectors.toList()); + + workflowCountResponse.setWorkflowCounts(projectMapper.queryAllWorkflowCounts(projectCodes)); + + putMsg(workflowCountResponse, Status.SUCCESS); + return workflowCountResponse; + } + + /** + * query all workflow States count + * @param loginUser login user + * @param statisticsStateRequest statisticsStateRequest + * @return workflow States count + */ + @Override + public Map<String, Object> countWorkflowStates(User loginUser, + StatisticsStateRequest statisticsStateRequest) { + Map<String, Object> result = new HashMap<>(); + Set<Integer> projectIds = resourcePermissionCheckService + .userOwnedResourceIdsAcquisition(AuthorizationType.PROJECTS, loginUser.getId(), logger); + if (projectIds.isEmpty()) { + putMsg(result, Status.SUCCESS); + return result; + } + String projectName = statisticsStateRequest.getProjectName(); + String workflowName = statisticsStateRequest.getWorkflowName(); + Date date = new Date(); + Date startTime = statisticsStateRequest.getStartTime() == null ? DateUtils.getMonthAgo(date) + : statisticsStateRequest.getStartTime(); + Date endTime = statisticsStateRequest.getEndTime() == null ? date : statisticsStateRequest.getEndTime(); + Integer model = Constants.QUERY_ALL_ON_SYSTEM; + if (!StringUtils.isBlank(projectName)) { + model = Constants.QUERY_ALL_ON_PROJECT; + } + if (!StringUtils.isBlank(workflowName)) { + model = Constants.QUERY_ALL_ON_WORKFLOW; + } + List<ExecuteStatusCount> executeStatusCounts = processInstanceMapper.countInstanceStateV2( + startTime, endTime, projectName, workflowName, model, projectIds); + TaskCountDto taskCountResult = new TaskCountDto(executeStatusCounts); + result.put(Constants.DATA_LIST, taskCountResult); + putMsg(result, Status.SUCCESS); + return result; + } + + @Override + public Map<String, Object> countOneWorkflowStates(User loginUser, String workflowName) { + Map<String, Object> result = new HashMap<>(); + Project project = projectMapper.queryByName(workflowName); + boolean hasProjectAndWritePerm = hasProjectAndWritePerm(loginUser, project, result); + if (!hasProjectAndWritePerm) { + return result; + } + List<ExecuteStatusCount> executeStatusCounts = processInstanceMapper.countInstanceStateV2( + null, null, null, workflowName, Constants.QUERY_ALL_ON_WORKFLOW, null); + if (executeStatusCounts != null) { + TaskCountDto taskCountResult = new TaskCountDto(executeStatusCounts); Review Comment: I think we should not use taskcout dto in workflow stats. but it is odd that the exists function `countProcessInstanceStateByProject` also use this function. do you know why they use them ? ########## dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProjectService.java: ########## @@ -204,4 +205,27 @@ Result queryProjectWithAuthorizedLevelListPaging(Integer userId, User loginUser, */ Result queryAllProjectListForDependent(); + /** + * query all workflow count + * @param loginUser login user + * @return workflow count + */ + Result queryAllWorkflowCounts(User loginUser); Review Comment: A question here, should we make three new interface return the same type? like a more general type about statistics data type for `List[Map<String, Long>]`? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
