xunliu commented on code in PR #5904:
URL: https://github.com/apache/gravitino/pull/5904#discussion_r1893547783


##########
authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcSecurableObjectMappingProvider.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.gravitino.authorization.jdbc;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.authorization.AuthorizationMetadataObject;
+import org.apache.gravitino.authorization.AuthorizationPrivilege;
+import 
org.apache.gravitino.authorization.AuthorizationPrivilegesMappingProvider;
+import org.apache.gravitino.authorization.AuthorizationSecurableObject;
+import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.authorization.SecurableObject;
+
+/**
+ * JdbcSecurableObjectMappingProvider is used for translating securable object 
to authorization
+ * securable object.
+ */
+public class JdbcSecurableObjectMappingProvider implements 
AuthorizationPrivilegesMappingProvider {
+
+  private final Map<Privilege.Name, Set<AuthorizationPrivilege>> 
privilegeMapping =
+      ImmutableMap.of(
+          Privilege.Name.CREATE_TABLE,
+              
Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)),
+          Privilege.Name.CREATE_SCHEMA,
+              
Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.CREATE)),
+          Privilege.Name.SELECT_TABLE,
+              
Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT)),
+          Privilege.Name.MODIFY_TABLE,
+              Sets.newHashSet(
+                  JdbcPrivilege.valueOf(JdbcPrivilege.Type.SELECT),
+                  JdbcPrivilege.valueOf(JdbcPrivilege.Type.UPDATE),
+                  JdbcPrivilege.valueOf(JdbcPrivilege.Type.DELETE),
+                  JdbcPrivilege.valueOf(JdbcPrivilege.Type.INSERT),
+                  JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALTER)),
+          Privilege.Name.USE_SCHEMA,
+              
Sets.newHashSet(JdbcPrivilege.valueOf(JdbcPrivilege.Type.USAGE)));
+
+  private final Map<Privilege.Name, MetadataObject.Type> privilegeScopeMapping 
=
+      ImmutableMap.of(
+          Privilege.Name.CREATE_TABLE, MetadataObject.Type.TABLE,
+          Privilege.Name.CREATE_SCHEMA, MetadataObject.Type.SCHEMA,
+          Privilege.Name.SELECT_TABLE, MetadataObject.Type.TABLE,
+          Privilege.Name.MODIFY_TABLE, MetadataObject.Type.TABLE,
+          Privilege.Name.USE_SCHEMA, MetadataObject.Type.SCHEMA);
+
+  private final Set<AuthorizationPrivilege> ownerPrivileges = 
ImmutableSet.of();
+
+  private final Set<MetadataObject.Type> allowObjectTypes =
+      ImmutableSet.of(
+          MetadataObject.Type.METALAKE,
+          MetadataObject.Type.CATALOG,
+          MetadataObject.Type.SCHEMA,
+          MetadataObject.Type.TABLE);
+
+  @Override
+  public Map<Privilege.Name, Set<AuthorizationPrivilege>> 
privilegesMappingRule() {
+    return privilegeMapping;
+  }
+
+  @Override
+  public Set<AuthorizationPrivilege> ownerMappingRule() {
+    return ownerPrivileges;
+  }
+
+  @Override
+  public Set<Privilege.Name> allowPrivilegesRule() {
+    return privilegeMapping.keySet();
+  }
+
+  @Override
+  public Set<MetadataObject.Type> allowMetadataObjectTypesRule() {
+    return allowObjectTypes;
+  }
+
+  @Override
+  public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject 
securableObject) {
+    List<AuthorizationSecurableObject> authObjects = Lists.newArrayList();
+    List<AuthorizationPrivilege> databasePrivileges = Lists.newArrayList();
+    List<AuthorizationPrivilege> tablePrivileges = Lists.newArrayList();
+    JdbcAuthorizationObject databaseObject;
+    JdbcAuthorizationObject tableObject;
+    switch (securableObject.type()) {
+      case METALAKE:
+      case CATALOG:
+        convertJdbcPrivileges(securableObject, databasePrivileges, 
tablePrivileges);
+
+        if (!databasePrivileges.isEmpty()) {
+          databaseObject =
+              new JdbcAuthorizationObject(JdbcAuthorizationObject.ALL, null, 
databasePrivileges);
+          authObjects.add(databaseObject);
+        }
+
+        if (!tablePrivileges.isEmpty()) {
+          tableObject =
+              new JdbcAuthorizationObject(
+                  JdbcAuthorizationObject.ALL, JdbcAuthorizationObject.ALL, 
tablePrivileges);
+          authObjects.add(tableObject);
+        }
+        break;
+
+      case SCHEMA:
+        convertJdbcPrivileges(securableObject, databasePrivileges, 
tablePrivileges);
+        if (!databasePrivileges.isEmpty()) {
+          databaseObject =
+              new JdbcAuthorizationObject(securableObject.name(), null, 
databasePrivileges);
+          authObjects.add(databaseObject);
+        }
+
+        if (!tablePrivileges.isEmpty()) {
+          tableObject =
+              new JdbcAuthorizationObject(
+                  securableObject.name(), JdbcAuthorizationObject.ALL, 
tablePrivileges);
+          authObjects.add(tableObject);
+        }
+        break;
+
+      case TABLE:
+        convertJdbcPrivileges(securableObject, databasePrivileges, 
tablePrivileges);
+        if (!tablePrivileges.isEmpty()) {
+          MetadataObject metadataObject =
+              MetadataObjects.parse(securableObject.parent(), 
MetadataObject.Type.SCHEMA);
+          tableObject =
+              new JdbcAuthorizationObject(
+                  metadataObject.name(), securableObject.name(), 
tablePrivileges);
+          authObjects.add(tableObject);
+        }
+        break;
+
+      default:
+        throw new IllegalArgumentException(
+            String.format("Don't support metadata object type %s", 
securableObject.type()));
+    }
+
+    return authObjects;
+  }
+
+  @Override
+  public List<AuthorizationSecurableObject> translateOwner(MetadataObject 
metadataObject) {
+    List<AuthorizationSecurableObject> objects = Lists.newArrayList();
+    JdbcPrivilege allPri = JdbcPrivilege.valueOf(JdbcPrivilege.Type.ALL);

Review Comment:
   I think it better to change `allPri` to `allPrivileges`?



##########
authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java:
##########
@@ -0,0 +1,468 @@
+/*
+ * 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.gravitino.authorization.jdbc;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.commons.pool2.impl.BaseObjectPoolConfig;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.annotation.Unstable;
+import org.apache.gravitino.authorization.AuthorizationPrivilege;
+import org.apache.gravitino.authorization.AuthorizationSecurableObject;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.MetadataObjectChange;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.RoleChange;
+import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
+import org.apache.gravitino.exceptions.AuthorizationPluginException;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.GroupEntity;
+import org.apache.gravitino.meta.UserEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * JdbcSQLBasedAuthorizationPlugin is the base class for all JDBC-based 
authorization plugins. For
+ * example, JdbcHiveAuthorizationPlugin is the JDBC-based authorization plugin 
for Hive. Different
+ * JDBC-based authorization plugins can inherit this class and implement their 
own SQL statements.
+ */
+@Unstable
+abstract class JdbcAuthorizationPlugin implements AuthorizationPlugin, 
JdbcAuthorizationSQL {
+
+  private static final String CONFIG_PREFIX = "authorization.jdbc.";
+  public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver";
+  public static final String JDBC_URL = CONFIG_PREFIX + "url";
+  public static final String JDBC_USERNAME = CONFIG_PREFIX + "username";
+  public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password";
+
+  private static final String GROUP_PREFIX = "GRAVITINO_GROUP_";
+  private static final Logger LOG = 
LoggerFactory.getLogger(JdbcAuthorizationPlugin.class);
+
+  protected BasicDataSource dataSource;
+  protected JdbcSecurableObjectMappingProvider mappingProvider;
+
+  public JdbcAuthorizationPlugin(Map<String, String> config) {
+    // Initialize the data source
+    dataSource = new BasicDataSource();
+    String jdbcUrl = config.get(JDBC_URL);
+    dataSource.setUrl(jdbcUrl);
+    dataSource.setDriverClassName(config.get(JDBC_DRIVER));
+    dataSource.setUsername(config.get(JDBC_USERNAME));
+    dataSource.setPassword(config.get(JDBC_PASSWORD));
+    dataSource.setDefaultAutoCommit(true);
+    dataSource.setMaxTotal(20);
+    dataSource.setMaxIdle(5);
+    dataSource.setMinIdle(0);
+    dataSource.setLogAbandoned(true);
+    dataSource.setRemoveAbandonedOnBorrow(true);
+    dataSource.setTestOnBorrow(BaseObjectPoolConfig.DEFAULT_TEST_ON_BORROW);
+    dataSource.setTestWhileIdle(BaseObjectPoolConfig.DEFAULT_TEST_WHILE_IDLE);
+    
dataSource.setNumTestsPerEvictionRun(BaseObjectPoolConfig.DEFAULT_NUM_TESTS_PER_EVICTION_RUN);
+    dataSource.setTestOnReturn(BaseObjectPoolConfig.DEFAULT_TEST_ON_RETURN);
+    dataSource.setLifo(BaseObjectPoolConfig.DEFAULT_LIFO);
+    mappingProvider = new JdbcSecurableObjectMappingProvider();
+  }
+
+  @Override
+  public void close() throws IOException {
+    if (dataSource != null) {
+      try {
+        dataSource.close();
+      } catch (SQLException e) {
+        throw new RuntimeException(e);
+      }
+    }
+  }
+
+  @Override
+  public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws 
RuntimeException {
+    // This interface mainly handles the metadata object rename change and 
delete change.
+    // The privilege for JdbcSQLBasedAuthorizationPlugin will be renamed or 
deleted automatically.
+    // We don't need to do any other things.
+    return true;
+  }
+
+  @Override
+  public Boolean onRoleCreated(Role role) throws AuthorizationPluginException {
+    beforeExecuteSQL();
+
+    String sql = getCreateRoleSQL(role.name());
+    executeUpdateSQL(sql, "already exists");
+
+    if (role.securableObjects() != null) {
+      for (SecurableObject object : role.securableObjects()) {
+        onRoleUpdated(role, RoleChange.addSecurableObject(role.name(), 
object));
+      }
+    }
+
+    return true;
+  }
+
+  @Override
+  public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException 
{
+    throw new UnsupportedOperationException("Doesn't support to acquired a 
role");
+  }
+
+  @Override
+  public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
+    beforeExecuteSQL();

Review Comment:
   I think we can merge `beforeExecuteSQL()` into `getXXXSQL()`, let 
`getXXXSQL()` return List<String> sqls.



##########
authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java:
##########
@@ -0,0 +1,340 @@
+/*
+ * 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.gravitino.authorization.jdbc;
+
+import com.google.common.collect.Lists;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.gravitino.Audit;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.Privileges;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.RoleChange;
+import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.authorization.SecurableObjects;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.GroupEntity;
+import org.apache.gravitino.meta.UserEntity;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class JdbcAuthorizationPluginTest {
+  private static List<String> expectSQLs = Lists.newArrayList();
+  private static List<MetadataObject.Type> expectTypes = Lists.newArrayList();
+  private static List<String> expectObjectNames = Lists.newArrayList();
+  private static List<Optional<Owner>> expectPreOwners = Lists.newArrayList();
+  private static List<Owner> expectNewOwners = Lists.newArrayList();
+  private static int currentSQLIndex = 0;
+  private static int currentIndex = 0;

Review Comment:
   I think we can add ` @AfterEach` to clean these variables.
   ```
     public void clean() {
     
     }
   ```



##########
authorizations/authorization-jdbc/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java:
##########
@@ -0,0 +1,468 @@
+/*
+ * 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.gravitino.authorization.jdbc;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.commons.pool2.impl.BaseObjectPoolConfig;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.annotation.Unstable;
+import org.apache.gravitino.authorization.AuthorizationPrivilege;
+import org.apache.gravitino.authorization.AuthorizationSecurableObject;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.MetadataObjectChange;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.RoleChange;
+import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
+import org.apache.gravitino.exceptions.AuthorizationPluginException;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.GroupEntity;
+import org.apache.gravitino.meta.UserEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * JdbcSQLBasedAuthorizationPlugin is the base class for all JDBC-based 
authorization plugins. For
+ * example, JdbcHiveAuthorizationPlugin is the JDBC-based authorization plugin 
for Hive. Different
+ * JDBC-based authorization plugins can inherit this class and implement their 
own SQL statements.
+ */
+@Unstable
+abstract class JdbcAuthorizationPlugin implements AuthorizationPlugin, 
JdbcAuthorizationSQL {
+
+  private static final String CONFIG_PREFIX = "authorization.jdbc.";
+  public static final String JDBC_DRIVER = CONFIG_PREFIX + "driver";
+  public static final String JDBC_URL = CONFIG_PREFIX + "url";
+  public static final String JDBC_USERNAME = CONFIG_PREFIX + "username";
+  public static final String JDBC_PASSWORD = CONFIG_PREFIX + "password";
+
+  private static final String GROUP_PREFIX = "GRAVITINO_GROUP_";
+  private static final Logger LOG = 
LoggerFactory.getLogger(JdbcAuthorizationPlugin.class);
+
+  protected BasicDataSource dataSource;
+  protected JdbcSecurableObjectMappingProvider mappingProvider;
+
+  public JdbcAuthorizationPlugin(Map<String, String> config) {
+    // Initialize the data source
+    dataSource = new BasicDataSource();
+    String jdbcUrl = config.get(JDBC_URL);
+    dataSource.setUrl(jdbcUrl);
+    dataSource.setDriverClassName(config.get(JDBC_DRIVER));
+    dataSource.setUsername(config.get(JDBC_USERNAME));
+    dataSource.setPassword(config.get(JDBC_PASSWORD));
+    dataSource.setDefaultAutoCommit(true);
+    dataSource.setMaxTotal(20);
+    dataSource.setMaxIdle(5);
+    dataSource.setMinIdle(0);
+    dataSource.setLogAbandoned(true);
+    dataSource.setRemoveAbandonedOnBorrow(true);
+    dataSource.setTestOnBorrow(BaseObjectPoolConfig.DEFAULT_TEST_ON_BORROW);
+    dataSource.setTestWhileIdle(BaseObjectPoolConfig.DEFAULT_TEST_WHILE_IDLE);
+    
dataSource.setNumTestsPerEvictionRun(BaseObjectPoolConfig.DEFAULT_NUM_TESTS_PER_EVICTION_RUN);
+    dataSource.setTestOnReturn(BaseObjectPoolConfig.DEFAULT_TEST_ON_RETURN);
+    dataSource.setLifo(BaseObjectPoolConfig.DEFAULT_LIFO);
+    mappingProvider = new JdbcSecurableObjectMappingProvider();
+  }

Review Comment:
   I think we need to provide `JdbcAuthorizationProperties.java` and provide 
`validate()` function to validate properties.
   just like this: 
https://github.com/apache/gravitino/blob/main/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationProperties.java



##########
authorizations/authorization-jdbc/src/test/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPluginTest.java:
##########
@@ -0,0 +1,340 @@
+/*
+ * 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.gravitino.authorization.jdbc;
+
+import com.google.common.collect.Lists;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.gravitino.Audit;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.Privileges;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.RoleChange;
+import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.authorization.SecurableObjects;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.GroupEntity;
+import org.apache.gravitino.meta.UserEntity;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class JdbcAuthorizationPluginTest {
+  private static List<String> expectSQLs = Lists.newArrayList();
+  private static List<MetadataObject.Type> expectTypes = Lists.newArrayList();
+  private static List<String> expectObjectNames = Lists.newArrayList();
+  private static List<Optional<Owner>> expectPreOwners = Lists.newArrayList();
+  private static List<Owner> expectNewOwners = Lists.newArrayList();
+  private static int currentSQLIndex = 0;
+  private static int currentIndex = 0;
+
+  private static final JdbcAuthorizationPlugin plugin =
+      new JdbcAuthorizationPlugin(Collections.emptyMap()) {
+
+        @Override
+        public List<String> getSetOwnerSQL(
+            MetadataObject.Type type, String objectName, Owner preOwner, Owner 
newOwner) {
+          Assertions.assertEquals(expectTypes.get(currentIndex), type);
+          Assertions.assertEquals(expectObjectNames.get(currentIndex), 
objectName);
+          Assertions.assertEquals(expectPreOwners.get(currentIndex), 
Optional.ofNullable(preOwner));
+          Assertions.assertEquals(expectNewOwners.get(currentIndex), newOwner);
+          currentIndex++;
+          return Collections.emptyList();
+        }
+
+        void executeUpdateSQL(String sql, String ignoreErrorMsg) {
+          Assertions.assertEquals(expectSQLs.get(currentSQLIndex), sql);
+          currentSQLIndex++;
+        }
+      };
+
+  @Test
+  public void testUserManagement() {
+    expectSQLs = Lists.newArrayList("CREATE USER tmp");
+    currentSQLIndex = 0;
+    plugin.onUserAdded(createUser("tmp"));
+
+    Assertions.assertThrows(
+        UnsupportedOperationException.class, () -> 
plugin.onUserAcquired(createUser("tmp")));
+
+    expectSQLs = Lists.newArrayList("DROP USER tmp");
+    currentSQLIndex = 0;
+    plugin.onUserRemoved(createUser("tmp"));
+  }
+
+  @Test
+  public void testGroupManagement() {
+    expectSQLs = Lists.newArrayList("CREATE USER GRAVITINO_GROUP_tmp");
+    currentSQLIndex = 0;
+    plugin.onGroupAdded(createGroup("tmp"));
+
+    Assertions.assertThrows(
+        UnsupportedOperationException.class, () -> 
plugin.onGroupAcquired(createGroup("tmp")));
+
+    expectSQLs = Lists.newArrayList("DROP USER GRAVITINO_GROUP_tmp");
+    currentSQLIndex = 0;
+    plugin.onGroupRemoved(createGroup("tmp"));
+  }
+
+  @Test
+  public void testRoleManagement() {
+    expectSQLs = Lists.newArrayList("CREATE ROLE tmp");
+    currentSQLIndex = 0;
+    Role role = new TemporaryRole("tmp");
+    plugin.onRoleCreated(role);
+
+    Assertions.assertThrows(UnsupportedOperationException.class, () -> 
plugin.onRoleAcquired(role));
+
+    currentSQLIndex = 0;
+    expectSQLs = Lists.newArrayList("DROP ROLE tmp");
+    plugin.onRoleDeleted(role);
+  }
+
+  @Test
+  public void testPermissionManagement() {
+    Role role = new TemporaryRole("tmp");
+    Group group = createGroup("tmp");
+    User user = createUser("tmp");
+
+    currentSQLIndex = 0;
+    expectSQLs =
+        Lists.newArrayList(
+            "CREATE USER GRAVITINO_GROUP_tmp",
+            "CREATE ROLE tmp",
+            "GRANT ROLE tmp TO USER GRAVITINO_GROUP_tmp");
+    plugin.onGrantedRolesToGroup(Lists.newArrayList(role), group);
+
+    currentSQLIndex = 0;
+    expectSQLs =
+        Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "GRANT ROLE 
tmp TO USER tmp");
+    plugin.onGrantedRolesToUser(Lists.newArrayList(role), user);
+
+    currentSQLIndex = 0;
+    expectSQLs =
+        Lists.newArrayList(
+            "CREATE USER GRAVITINO_GROUP_tmp",
+            "CREATE ROLE tmp",
+            "REVOKE ROLE tmp FROM USER GRAVITINO_GROUP_tmp");
+    plugin.onRevokedRolesFromGroup(Lists.newArrayList(role), group);
+
+    currentSQLIndex = 0;
+    expectSQLs =
+        Lists.newArrayList("CREATE USER tmp", "CREATE ROLE tmp", "REVOKE ROLE 
tmp FROM USER tmp");
+    plugin.onRevokedRolesFromUser(Lists.newArrayList(role), user);
+
+    // Test metalake object and different role change
+    currentSQLIndex = 0;
+    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE 
*.* TO ROLE tmp");
+    SecurableObject metalakeObject =
+        SecurableObjects.ofMetalake("metalake", 
Lists.newArrayList(Privileges.SelectTable.allow()));
+    RoleChange roleChange = RoleChange.addSecurableObject("tmp", 
metalakeObject);
+    plugin.onRoleUpdated(role, roleChange);
+
+    currentSQLIndex = 0;
+    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE SELECT ON TABLE 
*.* FROM ROLE tmp");
+    roleChange = RoleChange.removeSecurableObject("tmp", metalakeObject);
+    plugin.onRoleUpdated(role, roleChange);
+
+    currentSQLIndex = 0;
+    expectSQLs =
+        Lists.newArrayList(
+            "CREATE ROLE tmp",
+            "REVOKE SELECT ON TABLE *.* FROM ROLE tmp",
+            "GRANT CREATE ON TABLE *.* TO ROLE tmp");
+    SecurableObject newMetalakeObject =
+        SecurableObjects.ofMetalake("metalake", 
Lists.newArrayList(Privileges.CreateTable.allow()));
+    roleChange = RoleChange.updateSecurableObject("tmp", metalakeObject, 
newMetalakeObject);
+    plugin.onRoleUpdated(role, roleChange);
+
+    // Test catalog object
+    currentSQLIndex = 0;
+    SecurableObject catalogObject =
+        SecurableObjects.ofCatalog("catalog", 
Lists.newArrayList(Privileges.SelectTable.allow()));
+    roleChange = RoleChange.addSecurableObject("tmp", catalogObject);
+    expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE 
*.* TO ROLE tmp");
+    plugin.onRoleUpdated(role, roleChange);
+
+    // Test schema object
+    currentSQLIndex = 0;
+    SecurableObject schemaObject =
+        SecurableObjects.ofSchema(
+            catalogObject, "schema", 
Lists.newArrayList(Privileges.SelectTable.allow()));
+    roleChange = RoleChange.addSecurableObject("tmp", schemaObject);
+    expectSQLs =
+        Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.* 
TO ROLE tmp");
+    plugin.onRoleUpdated(role, roleChange);
+
+    // Test table object
+    currentSQLIndex = 0;
+    SecurableObject tableObject =
+        SecurableObjects.ofTable(
+            schemaObject, "table", 
Lists.newArrayList(Privileges.SelectTable.allow()));
+    roleChange = RoleChange.addSecurableObject("tmp", tableObject);
+    expectSQLs =
+        Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE 
schema.table TO ROLE tmp");
+    plugin.onRoleUpdated(role, roleChange);
+  }
+
+  @Test
+  public void testOwnerManagement() {
+
+    // Test metalake object
+    Owner owner = new TemporaryOwner("tmp", Owner.Type.USER);
+    MetadataObject metalakeObject =
+        MetadataObjects.of(null, "metalake", MetadataObject.Type.METALAKE);
+    expectSQLs = Lists.newArrayList("CREATE USER tmp");
+    currentSQLIndex = 0;
+    expectTypes.add(MetadataObject.Type.SCHEMA);
+    expectObjectNames.add("*");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+
+    expectTypes.add(MetadataObject.Type.TABLE);
+    expectObjectNames.add("*.*");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+    plugin.onOwnerSet(metalakeObject, null, owner);
+
+    // clean up
+    expectTypes.clear();
+    expectObjectNames.clear();
+    expectPreOwners.clear();
+    expectNewOwners.clear();
+    currentIndex = 0;
+    expectSQLs = Lists.newArrayList("CREATE USER tmp");
+    currentSQLIndex = 0;
+
+    // Test catalog object
+    MetadataObject catalogObject = MetadataObjects.of(null, "catalog", 
MetadataObject.Type.CATALOG);
+    expectTypes.add(MetadataObject.Type.SCHEMA);
+    expectObjectNames.add("*");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+
+    expectTypes.add(MetadataObject.Type.TABLE);
+    expectObjectNames.add("*.*");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+    plugin.onOwnerSet(catalogObject, null, owner);
+
+    // clean up
+    expectTypes.clear();
+    expectObjectNames.clear();
+    expectPreOwners.clear();
+    expectNewOwners.clear();
+    currentIndex = 0;
+    expectSQLs = Lists.newArrayList("CREATE USER tmp");
+    currentSQLIndex = 0;
+
+    // Test schema object
+    MetadataObject schemaObject =
+        MetadataObjects.of("catalog", "schema", MetadataObject.Type.SCHEMA);
+    expectTypes.add(MetadataObject.Type.SCHEMA);
+    expectObjectNames.add("schema");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+
+    expectTypes.add(MetadataObject.Type.TABLE);
+    expectObjectNames.add("schema.*");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+    plugin.onOwnerSet(schemaObject, null, owner);
+
+    // clean up
+    expectTypes.clear();
+    expectObjectNames.clear();
+    expectPreOwners.clear();
+    expectNewOwners.clear();
+    currentIndex = 0;
+    expectSQLs = Lists.newArrayList("CREATE USER tmp");
+    currentSQLIndex = 0;
+
+    // Test table object
+    MetadataObject tableObject =
+        MetadataObjects.of(
+            Lists.newArrayList("catalog", "schema", "table"), 
MetadataObject.Type.TABLE);
+
+    expectTypes.add(MetadataObject.Type.TABLE);
+    expectObjectNames.add("schema.table");
+    expectPreOwners.add(Optional.empty());
+    expectNewOwners.add(owner);
+    plugin.onOwnerSet(tableObject, null, owner);
+  }
+
+  private static class TemporaryRole implements Role {
+    private final String name;
+
+    public TemporaryRole(String name) {
+      this.name = name;
+    }
+
+    @Override
+    public Audit auditInfo() {
+      return AuditInfo.EMPTY;
+    }
+
+    @Override
+    public String name() {
+      return name;
+    }
+
+    @Override
+    public Map<String, String> properties() {
+      return Collections.emptyMap();
+    }
+
+    @Override
+    public List<SecurableObject> securableObjects() {
+      return Collections.emptyList();
+    }
+  }
+
+  private static class TemporaryOwner implements Owner {
+    private final String name;
+    private final Type type;
+
+    public TemporaryOwner(String name, Type type) {
+      this.name = name;
+      this.type = type;
+    }
+
+    @Override
+    public String name() {
+      return name;
+    }
+
+    @Override
+    public Type type() {
+      return type;
+    }
+  }

Review Comment:
   I think we can directly use `OwnerEnitity`, `RoleEntity` to instead 
`TemporaryRole ` and `TemporaryOwner `.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to