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 2022/04/17 09:48:23 UTC

[GitHub] [dolphinscheduler] leiwingqueen opened a new pull request, #9536: bugfix:relation_project_user duplicate

leiwingqueen opened a new pull request, #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536

   <!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
   
   
   ## Purpose of the pull request
   
   <!--(For example: This pull request adds checkstyle plugin).-->
   
   refer to issue [issues:9290](https://github.com/apache/dolphinscheduler/issues/9290)
   
   ## Brief change log
   
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## Verify this pull request
   
   
   <!--*(Please pick either of the following options)*-->
   
   This pull request is code cleanup without any test coverage.
   
   
   <!--*(example:)*
     - *Added dolphinscheduler-dao tests for end-to-end.*
     - *Added CronUtilsTest to verify the change.*
     - *Manually verified the change by testing locally.* -->
   


-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851860931


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/postgresql/dolphinscheduler_ddl.sql:
##########
@@ -45,4 +45,7 @@ $BODY$;
 
 select dolphin_update_metadata();
 
-d//
\ No newline at end of file
+d//
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   thanks,I try to fix it



-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852527885


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed

Review Comment:
   I see. I have change it



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851857643


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/postgresql/dolphinscheduler_ddl.sql:
##########
@@ -45,4 +45,7 @@ $BODY$;
 
 select dolphin_update_metadata();
 
-d//
\ No newline at end of file
+d//
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   Same here.



-- 
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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1100870436

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#9536](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad3a3bb) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/8440baa5e836d35dcebd22ef20b8d24c5e811d9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8440baa) will **increase** coverage by `0.06%`.
   > The diff coverage is `92.30%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #9536      +/-   ##
   ============================================
   + Coverage     39.86%   39.93%   +0.06%     
   - Complexity     4413     4417       +4     
   ============================================
     Files           826      826              
     Lines         33248    33243       -5     
     Branches       3680     3676       -4     
   ============================================
   + Hits          13255    13276      +21     
   + Misses        18763    18743      -20     
   + Partials       1230     1224       -6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...inscheduler/api/service/impl/UsersServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9Vc2Vyc1NlcnZpY2VJbXBsLmphdmE=) | `80.40% <92.30%> (-0.14%)` | :arrow_down: |
   | [...plugin/task/api/parameters/AbstractParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS9wYXJhbWV0ZXJzL0Fic3RyYWN0UGFyYW1ldGVycy5qYXZh) | `23.28% <0.00%> (-2.47%)` | :arrow_down: |
   | [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `53.33% <0.00%> (-1.34%)` | :arrow_down: |
   | [...che/dolphinscheduler/alert/AlertSenderService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC9BbGVydFNlbmRlclNlcnZpY2UuamF2YQ==) | `54.33% <0.00%> (-0.07%)` | :arrow_down: |
   | [...uler/server/worker/runner/WorkerManagerThread.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9ydW5uZXIvV29ya2VyTWFuYWdlclRocmVhZC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ler/server/worker/processor/TaskKillProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9wcm9jZXNzb3IvVGFza0tpbGxQcm9jZXNzb3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../server/worker/processor/TaskExecuteProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9wcm9jZXNzb3IvVGFza0V4ZWN1dGVQcm9jZXNzb3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...org/apache/dolphinscheduler/alert/AlertServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC9BbGVydFNlcnZlci5qYXZh) | `51.35% <0.00%> (ø)` | |
   | [...olphinscheduler/plugin/alert/email/MailSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZW1haWwvTWFpbFNlbmRlci5qYXZh) | `73.61% <0.00%> (+0.18%)` | :arrow_up: |
   | [...uler/api/service/impl/DataAnalysisServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhQW5hbHlzaXNTZXJ2aWNlSW1wbC5qYXZh) | `80.24% <0.00%> (+0.24%)` | :arrow_up: |
   | ... and [6 more](https://codecov.io/gh/apache/dolphinscheduler/pull/9536/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8440baa...ad3a3bb](https://codecov.io/gh/apache/dolphinscheduler/pull/9536?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855722022


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   thanks.but id is the property of project and should not change



-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855721491


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -557,20 +557,16 @@ public Map<String, Object> grantProject(User loginUser, int userId, String proje
         if (check(result, StringUtils.isEmpty(projectIds), Status.SUCCESS)) {
             return result;
         }
-
-        String[] projectIdArr = projectIds.split(",");
-
-        for (String projectId : projectIdArr) {
+        Arrays.stream(projectIds.split(",")).distinct().forEach(pId -> {
             Date now = new Date();
             ProjectUser projectUser = new ProjectUser();
             projectUser.setUserId(userId);
-            projectUser.setProjectId(Integer.parseInt(projectId));
+            projectUser.setProjectId(Integer.parseInt(pId));

Review Comment:
   @SbloodyS name change



-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855788518


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   got it.



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852019275


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -557,20 +557,16 @@ public Map<String, Object> grantProject(User loginUser, int userId, String proje
         if (check(result, StringUtils.isEmpty(projectIds), Status.SUCCESS)) {
             return result;
         }
-
-        String[] projectIdArr = projectIds.split(",");
-
-        for (String projectId : projectIdArr) {
+        Arrays.stream(projectIds.split(",")).distinct().forEach(pId -> {
             Date now = new Date();
             ProjectUser projectUser = new ProjectUser();
             projectUser.setUserId(userId);
-            projectUser.setProjectId(Integer.parseInt(projectId));
+            projectUser.setProjectId(Integer.parseInt(pId));

Review Comment:
   It is recommended to use ```projectId``` instead of ```pId```.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -557,20 +557,16 @@ public Map<String, Object> grantProject(User loginUser, int userId, String proje
         if (check(result, StringUtils.isEmpty(projectIds), Status.SUCCESS)) {
             return result;
         }
-
-        String[] projectIdArr = projectIds.split(",");
-
-        for (String projectId : projectIdArr) {
+        Arrays.stream(projectIds.split(",")).distinct().forEach(pId -> {
             Date now = new Date();
             ProjectUser projectUser = new ProjectUser();
             projectUser.setUserId(userId);
-            projectUser.setProjectId(Integer.parseInt(projectId));
+            projectUser.setProjectId(Integer.parseInt(pId));

Review Comment:
   It is recommended to use ```projectId``` instead of ```pId```.



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855740376


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   @leiwingqueen Hi,variable pu did not change to projectUser.



-- 
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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r854847735


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/mysql/dolphinscheduler_ddl.sql:
##########
@@ -57,3 +57,8 @@ d//
 delimiter ;
 CALL uc_dolphin_T_t_ds_alert_R_sign;
 DROP PROCEDURE uc_dolphin_T_t_ds_alert_R_sign;
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   i think we should add to new SQL directory named `3.0.0`?



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   but this `getId` is object `project`'s function



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851857573


##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_postgresql.sql:
##########
@@ -625,7 +625,8 @@ CREATE TABLE t_ds_relation_project_user (
   perm int DEFAULT '1' ,
   create_time timestamp DEFAULT NULL ,
   update_time timestamp DEFAULT NULL ,
-  PRIMARY KEY (id)
+  PRIMARY KEY (id),
+  UNIQUE KEY uniq_uid_pid(user_id,project_id)

Review Comment:
   There are syntax error in ```postgresql```.



-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1108130258

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=9536)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL)
   
   [![87.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '87.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list) [87.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r856874838


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/mysql/dolphinscheduler_ddl.sql:
##########
@@ -57,3 +57,8 @@ d//
 delimiter ;
 CALL uc_dolphin_T_t_ds_alert_R_sign;
 DROP PROCEDURE uc_dolphin_T_t_ds_alert_R_sign;
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   In general, I think change targe branch with `dev` database change should add to 3.0.0_schema, and committer judge whether should release in bug fix version. If it is only put in 2.0.5 and not in 3.0.0, it may be that the 3.0.0-bate release will miss this commit.
   
   But in case, `dev` branch without `3.0.0_schema` folder,(and that is my wrong, i forgot to add it after 3.0.0-release) so i think it could change into 2.0.6 or 3.0.0



-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1101296626

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=9536)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL)
   
   [![75.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '75.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list) [75.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1104803784

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=9536)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [37 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL)
   
   [![33.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list) [33.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list)  
   [![1.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '1.2%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list) [1.2% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855721491


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -557,20 +557,16 @@ public Map<String, Object> grantProject(User loginUser, int userId, String proje
         if (check(result, StringUtils.isEmpty(projectIds), Status.SUCCESS)) {
             return result;
         }
-
-        String[] projectIdArr = projectIds.split(",");
-
-        for (String projectId : projectIdArr) {
+        Arrays.stream(projectIds.split(",")).distinct().forEach(pId -> {
             Date now = new Date();
             ProjectUser projectUser = new ProjectUser();
             projectUser.setUserId(userId);
-            projectUser.setProjectId(Integer.parseInt(projectId));
+            projectUser.setProjectId(Integer.parseInt(pId));

Review Comment:
   @SbloodyS got it



-- 
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

[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1107445029

   code LGTM, but we still have discussion in https://github.com/apache/dolphinscheduler/pull/9536#discussion_r856874838


-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851754887


##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_postgresql.sql:
##########
@@ -622,7 +622,8 @@ CREATE TABLE t_ds_relation_project_user (
   perm int DEFAULT '1' ,
   create_time timestamp DEFAULT NULL ,
   update_time timestamp DEFAULT NULL ,
-  PRIMARY KEY (id)
+  PRIMARY KEY (id),

Review Comment:
   thank you. I have already fix it



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852021597


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed

Review Comment:
   Is it better to use ```maintain the relationship between project and user if not exists``` to avoid misunstanding?



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed

Review Comment:
   Is it better to use ```maintain the relationship between project and user if not exists``` to avoid misunstanding?



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852020565


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   Same as ```projectId```.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed
+        ProjectUser pu = projectUserMapper.queryProjectRelation(project.getId(), userId);

Review Comment:
   Same as ```projectId```.



-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852036781


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed

Review Comment:
   we won't change the relationship when the relationship already exist. 
   Do you mean that we should throw a error message to user?



-- 
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

[GitHub] [dolphinscheduler] litiliu commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
litiliu commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r857080449


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -609,15 +605,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
             return result;
         }
 
-        // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // 4. maintain the relationship between project and user if not exists
+        ProjectUser projectUser = projectUserMapper.queryProjectRelation(project.getId(), userId);
+        if (projectUser == null) {
+            final Date today = new Date();

Review Comment:
   Maybe the final identifier is not needed?



-- 
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

[GitHub] [dolphinscheduler] leiwingqueen commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
leiwingqueen commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r857282212


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -609,15 +605,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
             return result;
         }
 
-        // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // 4. maintain the relationship between project and user if not exists
+        ProjectUser projectUser = projectUserMapper.queryProjectRelation(project.getId(), userId);
+        if (projectUser == null) {
+            final Date today = new Date();

Review Comment:
   I agree. I try to remove it



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851750952


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -63,14 +63,7 @@
 
 import java.io.IOException;
 import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TimeZone;
+import java.util.*;

Review Comment:
   Please avoid ```import *```



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r852038403


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -610,14 +606,18 @@ public Map<String, Object> grantProjectByCode(final User loginUser, final int us
         }
 
         // 4. maintain the relationship between project and user
-        final Date today = new Date();
-        ProjectUser projectUser = new ProjectUser();
-        projectUser.setUserId(userId);
-        projectUser.setProjectId(project.getId());
-        projectUser.setPerm(7);
-        projectUser.setCreateTime(today);
-        projectUser.setUpdateTime(today);
-        this.projectUserMapper.insert(projectUser);
+        // check if already existed

Review Comment:
   I mean, change the content of the comment. @leiwingqueen 



-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r855960036


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/mysql/dolphinscheduler_ddl.sql:
##########
@@ -57,3 +57,8 @@ d//
 delimiter ;
 CALL uc_dolphin_T_t_ds_alert_R_sign;
 DROP PROCEDURE uc_dolphin_T_t_ds_alert_R_sign;
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   > i think we should add to new SQL directory named `3.0.0`?
   
   It's a bug fix. I think it should be included in 2.0.6-release. WDYT? @zhongjiajie 



-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1106236882

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=9536)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL)
   
   [![40.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40-16px.png '40.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list) [40.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#issuecomment-1100871522

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=9536)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=9536&resolved=false&types=CODE_SMELL)
   
   [![85.7%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '85.7%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list) [85.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=9536&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r851751226


##########
dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_postgresql.sql:
##########
@@ -622,7 +622,8 @@ CREATE TABLE t_ds_relation_project_user (
   perm int DEFAULT '1' ,
   create_time timestamp DEFAULT NULL ,
   update_time timestamp DEFAULT NULL ,
-  PRIMARY KEY (id)
+  PRIMARY KEY (id),

Review Comment:
   Please also add it to ```dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql``` and ```dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema```



-- 
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

[GitHub] [dolphinscheduler] zhongjiajie merged pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
zhongjiajie merged PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536


-- 
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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #9536: bugfix:relation_project_user duplicate

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #9536:
URL: https://github.com/apache/dolphinscheduler/pull/9536#discussion_r856881934


##########
dolphinscheduler-dao/src/main/resources/sql/upgrade/2.0.6_schema/mysql/dolphinscheduler_ddl.sql:
##########
@@ -57,3 +57,8 @@ d//
 delimiter ;
 CALL uc_dolphin_T_t_ds_alert_R_sign;
 DROP PROCEDURE uc_dolphin_T_t_ds_alert_R_sign;
+
+-- add unique key to t_ds_relation_project_user
+ALTER TABLE t_ds_relation_project_user ADD UNIQUE KEY uniq_uid_pid(user_id,project_id);

Review Comment:
   > In general, I think change targe branch with `dev` database change should add to 3.0.0_schema, and committer judge whether should release in bug fix version. If it is only put in 2.0.5 and not in 3.0.0, it may be that the 3.0.0-bate release will miss this commit.
   > 
   > But in case, `dev` branch without `3.0.0_schema` folder,(and that is my wrong, i forgot to add it after 3.0.0-release) so i think it could change into 2.0.6 or 3.0.0
   
   I agree with you. @zhongjiajie 



-- 
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