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


##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/OwnerMetaMapper.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.storage.relational.mapper;
+
+import org.apache.gravitino.storage.relational.po.OwnerRelPO;
+import org.apache.ibatis.annotations.Delete;
+import org.apache.ibatis.annotations.Insert;
+import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.annotations.Select;
+import org.apache.ibatis.annotations.Update;
+
+/**
+ * A MyBatis Mapper for owner meta operation SQLs.
+ *
+ * <p>This interface class is a specification defined by MyBatis. It requires 
this interface class
+ * to identify the corresponding SQLs for execution. We can write SQLs in an 
additional XML file, or
+ * write SQLs with annotations in this interface Mapper. See: <a
+ * href="https://mybatis.org/mybatis-3/getting-started.html";></a>
+ */
+public interface OwnerMetaMapper {
+
+  String OWNER_TABLE_NAME = "owner_meta";
+
+  @Select(
+      "SELECT metalake_id as metalakeId,"
+          + " owner_id as ownerId,"
+          + " owner_type as ownerType,"
+          + " metadata_object_id as metadataObjectId,"
+          + " metadata_object_type as metadataObjectType,"
+          + " audit_info as auditInfo,"
+          + " current_version as currentVersion, last_version as lastVersion,"
+          + " deleted_at as deletedAt"
+          + " FROM "
+          + OWNER_TABLE_NAME
+          + " WHERE metadata_object_id = #{metadataObjectId} AND"
+          + " metadata_object_type = #{metadataObjectType}"
+          + " AND deleted_at = 0")
+  OwnerRelPO selectOwnerMetaByMetadataObjectIdAndType(
+      @Param("metadataObjectId") Long metadataObjectId,
+      @Param("metadataObjectType") String metadataObjectType);
+
+  @Insert(
+      "INSERT INTO "
+          + OWNER_TABLE_NAME
+          + "(metalake_id, metadata_object_id, metadata_object_type, owner_id, 
owner_type,"
+          + " audit_info, current_version, last_version, deleted_at)"
+          + " VALUES ("
+          + " #{ownerRelPO.metalakeId},"
+          + " #{ownerRelPO.metadataObjectId},"
+          + " #{ownerRelPO.metadataObjectType},"
+          + " #{ownerRelPO.ownerId},"
+          + " #{ownerRelPO.ownerType},"
+          + " #{ownerRelPO.auditInfo},"
+          + " #{ownerRelPO.currentVersion},"
+          + " #{ownerRelPO.lastVersion},"
+          + " #{ownerRelPO.deletedAt}"
+          + ")")
+  void insertOwnerRel(@Param("ownerRelPO") OwnerRelPO ownerRelPO);
+
+  @Update(
+      "UPDATE "
+          + OWNER_TABLE_NAME
+          + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)"
+          + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000"
+          + " WHERE metadata_object_id = #{metadataObjectId} AND 
metadata_object_type = #{metadataObjectType} AND deleted_at = 0")
+  void softDeleteOwnerRelByMetadataObjectIdAndType(
+      @Param("metadataObjectId") Long metadataObjectId,
+      @Param("metadataObjectType") String metadataObjectType);

Review Comment:
   Can we use `MetadataObject.type` type instead `String` for 
`metadataObjectType` param?
   Beacuse if we can use `MetadataObject.type` type, the type parameters are 
very clear, 
   and we can only call once `MetadataObject.type.name()` function, currently 
we need to call `.name()` function repeatedly.



##########
core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java:
##########
@@ -0,0 +1,193 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.SupportsRelationOperations;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.lock.LockType;
+import org.apache.gravitino.lock.TreeLockUtils;
+import org.apache.gravitino.meta.GroupEntity;
+import org.apache.gravitino.meta.UserEntity;
+import org.apache.gravitino.storage.kv.KvEntityStore;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * OwnerManager is used for manage the owner of metadata object. The user and 
group don't have an
+ * owner
+ */
+public class OwnerManager {
+  private static final Logger LOG = 
LoggerFactory.getLogger(OwnerManager.class);
+  private final EntityStore store;
+
+  public OwnerManager(EntityStore store) {
+    if (store instanceof KvEntityStore) {
+      String errorMsg =
+          "OwnerManager cannot run with kv entity store, please configure the 
entity "
+              + "store to use relational entity store and restart the 
Gravitino server";
+      LOG.error(errorMsg);
+      throw new RuntimeException(errorMsg);
+    }
+
+    if (!(store instanceof SupportsRelationOperations)) {
+      String errorMsg =
+          "OwnerManager cannot run with entity store that does not support 
relation operations, "
+              + "please configure the entity store to use relational entity 
store and restart the Gravitino server";
+      LOG.error(errorMsg);
+      throw new RuntimeException(errorMsg);
+    }
+    this.store = store;

Review Comment:
   This codes logic have a little confusing.
   
   ```  
   public OwnerManager(EntityStore store) {
       if (store instanceof KvEntityStore) {
         String errorMsg =
             "OwnerManager cannot run with kv entity store, please configure 
the entity "
                 + "store to use relational entity store and restart the 
Gravitino server";
         LOG.error(errorMsg);
         throw new RuntimeException(errorMsg);
       } else if (store instanceof SupportsRelationOperations) {
         this.store = store;
       } else {
         String errorMsg = "OwnerManager currently only supports relational 
entity store, " +
               "please configure the entity store to use relational entity 
store and restart the Gravitino server";
         LOG.error(errorMsg);
         throw new RuntimeException(errorMsg);
       }
     }
   
   ```



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