You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/05/12 02:22:11 UTC

[GitHub] [shardingsphere] zhujunxxxxx opened a new pull request #10310: Users removed from StandardMetaDataContexts

zhujunxxxxx opened a new pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310


   Fixes #10251 .
   
   Changes proposed in this pull request:
   - users  removed from StandardMetaDataContexts
   


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

[GitHub] [shardingsphere] tristaZero commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841640571


   Hi @zhujunxxxxx How about this PR?


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

[GitHub] [shardingsphere] tristaZero merged pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310


   


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

[GitHub] [shardingsphere] zhujunxxxxx commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841805884


   @tristaZero  I'm hard working. ^_^


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

[GitHub] [shardingsphere] zhujunxxxxx commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841805884


   @tristaZero  I'm hard working. ^_^


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

[GitHub] [shardingsphere] zhujunxxxxx commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r633587443



##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {

Review comment:
       > Maybe the first argument of validate is possible to be password rather thangrantee?
   
   This would be better, but it would cause MySQL and PostgreSQL to be incompatible. so I extract the password validation rule.




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

[GitHub] [shardingsphere] zhujunxxxxx commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r633587443



##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {

Review comment:
       > Maybe the first argument of validate is possible to be password rather thangrantee?
   
   This would be better, but it would cause MySQL and PostgreSQL to be incompatible. so I extract the password validation rule.




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

[GitHub] [shardingsphere] huanghao495430759 commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
huanghao495430759 commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630778168



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {

Review comment:
       Hi.because UserChangedListener will be remove in the future,so this method all whould be remove




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

[GitHub] [shardingsphere] codecov-commenter commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841820920


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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 [#10310](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eeac38d) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ea2f7a390a811c9d0f96648d232bd897036a55be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea2f7a3) will **decrease** coverage by `0.08%`.
   > The diff coverage is `42.18%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10310/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10310      +/-   ##
   ============================================
   - Coverage     68.60%   68.52%   -0.09%     
     Complexity      691      691              
   ============================================
     Files          1727     1727              
     Lines         29438    29461      +23     
     Branches       5293     5301       +8     
   ============================================
   - Hits          20197    20188       -9     
   - Misses         7677     7708      +31     
   - Partials       1564     1565       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...dingsphere/authority/checker/AuthorityChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9jaGVja2VyL0F1dGhvcml0eUNoZWNrZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/shardingsphere/authority/rule/AuthorityRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9ydWxlL0F1dGhvcml0eVJ1bGUuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ingsphere/infra/executor/check/SQLCheckEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL2NoZWNrL1NRTENoZWNrRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ontext/metadata/impl/StandardMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9pbXBsL1N0YW5kYXJkTWV0YURhdGFDb250ZXh0cy5qYXZh) | `76.47% <50.00%> (-1.31%)` | `0.00 <0.00> (ø)` | |
   | [...sql/authentication/MySQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3Byb3h5L2Zyb250ZW5kL215c3FsL2F1dGhlbnRpY2F0aW9uL015U1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `92.00% <71.42%> (-3.66%)` | `0.00 <0.00> (ø)` | |
   | [...uthentication/PostgreSQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9hdXRoZW50aWNhdGlvbi9Qb3N0Z3JlU1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `88.46% <86.66%> (-1.02%)` | `0.00 <0.00> (ø)` | |
   | [...e/context/metadata/GovernanceMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9tZXRhZGF0YS9Hb3Zlcm5hbmNlTWV0YURhdGFDb250ZXh0cy5qYXZh) | `83.66% <87.50%> (-2.15%)` | `0.00 <0.00> (ø)` | |
   | [...nfra/context/metadata/MetaDataContextsBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9NZXRhRGF0YUNvbnRleHRzQnVpbGRlci5qYXZh) | `94.87% <100.00%> (+3.76%)` | `0.00 <0.00> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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/shardingsphere/pull/10310?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 [ea2f7a3...eeac38d](https://codecov.io/gh/apache/shardingsphere/pull/10310?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.

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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630955100



##########
File path: shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/authentication/MySQLAuthenticationHandler.java
##########
@@ -53,14 +57,24 @@
      * @return login success or failure
      */
     public Optional<MySQLServerErrorCode> login(final String username, final String hostname, final byte[] authenticationResponse, final String databaseName) {
-        Optional<ShardingSphereUser> user = ProxyContext.getInstance().getMetaDataContexts().getUsers().findUser(new Grantee(username, hostname));
+        Optional<ShardingSphereUser> user = getUsersFromGlobalRuleMetaData().findUser(new Grantee(username, hostname));
         if (!user.isPresent() || !isPasswordRight(user.get().getPassword(), authenticationResponse)) {
             return Optional.of(MySQLServerErrorCode.ER_ACCESS_DENIED_ERROR);
         }
         return null == databaseName || SQLCheckEngine.check(databaseName, getRules(databaseName), user.get().getGrantee())
                 ? Optional.empty() : Optional.of(MySQLServerErrorCode.ER_DBACCESS_DENIED_ERROR);
     }
-    
+
+    private ShardingSphereUsers getUsersFromGlobalRuleMetaData() {
+        for (RuleConfiguration ruleConfig : ProxyContext.getInstance().getMetaDataContexts().getGlobalRuleMetaData().getConfigurations()) {
+            if (ruleConfig instanceof AuthorityRuleConfiguration) {
+                AuthorityRuleConfiguration authorityRuleConfiguration = (AuthorityRuleConfiguration) ruleConfig;
+                return new ShardingSphereUsers(authorityRuleConfiguration.getUsers());
+            }
+        }
+        return new ShardingSphereUsers(Collections.emptyList());
+    }

Review comment:
       Hi,
   I guess you still have this sense that this approach is not so elegant, don't you? Here an idea FYI,
   We can add a new interface `check(Grantee)` in `SQLCheckEngine`, `SQLChecker` and its implementation, i.e., `AuthorityChecker`. Consequently, all authority checking processes will be hidden in `AuthorityChecker`.
   How do you think?

##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {
-        Collection<ShardingSphereUser> users = new HashSet<>(getNewUsers(event.getUsers()));
-        users.addAll(getModifiedUsers(event.getUsers()));
         metaDataContexts = new StandardMetaDataContexts(
-                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), new ShardingSphereUsers(users), metaDataContexts.getProps());
+                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), metaDataContexts.getProps());

Review comment:
       It is pointless to assign a copied `StandardMetaDataContexts` to `metaDataContexts`.
   getChangedMetaData() of `renew(final RuleConfigurationsChangedEvent event)` is a better method to create a new `global authority rule` to truly `renew`. (Not efficient, but correct)

##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/test/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContextsTest.java
##########
@@ -155,12 +154,12 @@ public void assertPropertiesChanged() {
         governanceMetaDataContexts.renew(event);
         assertThat(governanceMetaDataContexts.getProps().getProps().getProperty(ConfigurationPropertyKey.SQL_SHOW.getKey()), is("true"));
     }
-    
+
     @Test
     public void assertAuthorityChanged() {
         AuthorityChangedEvent event = new AuthorityChangedEvent(Collections.singleton(mock(ShardingSphereUser.class)));
         governanceMetaDataContexts.renew(event);
-        assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));
+        //assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));

Review comment:
       So why is this statement commented?




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

[GitHub] [shardingsphere] zhujunxxxxx commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-839706463


   Hi @tristaZero,  according to your advice, I removed `users` from  `StandardMetaDataContexts`
   please review this PR.


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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r634039265



##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,25 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        return user.isPresent() ? true : false;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        if (validate.test(user.get(), cipher)) {
+            return true;
+        }
+        return false;

Review comment:
       TODO-1,
   Please consider the ternary operator to make it clean.

##########
File path: shardingsphere-infra/shardingsphere-infra-context/src/main/java/org/apache/shardingsphere/infra/context/metadata/MetaDataContextsBuilder.java
##########
@@ -139,14 +135,4 @@ private ShardingSphereResource buildResource(final DatabaseType databaseType, fi
     private ShardingSphereSchema buildSchema(final DatabaseType databaseType, final Map<String, DataSource> dataSourceMap, final Collection<ShardingSphereRule> rules) throws SQLException {
         return SchemaBuilder.build(new SchemaBuilderMaterials(databaseType, dataSourceMap, rules, props));
     }
-
-    private Collection<ShardingSphereUser> getUsersFromAuthorityRule() {

Review comment:
       TODO-2,
   Please consider whether `shardingsphere-infra-authority-api` can be removed from `shardingsphere-infra/shardingsphere-infra-context/pom.xml`

##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {

Review comment:
       TODO-4,
   The third argument is likely to be `BiPredicate<Grantee, byte[]>` rather than `BiPredicate<Object, Object>`.  Thus, the argument `Object cipher` is supposed to be `byte[] cipher`.

##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {
-        Collection<ShardingSphereUser> users = new HashSet<>(getNewUsers(event.getUsers()));
-        users.addAll(getModifiedUsers(event.getUsers()));
         metaDataContexts = new StandardMetaDataContexts(
-                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), new ShardingSphereUsers(users), metaDataContexts.getProps());
+                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), metaDataContexts.getProps());

Review comment:
       TODO-3




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

[GitHub] [shardingsphere] tristaZero commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-842850381


   
   @zhujunxxxxx did a cool job. 👍  
   Please look at my comment TODO-3 for #10252  @huanghao495430759 


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

[GitHub] [shardingsphere] zhujunxxxxx commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630971183



##########
File path: shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/authentication/MySQLAuthenticationHandler.java
##########
@@ -53,14 +57,24 @@
      * @return login success or failure
      */
     public Optional<MySQLServerErrorCode> login(final String username, final String hostname, final byte[] authenticationResponse, final String databaseName) {
-        Optional<ShardingSphereUser> user = ProxyContext.getInstance().getMetaDataContexts().getUsers().findUser(new Grantee(username, hostname));
+        Optional<ShardingSphereUser> user = getUsersFromGlobalRuleMetaData().findUser(new Grantee(username, hostname));
         if (!user.isPresent() || !isPasswordRight(user.get().getPassword(), authenticationResponse)) {
             return Optional.of(MySQLServerErrorCode.ER_ACCESS_DENIED_ERROR);
         }
         return null == databaseName || SQLCheckEngine.check(databaseName, getRules(databaseName), user.get().getGrantee())
                 ? Optional.empty() : Optional.of(MySQLServerErrorCode.ER_DBACCESS_DENIED_ERROR);
     }
-    
+
+    private ShardingSphereUsers getUsersFromGlobalRuleMetaData() {
+        for (RuleConfiguration ruleConfig : ProxyContext.getInstance().getMetaDataContexts().getGlobalRuleMetaData().getConfigurations()) {
+            if (ruleConfig instanceof AuthorityRuleConfiguration) {
+                AuthorityRuleConfiguration authorityRuleConfiguration = (AuthorityRuleConfiguration) ruleConfig;
+                return new ShardingSphereUsers(authorityRuleConfiguration.getUsers());
+            }
+        }
+        return new ShardingSphereUsers(Collections.emptyList());
+    }

Review comment:
       Good idea.




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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r633232134



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {
-        Collection<ShardingSphereUser> users = new HashSet<>(getNewUsers(event.getUsers()));
-        users.addAll(getModifiedUsers(event.getUsers()));
         metaDataContexts = new StandardMetaDataContexts(
-                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), new ShardingSphereUsers(users), metaDataContexts.getProps());
+                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), metaDataContexts.getProps());

Review comment:
       @huanghao495430759 Please look at here, when `governanceMetaDataContexts` receives  `AuthorityChangedEvent`, we need to recreate `authority rule` instead of copying a `metaDataContexts`.
   I will accept @zhujunxxxxx 's change though, the current handling is incorrect. Please correct it after merged. 

##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }

Review comment:
       Hey, returen xxx ? :

##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {

Review comment:
       Maybe the first argument of `validate` is possible to be `password` rather than`grantee`?




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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r633232134



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {
-        Collection<ShardingSphereUser> users = new HashSet<>(getNewUsers(event.getUsers()));
-        users.addAll(getModifiedUsers(event.getUsers()));
         metaDataContexts = new StandardMetaDataContexts(
-                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), new ShardingSphereUsers(users), metaDataContexts.getProps());
+                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), metaDataContexts.getProps());

Review comment:
       TODO-3,
   @huanghao495430759 Please look at here, when `governanceMetaDataContexts` receives  `AuthorityChangedEvent`, we need to recreate `authority rule` instead of copying a `metaDataContexts`.
   I will accept @zhujunxxxxx 's change though, the current handling is incorrect. Please correct it after merged. 




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

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841820920


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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 [#10310](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (96efedd) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ea2f7a390a811c9d0f96648d232bd897036a55be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea2f7a3) will **increase** coverage by `0.30%`.
   > The diff coverage is `43.54%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10310/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10310      +/-   ##
   ============================================
   + Coverage     68.60%   68.90%   +0.30%     
   - Complexity      691      693       +2     
   ============================================
     Files          1727     1726       -1     
     Lines         29438    29401      -37     
     Branches       5293     5288       -5     
   ============================================
   + Hits          20197    20260      +63     
   + Misses         7677     7575     -102     
   - Partials       1564     1566       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...dingsphere/authority/checker/AuthorityChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9jaGVja2VyL0F1dGhvcml0eUNoZWNrZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/shardingsphere/authority/rule/AuthorityRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9ydWxlL0F1dGhvcml0eVJ1bGUuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ingsphere/infra/executor/check/SQLCheckEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL2NoZWNrL1NRTENoZWNrRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ontext/metadata/impl/StandardMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9pbXBsL1N0YW5kYXJkTWV0YURhdGFDb250ZXh0cy5qYXZh) | `76.47% <50.00%> (-1.31%)` | `0.00 <0.00> (ø)` | |
   | [...sql/authentication/MySQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3Byb3h5L2Zyb250ZW5kL215c3FsL2F1dGhlbnRpY2F0aW9uL015U1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `92.00% <71.42%> (-3.66%)` | `0.00 <0.00> (ø)` | |
   | [...uthentication/PostgreSQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9hdXRoZW50aWNhdGlvbi9Qb3N0Z3JlU1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `88.46% <86.66%> (-1.02%)` | `0.00 <0.00> (ø)` | |
   | [...e/context/metadata/GovernanceMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9tZXRhZGF0YS9Hb3Zlcm5hbmNlTWV0YURhdGFDb250ZXh0cy5qYXZh) | `83.66% <87.50%> (-2.15%)` | `0.00 <0.00> (ø)` | |
   | [...nfra/context/metadata/MetaDataContextsBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9NZXRhRGF0YUNvbnRleHRzQnVpbGRlci5qYXZh) | `94.87% <100.00%> (+3.76%)` | `0.00 <0.00> (ø)` | |
   | [...ontext/authority/listener/UserChangedListener.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9hdXRob3JpdHkvbGlzdGVuZXIvVXNlckNoYW5nZWRMaXN0ZW5lci5qYXZh) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | [...rce/ShardingSphereJDBCDataSourceConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9jb25maWcvZGF0YXNvdXJjZS9TaGFyZGluZ1NwaGVyZUpEQkNEYXRhU291cmNlQ29uZmlndXJhdGlvbi5qYXZh) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | ... and [39 more](https://codecov.io/gh/apache/shardingsphere/pull/10310/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/shardingsphere/pull/10310?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/shardingsphere/pull/10310?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 [ea2f7a3...96efedd](https://codecov.io/gh/apache/shardingsphere/pull/10310?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.

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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r633232134



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {
-        Collection<ShardingSphereUser> users = new HashSet<>(getNewUsers(event.getUsers()));
-        users.addAll(getModifiedUsers(event.getUsers()));
         metaDataContexts = new StandardMetaDataContexts(
-                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), new ShardingSphereUsers(users), metaDataContexts.getProps());
+                metaDataContexts.getMetaDataMap(), metaDataContexts.getGlobalRuleMetaData(), metaDataContexts.getExecutorEngine(), metaDataContexts.getProps());

Review comment:
       @huanghao495430759 Please look at here, when `governanceMetaDataContexts` receives  `AuthorityChangedEvent`, we need to recreate `authority rule` instead of copying a `metaDataContexts`.
   I will accept @zhujunxxxxx 's change though, the current handling is incorrect. Please correct it after merged. 

##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }

Review comment:
       Hey, returen xxx ? :

##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,28 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {

Review comment:
       Maybe the first argument of `validate` is possible to be `password` rather than`grantee`?




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

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841820920


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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 [#10310](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (96efedd) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ea2f7a390a811c9d0f96648d232bd897036a55be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea2f7a3) will **increase** coverage by `0.30%`.
   > The diff coverage is `43.54%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10310/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10310      +/-   ##
   ============================================
   + Coverage     68.60%   68.90%   +0.30%     
   - Complexity      691      693       +2     
   ============================================
     Files          1727     1726       -1     
     Lines         29438    29401      -37     
     Branches       5293     5288       -5     
   ============================================
   + Hits          20197    20260      +63     
   + Misses         7677     7575     -102     
   - Partials       1564     1566       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...dingsphere/authority/checker/AuthorityChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9jaGVja2VyL0F1dGhvcml0eUNoZWNrZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/shardingsphere/authority/rule/AuthorityRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9ydWxlL0F1dGhvcml0eVJ1bGUuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ingsphere/infra/executor/check/SQLCheckEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL2NoZWNrL1NRTENoZWNrRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ontext/metadata/impl/StandardMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9pbXBsL1N0YW5kYXJkTWV0YURhdGFDb250ZXh0cy5qYXZh) | `76.47% <50.00%> (-1.31%)` | `0.00 <0.00> (ø)` | |
   | [...sql/authentication/MySQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3Byb3h5L2Zyb250ZW5kL215c3FsL2F1dGhlbnRpY2F0aW9uL015U1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `92.00% <71.42%> (-3.66%)` | `0.00 <0.00> (ø)` | |
   | [...uthentication/PostgreSQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9hdXRoZW50aWNhdGlvbi9Qb3N0Z3JlU1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `88.46% <86.66%> (-1.02%)` | `0.00 <0.00> (ø)` | |
   | [...e/context/metadata/GovernanceMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9tZXRhZGF0YS9Hb3Zlcm5hbmNlTWV0YURhdGFDb250ZXh0cy5qYXZh) | `83.66% <87.50%> (-2.15%)` | `0.00 <0.00> (ø)` | |
   | [...nfra/context/metadata/MetaDataContextsBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9NZXRhRGF0YUNvbnRleHRzQnVpbGRlci5qYXZh) | `94.87% <100.00%> (+3.76%)` | `0.00 <0.00> (ø)` | |
   | [...ontext/authority/listener/UserChangedListener.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9hdXRob3JpdHkvbGlzdGVuZXIvVXNlckNoYW5nZWRMaXN0ZW5lci5qYXZh) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | [...rce/ShardingSphereJDBCDataSourceConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9jb25maWcvZGF0YXNvdXJjZS9TaGFyZGluZ1NwaGVyZUpEQkNEYXRhU291cmNlQ29uZmlndXJhdGlvbi5qYXZh) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | ... and [39 more](https://codecov.io/gh/apache/shardingsphere/pull/10310/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/shardingsphere/pull/10310?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/shardingsphere/pull/10310?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 [ea2f7a3...96efedd](https://codecov.io/gh/apache/shardingsphere/pull/10310?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.

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

[GitHub] [shardingsphere] zhujunxxxxx commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630969828



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/test/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContextsTest.java
##########
@@ -155,12 +154,12 @@ public void assertPropertiesChanged() {
         governanceMetaDataContexts.renew(event);
         assertThat(governanceMetaDataContexts.getProps().getProps().getProperty(ConfigurationPropertyKey.SQL_SHOW.getKey()), is("true"));
     }
-    
+
     @Test
     public void assertAuthorityChanged() {
         AuthorityChangedEvent event = new AuthorityChangedEvent(Collections.singleton(mock(ShardingSphereUser.class)));
         governanceMetaDataContexts.renew(event);
-        assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));
+        //assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));

Review comment:
       Because `users` have been removed from `GovernanceMetaDataContexts`, so don't need to verify users. But we can verify `users` from authority Rule




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

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r631705253



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/test/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContextsTest.java
##########
@@ -155,12 +154,12 @@ public void assertPropertiesChanged() {
         governanceMetaDataContexts.renew(event);
         assertThat(governanceMetaDataContexts.getProps().getProps().getProperty(ConfigurationPropertyKey.SQL_SHOW.getKey()), is("true"));
     }
-    
+
     @Test
     public void assertAuthorityChanged() {
         AuthorityChangedEvent event = new AuthorityChangedEvent(Collections.singleton(mock(ShardingSphereUser.class)));
         governanceMetaDataContexts.renew(event);
-        assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));
+        //assertThat(governanceMetaDataContexts.getUsers().getUsers().size(), is(1));

Review comment:
       If it is necessary no longer, we can consider removing this UT.




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

[GitHub] [shardingsphere] zhujunxxxxx commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
zhujunxxxxx commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r634073463



##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/checker/AuthorityChecker.java
##########
@@ -56,7 +58,25 @@ public SQLCheckResult check(final SQLStatement sqlStatement, final List<Object>
         // TODO add error msg
         return privileges.map(optional -> new SQLCheckResult(optional.hasPrivileges(Collections.singletonList(getPrivilege(sqlStatement))), "")).orElseGet(() -> new SQLCheckResult(false, ""));
     }
-    
+
+    @Override
+    public boolean check(final Grantee grantee, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        return user.isPresent() ? true : false;
+    }
+
+    @Override
+    public boolean check(final Grantee grantee, final BiPredicate<Object, Object> validate, final Object cipher, final AuthorityRule authorityRule) {
+        Optional<ShardingSphereUser> user = authorityRule.findUser(grantee);
+        if (!user.isPresent()) {
+            return false;
+        }
+        if (validate.test(user.get(), cipher)) {
+            return true;
+        }
+        return false;

Review comment:
       ok, I will fix later.




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

[GitHub] [shardingsphere] huanghao495430759 commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
huanghao495430759 commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630778878



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {

Review comment:
       Hi.because UserChangedListener will be remove in the future, so `renew(final AuthorityChangedEvent event)` also would be remove




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

[GitHub] [shardingsphere] codecov-commenter commented on pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#issuecomment-841820920


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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 [#10310](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eeac38d) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ea2f7a390a811c9d0f96648d232bd897036a55be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea2f7a3) will **decrease** coverage by `0.08%`.
   > The diff coverage is `42.18%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10310/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10310      +/-   ##
   ============================================
   - Coverage     68.60%   68.52%   -0.09%     
     Complexity      691      691              
   ============================================
     Files          1727     1727              
     Lines         29438    29461      +23     
     Branches       5293     5301       +8     
   ============================================
   - Hits          20197    20188       -9     
   - Misses         7677     7708      +31     
   - Partials       1564     1565       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...dingsphere/authority/checker/AuthorityChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9jaGVja2VyL0F1dGhvcml0eUNoZWNrZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/shardingsphere/authority/rule/AuthorityRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9ydWxlL0F1dGhvcml0eVJ1bGUuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ingsphere/infra/executor/check/SQLCheckEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL2NoZWNrL1NRTENoZWNrRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ontext/metadata/impl/StandardMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9pbXBsL1N0YW5kYXJkTWV0YURhdGFDb250ZXh0cy5qYXZh) | `76.47% <50.00%> (-1.31%)` | `0.00 <0.00> (ø)` | |
   | [...sql/authentication/MySQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3Byb3h5L2Zyb250ZW5kL215c3FsL2F1dGhlbnRpY2F0aW9uL015U1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `92.00% <71.42%> (-3.66%)` | `0.00 <0.00> (ø)` | |
   | [...uthentication/PostgreSQLAuthenticationHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9hdXRoZW50aWNhdGlvbi9Qb3N0Z3JlU1FMQXV0aGVudGljYXRpb25IYW5kbGVyLmphdmE=) | `88.46% <86.66%> (-1.02%)` | `0.00 <0.00> (ø)` | |
   | [...e/context/metadata/GovernanceMetaDataContexts.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9tZXRhZGF0YS9Hb3Zlcm5hbmNlTWV0YURhdGFDb250ZXh0cy5qYXZh) | `83.66% <87.50%> (-2.15%)` | `0.00 <0.00> (ø)` | |
   | [...nfra/context/metadata/MetaDataContextsBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10310/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29udGV4dC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvaW5mcmEvY29udGV4dC9tZXRhZGF0YS9NZXRhRGF0YUNvbnRleHRzQnVpbGRlci5qYXZh) | `94.87% <100.00%> (+3.76%)` | `0.00 <0.00> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10310?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/shardingsphere/pull/10310?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 [ea2f7a3...eeac38d](https://codecov.io/gh/apache/shardingsphere/pull/10310?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.

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

[GitHub] [shardingsphere] huanghao495430759 commented on a change in pull request #10310: Users removed from StandardMetaDataContexts

Posted by GitBox <gi...@apache.org>.
huanghao495430759 commented on a change in pull request #10310:
URL: https://github.com/apache/shardingsphere/pull/10310#discussion_r630778878



##########
File path: shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
##########
@@ -225,10 +217,8 @@ public synchronized void renew(final PropertiesChangedEvent event) {
      */
     @Subscribe
     public synchronized void renew(final AuthorityChangedEvent event) {

Review comment:
       Hi.because UserChangedListener will be remove in the future, so `renew(final AuthorityChangedEvent event)` also would be remove




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