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 2020/01/02 08:58:27 UTC

[GitHub] [incubator-dolphinscheduler] samz406 opened a new pull request #1684: fix get tenantCode my NPE

samz406 opened a new pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684
 
 
   For #1465
   1 fix get tenantCode my NPE
   2 add ResourcesServiceTest
   
   **modify class**
   
   ResourcesService
   
   **add UT**
   ResourcesServiceTest
   
   **modify root pom**
   
   `<include>**/api/service/ResourcesServiceTest.java</include>
   `

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r362805960
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -234,6 +234,12 @@ public Result updateResource(User loginUser,
             }
         }
 
+        // query tenant by user id
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
 
 Review comment:
   Is necessary to putMsg and log error?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] codecov-io commented on issue #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#issuecomment-570149474
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=h1) Report
   > Merging [#1684](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=desc) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/22a2d43d7c491d5a1acc5176c4ca26b842d1c99b?src=pr&el=desc) will **increase** coverage by `2.4%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/graphs/tree.svg?width=650&token=bv9iXXRLi9&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff            @@
   ##              dev   #1684     +/-   ##
   ========================================
   + Coverage   15.09%   17.5%   +2.4%     
   ========================================
     Files         283     283             
     Lines       13801   13817     +16     
     Branches     2244    2250      +6     
   ========================================
   + Hits         2083    2418    +335     
   + Misses      11496   11148    -348     
   - Partials      222     251     +29
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...dolphinscheduler/api/service/ResourcesService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvUmVzb3VyY2VzU2VydmljZS5qYXZh) | `79.85% <100%> (+79.85%)` | :arrow_up: |
   | [...ache/dolphinscheduler/api/service/BaseService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvQmFzZVNlcnZpY2UuamF2YQ==) | `77.77% <0%> (+22.22%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=footer). Last update [22a2d43...3260412](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
samz406 commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r363013870
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -644,18 +654,20 @@ public Result updateResourceContent(int resourceId, String content) {
         if (StringUtils.isNotEmpty(resourceViewSuffixs)) {
             List<String> strList = Arrays.asList(resourceViewSuffixs.split(","));
             if (!strList.contains(nameSuffix)) {
-                logger.error("resouce suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
+                logger.error("resource suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
                 putMsg(result, Status.RESOURCE_SUFFIX_NOT_SUPPORT_VIEW);
                 return result;
             }
         }
 
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
 
 Review comment:
   add error log  in getTenantCode method

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
samz406 commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r363013907
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -522,8 +529,11 @@ public Result readResource(int resourceId, int skipLineNum, int limit) {
             }
         }
 
-        User user = userMapper.queryDetailsById(resource.getUserId());
-        String tenantCode = tenantMapper.queryById(user.getTenantId()).getTenantCode();
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
+            return  result;
 
 Review comment:
   add error log in getTenantCode method
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
samz406 commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r362812224
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -644,18 +654,20 @@ public Result updateResourceContent(int resourceId, String content) {
         if (StringUtils.isNotEmpty(resourceViewSuffixs)) {
             List<String> strList = Arrays.asList(resourceViewSuffixs.split(","));
             if (!strList.contains(nameSuffix)) {
-                logger.error("resouce suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
+                logger.error("resource suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
                 putMsg(result, Status.RESOURCE_SUFFIX_NOT_SUPPORT_VIEW);
                 return result;
             }
         }
 
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
 
 Review comment:
   Since a tenant is required to upload a resource, the resource and tenant are associated through the user. If the user on resouce is illegal. Then the tenant cannot be obtained, and the null pointer exception will be thrown when the tenantCode is obtained.
   
   由于上传resource需要tenant，resource和tenant是通过用户关联，如果resource上的用户是非法的。那么就获取不到tenant了， 获取tenantCode也就会抛空指针异常

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r362806342
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -644,18 +654,20 @@ public Result updateResourceContent(int resourceId, String content) {
         if (StringUtils.isNotEmpty(resourceViewSuffixs)) {
             List<String> strList = Arrays.asList(resourceViewSuffixs.split(","));
             if (!strList.contains(nameSuffix)) {
-                logger.error("resouce suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
+                logger.error("resource suffix {} not support updateProcessInstance,  resource id {}", nameSuffix, resourceId);
                 putMsg(result, Status.RESOURCE_SUFFIX_NOT_SUPPORT_VIEW);
                 return result;
             }
         }
 
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
 
 Review comment:
   Is necessary to putMsg and log error?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
khadgarmage commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r362806265
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -522,8 +529,11 @@ public Result readResource(int resourceId, int skipLineNum, int limit) {
             }
         }
 
-        User user = userMapper.queryDetailsById(resource.getUserId());
-        String tenantCode = tenantMapper.queryById(user.getTenantId()).getTenantCode();
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
+            return  result;
 
 Review comment:
   Is necessary to putMsg and log error?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
samz406 commented on a change in pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#discussion_r363013901
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java
 ##########
 @@ -234,6 +234,12 @@ public Result updateResource(User loginUser,
             }
         }
 
+        // query tenant by user id
+        String tenantCode = getTenantCode(resource.getUserId(),result);
+        if (StringUtils.isEmpty(tenantCode)){
 
 Review comment:
   add error log in getTenantCode method
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] codecov-io edited a comment on issue #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684#issuecomment-570149474
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`dev@d5fe55a`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `37.2%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/graphs/tree.svg?width=650&token=bv9iXXRLi9&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=tree)
   
   ```diff
   @@          Coverage Diff           @@
   ##             dev    #1684   +/-   ##
   ======================================
     Coverage       ?   19.55%           
   ======================================
     Files          ?      285           
     Lines          ?    13923           
     Branches       ?     2279           
   ======================================
     Hits           ?     2722           
     Misses         ?    10897           
     Partials       ?      304
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...inscheduler/common/job/db/DB2ServerDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9EQjJTZXJ2ZXJEYXRhU291cmNlLmphdmE=) | `0% <ø> (ø)` | |
   | [...apache/dolphinscheduler/common/model/TaskNode.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL21vZGVsL1Rhc2tOb2RlLmphdmE=) | `0% <ø> (ø)` | |
   | [...inscheduler/common/job/db/SQLServerDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9TUUxTZXJ2ZXJEYXRhU291cmNlLmphdmE=) | `0% <ø> (ø)` | |
   | [...a/org/apache/dolphinscheduler/dao/entity/User.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9Vc2VyLmphdmE=) | `0% <ø> (ø)` | |
   | [...phinscheduler/common/job/db/PostgreDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9Qb3N0Z3JlRGF0YVNvdXJjZS5qYXZh) | `0% <ø> (ø)` | |
   | [...olphinscheduler/common/job/db/SparkDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9TcGFya0RhdGFTb3VyY2UuamF2YQ==) | `0% <ø> (ø)` | |
   | [...dolphinscheduler/common/utils/EncryptionUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0VuY3J5cHRpb25VdGlscy5qYXZh) | `100% <ø> (ø)` | |
   | [...e/dolphinscheduler/common/zk/AbstractZKClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3prL0Fic3RyYWN0WktDbGllbnQuamF2YQ==) | `0% <ø> (ø)` | |
   | [...nscheduler/common/job/db/ClickHouseDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9DbGlja0hvdXNlRGF0YVNvdXJjZS5qYXZh) | `0% <ø> (ø)` | |
   | [...olphinscheduler/common/job/db/MySQLDataSource.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2pvYi9kYi9NeVNRTERhdGFTb3VyY2UuamF2YQ==) | `0% <ø> (ø)` | |
   | ... and [13 more](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=footer). Last update [d5fe55a...b0d73fb](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1684?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] khadgarmage merged pull request #1684: fix get tenantCode my NPE

Posted by GitBox <gi...@apache.org>.
khadgarmage merged pull request #1684: fix get tenantCode my NPE
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1684
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services