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/06/22 01:24:39 UTC
[GitHub] [shardingsphere] lwclover opened a new pull request #10899: custom authority，schema can be assigned to proxy users
lwclover opened a new pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899
authority enhance，sample：
rules:
!AUTHORITY
users:
root@:root
root1@:root1
provider:
type: CUSTOM_PRIVILEGES_PERMITTED
props:
user-schema-mappings: root=test, root1=db_dal_admin, root1=test
when using root login proxy，you can use database test.
when using root1 login proxy，you can use database db_dal_admin and test.
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865656769
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 [#10899](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cfef355) into [5.0.0-beta-release](https://codecov.io/gh/apache/shardingsphere/commit/f60bb6f90df58dfb29b6f5c0217fec24306315a0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f60bb6f) will **decrease** coverage by `0.03%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 @@
## 5.0.0-beta-release #10899 +/- ##
========================================================
- Coverage 65.60% 65.57% -0.04%
Complexity 691 691
========================================================
Files 1777 1780 +3
Lines 30824 30851 +27
Branches 5558 5560 +2
========================================================
+ Hits 20222 20230 +8
- Misses 8990 9008 +18
- Partials 1612 1613 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...PrivilegesPermittedAuthorityProviderAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vQ3VzdG9tUHJpdmlsZWdlc1Blcm1pdHRlZEF1dGhvcml0eVByb3ZpZGVyQWxnb3JpdGhtLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rovider/custom/builder/CustomPrivilegeBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vYnVpbGRlci9DdXN0b21Qcml2aWxlZ2VCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...omPrivilegesPermittedShardingSpherePrivileges.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vbW9kZWwvcHJpdmlsZWdlL0N1c3RvbVByaXZpbGVnZXNQZXJtaXR0ZWRTaGFyZGluZ1NwaGVyZVByaXZpbGVnZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...xt/admin/mysql/executor/ShowDatabasesExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dEYXRhYmFzZXNFeGVjdXRvci5qYXZh) | `88.88% <87.50%> (+44.44%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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/10899?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 [f60bb6f...cfef355](https://codecov.io/gh/apache/shardingsphere/pull/10899?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] lwclover commented on pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-867609809
changes submit to master
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657579237
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
Hey @terrymanu
@lwclover and I were confused here, actually it is not straightforward enough, could you have a look?
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865656769
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 [#10899](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f6e16da) into [5.0.0-beta-release](https://codecov.io/gh/apache/shardingsphere/commit/f60bb6f90df58dfb29b6f5c0217fec24306315a0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f60bb6f) will **decrease** coverage by `0.05%`.
> The diff coverage is `14.89%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 @@
## 5.0.0-beta-release #10899 +/- ##
========================================================
- Coverage 65.60% 65.54% -0.06%
Complexity 691 691
========================================================
Files 1777 1780 +3
Lines 30824 30863 +39
Branches 5558 5562 +4
========================================================
+ Hits 20222 20230 +8
- Misses 8990 9020 +30
- Partials 1612 1613 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...PrivilegesPermittedAuthorityProviderAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvU2NoZW1hUHJpdmlsZWdlc1Blcm1pdHRlZEF1dGhvcml0eVByb3ZpZGVyQWxnb3JpdGhtLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rovider/schema/builder/SchemaPrivilegeBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvYnVpbGRlci9TY2hlbWFQcml2aWxlZ2VCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...maPrivilegesPermittedShardingSpherePrivileges.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvbW9kZWwvcHJpdmlsZWdlL1NjaGVtYVByaXZpbGVnZXNQZXJtaXR0ZWRTaGFyZGluZ1NwaGVyZVByaXZpbGVnZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...xt/admin/mysql/executor/ShowDatabasesExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dEYXRhYmFzZXNFeGVjdXRvci5qYXZh) | `88.88% <87.50%> (+44.44%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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/10899?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 [f60bb6f...f6e16da](https://codecov.io/gh/apache/shardingsphere/pull/10899?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] lwclover closed pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover closed pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899
--
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] lwclover commented on pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865848789
> Hi @lwclover ,
>
> Thanks for your PR, which attracts my interest. Let set aside your coding, and firstly target your idea and plan.
>
> It basically looks nice, but there is an oversight, i.e. when users change the map of user and privileges, refresh() just refresh the runtime. All these changes will lose once users restart ShardingProxy. What do you think?
user-schema-mappings persists on registry center like other configs.
--
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] lwclover commented on a change in pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657695849
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
I think codes below are redundant lines.
```
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
backendConnection.getGrantee());
```
I think method getSchemaNames should be streamlined like this, it can get the same result
```
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
Collection<Object> result = new LinkedList<>();
for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
result.add(each);
}
}
return result;
}
```
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865656769
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 [#10899](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cfef355) into [5.0.0-beta-release](https://codecov.io/gh/apache/shardingsphere/commit/f60bb6f90df58dfb29b6f5c0217fec24306315a0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f60bb6f) will **decrease** coverage by `0.03%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 @@
## 5.0.0-beta-release #10899 +/- ##
========================================================
- Coverage 65.60% 65.57% -0.04%
Complexity 691 691
========================================================
Files 1777 1780 +3
Lines 30824 30851 +27
Branches 5558 5560 +2
========================================================
+ Hits 20222 20230 +8
- Misses 8990 9008 +18
- Partials 1612 1613 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...PrivilegesPermittedAuthorityProviderAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vQ3VzdG9tUHJpdmlsZWdlc1Blcm1pdHRlZEF1dGhvcml0eVByb3ZpZGVyQWxnb3JpdGhtLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rovider/custom/builder/CustomPrivilegeBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vYnVpbGRlci9DdXN0b21Qcml2aWxlZ2VCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...omPrivilegesPermittedShardingSpherePrivileges.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9jdXN0b20vbW9kZWwvcHJpdmlsZWdlL0N1c3RvbVByaXZpbGVnZXNQZXJtaXR0ZWRTaGFyZGluZ1NwaGVyZVByaXZpbGVnZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...xt/admin/mysql/executor/ShowDatabasesExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dEYXRhYmFzZXNFeGVjdXRvci5qYXZh) | `88.88% <87.50%> (+44.44%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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/10899?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 [f60bb6f...cfef355](https://codecov.io/gh/apache/shardingsphere/pull/10899?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] lwclover commented on a change in pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657695849
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
I think codes below are redundant lines.
```
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
backendConnection.getGrantee());
```
```
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
Collection<Object> result = new LinkedList<>();
for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
result.add(each);
}
}
return result;
}
```
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865656769
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 [#10899](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a593a5b) into [5.0.0-beta-release](https://codecov.io/gh/apache/shardingsphere/commit/f60bb6f90df58dfb29b6f5c0217fec24306315a0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f60bb6f) will **decrease** coverage by `0.05%`.
> The diff coverage is `9.09%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/10899?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 @@
## 5.0.0-beta-release #10899 +/- ##
========================================================
- Coverage 65.60% 65.54% -0.06%
Complexity 691 691
========================================================
Files 1777 1780 +3
Lines 30824 30858 +34
Branches 5558 5562 +4
========================================================
+ Hits 20222 20226 +4
- Misses 8990 9019 +29
- Partials 1612 1613 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10899?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...PrivilegesPermittedAuthorityProviderAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvU2NoZW1hUHJpdmlsZWdlc1Blcm1pdHRlZEF1dGhvcml0eVByb3ZpZGVyQWxnb3JpdGhtLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rovider/schema/builder/SchemaPrivilegeBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvYnVpbGRlci9TY2hlbWFQcml2aWxlZ2VCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...maPrivilegesPermittedShardingSpherePrivileges.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYXV0aG9yaXR5L3NoYXJkaW5nc3BoZXJlLWluZnJhLWF1dGhvcml0eS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2F1dGhvcml0eS9wcm92aWRlci9zY2hlbWEvbW9kZWwvcHJpdmlsZWdlL1NjaGVtYVByaXZpbGVnZXNQZXJtaXR0ZWRTaGFyZGluZ1NwaGVyZVByaXZpbGVnZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...xt/admin/mysql/executor/ShowDatabasesExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/10899/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dEYXRhYmFzZXNFeGVjdXRvci5qYXZh) | `92.30% <80.00%> (+47.86%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10899?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/10899?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 [f60bb6f...a593a5b](https://codecov.io/gh/apache/shardingsphere/pull/10899?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] lwclover commented on a change in pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657695849
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
I think codes below are redundant lines.
```
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
backendConnection.getGrantee());
```
--
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] lwclover commented on a change in pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
lwclover commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657695849
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
I think codes below are redundant lines.
```
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
backendConnection.getGrantee());
```
I think method getSchemaNames should like this：
```
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
Collection<Object> result = new LinkedList<>();
for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
result.add(each);
}
}
return result;
}
```
--
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] terrymanu commented on a change in pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657605694
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
Maybe cause problem if SQLCheckException occur, why ignore the exception 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865682691
Hi @lwclover ,
Thanks for your PR, which attracts my interest. Let firstly set aside your coding, and target your idea and plan.
It basically looks nice to me, but there is an oversight, i.e. when users change the map of user and privileges, `refresh()` just refresh the runtime. All these changes will lose once users restart ShardingProxy. What do you think?
--
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 edited a comment on pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865682691
Hi @lwclover ,
Thanks for your PR, which attracts my interest. Let set aside your coding, and firstly target your idea and plan.
It basically looks nice, but there is an oversight, i.e. when users change the map of user and privileges, `refresh()` just refresh the runtime. All these changes will lose once users restart ShardingProxy. What do you think?
--
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 removed a comment on pull request #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
tristaZero removed a comment on pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#issuecomment-865682691
Hi @lwclover ,
Thanks for your PR, which attracts my interest. Let set aside your coding, and firstly target your idea and plan.
It basically looks nice, but there is an oversight, i.e. when users change the map of user and privileges, `refresh()` just refresh the runtime. All these changes will lose once users restart ShardingProxy. What do you think?
--
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 #10899: custom authority，schema can be assigned to proxy users
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #10899:
URL: https://github.com/apache/shardingsphere/pull/10899#discussion_r657012460
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/model/privilege/SchemaPrivilegesPermittedShardingSpherePrivileges.java
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.model.privilege;
+
+import java.util.Collection;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.AccessSubject;
+import org.apache.shardingsphere.authority.model.PrivilegeType;
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.natived.model.subject.SchemaAccessSubject;
+
+import lombok.AllArgsConstructor;
+
+@AllArgsConstructor
+public class SchemaPrivilegesPermittedShardingSpherePrivileges implements ShardingSpherePrivileges {
Review comment:
Hi suppose it has no possibility to have chid class, please define it as a final class.
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/builder/SchemaPrivilegeBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.builder;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.schema.SchemaPrivilegesPermittedAuthorityProviderAlgorithm;
+import org.apache.shardingsphere.authority.provider.schema.model.privilege.SchemaPrivilegesPermittedShardingSpherePrivileges;
+import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+
+import com.google.common.base.Preconditions;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaPrivilegeBuilder {
+ /**
+ * Build privileges.
+ *
+ * @param users users
+ * @param props props
+ * @return privileges
+ */
+ public static Map<ShardingSphereUser, ShardingSpherePrivileges> build(final Collection<ShardingSphereUser> users, final Properties props) {
+ String mappingProp = props.getProperty(SchemaPrivilegesPermittedAuthorityProviderAlgorithm.PROP_USER_SCHEMA_MAPPINGS, "");
+ Preconditions.checkArgument(!"".equals(mappingProp), "user-schema-mappings configuration `%s` can not be null", mappingProp);
+
Review comment:
Could you remove this redundant line?
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/model/privilege/SchemaPrivilegesPermittedShardingSpherePrivileges.java
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.model.privilege;
+
+import java.util.Collection;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.AccessSubject;
+import org.apache.shardingsphere.authority.model.PrivilegeType;
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.natived.model.subject.SchemaAccessSubject;
+
+import lombok.AllArgsConstructor;
+
+@AllArgsConstructor
+public class SchemaPrivilegesPermittedShardingSpherePrivileges implements ShardingSpherePrivileges {
+
+ private Set<String> schemas;
+
+ @Override
+ public void setSuperPrivilege() {
+
+ }
+
+ @Override
+ public boolean hasPrivileges(final String schema) {
+ return schemas.contains(schema);
+ }
+
+ @Override
+ public boolean hasPrivileges(final Collection<PrivilegeType> privileges) {
+ return true;
+ }
+
+ @Override
+ public boolean hasPrivileges(final AccessSubject accessSubject, final Collection<PrivilegeType> privileges) {
+ if (accessSubject instanceof SchemaAccessSubject) {
+ return hasPrivileges(((SchemaAccessSubject) accessSubject).getSchema());
+ }
+ throw new UnsupportedOperationException(accessSubject.getClass().getCanonicalName());
+ }
+
Review comment:
Could you remove this redundant line?
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/SchemaPrivilegesPermittedAuthorityProviderAlgorithm.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.schema.builder.SchemaPrivilegeBuilder;
+import org.apache.shardingsphere.authority.spi.AuthorityProvideAlgorithm;
+import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
+import org.apache.shardingsphere.infra.metadata.user.Grantee;
+import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+
+public final class SchemaPrivilegesPermittedAuthorityProviderAlgorithm implements AuthorityProvideAlgorithm {
+
+ public static final String PROP_USER_SCHEMA_MAPPINGS = "user-schema-mappings";
+
+ private final Map<ShardingSphereUser, ShardingSpherePrivileges> userPrivilegeMap = new ConcurrentHashMap<>();
+
+ private Properties props;
+
+ @Override
+ public void setProps(final Properties props) {
+ this.props = props;
+ }
+
+ @Override
+ public Properties getProps() {
+ return this.props;
+ }
+
+ @Override
+ public void init(final Map<String, ShardingSphereMetaData> mataDataMap, final Collection<ShardingSphereUser> users) {
+ this.userPrivilegeMap.putAll(SchemaPrivilegeBuilder.build(users, props));
+ }
+
+ @Override
+ public void refresh(final Map<String, ShardingSphereMetaData> mataDataMap, final Collection<ShardingSphereUser> users) {
+ this.userPrivilegeMap.putAll(SchemaPrivilegeBuilder.build(users, props));
+ }
+
+ @Override
+ public Optional<ShardingSpherePrivileges> findPrivileges(final Grantee grantee) {
+ return userPrivilegeMap.keySet().stream().filter(each -> each.getGrantee().equals(grantee)).findFirst().map(userPrivilegeMap::get);
+ }
+
+ @Override
+ public String getType() {
+ return "SCHEMA_PRIVILEGES_PERMITTED";
+ }
+
Review comment:
Could you remove this redundant line?
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/mysql/executor/ShowDatabasesExecutor.java
##########
@@ -54,18 +54,18 @@ public void execute(final BackendConnection backendConnection) {
private Collection<Object> getSchemaNames(final BackendConnection backendConnection) {
try {
MetaDataContexts contexts = ProxyContext.getInstance().getMetaDataContexts();
- SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(),
- contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(), backendConnection.getGrantee());
- return new ArrayList<>(ProxyContext.getInstance().getAllSchemaNames());
+ SQLCheckEngine.check(new MySQLShowDatabasesStatement(), Collections.emptyList(), contexts.getGlobalRuleMetaData().getRules(), backendConnection.getSchemaName(), contexts.getMetaDataMap(),
+ backendConnection.getGrantee());
} catch (final SQLCheckException ex) {
- Collection<Object> result = new LinkedList<>();
- for (String each : ProxyContext.getInstance().getAllSchemaNames()) {
- if (SQLCheckEngine.check(each, getRules(each), backendConnection.getGrantee())) {
- result.add(each);
- }
+ // ignore
+ }
Review comment:
Is there a need to do such modification?
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/builder/SchemaPrivilegeBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.builder;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.schema.SchemaPrivilegesPermittedAuthorityProviderAlgorithm;
+import org.apache.shardingsphere.authority.provider.schema.model.privilege.SchemaPrivilegesPermittedShardingSpherePrivileges;
+import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+
+import com.google.common.base.Preconditions;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaPrivilegeBuilder {
Review comment:
Hi it is suggested to define it as a final one.
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/builder/SchemaPrivilegeBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.builder;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.schema.SchemaPrivilegesPermittedAuthorityProviderAlgorithm;
+import org.apache.shardingsphere.authority.provider.schema.model.privilege.SchemaPrivilegesPermittedShardingSpherePrivileges;
+import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+
+import com.google.common.base.Preconditions;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaPrivilegeBuilder {
+ /**
+ * Build privileges.
+ *
+ * @param users users
+ * @param props props
+ * @return privileges
+ */
+ public static Map<ShardingSphereUser, ShardingSpherePrivileges> build(final Collection<ShardingSphereUser> users, final Properties props) {
+ String mappingProp = props.getProperty(SchemaPrivilegesPermittedAuthorityProviderAlgorithm.PROP_USER_SCHEMA_MAPPINGS, "");
+ Preconditions.checkArgument(!"".equals(mappingProp), "user-schema-mappings configuration `%s` can not be null", mappingProp);
+
+ String[] mappings = mappingProp.split(",");
+ Arrays.asList(mappings).stream().forEach(each -> Preconditions.checkArgument(0 < each.indexOf("@") && 0 < each.indexOf("="),
+ "user-schema-mappings configuration `%s` is invalid, the configuration format should be like `username@hostname=schema`", each));
+
Review comment:
Could you remove this redundant line?
##########
File path: shardingsphere-infra/shardingsphere-infra-authority/shardingsphere-infra-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/schema/builder/SchemaPrivilegeBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.authority.provider.schema.builder;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Set;
+
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.provider.schema.SchemaPrivilegesPermittedAuthorityProviderAlgorithm;
+import org.apache.shardingsphere.authority.provider.schema.model.privilege.SchemaPrivilegesPermittedShardingSpherePrivileges;
+import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
+
+import com.google.common.base.Preconditions;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaPrivilegeBuilder {
+ /**
+ * Build privileges.
+ *
+ * @param users users
+ * @param props props
+ * @return privileges
+ */
+ public static Map<ShardingSphereUser, ShardingSpherePrivileges> build(final Collection<ShardingSphereUser> users, final Properties props) {
+ String mappingProp = props.getProperty(SchemaPrivilegesPermittedAuthorityProviderAlgorithm.PROP_USER_SCHEMA_MAPPINGS, "");
+ Preconditions.checkArgument(!"".equals(mappingProp), "user-schema-mappings configuration `%s` can not be null", mappingProp);
+
+ String[] mappings = mappingProp.split(",");
+ Arrays.asList(mappings).stream().forEach(each -> Preconditions.checkArgument(0 < each.indexOf("@") && 0 < each.indexOf("="),
+ "user-schema-mappings configuration `%s` is invalid, the configuration format should be like `username@hostname=schema`", each));
+
+ Map<ShardingSphereUser, ShardingSpherePrivileges> result = new HashMap<>();
+
Review comment:
Could you remove this redundant line?
--
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