You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2021/07/06 07:01:50 UTC

[GitHub] [dolphinscheduler] kyoty commented on a change in pull request #5651: [Improvement]The owner field in file management is always displayed empty.

kyoty commented on a change in pull request #5651:
URL: https://github.com/apache/dolphinscheduler/pull/5651#discussion_r664287027



##########
File path: dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/entity/Resource.java
##########
@@ -68,10 +68,15 @@
    */
   private int userId;
 
-  /**
-   * resource type
-   */
-  private ResourceType type;
+    /**
+     * user Name
+     */
+    private String userName;

Review comment:
       Thanks @CalvinKirs 
   + I have tried to use a new class, named `ResourceWrapper`, to wrap the Resource and maintain a field like userName, but I found that this may lead to the code to be more difficult to maintain. As the owner must be updated in both `queryResourceListPaging` and `updateResource`. 
   
   + The `updateResource` method is very complicated (this method has nearly 200 lines)and includes the processing of `File Resource` and `UDF Resource`. 
   + If I use `ResourceWrapper` in this method, I also need to determine whether the currently processed resource is FileResource, which will make the code more difficult to maintain. 
   So I gave up this idea, What do you think is more appropriate?
   
   ```java
       @Override
       public Map<String, Object> queryResourceListPaging(User loginUser, int directoryId, ResourceType type, String searchVal, Integer pageNo, Integer pageSize) {
   
           HashMap<String, Object> result = new HashMap<>();
           Page<Resource> page = new Page<>(pageNo, pageSize);
           int userId = loginUser.getId();
           if (isAdmin(loginUser)) {
               userId = 0;
           }
           if (directoryId != -1) {
               Resource directory = resourcesMapper.selectById(directoryId);
               if (directory == null) {
                   putMsg(result, Status.RESOURCE_NOT_EXIST);
                   return result;
               }
           }
   
           List<Integer> resourcesIds = resourceUserMapper.queryResourcesIdListByUserIdAndPerm(userId, 0);
   
           IPage<Resource> resourceIPage = resourcesMapper.queryResourcePaging(page, userId, directoryId, type.ordinal(), searchVal, resourcesIds);
   
           PageInfo<Resource> pageInfo = new PageInfo<>(pageNo, pageSize);
           pageInfo.setTotalCount((int) resourceIPage.getTotal());
           pageInfo.setLists(resourceIPage.getRecords());
           result.put(Constants.DATA_LIST, pageInfo);
           putMsg(result, Status.SUCCESS);
           return result;
       }
   
   
       @Override
       @Transactional(rollbackFor = Exception.class)
       public Result<Object> updateResource(User loginUser,
                                            int resourceId,
                                            String name,
                                            String desc,
                                            ResourceType type,
                                            MultipartFile file) {
           Result<Object> result = checkResourceUploadStartupState();
           if (result.isFailed()) {
               return result;
           }
   
           Resource resource = resourcesMapper.queryResourceAndOwnerByResourceId(resourceId);
          // 省略
       }
   
   ```
   
   
   




-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org