This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-0.6
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.6 by this push:
new c5c103db8 [#5070] improvement(core): Add check for the full name of
the metadata object (#5091)
c5c103db8 is described below
commit c5c103db8ec273de37519685e603a2469f3132f8
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Oct 11 09:57:23 2024 +0800
[#5070] improvement(core): Add check for the full name of the metadata
object (#5091)
### What changes were proposed in this pull request?
Add check for full name of the metadata object.
### Why are the changes needed?
Fix: #5070
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Add UTs.
Co-authored-by: roryqi <[email protected]>
---
.../authorization/AuthorizationUtils.java | 62 ---------------
.../java/org/apache/gravitino/tag/TagManager.java | 62 +++------------
.../apache/gravitino/utils/MetadataObjectUtil.java | 78 +++++++++++++++++++
.../org/apache/gravitino/tag/TestTagManager.java | 29 ++++++--
.../gravitino/server/web/rest/OwnerOperations.java | 2 +
.../server/web/rest/PermissionOperations.java | 5 +-
.../gravitino/server/web/rest/RoleOperations.java | 3 +-
.../server/web/rest/TestOwnerOperations.java | 47 ++++++++++++
.../server/web/rest/TestRoleOperations.java | 87 ++++++++++++++++++----
9 files changed, 241 insertions(+), 134 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
index 66019ee04..51f5cae62 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -18,15 +18,12 @@
*/
package org.apache.gravitino.authorization;
-import static com.google.common.base.Preconditions.checkNotNull;
-
import com.google.common.collect.Sets;
import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.Consumer;
-import java.util.function.Supplier;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
@@ -216,58 +213,6 @@ public class AuthorizationUtils {
return false;
}
- // Check every securable object whether exists and is imported.
- public static void checkSecurableObject(String metalake, MetadataObject
object) {
- NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalake,
object);
-
- Supplier<NoSuchMetadataObjectException> exceptionToThrowSupplier =
- () ->
- new NoSuchMetadataObjectException(
- "Securable object %s type %s doesn't exist",
object.fullName(), object.type());
-
- switch (object.type()) {
- case METALAKE:
- check(
-
GravitinoEnv.getInstance().metalakeDispatcher().metalakeExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- case CATALOG:
- check(
-
GravitinoEnv.getInstance().catalogDispatcher().catalogExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- case SCHEMA:
- check(
-
GravitinoEnv.getInstance().schemaDispatcher().schemaExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- case FILESET:
- check(
-
GravitinoEnv.getInstance().filesetDispatcher().filesetExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- case TABLE:
- check(
-
GravitinoEnv.getInstance().tableDispatcher().tableExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- case TOPIC:
- check(
-
GravitinoEnv.getInstance().topicDispatcher().topicExists(identifier),
- exceptionToThrowSupplier);
- break;
-
- default:
- throw new IllegalArgumentException(
- String.format("Doesn't support the type %s", object.type()));
- }
- }
-
public static void checkDuplicatedNamePrivilege(Collection<Privilege>
privileges) {
Set<Privilege.Name> privilegeNameSet = Sets.newHashSet();
for (Privilege privilege : privileges) {
@@ -313,13 +258,6 @@ public class AuthorizationUtils {
}
}
- private static void check(
- final boolean expression, Supplier<? extends RuntimeException>
exceptionToThrowSupplier) {
- if (!expression) {
- throw checkNotNull(exceptionToThrowSupplier).get();
- }
- }
-
private static void checkCatalogType(
NameIdentifier catalogIdent, Catalog.Type type, Privilege privilege) {
Catalog catalog =
GravitinoEnv.getInstance().catalogDispatcher().loadCatalog(catalogIdent);
diff --git a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
index 040ac9f19..aaffd35b5 100644
--- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
+++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
@@ -18,7 +18,6 @@
*/
package org.apache.gravitino.tag;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -31,11 +30,11 @@ import java.util.Set;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
-import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchTagException;
import org.apache.gravitino.exceptions.NotFoundException;
@@ -240,14 +239,11 @@ public class TagManager {
}
public Tag[] listTagsInfoForMetadataObject(String metalake, MetadataObject
metadataObject)
- throws NotFoundException {
+ throws NoSuchMetadataObjectException {
NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
- if (!checkAndImportEntity(metalake, metadataObject,
GravitinoEnv.getInstance())) {
- throw new NotFoundException(
- "Failed to list tags for metadata object %s due to not found",
metadataObject);
- }
+ MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
return TreeLockUtils.doWithTreeLock(
entityIdent,
@@ -258,7 +254,7 @@ public class TagManager {
.listAssociatedTagsForMetadataObject(entityIdent, entityType)
.toArray(new Tag[0]);
} catch (NoSuchEntityException e) {
- throw new NotFoundException(
+ throw new NoSuchMetadataObjectException(
e, "Failed to list tags for metadata object %s due to not
found", metadataObject);
} catch (IOException e) {
LOG.error("Failed to list tags for metadata object {}",
metadataObject, e);
@@ -268,15 +264,12 @@ public class TagManager {
}
public Tag getTagForMetadataObject(String metalake, MetadataObject
metadataObject, String name)
- throws NotFoundException {
+ throws NoSuchMetadataObjectException {
NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
NameIdentifier tagIdent = ofTagIdent(metalake, name);
- if (!checkAndImportEntity(metalake, metadataObject,
GravitinoEnv.getInstance())) {
- throw new NotFoundException(
- "Failed to get tag for metadata object %s due to not found",
metadataObject);
- }
+ MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
return TreeLockUtils.doWithTreeLock(
entityIdent,
@@ -289,7 +282,7 @@ public class TagManager {
throw new NoSuchTagException(
e, "Tag %s does not exist for metadata object %s", name,
metadataObject);
} else {
- throw new NotFoundException(
+ throw new NoSuchMetadataObjectException(
e, "Failed to get tag for metadata object %s due to not
found", metadataObject);
}
} catch (IOException e) {
@@ -301,20 +294,18 @@ public class TagManager {
public String[] associateTagsForMetadataObject(
String metalake, MetadataObject metadataObject, String[] tagsToAdd,
String[] tagsToRemove)
- throws NotFoundException, TagAlreadyAssociatedException {
+ throws NoSuchMetadataObjectException, TagAlreadyAssociatedException {
Preconditions.checkArgument(
!metadataObject.type().equals(MetadataObject.Type.METALAKE)
- && !metadataObject.type().equals(MetadataObject.Type.COLUMN),
+ && !metadataObject.type().equals(MetadataObject.Type.COLUMN)
+ && !metadataObject.type().equals(MetadataObject.Type.ROLE),
"Cannot associate tags for unsupported metadata object type %s",
metadataObject.type());
NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
- if (!checkAndImportEntity(metalake, metadataObject,
GravitinoEnv.getInstance())) {
- throw new NotFoundException(
- "Failed to associate tags for metadata object %s due to not found",
metadataObject);
- }
+ MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
// Remove all the tags that are both set to add and remove
Set<String> tagsToAddSet = tagsToAdd == null ? Sets.newHashSet() :
Sets.newHashSet(tagsToAdd);
@@ -347,7 +338,7 @@ public class TagManager {
.map(Tag::name)
.toArray(String[]::new);
} catch (NoSuchEntityException e) {
- throw new NotFoundException(
+ throw new NoSuchMetadataObjectException(
e,
"Failed to associate tags for metadata object %s due
to not found",
metadataObject);
@@ -425,33 +416,4 @@ public class TagManager {
.build())
.build();
}
-
- // This method will check if the entity is existed explicitly, internally
this check will load
- // the entity from underlying sources to entity store if not stored, and
will allocate an uid
- // for this entity, with this uid tags can be associated with this entity.
- // This method should be called out of the tree lock, otherwise it will
cause deadlock.
- @VisibleForTesting
- boolean checkAndImportEntity(String metalake, MetadataObject metadataObject,
GravitinoEnv env) {
- NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
- Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
-
- switch (entityType) {
- case METALAKE:
- return env.metalakeDispatcher().metalakeExists(entityIdent);
- case CATALOG:
- return env.catalogDispatcher().catalogExists(entityIdent);
- case SCHEMA:
- return env.schemaDispatcher().schemaExists(entityIdent);
- case TABLE:
- return env.tableDispatcher().tableExists(entityIdent);
- case TOPIC:
- return env.topicDispatcher().topicExists(entityIdent);
- case FILESET:
- return env.filesetDispatcher().filesetExists(entityIdent);
- case COLUMN:
- default:
- throw new IllegalArgumentException(
- "Unsupported metadata object type: " + metadataObject.type());
- }
- }
}
diff --git
a/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
b/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
index 42878ef09..014ae3a18 100644
--- a/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
+++ b/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
@@ -18,16 +18,22 @@
*/
package org.apache.gravitino.utils;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
import java.util.Optional;
+import java.util.function.Supplier;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationUtils;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
+import org.apache.gravitino.exceptions.NoSuchRoleException;
public class MetadataObjectUtil {
@@ -98,4 +104,76 @@ public class MetadataObjectUtil {
"Unknown metadata object type: " + metadataObject.type());
}
}
+
+ /**
+ * This method will check if the entity is existed explicitly, internally
this check will load the
+ * entity from underlying sources to entity store if not stored, and will
allocate an uid for this
+ * entity, with this uid tags can be associated with this entity. This
method should be called out
+ * of the tree lock, otherwise it will cause deadlock.
+ *
+ * @param metalake The metalake name
+ * @param object The metadata object
+ * @throws NoSuchMetadataObjectException if the metadata object type doesn't
exist.
+ */
+ public static void checkMetadataObject(String metalake, MetadataObject
object) {
+ GravitinoEnv env = GravitinoEnv.getInstance();
+ NameIdentifier identifier = toEntityIdent(metalake, object);
+
+ Supplier<NoSuchMetadataObjectException> exceptionToThrowSupplier =
+ () ->
+ new NoSuchMetadataObjectException(
+ "Metadata object %s type %s doesn't exist", object.fullName(),
object.type());
+
+ switch (object.type()) {
+ case METALAKE:
+ NameIdentifierUtil.checkMetalake(identifier);
+ check(env.metalakeDispatcher().metalakeExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case CATALOG:
+ NameIdentifierUtil.checkCatalog(identifier);
+ check(env.catalogDispatcher().catalogExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case SCHEMA:
+ NameIdentifierUtil.checkSchema(identifier);
+ check(env.schemaDispatcher().schemaExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case FILESET:
+ NameIdentifierUtil.checkFileset(identifier);
+ check(env.filesetDispatcher().filesetExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case TABLE:
+ NameIdentifierUtil.checkTable(identifier);
+ check(env.tableDispatcher().tableExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case TOPIC:
+ NameIdentifierUtil.checkTopic(identifier);
+ check(env.topicDispatcher().topicExists(identifier),
exceptionToThrowSupplier);
+ break;
+
+ case ROLE:
+ AuthorizationUtils.checkRole(identifier);
+ try {
+ env.accessControlDispatcher().getRole(metalake, object.fullName());
+ } catch (NoSuchRoleException nsr) {
+ throw checkNotNull(exceptionToThrowSupplier).get();
+ }
+ break;
+
+ default:
+ throw new IllegalArgumentException(
+ String.format("Doesn't support the type %s", object.type()));
+ }
+ }
+
+ private static void check(
+ final boolean expression, Supplier<? extends RuntimeException>
exceptionToThrowSupplier) {
+ if (!expression) {
+ throw checkNotNull(exceptionToThrowSupplier).get();
+ }
+ }
}
diff --git a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
index 3dc298cdb..82ed55eed 100644
--- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
+++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
@@ -31,8 +31,9 @@ import static
org.apache.gravitino.Configs.TREE_LOCK_CLEAN_INTERVAL;
import static org.apache.gravitino.Configs.TREE_LOCK_MAX_NODE_IN_MEMORY;
import static org.apache.gravitino.Configs.TREE_LOCK_MIN_NODE_IN_MEMORY;
import static org.apache.gravitino.Configs.VERSION_RETENTION_COUNT;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.spy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -54,6 +55,9 @@ import org.apache.gravitino.EntityStoreFactory;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.CatalogDispatcher;
+import org.apache.gravitino.catalog.SchemaDispatcher;
+import org.apache.gravitino.catalog.TableDispatcher;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchTagException;
import org.apache.gravitino.exceptions.NotFoundException;
@@ -66,6 +70,7 @@ import org.apache.gravitino.meta.CatalogEntity;
import org.apache.gravitino.meta.SchemaEntity;
import org.apache.gravitino.meta.SchemaVersion;
import org.apache.gravitino.meta.TableEntity;
+import org.apache.gravitino.metalake.MetalakeDispatcher;
import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.storage.RandomIdGenerator;
import org.apache.gravitino.utils.NameIdentifierUtil;
@@ -91,6 +96,10 @@ public class TestTagManager {
private static final String SCHEMA = "schema_for_tag_test";
private static final String TABLE = "table_for_tag_test";
+ private static final MetalakeDispatcher metalakeDispatcher =
mock(MetalakeDispatcher.class);
+ private static final CatalogDispatcher catalogDispatcher =
mock(CatalogDispatcher.class);
+ private static final SchemaDispatcher schemaDispatcher =
mock(SchemaDispatcher.class);
+ private static final TableDispatcher tableDispatcher =
mock(TableDispatcher.class);
private static EntityStore entityStore;
@@ -166,10 +175,18 @@ public class TestTagManager {
.build();
entityStore.put(table, false /* overwritten */);
- tagManager = spy(new TagManager(idGenerator, entityStore));
- doReturn(true)
- .when(tagManager)
- .checkAndImportEntity(Mockito.any(), Mockito.any(), Mockito.any());
+ tagManager = new TagManager(idGenerator, entityStore);
+
+ FieldUtils.writeField(
+ GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher,
true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogDispatcher",
catalogDispatcher, true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "schemaDispatcher",
schemaDispatcher, true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "tableDispatcher",
tableDispatcher, true);
+
+ when(metalakeDispatcher.metalakeExists(any())).thenReturn(true);
+ when(catalogDispatcher.catalogExists(any())).thenReturn(true);
+ when(schemaDispatcher.schemaExists(any())).thenReturn(true);
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
}
@AfterAll
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
index d4fe66b7f..ea5684b55 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
@@ -78,6 +78,7 @@ public class OwnerOperations {
return Utils.doAs(
httpRequest,
() -> {
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
object);
Optional<Owner> owner =
TreeLockUtils.doWithTreeLock(
@@ -112,6 +113,7 @@ public class OwnerOperations {
return Utils.doAs(
httpRequest,
() -> {
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
NameIdentifier objectIdent =
MetadataObjectUtil.toEntityIdent(metalake, object);
TreeLockUtils.doWithTreeLock(
objectIdent,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
index 94ace77aa..38fcd7380 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
@@ -50,6 +50,7 @@ import org.apache.gravitino.lock.TreeLockUtils;
import org.apache.gravitino.metrics.MetricNames;
import org.apache.gravitino.server.authorization.NameBindings;
import org.apache.gravitino.server.web.Utils;
+import org.apache.gravitino.utils.MetadataObjectUtil;
@NameBindings.AccessControlInterfaces
@Path("/metalakes/{metalake}/permissions")
@@ -217,7 +218,7 @@ public class PermissionOperations {
AuthorizationUtils.checkPrivilege(privilegeDTO, object,
metalake);
}
- AuthorizationUtils.checkSecurableObject(metalake, object);
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
return TreeLockUtils.doWithTreeLock(
AuthorizationUtils.ofRole(metalake, role),
LockType.WRITE,
@@ -262,7 +263,7 @@ public class PermissionOperations {
AuthorizationUtils.checkPrivilege(privilegeDTO, object,
metalake);
}
- AuthorizationUtils.checkSecurableObject(metalake, object);
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
return TreeLockUtils.doWithTreeLock(
AuthorizationUtils.ofRole(metalake, role),
LockType.WRITE,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
index e20f6b451..91ebaf5b4 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
@@ -55,6 +55,7 @@ import org.apache.gravitino.lock.TreeLockUtils;
import org.apache.gravitino.metrics.MetricNames;
import org.apache.gravitino.server.authorization.NameBindings;
import org.apache.gravitino.server.web.Utils;
+import org.apache.gravitino.utils.MetadataObjectUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -142,7 +143,7 @@ public class RoleOperations {
for (Privilege privilege : object.privileges()) {
AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege,
object, metalake);
}
- AuthorizationUtils.checkSecurableObject(metalake, object);
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
}
List<SecurableObject> securableObjects =
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
index 2b0b3ff4a..0643ed9bf 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
@@ -36,17 +36,25 @@ import javax.ws.rs.core.Response;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.authorization.AccessControlDispatcher;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerManager;
+import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.dto.authorization.OwnerDTO;
import org.apache.gravitino.dto.requests.OwnerSetRequest;
import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.dto.responses.OwnerResponse;
import org.apache.gravitino.dto.responses.SetResponse;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
+import org.apache.gravitino.exceptions.NoSuchRoleException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.lock.LockManager;
+import org.apache.gravitino.metalake.MetalakeDispatcher;
import org.apache.gravitino.rest.RESTUtils;
+import org.apache.gravitino.utils.MetadataObjectUtil;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
@@ -58,6 +66,9 @@ import org.mockito.Mockito;
class TestOwnerOperations extends JerseyTest {
private static final OwnerManager manager = mock(OwnerManager.class);
+ private static final MetalakeDispatcher metalakeDispatcher =
mock(MetalakeDispatcher.class);
+ private static final AccessControlDispatcher accessControlDispatcher =
+ mock(AccessControlDispatcher.class);
private static class MockServletRequestFactory extends
ServletRequestFactoryBase {
@Override
@@ -76,6 +87,10 @@ class TestOwnerOperations extends JerseyTest {
Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL);
FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new
LockManager(config), true);
FieldUtils.writeField(GravitinoEnv.getInstance(), "ownerManager", manager,
true);
+ FieldUtils.writeField(
+ GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher,
true);
+ FieldUtils.writeField(
+ GravitinoEnv.getInstance(), "accessControlDispatcher",
accessControlDispatcher, true);
}
@Override
@@ -116,6 +131,7 @@ class TestOwnerOperations extends JerseyTest {
};
when(manager.getOwner(any(), any())).thenReturn(Optional.of(owner));
+ when(metalakeDispatcher.metalakeExists(any())).thenReturn(true);
Response resp =
target("/metalakes/metalake1/owners/metalake/metalake1")
@@ -172,10 +188,20 @@ class TestOwnerOperations extends JerseyTest {
ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class);
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse2.getCode());
+
+ // Test to throw IllegalNamespaceException
+ Response resp4 =
+ target("/metalakes/metalake1/owners/catalog/metalake1.catalog1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .get();
+ ErrorResponse errorResponse3 = resp4.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse3.getCode());
}
@Test
void testSetOwnerForObject() {
+ when(metalakeDispatcher.metalakeExists(any())).thenReturn(true);
OwnerSetRequest request = new OwnerSetRequest("test", Owner.Type.USER);
Response resp =
target("/metalakes/metalake1/owners/metalake/metalake1")
@@ -216,5 +242,26 @@ class TestOwnerOperations extends JerseyTest {
ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class);
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse2.getCode());
+
+ // Test to throw IllegalNamespaceException
+ Response resp4 =
+ target("/metalakes/metalake1/owners/catalog/metalake1.catalog1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
+ ErrorResponse errorResponse3 = resp4.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse3.getCode());
+ }
+
+ @Test
+ public void testRoleObject() {
+ MetadataObject role = MetadataObjects.of(null, "role",
MetadataObject.Type.ROLE);
+ when(accessControlDispatcher.getRole(any(),
any())).thenReturn(mock(Role.class));
+ Assertions.assertDoesNotThrow(() ->
MetadataObjectUtil.checkMetadataObject("metalake", role));
+
+ doThrow(new
NoSuchRoleException("test")).when(accessControlDispatcher).getRole(any(),
any());
+ Assertions.assertThrows(
+ NoSuchMetadataObjectException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake", role));
}
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
index 265fb6073..55fa7dd3a 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
@@ -31,6 +31,8 @@ import com.google.common.collect.Lists;
import java.io.IOException;
import java.time.Instant;
import java.util.Collections;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Application;
@@ -39,8 +41,8 @@ import javax.ws.rs.core.Response;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.AccessControlManager;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.authorization.SecurableObject;
@@ -59,6 +61,7 @@ import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.dto.responses.NameListResponse;
import org.apache.gravitino.dto.responses.RoleResponse;
import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.exceptions.IllegalNamespaceException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -69,6 +72,7 @@ import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.metalake.MetalakeDispatcher;
import org.apache.gravitino.rest.RESTUtils;
+import org.apache.gravitino.utils.MetadataObjectUtil;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
@@ -105,8 +109,6 @@ public class TestRoleOperations extends JerseyTest {
Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL);
FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new
LockManager(config), true);
FieldUtils.writeField(GravitinoEnv.getInstance(),
"accessControlDispatcher", manager, true);
- FieldUtils.writeField(
- GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher,
true);
FieldUtils.writeField(
GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher,
true);
FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogDispatcher",
catalogDispatcher, true);
@@ -439,11 +441,11 @@ public class TestRoleOperations extends JerseyTest {
SecurableObjects.ofCatalog("catalog",
Lists.newArrayList(Privileges.UseCatalog.allow()));
when(catalogDispatcher.catalogExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(catalog)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(catalog)));
when(catalogDispatcher.catalogExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(catalog)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(catalog)));
// check the schema
SecurableObject schema =
@@ -451,11 +453,11 @@ public class TestRoleOperations extends JerseyTest {
catalog, "schema",
Lists.newArrayList(Privileges.UseSchema.allow()));
when(schemaDispatcher.schemaExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(schema)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(schema)));
when(schemaDispatcher.schemaExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(schema)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(schema)));
// check the table
SecurableObject table =
@@ -463,11 +465,11 @@ public class TestRoleOperations extends JerseyTest {
schema, "table",
Lists.newArrayList(Privileges.SelectTable.allow()));
when(tableDispatcher.tableExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(table)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(table)));
when(tableDispatcher.tableExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(table)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(table)));
// check the topic
SecurableObject topic =
@@ -475,11 +477,11 @@ public class TestRoleOperations extends JerseyTest {
schema, "topic",
Lists.newArrayList(Privileges.ConsumeTopic.allow()));
when(topicDispatcher.topicExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(topic)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(topic)));
when(topicDispatcher.topicExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(topic)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(topic)));
// check the fileset
SecurableObject fileset =
@@ -487,11 +489,70 @@ public class TestRoleOperations extends JerseyTest {
schema, "fileset",
Lists.newArrayList(Privileges.ReadFileset.allow()));
when(filesetDispatcher.filesetExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(fileset)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(fileset)));
when(filesetDispatcher.filesetExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
- () -> AuthorizationUtils.checkSecurableObject("metalake",
DTOConverters.toDTO(fileset)));
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
DTOConverters.toDTO(fileset)));
+
+ AtomicReference<String> wrongParent = new AtomicReference<>();
+ AtomicReference<String> wrongName = new AtomicReference<>();
+ AtomicReference<MetadataObject.Type> wrongType = new AtomicReference<>();
+ wrongParent.set("catalog1");
+ wrongName.set("schema1");
+
+ MetadataObject wrongMetadataObject =
+ new MetadataObject() {
+ @Nullable
+ @Override
+ public String parent() {
+ return wrongParent.get();
+ }
+
+ @Override
+ public String name() {
+ return wrongName.get();
+ }
+
+ @Override
+ public Type type() {
+ return wrongType.get();
+ }
+ };
+
+ // Test catalog object
+ wrongType.set(MetadataObject.Type.CATALOG);
+ Assertions.assertThrows(
+ IllegalNamespaceException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
wrongMetadataObject));
+
+ // Test schema object
+ wrongType.set(MetadataObject.Type.CATALOG);
+ wrongParent.set("catalog1.schema1");
+ Assertions.assertThrows(
+ IllegalNamespaceException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
wrongMetadataObject));
+
+ // Test table object
+ wrongType.set(MetadataObject.Type.TABLE);
+ wrongParent.set("catalog1");
+ Assertions.assertThrows(
+ IllegalNamespaceException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
wrongMetadataObject));
+
+ // Test fileset object
+ wrongType.set(MetadataObject.Type.FILESET);
+ wrongParent.set("catalog1");
+ Assertions.assertThrows(
+ IllegalNamespaceException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
wrongMetadataObject));
+
+ // Test topic object
+ wrongType.set(MetadataObject.Type.TOPIC);
+ wrongParent.set("catalog1");
+ Assertions.assertThrows(
+ IllegalNamespaceException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("metalake",
wrongMetadataObject));
}
@Test
