Browse Source

Fix homepage user wrong statistics result for new user (#7242)

* Fix homepage user wrong statistics result for new user

For now, when we create a new user, it would see homepage
statistic info as admin user like, cause `projectCodeArray`
would not filter when we render SQL. So I add prefix conditions
to handle this situation.

Also, I do some minimal refactor here. including change function
name no quit fit

fix: #7182

* Fix UT
3.0.0/version-upgrade
Jiajie Zhong 3 years ago committed by GitHub
parent
commit
29414a080f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java
  2. 30
      dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataAnalysisServiceImpl.java
  3. 12
      dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataAnalysisServiceTest.java
  4. 13
      dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapper.java
  5. 6
      dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.java
  6. 12
      dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapper.java
  7. 2
      dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapper.xml
  8. 2
      dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
  9. 2
      dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapper.xml
  10. 6
      dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapperTest.java
  11. 2
      dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapperTest.java
  12. 7
      dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapperTest.java

4
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java

@ -49,7 +49,9 @@ public interface DataAnalysisService {
Map<String, Object> countProcessInstanceStateByProject(User loginUser, long projectCode, String startDate, String endDate); Map<String, Object> countProcessInstanceStateByProject(User loginUser, long projectCode, String startDate, String endDate);
/** /**
* statistics the process definition quantities of certain person * statistics the process definition quantities of a certain person
*
* We only need projects which users have permission to see to determine whether the definition belongs to the user or not.
* *
* @param loginUser login user * @param loginUser login user
* @param projectCode project code * @param projectCode project code

30
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataAnalysisServiceImpl.java

@ -44,6 +44,7 @@ import org.apache.dolphinscheduler.service.process.ProcessService;
import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.StringUtils;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Date; import java.util.Date;
import java.util.HashMap; import java.util.HashMap;
@ -104,7 +105,7 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
projectCode, projectCode,
startDate, startDate,
endDate, endDate,
(start, end, projectCodes) -> this.taskInstanceMapper.countTaskInstanceStateByUser(start, end, projectCodes)); (start, end, projectCodes) -> this.taskInstanceMapper.countTaskInstanceStateByProjectCodes(start, end, projectCodes));
} }
/** /**
@ -123,7 +124,7 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
projectCode, projectCode,
startDate, startDate,
endDate, endDate,
(start, end, projectCodes) -> this.processInstanceMapper.countInstanceStateByUser(start, end, projectCodes)); (start, end, projectCodes) -> this.processInstanceMapper.countInstanceStateByProjectCodes(start, end, projectCodes));
// process state count needs to remove state of forced success // process state count needs to remove state of forced success
if (result.containsKey(Constants.STATUS) && result.get(Constants.STATUS).equals(Status.SUCCESS)) { if (result.containsKey(Constants.STATUS) && result.get(Constants.STATUS).equals(Status.SUCCESS)) {
((TaskCountDto)result.get(Constants.DATA_LIST)).removeStateFromCountList(ExecutionStatus.FORCED_SUCCESS); ((TaskCountDto)result.get(Constants.DATA_LIST)).removeStateFromCountList(ExecutionStatus.FORCED_SUCCESS);
@ -131,6 +132,14 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
return result; return result;
} }
/**
* Wrapper function of counting process instance state and task state
*
* @param loginUser login user
* @param projectCode project code
* @param startDate start date
* @param endDate end date
*/
private Map<String, Object> countStateByProject(User loginUser, long projectCode, String startDate, String endDate private Map<String, Object> countStateByProject(User loginUser, long projectCode, String startDate, String endDate
, TriFunction<Date, Date, Long[], List<ExecuteStatusCount>> instanceStateCounter) { , TriFunction<Date, Date, Long[], List<ExecuteStatusCount>> instanceStateCounter) {
Map<String, Object> result = new HashMap<>(); Map<String, Object> result = new HashMap<>();
@ -154,10 +163,13 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
} }
} }
List<ExecuteStatusCount> processInstanceStateCounts = new ArrayList<>();
Long[] projectCodeArray = projectCode == 0 ? getProjectCodesArrays(loginUser) Long[] projectCodeArray = projectCode == 0 ? getProjectCodesArrays(loginUser)
: new Long[] {projectCode}; : new Long[] {projectCode};
List<ExecuteStatusCount> processInstanceStateCounts =
instanceStateCounter.apply(start, end, projectCodeArray); if (projectCodeArray.length != 0 || loginUser.getUserType() == UserType.ADMIN_USER) {
processInstanceStateCounts = instanceStateCounter.apply(start, end, projectCodeArray);
}
if (processInstanceStateCounts != null) { if (processInstanceStateCounts != null) {
TaskCountDto taskCountResult = new TaskCountDto(processInstanceStateCounts); TaskCountDto taskCountResult = new TaskCountDto(processInstanceStateCounts);
@ -169,7 +181,9 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
/** /**
* statistics the process definition quantities of certain person * statistics the process definition quantities of a certain person
* <p>
* We only need projects which users have permission to see to determine whether the definition belongs to the user or not.
* *
* @param loginUser login user * @param loginUser login user
* @param projectCode project code * @param projectCode project code
@ -187,10 +201,12 @@ public class DataAnalysisServiceImpl extends BaseServiceImpl implements DataAnal
} }
} }
List<DefinitionGroupByUser> defineGroupByUsers = new ArrayList<>();
Long[] projectCodeArray = projectCode == 0 ? getProjectCodesArrays(loginUser) Long[] projectCodeArray = projectCode == 0 ? getProjectCodesArrays(loginUser)
: new Long[] {projectCode}; : new Long[] {projectCode};
List<DefinitionGroupByUser> defineGroupByUsers = processDefinitionMapper.countDefinitionGroupByUser( if (projectCodeArray.length != 0 || loginUser.getUserType() == UserType.ADMIN_USER) {
loginUser.getId(), projectCodeArray, isAdmin(loginUser)); defineGroupByUsers = processDefinitionMapper.countDefinitionByProjectCodes(projectCodeArray);
}
DefineUserDto dto = new DefineUserDto(defineGroupByUsers); DefineUserDto dto = new DefineUserDto(defineGroupByUsers);
result.put(Constants.DATA_LIST, dto); result.put(Constants.DATA_LIST, dto);

12
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataAnalysisServiceTest.java

@ -130,7 +130,7 @@ public class DataAnalysisServiceTest {
Mockito.when(projectMapper.queryByCode(1L)).thenReturn(getProject("test")); Mockito.when(projectMapper.queryByCode(1L)).thenReturn(getProject("test"));
//SUCCESS //SUCCESS
Mockito.when(taskInstanceMapper.countTaskInstanceStateByUser(DateUtils.getScheduleDate(startDate), Mockito.when(taskInstanceMapper.countTaskInstanceStateByProjectCodes(DateUtils.getScheduleDate(startDate),
DateUtils.getScheduleDate(endDate), new Long[]{1L})).thenReturn(getTaskInstanceStateCounts()); DateUtils.getScheduleDate(endDate), new Long[]{1L})).thenReturn(getTaskInstanceStateCounts());
Mockito.when(projectMapper.selectById(Mockito.any())).thenReturn(getProject("test")); Mockito.when(projectMapper.selectById(Mockito.any())).thenReturn(getProject("test"));
Mockito.when(projectService.hasProjectAndPerm(Mockito.any(), Mockito.any(), (Map<String, Object>)Mockito.any())).thenReturn(true); Mockito.when(projectService.hasProjectAndPerm(Mockito.any(), Mockito.any(), (Map<String, Object>)Mockito.any())).thenReturn(true);
@ -187,7 +187,7 @@ public class DataAnalysisServiceTest {
// when general user doesn't have any task then return all count are 0 // when general user doesn't have any task then return all count are 0
user.setUserType(UserType.GENERAL_USER); user.setUserType(UserType.GENERAL_USER);
Mockito.when(taskInstanceMapper.countTaskInstanceStateByUser(any(), any(), any())).thenReturn( Mockito.when(taskInstanceMapper.countTaskInstanceStateByProjectCodes(any(), any(), any())).thenReturn(
Collections.emptyList()); Collections.emptyList());
result = dataAnalysisServiceImpl.countTaskStateByProject(user, 1, null, null); result = dataAnalysisServiceImpl.countTaskStateByProject(user, 1, null, null);
assertThat(result.get(Constants.DATA_LIST)).extracting("totalCount").isEqualTo(0); assertThat(result.get(Constants.DATA_LIST)).extracting("totalCount").isEqualTo(0);
@ -206,7 +206,7 @@ public class DataAnalysisServiceTest {
// when instanceStateCounter return null, then return nothing // when instanceStateCounter return null, then return nothing
user.setUserType(UserType.GENERAL_USER); user.setUserType(UserType.GENERAL_USER);
PowerMockito.when(taskInstanceMapper.countTaskInstanceStateByUser(any(), any(), any())).thenReturn(null); PowerMockito.when(taskInstanceMapper.countTaskInstanceStateByProjectCodes(any(), any(), any())).thenReturn(null);
result = dataAnalysisServiceImpl.countTaskStateByProject(user, 1, null, null); result = dataAnalysisServiceImpl.countTaskStateByProject(user, 1, null, null);
Assert.assertNull(result.get(Constants.DATA_LIST)); Assert.assertNull(result.get(Constants.DATA_LIST));
} }
@ -230,7 +230,7 @@ public class DataAnalysisServiceTest {
Mockito.when(projectService.checkProjectAndAuth(any(), any(), anyLong())).thenReturn(result); Mockito.when(projectService.checkProjectAndAuth(any(), any(), anyLong())).thenReturn(result);
//SUCCESS //SUCCESS
Mockito.when(processInstanceMapper.countInstanceStateByUser(DateUtils.getScheduleDate(startDate), Mockito.when(processInstanceMapper.countInstanceStateByProjectCodes(DateUtils.getScheduleDate(startDate),
DateUtils.getScheduleDate(endDate), new Long[]{1L})).thenReturn(getTaskInstanceStateCounts()); DateUtils.getScheduleDate(endDate), new Long[]{1L})).thenReturn(getTaskInstanceStateCounts());
Mockito.when(projectService.hasProjectAndPerm(Mockito.any(), Mockito.any(), (Map<String, Object>)Mockito.any())).thenReturn(true); Mockito.when(projectService.hasProjectAndPerm(Mockito.any(), Mockito.any(), (Map<String, Object>)Mockito.any())).thenReturn(true);
@ -246,8 +246,8 @@ public class DataAnalysisServiceTest {
putMsg(result, Status.SUCCESS, null); putMsg(result, Status.SUCCESS, null);
Mockito.when(projectService.checkProjectAndAuth(any(), any(), anyLong())).thenReturn(result); Mockito.when(projectService.checkProjectAndAuth(any(), any(), anyLong())).thenReturn(result);
Mockito.when(processDefinitionMapper.countDefinitionGroupByUser(Mockito.anyInt(), Mockito.any(Long[].class), Mockito.when(processDefinitionMapper.countDefinitionByProjectCodes(
Mockito.anyBoolean())).thenReturn(new ArrayList<DefinitionGroupByUser>()); Mockito.any(Long[].class))).thenReturn(new ArrayList<DefinitionGroupByUser>());
result = dataAnalysisServiceImpl.countDefinitionByUser(user, 0); result = dataAnalysisServiceImpl.countDefinitionByUser(user, 0);
Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
} }

13
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapper.java

@ -142,17 +142,14 @@ public interface ProcessDefinitionMapper extends BaseMapper<ProcessDefinition> {
List<ProcessDefinition> queryDefinitionListByTenant(@Param("tenantId") int tenantId); List<ProcessDefinition> queryDefinitionListByTenant(@Param("tenantId") int tenantId);
/** /**
* count process definition group by user * Statistics process definition group by project codes list
* <p>
* We only need project codes to determine whether the definition belongs to the user or not.
* *
* @param userId userId
* @param projectCodes projectCodes * @param projectCodes projectCodes
* @param isAdmin isAdmin * @return definition group by user
* @return process definition list
*/ */
List<DefinitionGroupByUser> countDefinitionGroupByUser( List<DefinitionGroupByUser> countDefinitionByProjectCodes(@Param("projectCodes") Long[] projectCodes);
@Param("userId") Integer userId,
@Param("projectCodes") Long[] projectCodes,
@Param("isAdmin") boolean isAdmin);
/** /**
* list all resource ids * list all resource ids

6
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.java

@ -150,14 +150,16 @@ public interface ProcessInstanceMapper extends BaseMapper<ProcessInstance> {
@Param("destWorkerGroupName") String destWorkerGroupName); @Param("destWorkerGroupName") String destWorkerGroupName);
/** /**
* count process instance state by user * Statistics process instance state by given project codes list
* <p>
* We only need project codes to determine whether the process instance belongs to the user or not.
* *
* @param startTime startTime * @param startTime startTime
* @param endTime endTime * @param endTime endTime
* @param projectCodes projectCodes * @param projectCodes projectCodes
* @return ExecuteStatusCount list * @return ExecuteStatusCount list
*/ */
List<ExecuteStatusCount> countInstanceStateByUser( List<ExecuteStatusCount> countInstanceStateByProjectCodes(
@Param("startTime") Date startTime, @Param("startTime") Date startTime,
@Param("endTime") Date endTime, @Param("endTime") Date endTime,
@Param("projectCodes") Long[] projectCodes); @Param("projectCodes") Long[] projectCodes);

12
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapper.java

@ -57,7 +57,17 @@ public interface TaskInstanceMapper extends BaseMapper<TaskInstance> {
Integer countTask(@Param("projectCodes") Long[] projectCodes, Integer countTask(@Param("projectCodes") Long[] projectCodes,
@Param("taskIds") int[] taskIds); @Param("taskIds") int[] taskIds);
List<ExecuteStatusCount> countTaskInstanceStateByUser(@Param("startTime") Date startTime, /**
* Statistics task instance group by given project codes list
* <p>
* We only need project codes to determine whether the task instance belongs to the user or not.
*
* @param startTime Statistics start time
* @param endTime Statistics end time
* @param projectCodes Project codes list to filter
* @return List of ExecuteStatusCount
*/
List<ExecuteStatusCount> countTaskInstanceStateByProjectCodes(@Param("startTime") Date startTime,
@Param("endTime") Date endTime, @Param("endTime") Date endTime,
@Param("projectCodes") Long[] projectCodes); @Param("projectCodes") Long[] projectCodes);

2
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapper.xml

@ -109,7 +109,7 @@
#{i} #{i}
</foreach> </foreach>
</select> </select>
<select id="countDefinitionGroupByUser" resultType="org.apache.dolphinscheduler.dao.entity.DefinitionGroupByUser"> <select id="countDefinitionByProjectCodes" resultType="org.apache.dolphinscheduler.dao.entity.DefinitionGroupByUser">
SELECT td.user_id as user_id, tu.user_name as user_name, count(0) as count SELECT td.user_id as user_id, tu.user_name as user_name, count(0) as count
FROM t_ds_process_definition td FROM t_ds_process_definition td
JOIN t_ds_user tu on tu.id=td.user_id JOIN t_ds_user tu on tu.id=td.user_id

2
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml

@ -147,7 +147,7 @@
where worker_group = #{originWorkerGroupName} where worker_group = #{originWorkerGroupName}
</update> </update>
<select id="countInstanceStateByUser" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount"> <select id="countInstanceStateByProjectCodes" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount">
select t.state, count(0) as count select t.state, count(0) as count
from t_ds_process_instance t from t_ds_process_instance t
join t_ds_process_definition d on d.code=t.process_definition_code join t_ds_process_definition d on d.code=t.process_definition_code

2
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapper.xml

@ -69,7 +69,7 @@
</foreach> </foreach>
</if> </if>
</select> </select>
<select id="countTaskInstanceStateByUser" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount"> <select id="countTaskInstanceStateByProjectCodes" resultType="org.apache.dolphinscheduler.dao.entity.ExecuteStatusCount">
select state, count(0) as count select state, count(0) as count
from t_ds_task_instance t from t_ds_task_instance t
left join t_ds_task_definition_log d on d.code=t.task_code and d.version=t.task_definition_version left join t_ds_task_definition_log d on d.code=t.task_code and d.version=t.task_definition_version

6
dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessDefinitionMapperTest.java

@ -318,11 +318,7 @@ public class ProcessDefinitionMapperTest extends BaseDaoTest {
Long[] projectCodes = new Long[1]; Long[] projectCodes = new Long[1];
projectCodes[0] = processDefinition.getProjectCode(); projectCodes[0] = processDefinition.getProjectCode();
List<DefinitionGroupByUser> processDefinitions = processDefinitionMapper.countDefinitionGroupByUser( List<DefinitionGroupByUser> processDefinitions = processDefinitionMapper.countDefinitionByProjectCodes(projectCodes);
processDefinition.getUserId(),
projectCodes,
user.getUserType() == UserType.ADMIN_USER
);
Assert.assertNotEquals(processDefinitions.size(), 0); Assert.assertNotEquals(processDefinitions.size(), 0);
} }

2
dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapperTest.java

@ -266,7 +266,7 @@ public class ProcessInstanceMapperTest extends BaseDaoTest {
Long[] projectCodes = new Long[]{processDefinition.getProjectCode()}; Long[] projectCodes = new Long[]{processDefinition.getProjectCode()};
List<ExecuteStatusCount> executeStatusCounts = processInstanceMapper.countInstanceStateByUser(null, null, projectCodes); List<ExecuteStatusCount> executeStatusCounts = processInstanceMapper.countInstanceStateByProjectCodes(null, null, projectCodes);
Assert.assertNotEquals(executeStatusCounts.size(), 0); Assert.assertNotEquals(executeStatusCounts.size(), 0);

7
dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskInstanceMapperTest.java

@ -32,12 +32,7 @@ import java.util.List;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.Rollback;
import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.transaction.annotation.Transactional;
import com.baomidou.mybatisplus.core.metadata.IPage; import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page; import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
@ -342,7 +337,7 @@ public class TaskInstanceMapperTest extends BaseDaoTest {
taskInstanceMapper.updateById(task); taskInstanceMapper.updateById(task);
List<ExecuteStatusCount> count = taskInstanceMapper.countTaskInstanceStateByUser( List<ExecuteStatusCount> count = taskInstanceMapper.countTaskInstanceStateByProjectCodes(
null, null, null, null,
new Long[]{definition.getProjectCode()} new Long[]{definition.getProjectCode()}
); );

Loading…
Cancel
Save