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/04/19 02:49:52 UTC

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10115: Implement PostgreSQLPrivilegeHandler and add unit test

tristaZero commented on a change in pull request #10115:
URL: https://github.com/apache/shardingsphere/pull/10115#discussion_r615510335



##########
File path: shardingsphere-features/shardingsphere-authority/shardingsphere-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/natived/builder/dialect/PostgreSQLPrivilegeHandler.java
##########
@@ -46,25 +45,56 @@
  * PostgreSQL privilege handler.
  */
 public final class PostgreSQLPrivilegeHandler implements StoragePrivilegeHandler {
-    
+
+    private static final String CREATE_USER_SQL = "CREATE USER %s WITH PASSWORD '%s'";
+
+    private static final String GRANT_ALL_SQL = "GRANT ALL ON ALL TABLES IN SCHEMA public TO %s";
+
     private static final String ROLES_SQL = "select * from pg_roles WHERE rolname IN (%s)";
-    
-    private static final String TABLE_PRIVILEGE_SQL = 
+
+    private static final String TABLE_PRIVILEGE_SQL =
             "SELECT grantor, grantee, table_catalog, table_name, privilege_type, is_grantable from information_schema.table_privileges WHERE grantee IN (%s)";
-    
+
     @Override
     public Collection<ShardingSphereUser> diff(final Collection<ShardingSphereUser> users, final DataSource dataSource) throws SQLException {
-        return Collections.emptyList();
+        Collection<Grantee> grantees = new LinkedList<>();
+        try (Connection connection = dataSource.getConnection()) {
+            Statement statement = connection.createStatement();
+            try (ResultSet resultSet = statement.executeQuery(getRolePrivilegesSQL(users))) {
+                while (resultSet.next()) {
+                    grantees.add(new Grantee(resultSet.getString("rolname"), ""));
+                }
+            }
+        }
+        return users.stream().filter(each -> !grantees.contains(each.getGrantee())).collect(Collectors.toList());
     }
-    
+
     @Override
     public void create(final Collection<ShardingSphereUser> users, final DataSource dataSource) throws SQLException {
+        try (Connection connection = dataSource.getConnection(); Statement statement = connection.createStatement()) {
+            for (ShardingSphereUser each : users) {
+                statement.execute(getCreateUsersSQL(each));
+            }
+        }
+    }
+
+    private String getCreateUsersSQL(final ShardingSphereUser users) {
+        return String.format(CREATE_USER_SQL, users.getGrantee(), users.getPassword());

Review comment:
       Hi, `user.getGrantee()` will returen `user@host` and `host` is always empty string, is that what you expect? 

##########
File path: shardingsphere-features/shardingsphere-authority/shardingsphere-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/natived/builder/dialect/PostgreSQLPrivilegeHandler.java
##########
@@ -130,19 +160,19 @@ private void fillRolePrivileges(final Map<ShardingSphereUser, NativePrivileges>
             }
         }
     }
-    
+
     private void fillRolePrivileges(final Map<ShardingSphereUser, NativePrivileges> userPrivilegeMap, final ResultSet resultSet) throws SQLException {
         Optional<ShardingSphereUser> user = findShardingSphereUser(userPrivilegeMap, resultSet);
         if (user.isPresent()) {
             userPrivilegeMap.get(user.get()).getAdministrativePrivileges().getPrivileges().addAll(loadRolePrivileges(resultSet));
         }
     }
-    
+

Review comment:
       Please give a look at these blank space changes.

##########
File path: shardingsphere-features/shardingsphere-authority/shardingsphere-authority-common/src/main/java/org/apache/shardingsphere/authority/provider/natived/builder/dialect/PostgreSQLPrivilegeHandler.java
##########
@@ -46,25 +45,56 @@
  * PostgreSQL privilege handler.
  */
 public final class PostgreSQLPrivilegeHandler implements StoragePrivilegeHandler {
-    
+
+    private static final String CREATE_USER_SQL = "CREATE USER %s WITH PASSWORD '%s'";
+
+    private static final String GRANT_ALL_SQL = "GRANT ALL ON ALL TABLES IN SCHEMA public TO %s";
+
     private static final String ROLES_SQL = "select * from pg_roles WHERE rolname IN (%s)";
-    
-    private static final String TABLE_PRIVILEGE_SQL = 
+
+    private static final String TABLE_PRIVILEGE_SQL =
             "SELECT grantor, grantee, table_catalog, table_name, privilege_type, is_grantable from information_schema.table_privileges WHERE grantee IN (%s)";
-    
+
     @Override
     public Collection<ShardingSphereUser> diff(final Collection<ShardingSphereUser> users, final DataSource dataSource) throws SQLException {
-        return Collections.emptyList();
+        Collection<Grantee> grantees = new LinkedList<>();
+        try (Connection connection = dataSource.getConnection()) {
+            Statement statement = connection.createStatement();
+            try (ResultSet resultSet = statement.executeQuery(getRolePrivilegesSQL(users))) {
+                while (resultSet.next()) {
+                    grantees.add(new Grantee(resultSet.getString("rolname"), ""));

Review comment:
       Does that mean `username` equals `rolename`?




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