diff --git a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java index c48e0fcf02c..447717d77cb 100644 --- a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java +++ b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java @@ -121,6 +121,16 @@ public void setAccessControlManager(AccessControlManager accessControlManager) { this.accessControlManager = accessControlManager; } + /** + * This method is used for testing purposes only to set the metalake dispatcher for test. + * + * @param metalakeDispatcher The metalake dispatcher to be set. + */ + @VisibleForTesting + public void setMetalakeDispatcher(MetalakeDispatcher metalakeDispatcher) { + this.metalakeDispatcher = metalakeDispatcher; + } + /** * This method is used for testing purposes only to set the access manager for test in package * `com.datastrato.gravitino.server.web.rest`. @@ -132,6 +142,46 @@ public void setCatalogDispatcher(CatalogDispatcher catalogDispatcher) { this.catalogDispatcher = catalogDispatcher; } + /** + * This method is used for testing purposes only to set the schema dispatcher for test. + * + * @param schemaDispatcher The schema dispatcher to be set. + */ + @VisibleForTesting + public void setSchemaDispatcher(SchemaDispatcher schemaDispatcher) { + this.schemaDispatcher = schemaDispatcher; + } + + /** + * This method is used for testing purposes only to set the table dispatcher for test. + * + * @param tableDispatcher The table dispatcher to be set. + */ + @VisibleForTesting + public void setTableDispatcher(TableDispatcher tableDispatcher) { + this.tableDispatcher = tableDispatcher; + } + + /** + * This method is used for testing purposes only to set the topic dispatcher for test. + * + * @param topicDispatcher The topic dispatcher to be set. + */ + @VisibleForTesting + public void setTopicDispatcher(TopicDispatcher topicDispatcher) { + this.topicDispatcher = topicDispatcher; + } + + /** + * This method is used for testing purposes only to set the filset dispatcher for test. + * + * @param filesetDispatcher The fileset dispatcher to be set. + */ + @VisibleForTesting + public void setFilesetDispatcher(FilesetDispatcher filesetDispatcher) { + this.filesetDispatcher = filesetDispatcher; + } + /** * This method is used for testing purposes only to set the entity store for test in package * `com.datastrato.gravitino.authorization`. diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java index 178b0f921fc..cbf5e279975 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java @@ -214,8 +214,7 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(true); + alteredSchema.properties())); } StringIdentifier stringId = getStringIdFromProperties(alteredSchema.properties()); @@ -226,8 +225,7 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(isEntityExist(ident)); + alteredSchema.properties())); } SchemaEntity updatedSchemaEntity = @@ -255,15 +253,12 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) "UPDATE", stringId.id()); - boolean imported = updatedSchemaEntity != null; - return EntityCombinedSchema.of(alteredSchema, updatedSchemaEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(imported); + alteredSchema.properties())); } /** diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java index 52650303f33..836c3aa7237 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java @@ -10,6 +10,7 @@ import static com.datastrato.gravitino.rel.expressions.transforms.Transforms.EMPTY_TRANSFORM; import com.datastrato.gravitino.EntityStore; +import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.StringIdentifier; @@ -88,8 +89,12 @@ public Table loadTable(NameIdentifier ident) throws NoSuchTableException { return table; } - TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTable(ident)); + if (GravitinoEnv.getInstance() + .schemaDispatcher() + .schemaExists(NameIdentifier.of(ident.namespace().levels()))) { + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTable(ident)); + } return table; } @@ -231,8 +236,7 @@ public Table alterTable(NameIdentifier ident, TableChange... changes) getHiddenPropertyNames( getCatalogIdentifier(ident), HasPropertyMetadata::tablePropertiesMetadata, - alteredTable.properties())) - .withImported(isEntityExist(ident)); + alteredTable.properties())); } TableEntity updatedTableEntity = @@ -267,15 +271,12 @@ public Table alterTable(NameIdentifier ident, TableChange... changes) "UPDATE", stringId.id()); - boolean imported = updatedTableEntity != null; - return EntityCombinedTable.of(alteredTable, updatedTableEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( getCatalogIdentifier(ident), HasPropertyMetadata::tablePropertiesMetadata, - alteredTable.properties())) - .withImported(imported); + alteredTable.properties())); } /** diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java index c0a43f73fe0..1bc69760633 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java @@ -9,6 +9,7 @@ import static com.datastrato.gravitino.catalog.PropertiesMetadataHelpers.validatePropertyForCreate; import com.datastrato.gravitino.EntityStore; +import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.StringIdentifier; @@ -79,8 +80,12 @@ public Topic loadTopic(NameIdentifier ident) throws NoSuchTopicException { return topic; } - TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTopic(ident)); + if (GravitinoEnv.getInstance() + .schemaDispatcher() + .schemaExists(NameIdentifier.of(ident.namespace().levels()))) { + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTopic(ident)); + } return topic; } @@ -219,15 +224,12 @@ public Topic alterTopic(NameIdentifier ident, TopicChange... changes) "UPDATE", getStringIdFromProperties(alteredTopic.properties()).id()); - boolean imported = updatedTopicEntity != null; - return EntityCombinedTopic.of(alteredTopic, updatedTopicEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, - alteredTopic.properties())) - .withImported(imported); + alteredTopic.properties())); } /** diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java index b8e8e303fa7..1cf998e565c 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java @@ -16,6 +16,7 @@ import com.datastrato.gravitino.Config; import com.datastrato.gravitino.Configs; +import com.datastrato.gravitino.Entity; import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; @@ -118,6 +119,74 @@ public void testCreateAndListSchemas() throws IOException { Assertions.assertEquals("test", schema2.auditInfo().creator()); } + @Test + public void testCreateAndLoadSchema() throws IOException { + NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalog, "schema20"); + Map props = ImmutableMap.of("k1", "v1", "k2", "v2"); + Schema schema = dispatcher.createSchema(schemaIdent, "comment", props); + Assertions.assertEquals("schema20", schema.name()); + Assertions.assertEquals("comment", schema.comment()); + testProperties(props, schema.properties()); + + Schema loadedSchema = dispatcher.loadSchema(schemaIdent); + Assertions.assertEquals(loadedSchema.name(), schema.name()); + Assertions.assertEquals(loadedSchema.comment(), schema.comment()); + testProperties(loadedSchema.properties(), schema.properties()); + // Audit info is gotten from the entity store + Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, loadedSchema.auditInfo().creator()); + + // Case 2: Test if the schema is not found in entity store + doThrow(new NoSuchEntityException("mock error")).when(entityStore).get(any(), any(), any()); + entityStore.delete(schemaIdent, Entity.EntityType.SCHEMA); + Schema loadedSchema1 = dispatcher.loadSchema(schemaIdent); + Assertions.assertEquals(schema.name(), loadedSchema1.name()); + Assertions.assertEquals(schema.comment(), loadedSchema1.comment()); + testProperties(props, loadedSchema1.properties()); + // Succeed to import the topic entity + Assertions.assertTrue(entityStore.exists(schemaIdent, SCHEMA)); + + // Audit info is gotten from catalog, not from the entity store + Assertions.assertEquals("test", loadedSchema1.auditInfo().creator()); + + // Case 3: Test if entity store is failed to get the schema entity + reset(entityStore); + doThrow(new IOException()).when(entityStore).get(any(), any(), any()); + entityStore.delete(schemaIdent, Entity.EntityType.SCHEMA); + Schema loadedSchema2 = dispatcher.loadSchema(schemaIdent); + // Succeed to import the topic entity + Assertions.assertTrue(entityStore.exists(schemaIdent, SCHEMA)); + Assertions.assertEquals(schema.name(), loadedSchema2.name()); + Assertions.assertEquals(schema.comment(), loadedSchema2.comment()); + testProperties(props, loadedSchema2.properties()); + // Audit info is gotten from catalog, not from the entity store + Assertions.assertEquals("test", loadedSchema2.auditInfo().creator()); + + // Case 4: Test if the fetched schema entity is matched. + reset(entityStore); + SchemaEntity unmatchedEntity = + SchemaEntity.builder() + .withId(1L) + .withName("schema21") + .withNamespace(Namespace.of(metalake, catalog)) + .withAuditInfo( + AuditInfo.builder() + .withCreator(AuthConstants.ANONYMOUS_USER) + .withCreateTime(Instant.now()) + .build()) + .build(); + doReturn(unmatchedEntity).when(entityStore).get(any(), any(), any()); + Schema loadedSchema3 = dispatcher.loadSchema(schemaIdent); + // Succeed to import the schema entity + reset(entityStore); + SchemaEntity schemaEntity = entityStore.get(schemaIdent, SCHEMA, SchemaEntity.class); + Assertions.assertEquals("test", schemaEntity.auditInfo().creator()); + Assertions.assertEquals(schema.name(), loadedSchema3.name()); + Assertions.assertEquals(schema.comment(), loadedSchema3.comment()); + testProperties(props, loadedSchema3.properties()); + // Audit info is gotten from catalog, not from the entity store + Assertions.assertEquals("test", loadedSchema3.auditInfo().creator()); + } + @Test public void testCreateAndAlterSchema() throws IOException { NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalog, "schema21"); diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java index f3c1324b671..a37e7e5acdb 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java @@ -7,6 +7,7 @@ import static com.datastrato.gravitino.Configs.TREE_LOCK_CLEAN_INTERVAL; import static com.datastrato.gravitino.Configs.TREE_LOCK_MAX_NODE_IN_MEMORY; import static com.datastrato.gravitino.Configs.TREE_LOCK_MIN_NODE_IN_MEMORY; +import static com.datastrato.gravitino.Entity.EntityType.SCHEMA; import static com.datastrato.gravitino.Entity.EntityType.TABLE; import static com.datastrato.gravitino.StringIdentifier.ID_KEY; import static com.datastrato.gravitino.TestBasePropertiesMetadata.COMMENT_KEY; @@ -59,6 +60,7 @@ public static void initialize() throws IOException { doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + GravitinoEnv.getInstance().setSchemaDispatcher(schemaOperationDispatcher); } @Test @@ -167,15 +169,25 @@ public void testCreateAndLoadTable() throws IOException { // Case 2: Test if the table entity is not found in the entity store reset(entityStore); + entityStore.delete(tableIdent1, TABLE); + entityStore.delete(NameIdentifier.of(tableNs.levels()), SCHEMA); doThrow(new NoSuchEntityException("")).when(entityStore).get(any(), any(), any()); Table loadedTable2 = tableOperationDispatcher.loadTable(tableIdent1); + // Succeed to import the topic entity + Assertions.assertTrue(entityStore.exists(NameIdentifier.of(tableNs.levels()), SCHEMA)); + Assertions.assertTrue(entityStore.exists(tableIdent1, TABLE)); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTable2.auditInfo().creator()); // Case 3: Test if the entity store is failed to get the table entity reset(entityStore); + entityStore.delete(tableIdent1, TABLE); + entityStore.delete(NameIdentifier.of(tableNs.levels()), SCHEMA); doThrow(new IOException()).when(entityStore).get(any(), any(), any()); Table loadedTable3 = tableOperationDispatcher.loadTable(tableIdent1); + // Succeed to import the topic entity + Assertions.assertTrue(entityStore.exists(NameIdentifier.of(tableNs.levels()), SCHEMA)); + Assertions.assertTrue(entityStore.exists(tableIdent1, TABLE)); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTable3.auditInfo().creator()); @@ -191,6 +203,10 @@ public void testCreateAndLoadTable() throws IOException { .build(); doReturn(tableEntity).when(entityStore).get(any(), any(), any()); Table loadedTable4 = tableOperationDispatcher.loadTable(tableIdent1); + // Succeed to import the topic entity + reset(entityStore); + TableEntity tableImportedEntity = entityStore.get(tableIdent1, TABLE, TableEntity.class); + Assertions.assertEquals("test", tableImportedEntity.auditInfo().creator()); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTable4.auditInfo().creator()); } diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java index ccf55f1a19d..0371e251d69 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java @@ -17,6 +17,7 @@ import static org.mockito.Mockito.reset; import com.datastrato.gravitino.Config; +import com.datastrato.gravitino.Entity; import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; @@ -52,6 +53,7 @@ public static void initialize() throws IOException { doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + GravitinoEnv.getInstance().setSchemaDispatcher(schemaOperationDispatcher); } @Test @@ -103,15 +105,27 @@ public void testCreateAndLoadTopic() throws IOException { // Case 2: Test if the topic entity is not found in the entity store reset(entityStore); + entityStore.delete(topicIdent1, Entity.EntityType.TOPIC); + entityStore.delete(NameIdentifier.of(topicNs.levels()), Entity.EntityType.SCHEMA); doThrow(new NoSuchEntityException("")).when(entityStore).get(any(), any(), any()); Topic loadedTopic2 = topicOperationDispatcher.loadTopic(topicIdent1); + // Succeed to import the topic entity + Assertions.assertTrue(entityStore.exists(topicIdent1, Entity.EntityType.TOPIC)); + Assertions.assertTrue( + entityStore.exists(NameIdentifier.of(topicNs.levels()), Entity.EntityType.SCHEMA)); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTopic2.auditInfo().creator()); // Case 3: Test if the entity store is failed to get the topic entity reset(entityStore); + entityStore.delete(topicIdent1, Entity.EntityType.TOPIC); + entityStore.delete(NameIdentifier.of(topicNs.levels()), Entity.EntityType.SCHEMA); doThrow(new IOException()).when(entityStore).get(any(), any(), any()); Topic loadedTopic3 = topicOperationDispatcher.loadTopic(topicIdent1); + // Succeed to import the topic entity + Assertions.assertTrue( + entityStore.exists(NameIdentifier.of(topicNs.levels()), Entity.EntityType.SCHEMA)); + Assertions.assertTrue(entityStore.exists(topicIdent1, Entity.EntityType.TOPIC)); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTopic3.auditInfo().creator()); @@ -127,6 +141,11 @@ public void testCreateAndLoadTopic() throws IOException { .build(); doReturn(unmatchedEntity).when(entityStore).get(any(), any(), any()); Topic loadedTopic4 = topicOperationDispatcher.loadTopic(topicIdent1); + // Succeed to import the topic entity + reset(entityStore); + TopicEntity topicEntity = + entityStore.get(topicIdent1, Entity.EntityType.TOPIC, TopicEntity.class); + Assertions.assertEquals("test", topicEntity.auditInfo().creator()); // Audit info is gotten from the catalog, not from the entity store Assertions.assertEquals("test", loadedTopic4.auditInfo().creator()); } diff --git a/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java b/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java index bfc84ee0936..e8575c56341 100644 --- a/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java +++ b/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java @@ -14,9 +14,6 @@ import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; -import com.datastrato.gravitino.catalog.SchemaDispatcher; -import com.datastrato.gravitino.catalog.TableDispatcher; -import com.datastrato.gravitino.catalog.TopicDispatcher; import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; import com.datastrato.gravitino.dto.responses.DeleteResponse; @@ -43,6 +40,7 @@ @Path("/metalakes/{metalake}/roles") public class RoleOperations { private static final Logger LOG = LoggerFactory.getLogger(RoleOperations.class); + private static final String ALL_METALAKES = "*"; private final AccessControlManager accessControlManager; @@ -86,7 +84,7 @@ public Response createRole(@PathParam("metalake") String metalake, RoleCreateReq "The size of securable objects must be 1"); for (SecurableObjectDTO object : request.getSecurableObjects()) { - checkSecurableObjects(metalake, object); + checkSecurableObject(metalake, object); } return Utils.doAs( @@ -146,11 +144,15 @@ public Response deleteRole( } // Check every securable object whether exists and is imported. - private void checkSecurableObjects(String metalake, SecurableObjectDTO object) { + static void checkSecurableObject(String metalake, SecurableObjectDTO object) { NameIdentifier identifier; // Securable object ignores the metalake namespace, so we should add it back. if (object.type() == MetadataObject.Type.METALAKE) { + // All metalakes don't need to check the securable object whether exists. + if (object.name().equals(ALL_METALAKES)) { + return; + } identifier = NameIdentifier.parse(object.fullName()); } else { identifier = NameIdentifier.parse(String.format("%s.%s", metalake, object.fullName())); @@ -169,20 +171,21 @@ private void checkSecurableObjects(String metalake, SecurableObjectDTO object) { } break; + case CATALOG: if (!GravitinoEnv.getInstance().catalogDispatcher().catalogExists(identifier)) { throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); } break; + case SCHEMA: - { - SchemaDispatcher dispatcher = GravitinoEnv.getInstance().schemaDispatcher(); - if (!dispatcher.schemaExists(identifier)) { - throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); - } - break; + if (!GravitinoEnv.getInstance().schemaDispatcher().schemaExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); } + + break; + case FILESET: if (!GravitinoEnv.getInstance().filesetDispatcher().filesetExists(identifier)) { throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); @@ -190,23 +193,19 @@ private void checkSecurableObjects(String metalake, SecurableObjectDTO object) { break; case TABLE: - { - TableDispatcher dispatcher = GravitinoEnv.getInstance().tableDispatcher(); - if (!dispatcher.tableExists(identifier)) { - throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); - } - - break; + if (!GravitinoEnv.getInstance().tableDispatcher().tableExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); } - case TOPIC: - { - TopicDispatcher dispatcher = GravitinoEnv.getInstance().topicDispatcher(); - if (!dispatcher.topicExists(identifier)) { - throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); - } - break; + break; + + case TOPIC: + if (!GravitinoEnv.getInstance().topicDispatcher().topicExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); } + + break; + default: throw new IllegalArgumentException( String.format("Doesn't support the type %s", object.type())); diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java index e8cdced23e6..a677d226383 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java @@ -20,6 +20,10 @@ import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.catalog.CatalogDispatcher; +import com.datastrato.gravitino.catalog.FilesetDispatcher; +import com.datastrato.gravitino.catalog.SchemaDispatcher; +import com.datastrato.gravitino.catalog.TableDispatcher; +import com.datastrato.gravitino.catalog.TopicDispatcher; import com.datastrato.gravitino.dto.authorization.RoleDTO; import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; @@ -34,6 +38,7 @@ import com.datastrato.gravitino.lock.LockManager; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.RoleEntity; +import com.datastrato.gravitino.metalake.MetalakeDispatcher; import com.datastrato.gravitino.rest.RESTUtils; import com.google.common.collect.Lists; import java.io.IOException; @@ -56,7 +61,12 @@ public class TestRoleOperations extends JerseyTest { private static final AccessControlManager manager = mock(AccessControlManager.class); - private static final CatalogDispatcher dispatcher = mock(CatalogDispatcher.class); + 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 final TopicDispatcher topicDispatcher = mock(TopicDispatcher.class); + private static final FilesetDispatcher filesetDispatcher = mock(FilesetDispatcher.class); private static class MockServletRequestFactory extends ServletRequestFactoryBase { @Override @@ -75,7 +85,12 @@ public static void setup() { Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); GravitinoEnv.getInstance().setLockManager(new LockManager(config)); GravitinoEnv.getInstance().setAccessControlManager(manager); - GravitinoEnv.getInstance().setCatalogDispatcher(dispatcher); + GravitinoEnv.getInstance().setMetalakeDispatcher(metalakeDispatcher); + GravitinoEnv.getInstance().setCatalogDispatcher(catalogDispatcher); + GravitinoEnv.getInstance().setSchemaDispatcher(schemaDispatcher); + GravitinoEnv.getInstance().setTableDispatcher(tableDispatcher); + GravitinoEnv.getInstance().setTopicDispatcher(topicDispatcher); + GravitinoEnv.getInstance().setFilesetDispatcher(filesetDispatcher); } @Override @@ -113,7 +128,7 @@ public void testCreateRole() { Role role = buildRole("role1"); when(manager.createRole(any(), any(), any(), any())).thenReturn(role); - when(dispatcher.catalogExists(any())).thenReturn(true); + when(catalogDispatcher.catalogExists(any())).thenReturn(true); Response resp = target("/metalakes/metalake1/roles") @@ -141,7 +156,20 @@ public void testCreateRole() { Privileges.UseCatalog.allow().condition(), roleDTO.securableObjects().get(0).privileges().get(0).condition()); + // Test to a catalog which doesn't exist + when(catalogDispatcher.catalogExists(any())).thenReturn(false); + Response respNotExist = + target("/metalakes/metalake1/roles") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), respNotExist.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, respNotExist.getMediaType()); + ErrorResponse notExistResponse = respNotExist.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, notExistResponse.getCode()); + // Test to throw NoSuchMetalakeException + when(catalogDispatcher.catalogExists(any())).thenReturn(true); doThrow(new NoSuchMetalakeException("mock error")) .when(manager) .createRole(any(), any(), any(), any()); @@ -323,4 +351,85 @@ public void testDeleteRole() { Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse.getType()); } + + @Test + public void testCheckSecurableObjects() { + // check all metalakes + SecurableObject allMetalake = + SecurableObjects.ofAllMetalakes(Lists.newArrayList(Privileges.UseMetalake.allow())); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(allMetalake))); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(false); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(allMetalake))); + + // check the metalake + SecurableObject metalake = + SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.UseMetalake.allow())); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(metalake))); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(metalake))); + + // check the catalog + SecurableObject catalog = + SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.UseCatalog.allow())); + when(catalogDispatcher.catalogExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); + when(catalogDispatcher.catalogExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); + + // check the schema + SecurableObject schema = + SecurableObjects.ofSchema( + catalog, "schema", Lists.newArrayList(Privileges.UseSchema.allow())); + when(schemaDispatcher.schemaExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); + when(schemaDispatcher.schemaExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); + + // check the table + SecurableObject table = + SecurableObjects.ofTable(schema, "table", Lists.newArrayList(Privileges.ReadTable.allow())); + when(tableDispatcher.tableExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(table))); + when(tableDispatcher.tableExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(table))); + + // check the topic + SecurableObject topic = + SecurableObjects.ofTopic(schema, "topic", Lists.newArrayList(Privileges.ReadTopic.allow())); + when(topicDispatcher.topicExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); + when(topicDispatcher.topicExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); + + // check the fileset + SecurableObject fileset = + SecurableObjects.ofFileset( + schema, "fileset", Lists.newArrayList(Privileges.ReadFileset.allow())); + when(filesetDispatcher.filesetExists(any())).thenReturn(true); + Assertions.assertDoesNotThrow( + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); + when(filesetDispatcher.filesetExists(any())).thenReturn(false); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); + } }