diff --git a/common/src/main/java/com/datastrato/gravitino/dto/rel/TableDTO.java b/common/src/main/java/com/datastrato/gravitino/dto/rel/TableDTO.java index 84dd54b9917..2633ce34faf 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/rel/TableDTO.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/rel/TableDTO.java @@ -58,7 +58,7 @@ private TableDTO() {} * @param properties The properties associated with the table. * @param audit The audit information for the table. * @param partitioning The partitioning of the table. - * @param indexes Teh indexes of the table. + * @param indexes The indexes of the table. */ private TableDTO( String name, diff --git a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java index a05bb3662ee..326e75f721a 100644 --- a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java +++ b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java @@ -40,7 +40,6 @@ import com.datastrato.gravitino.metrics.source.JVMMetricsSource; import com.datastrato.gravitino.storage.IdGenerator; import com.datastrato.gravitino.storage.RandomIdGenerator; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -98,40 +97,6 @@ public static GravitinoEnv getInstance() { return InstanceHolder.INSTANCE; } - /** - * This method is used for testing purposes only to set the lock manager for test in package - * `com.datastrato.gravitino.server.web.rest`, as tree lock depends on the lock manager and we did - * not mock the lock manager in the test, so we need to set the lock manager for test. - * - * @param lockManager The lock manager to be set. - */ - @VisibleForTesting - public void setLockManager(LockManager lockManager) { - this.lockManager = lockManager; - } - - /** - * This method is used for testing purposes only to set the access manager for test in package - * `com.datastrato.gravitino.server.web.rest` and `com.datastrato.gravitino.authorization`. - * - * @param accessControlManager The access control manager to be set. - */ - @VisibleForTesting - public void setAccessControlManager(AccessControlManager accessControlManager) { - this.accessControlManager = accessControlManager; - } - - /** - * This method is used for testing purposes only to set the entity store for test in package - * `com.datastrato.gravitino.authorization`. - * - * @param entityStore The entity store to be set. - */ - @VisibleForTesting - public void setEntityStore(EntityStore entityStore) { - this.entityStore = entityStore; - } - /** * Initialize the Gravitino environment. * diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedSchema.java b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedSchema.java index 02e45e6eb81..831daf6a5ac 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedSchema.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedSchema.java @@ -6,6 +6,7 @@ import com.datastrato.gravitino.Audit; import com.datastrato.gravitino.Schema; +import com.datastrato.gravitino.StringIdentifier; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.SchemaEntity; import java.util.Map; @@ -19,14 +20,23 @@ public final class EntityCombinedSchema implements Schema { private final Schema schema; + private final SchemaEntity schemaEntity; // Sets of properties that should be hidden from the user. private Set hiddenProperties; + // Field "imported" is used to indicate whether the entity has been imported to Gravitino + // managed storage backend. If "imported" is true, it means that storage backend have stored + // the correct entity. Otherwise, we should import the external entity to the storage backend. + // This is used for tag/access control related purposes, only the imported entities have the + // unique id, and based on this id, we can label and control the access to the entities. + private boolean imported; + private EntityCombinedSchema(Schema schema, SchemaEntity schemaEntity) { this.schema = schema; this.schemaEntity = schemaEntity; + this.imported = false; } public static EntityCombinedSchema of(Schema schema, SchemaEntity schemaEntity) { @@ -42,6 +52,11 @@ public EntityCombinedSchema withHiddenPropertiesSet(Set hiddenProperties return this; } + public EntityCombinedSchema withImported(boolean imported) { + this.imported = imported; + return this; + } + @Override public String name() { return schema.name(); @@ -73,4 +88,12 @@ public Audit auditInfo() { ? schema.auditInfo() : mergedAudit.merge(schemaEntity.auditInfo(), true /* overwrite */); } + + public boolean imported() { + return imported; + } + + StringIdentifier stringIdentifier() { + return StringIdentifier.fromProperties(schema.properties()); + } } diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTable.java b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTable.java index 593508f9e6e..3edfe491390 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTable.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTable.java @@ -5,6 +5,7 @@ package com.datastrato.gravitino.catalog; import com.datastrato.gravitino.Audit; +import com.datastrato.gravitino.StringIdentifier; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.TableEntity; import com.datastrato.gravitino.rel.Column; @@ -31,9 +32,17 @@ public final class EntityCombinedTable implements Table { // Sets of properties that should be hidden from the user. private Set hiddenProperties; + // Field "imported" is used to indicate whether the entity has been imported to Gravitino + // managed storage backend. If "imported" is true, it means that storage backend have stored + // the correct entity. Otherwise, we should import the external entity to the storage backend. + // This is used for tag/access control related purposes, only the imported entities have the + // unique id, and based on this id, we can label and control the access to the entities. + private boolean imported; + private EntityCombinedTable(Table table, TableEntity tableEntity) { this.table = table; this.tableEntity = tableEntity; + this.imported = false; } public static EntityCombinedTable of(Table table, TableEntity tableEntity) { @@ -49,6 +58,11 @@ public EntityCombinedTable withHiddenPropertiesSet(Set hiddenProperties) return this; } + public EntityCombinedTable withImported(boolean imported) { + this.imported = imported; + return this; + } + @Override public String name() { return table.name(); @@ -96,6 +110,10 @@ public Index[] index() { return table.index(); } + public boolean imported() { + return imported; + } + @Override public Audit auditInfo() { AuditInfo mergedAudit = @@ -110,4 +128,8 @@ public Audit auditInfo() { ? table.auditInfo() : mergedAudit.merge(tableEntity.auditInfo(), true /* overwrite */); } + + StringIdentifier stringIdentifier() { + return StringIdentifier.fromProperties(table.properties()); + } } diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTopic.java b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTopic.java index b6fed19ec32..bf24cb74792 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTopic.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/EntityCombinedTopic.java @@ -5,6 +5,7 @@ package com.datastrato.gravitino.catalog; import com.datastrato.gravitino.Audit; +import com.datastrato.gravitino.StringIdentifier; import com.datastrato.gravitino.messaging.Topic; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.TopicEntity; @@ -24,9 +25,17 @@ public class EntityCombinedTopic implements Topic { // Sets of properties that should be hidden from the user. private Set hiddenProperties; + // Field "imported" is used to indicate whether the entity has been imported to Gravitino + // managed storage backend. If "imported" is true, it means that storage backend have stored + // the correct entity. Otherwise, we should import the external entity to the storage backend. + // This is used for tag/access control related purposes, only the imported entities have the + // unique id, and based on this id, we can label and control the access to the entities. + private boolean imported; + private EntityCombinedTopic(Topic topic, TopicEntity topicEntity) { this.topic = topic; this.topicEntity = topicEntity; + this.imported = false; } public static EntityCombinedTopic of(Topic topic, TopicEntity topicEntity) { @@ -42,6 +51,11 @@ public EntityCombinedTopic withHiddenPropertiesSet(Set hiddenProperties) return this; } + public EntityCombinedTopic withImported(boolean imported) { + this.imported = imported; + return this; + } + @Override public String name() { return topic.name(); @@ -73,4 +87,12 @@ public Audit auditInfo() { ? topic.auditInfo() : mergedAudit.merge(topicEntity.auditInfo(), true /* overwrite */); } + + public boolean imported() { + return imported; + } + + StringIdentifier stringIdentifier() { + return StringIdentifier.fromProperties(topic.properties()); + } } diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/OperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/OperationDispatcher.java index a24a258d398..4272b5c24f9 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/OperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/OperationDispatcher.java @@ -6,6 +6,7 @@ import static com.datastrato.gravitino.catalog.PropertiesMetadataHelpers.validatePropertyForAlter; +import com.datastrato.gravitino.Entity; import com.datastrato.gravitino.EntityStore; import com.datastrato.gravitino.HasIdentifier; import com.datastrato.gravitino.NameIdentifier; @@ -44,7 +45,7 @@ public abstract class OperationDispatcher { protected final EntityStore store; - final IdGenerator idGenerator; + protected final IdGenerator idGenerator; /** * Creates a new CatalogOperationDispatcher instance. @@ -60,7 +61,21 @@ public OperationDispatcher( this.idGenerator = idGenerator; } - R doWithTable( + protected Capability getCatalogCapability(NameIdentifier ident) { + return doWithCatalog( + getCatalogIdentifier(ident), + CatalogManager.CatalogWrapper::capabilities, + IllegalArgumentException.class); + } + + protected Capability getCatalogCapability(Namespace namespace) { + return doWithCatalog( + getCatalogIdentifier(NameIdentifier.of(namespace.levels())), + CatalogManager.CatalogWrapper::capabilities, + IllegalArgumentException.class); + } + + protected R doWithTable( NameIdentifier tableIdent, ThrowableFunction fn, Class ex) throws E { try { @@ -78,7 +93,7 @@ R doWithTable( } } - R doWithCatalog( + protected R doWithCatalog( NameIdentifier ident, ThrowableFunction fn, Class ex) throws E { try { @@ -95,7 +110,7 @@ R doWithCatalog( } } - R doWithCatalog( + protected R doWithCatalog( NameIdentifier ident, ThrowableFunction fn, Class ex1, @@ -118,21 +133,7 @@ R doWithCatalog( } } - Capability getCatalogCapability(NameIdentifier ident) { - return doWithCatalog( - getCatalogIdentifier(ident), - CatalogManager.CatalogWrapper::capabilities, - IllegalArgumentException.class); - } - - Capability getCatalogCapability(Namespace namespace) { - return doWithCatalog( - getCatalogIdentifier(NameIdentifier.of(namespace.levels())), - CatalogManager.CatalogWrapper::capabilities, - IllegalArgumentException.class); - } - - Set getHiddenPropertyNames( + protected Set getHiddenPropertyNames( NameIdentifier catalogIdent, ThrowableFunction provider, Map properties) { @@ -149,7 +150,7 @@ Set getHiddenPropertyNames( IllegalArgumentException.class); } - void validateAlterProperties( + protected void validateAlterProperties( NameIdentifier ident, ThrowableFunction provider, T... changes) { @@ -166,27 +167,6 @@ void validateAlterProperties( IllegalArgumentException.class); } - private Map getPropertiesForSet(T... t) { - Map properties = Maps.newHashMap(); - for (T item : t) { - if (item instanceof TableChange.SetProperty) { - TableChange.SetProperty setProperty = (TableChange.SetProperty) item; - properties.put(setProperty.getProperty(), setProperty.getValue()); - } else if (item instanceof SchemaChange.SetProperty) { - SchemaChange.SetProperty setProperty = (SchemaChange.SetProperty) item; - properties.put(setProperty.getProperty(), setProperty.getValue()); - } else if (item instanceof FilesetChange.SetProperty) { - FilesetChange.SetProperty setProperty = (FilesetChange.SetProperty) item; - properties.put(setProperty.getProperty(), setProperty.getValue()); - } else if (item instanceof TopicChange.SetProperty) { - TopicChange.SetProperty setProperty = (TopicChange.SetProperty) item; - properties.put(setProperty.getProperty(), setProperty.getValue()); - } - } - - return properties; - } - private Map getPropertiesForDelete(T... t) { Map properties = Maps.newHashMap(); for (T item : t) { @@ -208,7 +188,7 @@ private Map getPropertiesForDelete(T... t) { return properties; } - StringIdentifier getStringIdFromProperties(Map properties) { + protected StringIdentifier getStringIdFromProperties(Map properties) { try { StringIdentifier stringId = StringIdentifier.fromProperties(properties); if (stringId == null) { @@ -221,7 +201,7 @@ StringIdentifier getStringIdFromProperties(Map properties) { } } - R operateOnEntity( + protected R operateOnEntity( NameIdentifier ident, ThrowableFunction fn, String opName, long id) { R ret = null; try { @@ -247,7 +227,7 @@ R operateOnEntity( // TODO(xun): Remove this method when we implement a better way to get the catalog identifier // [#257] Add an explicit get catalog functions in NameIdentifier - NameIdentifier getCatalogIdentifier(NameIdentifier ident) { + protected NameIdentifier getCatalogIdentifier(NameIdentifier ident) { NameIdentifier.check( ident.name() != null, "The name variable in the NameIdentifier must have value."); Namespace.check( @@ -272,6 +252,36 @@ boolean isManagedEntity(NameIdentifier catalogIdent, Capability.Scope scope) { IllegalArgumentException.class); } + protected boolean isEntityExist(NameIdentifier ident, Entity.EntityType type) { + try { + return store.exists(ident, type); + } catch (Exception e) { + LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "exists", ident, e); + throw new RuntimeException("Fail to check if entity is existed", e); + } + } + + private Map getPropertiesForSet(T... t) { + Map properties = Maps.newHashMap(); + for (T item : t) { + if (item instanceof TableChange.SetProperty) { + TableChange.SetProperty setProperty = (TableChange.SetProperty) item; + properties.put(setProperty.getProperty(), setProperty.getValue()); + } else if (item instanceof SchemaChange.SetProperty) { + SchemaChange.SetProperty setProperty = (SchemaChange.SetProperty) item; + properties.put(setProperty.getProperty(), setProperty.getValue()); + } else if (item instanceof FilesetChange.SetProperty) { + FilesetChange.SetProperty setProperty = (FilesetChange.SetProperty) item; + properties.put(setProperty.getProperty(), setProperty.getValue()); + } else if (item instanceof TopicChange.SetProperty) { + TopicChange.SetProperty setProperty = (TopicChange.SetProperty) item; + properties.put(setProperty.getProperty(), setProperty.getValue()); + } + } + + return properties; + } + static final class FormattedErrorMessages { static final String STORE_OP_FAILURE = "Failed to {} entity for {} in " 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 1575ded111d..c22cf5cef09 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java @@ -20,6 +20,8 @@ import com.datastrato.gravitino.exceptions.NoSuchSchemaException; import com.datastrato.gravitino.exceptions.NonEmptySchemaException; import com.datastrato.gravitino.exceptions.SchemaAlreadyExistsException; +import com.datastrato.gravitino.lock.LockType; +import com.datastrato.gravitino.lock.TreeLockUtils; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.SchemaEntity; import com.datastrato.gravitino.storage.IdGenerator; @@ -159,47 +161,21 @@ public Schema createSchema(NameIdentifier ident, String comment, Map c.doWithSchemaOps(s -> s.loadSchema(ident)), - NoSuchSchemaException.class); - - // If the Schema is maintained by the Gravitino's store, we don't have to load again. - boolean isManagedSchema = isManagedEntity(catalogIdentifier, Capability.Scope.SCHEMA); - if (isManagedSchema) { - return EntityCombinedSchema.of(schema) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdentifier, - HasPropertyMetadata::schemaPropertiesMetadata, - schema.properties())); - } + // Load the schema and check if this schema is already imported. + EntityCombinedSchema schema = + TreeLockUtils.doWithTreeLock(ident, LockType.READ, () -> internalLoadSchema(ident)); - StringIdentifier stringId = getStringIdFromProperties(schema.properties()); - // Case 1: The schema is not created by Gravitino. - if (stringId == null) { - return EntityCombinedSchema.of(schema) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdentifier, - HasPropertyMetadata::schemaPropertiesMetadata, - schema.properties())); + if (!schema.imported()) { + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), + LockType.WRITE, + () -> { + importSchema(ident); + return null; + }); } - SchemaEntity schemaEntity = - operateOnEntity( - ident, - identifier -> store.get(identifier, SCHEMA, SchemaEntity.class), - "GET", - stringId.id()); - return EntityCombinedSchema.of(schema, schemaEntity) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdentifier, - HasPropertyMetadata::schemaPropertiesMetadata, - schema.properties())); + return schema; } /** @@ -280,6 +256,7 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) .build()), "UPDATE", stringId.id()); + return EntityCombinedSchema.of(alteredSchema, updatedSchemaEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( @@ -330,4 +307,106 @@ public boolean dropSchema(NameIdentifier ident, boolean cascade) throws NonEmpty ? droppedFromStore : droppedFromCatalog; } + + private void importSchema(NameIdentifier identifier) { + EntityCombinedSchema schema = internalLoadSchema(identifier); + if (schema.imported()) { + return; + } + + StringIdentifier stringId = null; + try { + stringId = schema.stringIdentifier(); + } catch (IllegalArgumentException ie) { + LOG.warn(FormattedErrorMessages.STRING_ID_PARSE_ERROR, ie.getMessage()); + } + + long uid; + if (stringId != null) { + // If the entity in the store doesn't match the one in the external system, we use the data + // of external system to correct it. + LOG.warn( + "The Schema uid {} existed but still needs to be imported, this could be happened " + + "when Schema is renamed by external systems not controlled by Gravitino. In this case, " + + "we need to overwrite the stored entity to keep consistency.", + stringId); + uid = stringId.id(); + } else { + // If entity doesn't exist, we import the entity from the external system. + uid = idGenerator.nextId(); + } + + SchemaEntity schemaEntity = + SchemaEntity.builder() + .withId(uid) + .withName(identifier.name()) + .withNamespace(identifier.namespace()) + .withAuditInfo( + AuditInfo.builder() + .withCreator(schema.auditInfo().creator()) + .withCreateTime(schema.auditInfo().createTime()) + .withLastModifier(schema.auditInfo().lastModifier()) + .withLastModifiedTime(schema.auditInfo().lastModifiedTime()) + .build()) + .build(); + try { + store.put(schemaEntity, true); + } catch (Exception e) { + LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "put", identifier, e); + throw new RuntimeException("Fail to import schema entity to the store.", e); + } + } + + private EntityCombinedSchema internalLoadSchema(NameIdentifier ident) { + NameIdentifier catalogIdentifier = getCatalogIdentifier(ident); + Schema schema = + doWithCatalog( + catalogIdentifier, + c -> c.doWithSchemaOps(s -> s.loadSchema(ident)), + NoSuchSchemaException.class); + + // If the Schema is maintained by the entity store, we don't have to import. + boolean isManagedSchema = isManagedEntity(catalogIdentifier, Capability.Scope.SCHEMA); + if (isManagedSchema) { + return EntityCombinedSchema.of(schema) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdentifier, + HasPropertyMetadata::schemaPropertiesMetadata, + schema.properties())) + // The meta of managed schema is stored by Gravitino, we don't need to import it. + .withImported(true /* imported */); + } + + StringIdentifier stringId = getStringIdFromProperties(schema.properties()); + // Case 1: The schema is not created by Gravitino or the external system does not support + // storing string identifiers. + if (stringId == null) { + return EntityCombinedSchema.of(schema) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdentifier, + HasPropertyMetadata::schemaPropertiesMetadata, + schema.properties())) + // For some catalogs like PG, the identifier information is not stored in the schema's + // metadata, we need to check if this schema is existed in the store, if so we don't + // need to import. + .withImported(isEntityExist(ident, SCHEMA)); + } + + SchemaEntity schemaEntity = + operateOnEntity( + ident, + identifier -> store.get(identifier, SCHEMA, SchemaEntity.class), + "GET", + stringId.id()); + + return EntityCombinedSchema.of(schema, schemaEntity) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdentifier, + HasPropertyMetadata::schemaPropertiesMetadata, + schema.properties())) + .withImported(schemaEntity != null); + } } 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 df41cdc2634..fd1cafa3869 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; @@ -19,6 +20,8 @@ import com.datastrato.gravitino.exceptions.NoSuchSchemaException; import com.datastrato.gravitino.exceptions.NoSuchTableException; import com.datastrato.gravitino.exceptions.TableAlreadyExistsException; +import com.datastrato.gravitino.lock.LockType; +import com.datastrato.gravitino.lock.TreeLockUtils; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.TableEntity; import com.datastrato.gravitino.rel.Column; @@ -79,37 +82,26 @@ public NameIdentifier[] listTables(Namespace namespace) throws NoSuchSchemaExcep */ @Override public Table loadTable(NameIdentifier ident) throws NoSuchTableException { - NameIdentifier catalogIdentifier = getCatalogIdentifier(ident); - Table table = - doWithCatalog( - catalogIdentifier, - c -> c.doWithTableOps(t -> t.loadTable(ident)), - NoSuchTableException.class); + EntityCombinedTable table = + TreeLockUtils.doWithTreeLock(ident, LockType.READ, () -> internalLoadTable(ident)); - StringIdentifier stringId = getStringIdFromProperties(table.properties()); - // Case 1: The table is not created by Gravitino. - if (stringId == null) { - return EntityCombinedTable.of(table) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdentifier, - HasPropertyMetadata::tablePropertiesMetadata, - table.properties())); - } + if (!table.imported()) { + // Load the schema to make sure the schema is imported. + SchemaDispatcher schemaDispatcher = GravitinoEnv.getInstance().schemaDispatcher(); + NameIdentifier schemaIdent = NameIdentifier.of(ident.namespace().levels()); + schemaDispatcher.loadSchema(schemaIdent); - TableEntity tableEntity = - operateOnEntity( - ident, - identifier -> store.get(identifier, TABLE, TableEntity.class), - "GET", - stringId.id()); + // Import the table. + TreeLockUtils.doWithTreeLock( + schemaIdent, + LockType.WRITE, + () -> { + importTable(ident); + return null; + }); + } - return EntityCombinedTable.of(table, tableEntity) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdentifier, - HasPropertyMetadata::tablePropertiesMetadata, - table.properties())); + return table; } /** @@ -379,4 +371,94 @@ public boolean purgeTable(NameIdentifier ident) throws UnsupportedOperationExcep ? droppedFromStore : droppedFromCatalog; } + + private void importTable(NameIdentifier identifier) { + EntityCombinedTable table = internalLoadTable(identifier); + + if (table.imported()) { + return; + } + + StringIdentifier stringId = null; + try { + stringId = table.stringIdentifier(); + } catch (IllegalArgumentException ie) { + LOG.warn(FormattedErrorMessages.STRING_ID_PARSE_ERROR, ie.getMessage()); + } + + long uid; + if (stringId != null) { + // If the entity in the store doesn't match the external system, we use the data + // of external system to correct it. + LOG.warn( + "The Table uid {} existed but still need to be imported, this could be happened " + + "when Table is renamed by external systems not controlled by Gravitino. In this case, " + + "we need to overwrite the stored entity to keep the consistency.", + stringId); + uid = stringId.id(); + } else { + // If entity doesn't exist, we import the entity from the external system. + uid = idGenerator.nextId(); + } + + TableEntity tableEntity = + TableEntity.builder() + .withId(uid) + .withName(identifier.name()) + .withNamespace(identifier.namespace()) + .withAuditInfo( + AuditInfo.builder() + .withCreator(table.auditInfo().creator()) + .withCreateTime(table.auditInfo().createTime()) + .withLastModifier(table.auditInfo().lastModifier()) + .withLastModifiedTime(table.auditInfo().lastModifiedTime()) + .build()) + .build(); + try { + store.put(tableEntity, true); + } catch (Exception e) { + LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "put", identifier, e); + throw new RuntimeException("Fail to import the table entity to the store.", e); + } + } + + private EntityCombinedTable internalLoadTable(NameIdentifier ident) { + NameIdentifier catalogIdentifier = getCatalogIdentifier(ident); + Table table = + doWithCatalog( + catalogIdentifier, + c -> c.doWithTableOps(t -> t.loadTable(ident)), + NoSuchTableException.class); + + StringIdentifier stringId = getStringIdFromProperties(table.properties()); + // Case 1: The table is not created by Gravitino or the external system does not support storing + // string identifier. + if (stringId == null) { + return EntityCombinedTable.of(table) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdentifier, + HasPropertyMetadata::tablePropertiesMetadata, + table.properties())) + // Some tables don't have properties or are not created by Gravitino, + // we can't use stringIdentifier to judge whether schema is ever imported or not. + // We need to check whether the entity exists. + .withImported(isEntityExist(ident, TABLE)); + } + + TableEntity tableEntity = + operateOnEntity( + ident, + identifier -> store.get(identifier, TABLE, TableEntity.class), + "GET", + stringId.id()); + + return EntityCombinedTable.of(table, tableEntity) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdentifier, + HasPropertyMetadata::tablePropertiesMetadata, + table.properties())) + .withImported(tableEntity != null); + } } 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 6d52e92d75a..d8d31425f23 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; @@ -18,6 +19,8 @@ import com.datastrato.gravitino.exceptions.NoSuchSchemaException; import com.datastrato.gravitino.exceptions.NoSuchTopicException; import com.datastrato.gravitino.exceptions.TopicAlreadyExistsException; +import com.datastrato.gravitino.lock.LockType; +import com.datastrato.gravitino.lock.TreeLockUtils; import com.datastrato.gravitino.messaging.DataLayout; import com.datastrato.gravitino.messaging.Topic; import com.datastrato.gravitino.messaging.TopicChange; @@ -70,36 +73,27 @@ public NameIdentifier[] listTopics(Namespace namespace) throws NoSuchSchemaExcep */ @Override public Topic loadTopic(NameIdentifier ident) throws NoSuchTopicException { - NameIdentifier catalogIdent = getCatalogIdentifier(ident); - Topic topic = - doWithCatalog( - catalogIdent, - c -> c.doWithTopicOps(t -> t.loadTopic(ident)), - NoSuchTopicException.class); + EntityCombinedTopic topic = + TreeLockUtils.doWithTreeLock(ident, LockType.READ, () -> internalLoadTopic(ident)); - StringIdentifier stringId = getStringIdFromProperties(topic.properties()); - // Case 1: The topic is not created by Gravitino. - // Note: for Kafka catalog, stringId will not be null. Because there is no way to store the - // Gravitino - // ID in Kafka, therefor we use the topic ID as the Gravitino ID - if (stringId == null) { - return EntityCombinedTopic.of(topic) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, topic.properties())); - } + if (!topic.imported()) { + // Load the schema to make sure the schema is imported. + // This is not necessary for Kafka catalog. + SchemaDispatcher schemaDispatcher = GravitinoEnv.getInstance().schemaDispatcher(); + NameIdentifier schemaIdent = NameIdentifier.of(ident.namespace().levels()); + schemaDispatcher.loadSchema(schemaIdent); - TopicEntity topicEntity = - operateOnEntity( - ident, - identifier -> store.get(identifier, TOPIC, TopicEntity.class), - "GET", - getStringIdFromProperties(topic.properties()).id()); + // Import the topic + TreeLockUtils.doWithTreeLock( + schemaIdent, + LockType.WRITE, + () -> { + importTopic(ident); + return null; + }); + } - return EntityCombinedTopic.of(topic, topicEntity) - .withHiddenPropertiesSet( - getHiddenPropertyNames( - catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, topic.properties())); + return topic; } /** @@ -282,4 +276,87 @@ public boolean dropTopic(NameIdentifier ident) { ? droppedFromStore : droppedFromCatalog; } + + private void importTopic(NameIdentifier identifier) { + + EntityCombinedTopic topic = internalLoadTopic(identifier); + + if (topic.imported()) { + return; + } + + StringIdentifier stringId = null; + try { + stringId = topic.stringIdentifier(); + } catch (IllegalArgumentException ie) { + LOG.warn(FormattedErrorMessages.STRING_ID_PARSE_ERROR, ie.getMessage()); + } + + long uid; + if (stringId != null) { + // For Kafka topic, the uid is coming from topic UUID, which is always existed. + LOG.warn( + "The Topic uid {} existed but still needs to be imported, this could be happened " + + "when Topic is created externally without leveraging Gravitino. In this " + + "case, we need to store the stored entity to keep consistency.", + stringId); + uid = stringId.id(); + } else { + // This will not be happened for now, since we only support Kafka, and it always has an uid. + uid = idGenerator.nextId(); + } + + TopicEntity topicEntity = + TopicEntity.builder() + .withId(uid) + .withName(topic.name()) + .withComment(topic.comment()) + .withNamespace(identifier.namespace()) + .withAuditInfo( + AuditInfo.builder() + .withCreator(topic.auditInfo().creator()) + .withCreateTime(topic.auditInfo().createTime()) + .withLastModifier(topic.auditInfo().lastModifier()) + .withLastModifiedTime(topic.auditInfo().lastModifiedTime()) + .build()) + .build(); + + try { + store.put(topicEntity, true); + } catch (Exception e) { + LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "put", identifier, e); + throw new RuntimeException("Fail to import topic entity to store.", e); + } + } + + private EntityCombinedTopic internalLoadTopic(NameIdentifier ident) { + NameIdentifier catalogIdent = getCatalogIdentifier(ident); + Topic topic = + doWithCatalog( + catalogIdent, + c -> c.doWithTopicOps(t -> t.loadTopic(ident)), + NoSuchTopicException.class); + + StringIdentifier stringId = getStringIdFromProperties(topic.properties()); + if (stringId == null) { + return EntityCombinedTopic.of(topic) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, topic.properties())) + .withImported(isEntityExist(ident, TOPIC)); + } + + TopicEntity topicEntity = + operateOnEntity( + ident, + identifier -> store.get(identifier, TOPIC, TopicEntity.class), + "GET", + getStringIdFromProperties(topic.properties()).id()); + + return EntityCombinedTopic.of(topic, topicEntity) + .withHiddenPropertiesSet( + getHiddenPropertyNames( + catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, topic.properties())) + .withImported(topicEntity != null); + } } diff --git a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java index a78ea934d1b..c7f5b6058dc 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.time.Instant; import java.util.Map; +import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -63,8 +64,9 @@ public static void setUp() throws Exception { entityStore.put(metalakeEntity, true); accessControlManager = new AccessControlManager(entityStore, new RandomIdGenerator(), config); - GravitinoEnv.getInstance().setEntityStore(entityStore); - GravitinoEnv.getInstance().setAccessControlManager(accessControlManager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "entityStore", entityStore, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "accessControlManager", accessControlManager, true); } @AfterAll diff --git a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java index f67437dfe6d..0a90ecf0fa8 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.time.Instant; import java.util.List; +import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -108,8 +109,9 @@ public static void setUp() throws Exception { accessControlManager = new AccessControlManager(entityStore, new RandomIdGenerator(), config); - GravitinoEnv.getInstance().setEntityStore(entityStore); - GravitinoEnv.getInstance().setAccessControlManager(accessControlManager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "entityStore", entityStore, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "accessControlManager", accessControlManager, true); } @AfterAll diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaNormalizeDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaNormalizeDispatcher.java index 44a8c21f3ad..c696e872d8c 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaNormalizeDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaNormalizeDispatcher.java @@ -21,7 +21,7 @@ public class TestSchemaNormalizeDispatcher extends TestOperationDispatcher { private static SchemaNormalizeDispatcher schemaNormalizeDispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { TestSchemaOperationDispatcher.initialize(); schemaNormalizeDispatcher = new SchemaNormalizeDispatcher(TestSchemaOperationDispatcher.dispatcher); 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 c04b8e1a72b..0517c6195f3 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestSchemaOperationDispatcher.java @@ -11,14 +11,20 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +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; import com.datastrato.gravitino.Schema; import com.datastrato.gravitino.SchemaChange; import com.datastrato.gravitino.auth.AuthConstants; import com.datastrato.gravitino.exceptions.NoSuchEntityException; +import com.datastrato.gravitino.lock.LockManager; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.SchemaEntity; import com.google.common.collect.ImmutableMap; @@ -28,6 +34,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -37,8 +44,14 @@ public class TestSchemaOperationDispatcher extends TestOperationDispatcher { static SchemaOperationDispatcher dispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { dispatcher = new SchemaOperationDispatcher(catalogManager, entityStore, idGenerator); + + Config config = mock(Config.class); + doReturn(100000L).when(config).get(Configs.TREE_LOCK_MAX_NODE_IN_MEMORY); + doReturn(1000L).when(config).get(Configs.TREE_LOCK_MIN_NODE_IN_MEMORY); + doReturn(36000L).when(config).get(Configs.TREE_LOCK_CLEAN_INTERVAL); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Test @@ -107,6 +120,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/TestTableNormalizeDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableNormalizeDispatcher.java index 3f369df1413..fd6f0f2bb56 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableNormalizeDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableNormalizeDispatcher.java @@ -40,7 +40,7 @@ public class TestTableNormalizeDispatcher extends TestOperationDispatcher { private static SchemaNormalizeDispatcher schemaNormalizeDispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { TestTableOperationDispatcher.initialize(); tableNormalizeDispatcher = new TableNormalizeDispatcher(TestTableOperationDispatcher.tableOperationDispatcher); 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 83cee605232..f2d9678227d 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java @@ -4,6 +4,10 @@ */ package com.datastrato.gravitino.catalog; +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; @@ -11,13 +15,17 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +import com.datastrato.gravitino.Config; +import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.TestColumn; import com.datastrato.gravitino.auth.AuthConstants; import com.datastrato.gravitino.exceptions.NoSuchEntityException; +import com.datastrato.gravitino.lock.LockManager; import com.datastrato.gravitino.meta.AuditInfo; import com.datastrato.gravitino.meta.TableEntity; import com.datastrato.gravitino.rel.Column; @@ -32,6 +40,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -41,11 +50,19 @@ public class TestTableOperationDispatcher extends TestOperationDispatcher { static SchemaOperationDispatcher schemaOperationDispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { schemaOperationDispatcher = new SchemaOperationDispatcher(catalogManager, entityStore, idGenerator); tableOperationDispatcher = new TableOperationDispatcher(catalogManager, entityStore, idGenerator); + + Config config = mock(Config.class); + doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); + doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); + doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "schemaDispatcher", schemaOperationDispatcher, true); } @Test @@ -154,15 +171,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()); @@ -178,6 +205,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/TestTopicNormalizeDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicNormalizeDispatcher.java index 3772876d81a..d15c18a9ebf 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicNormalizeDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicNormalizeDispatcher.java @@ -22,7 +22,7 @@ public class TestTopicNormalizeDispatcher extends TestOperationDispatcher { private static SchemaNormalizeDispatcher schemaNormalizeDispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { TestTopicOperationDispatcher.initialize(); schemaNormalizeDispatcher = new SchemaNormalizeDispatcher(TestTopicOperationDispatcher.schemaOperationDispatcher); 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 fd06db7614a..4c5d00410b5 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java @@ -4,18 +4,26 @@ */ package com.datastrato.gravitino.catalog; +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.StringIdentifier.ID_KEY; import static com.datastrato.gravitino.TestBasePropertiesMetadata.COMMENT_KEY; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; 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; import com.datastrato.gravitino.auth.AuthConstants; import com.datastrato.gravitino.exceptions.NoSuchEntityException; +import com.datastrato.gravitino.lock.LockManager; import com.datastrato.gravitino.messaging.Topic; import com.datastrato.gravitino.messaging.TopicChange; import com.datastrato.gravitino.meta.AuditInfo; @@ -24,6 +32,7 @@ import java.io.IOException; import java.time.Instant; import java.util.Map; +import org.apache.commons.lang3.reflect.FieldUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -34,11 +43,19 @@ public class TestTopicOperationDispatcher extends TestOperationDispatcher { static TopicOperationDispatcher topicOperationDispatcher; @BeforeAll - public static void initialize() throws IOException { + public static void initialize() throws IOException, IllegalAccessException { schemaOperationDispatcher = new SchemaOperationDispatcher(catalogManager, entityStore, idGenerator); topicOperationDispatcher = new TopicOperationDispatcher(catalogManager, entityStore, idGenerator); + + Config config = mock(Config.class); + doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); + doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); + doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "schemaDispatcher", schemaOperationDispatcher, true); } @Test @@ -90,15 +107,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()); @@ -114,6 +143,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 3d455b041ea..39d35b5d1d3 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 @@ -6,16 +6,22 @@ import com.codahale.metrics.annotation.ResponseMetered; import com.codahale.metrics.annotation.Timed; +import com.datastrato.gravitino.Entity; import com.datastrato.gravitino.GravitinoEnv; +import com.datastrato.gravitino.MetadataObject; +import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.authorization.AccessControlManager; import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; +import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; import com.datastrato.gravitino.dto.responses.DeleteResponse; import com.datastrato.gravitino.dto.responses.RoleResponse; import com.datastrato.gravitino.dto.util.DTOConverters; +import com.datastrato.gravitino.lock.LockType; +import com.datastrato.gravitino.lock.TreeLockUtils; import com.datastrato.gravitino.metrics.MetricNames; import com.datastrato.gravitino.server.web.Utils; import java.util.Arrays; @@ -69,6 +75,11 @@ public Response getRole(@PathParam("metalake") String metalake, @PathParam("role @ResponseMetered(name = "create-role", absolute = true) public Response createRole(@PathParam("metalake") String metalake, RoleCreateRequest request) { try { + + for (SecurableObjectDTO object : request.getSecurableObjects()) { + checkSecurableObject(metalake, object); + } + return Utils.doAs( httpRequest, () -> { @@ -130,4 +141,76 @@ public Response deleteRole( return ExceptionHandlers.handleRoleException(OperationType.DELETE, role, metalake, e); } } + + // Check every securable object whether exists and is imported. + 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(Entity.SECURABLE_ENTITY_RESERVED_NAME)) { + return; + } + identifier = NameIdentifier.parse(object.fullName()); + } else { + identifier = NameIdentifier.parse(String.format("%s.%s", metalake, object.fullName())); + } + + String existErrMsg = "Securable object % doesn't exist"; + + TreeLockUtils.doWithTreeLock( + identifier, + LockType.READ, + () -> { + switch (object.type()) { + case METALAKE: + if (!GravitinoEnv.getInstance().metalakeDispatcher().metalakeExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); + } + + break; + + case CATALOG: + if (!GravitinoEnv.getInstance().catalogDispatcher().catalogExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); + } + + break; + + case SCHEMA: + 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())); + } + + break; + case TABLE: + if (!GravitinoEnv.getInstance().tableDispatcher().tableExists(identifier)) { + throw new IllegalArgumentException(String.format(existErrMsg, object.fullName())); + } + + 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())); + } + + return null; + }); + } } diff --git a/server/src/main/java/com/datastrato/gravitino/server/web/rest/SchemaOperations.java b/server/src/main/java/com/datastrato/gravitino/server/web/rest/SchemaOperations.java index 4bdcb1c09c3..33531102b7c 100644 --- a/server/src/main/java/com/datastrato/gravitino/server/web/rest/SchemaOperations.java +++ b/server/src/main/java/com/datastrato/gravitino/server/web/rest/SchemaOperations.java @@ -133,9 +133,7 @@ public Response loadSchema( httpRequest, () -> { NameIdentifier ident = NameIdentifierUtil.ofSchema(metalake, catalog, schema); - Schema s = - TreeLockUtils.doWithTreeLock( - ident, LockType.READ, () -> dispatcher.loadSchema(ident)); + Schema s = dispatcher.loadSchema(ident); Response response = Utils.ok(new SchemaResponse(DTOConverters.toDTO(s))); LOG.info("Schema loaded: {}.{}.{}", metalake, catalog, s.name()); return response; diff --git a/server/src/main/java/com/datastrato/gravitino/server/web/rest/TableOperations.java b/server/src/main/java/com/datastrato/gravitino/server/web/rest/TableOperations.java index b0093fd2b9d..42630da8c36 100644 --- a/server/src/main/java/com/datastrato/gravitino/server/web/rest/TableOperations.java +++ b/server/src/main/java/com/datastrato/gravitino/server/web/rest/TableOperations.java @@ -148,9 +148,7 @@ public Response loadTable( httpRequest, () -> { NameIdentifier ident = NameIdentifierUtil.ofTable(metalake, catalog, schema, table); - Table t = - TreeLockUtils.doWithTreeLock( - ident, LockType.READ, () -> dispatcher.loadTable(ident)); + Table t = dispatcher.loadTable(ident); Response response = Utils.ok(new TableResponse(DTOConverters.toDTO(t))); LOG.info("Table loaded: {}.{}.{}.{}", metalake, catalog, schema, table); return response; diff --git a/server/src/main/java/com/datastrato/gravitino/server/web/rest/TopicOperations.java b/server/src/main/java/com/datastrato/gravitino/server/web/rest/TopicOperations.java index c49552ac8b5..1932585ef36 100644 --- a/server/src/main/java/com/datastrato/gravitino/server/web/rest/TopicOperations.java +++ b/server/src/main/java/com/datastrato/gravitino/server/web/rest/TopicOperations.java @@ -142,9 +142,7 @@ public Response loadTopic( () -> { LOG.info("Loading topic: {}.{}.{}.{}", metalake, catalog, schema, topic); NameIdentifier ident = NameIdentifierUtil.ofTopic(metalake, catalog, schema, topic); - Topic t = - TreeLockUtils.doWithTreeLock( - ident, LockType.READ, () -> dispatcher.loadTopic(ident)); + Topic t = dispatcher.loadTopic(ident); Response response = Utils.ok(new TopicResponse(DTOConverters.toDTO(t))); LOG.info("Topic loaded: {}.{}.{}.{}", metalake, catalog, schema, topic); return response; diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestCatalogOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestCatalogOperations.java index 2ffb138a452..e294b05f093 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestCatalogOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestCatalogOperations.java @@ -46,6 +46,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -69,12 +70,12 @@ public HttpServletRequest get() { private CatalogManager manager = mock(CatalogManager.class); @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestFilesetOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestFilesetOperations.java index 96ebd196c2c..c1f342de58c 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestFilesetOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestFilesetOperations.java @@ -44,6 +44,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -72,12 +73,12 @@ public HttpServletRequest get() { private final String schema = "schema1"; @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestGroupOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestGroupOperations.java index ecba282167d..4b232ca1016 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestGroupOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestGroupOperations.java @@ -37,6 +37,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -60,13 +61,13 @@ public HttpServletRequest get() { } @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); - GravitinoEnv.getInstance().setAccessControlManager(manager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlManager", manager, true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeAdminOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeAdminOperations.java index 8437626dc19..2ffbad2edfc 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeAdminOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeAdminOperations.java @@ -32,6 +32,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -55,13 +56,13 @@ public HttpServletRequest get() { } @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = Mockito.mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); - GravitinoEnv.getInstance().setAccessControlManager(manager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlManager", manager, true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeOperations.java index a00afd62166..a4fe3eca64c 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestMetalakeOperations.java @@ -42,6 +42,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -65,12 +66,12 @@ public HttpServletRequest get() { private MetalakeManager metalakeManager = mock(MetalakeManager.class); @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPartitionOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPartitionOperations.java index 9de31fb91f5..00d3027dc83 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPartitionOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPartitionOperations.java @@ -40,6 +40,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -84,12 +85,12 @@ public HttpServletRequest get() { private final String table = "table1"; @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPermissionOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPermissionOperations.java index 9e8ee453b7c..501ceccaeb3 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPermissionOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestPermissionOperations.java @@ -39,6 +39,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -62,13 +63,13 @@ public HttpServletRequest get() { } @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); - GravitinoEnv.getInstance().setAccessControlManager(manager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlManager", manager, true); } @Override 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 e17977b0fb9..3e0194e36f2 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 @@ -19,6 +19,11 @@ import com.datastrato.gravitino.authorization.Role; 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; @@ -33,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; @@ -43,6 +49,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -55,6 +62,12 @@ public class TestRoleOperations extends JerseyTest { private static final AccessControlManager manager = mock(AccessControlManager.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 @@ -66,13 +79,22 @@ public HttpServletRequest get() { } @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); - GravitinoEnv.getInstance().setAccessControlManager(manager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlManager", manager, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "metalakeDispatcher", metalakeDispatcher, true); + 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); + FieldUtils.writeField(GravitinoEnv.getInstance(), "topicDispatcher", topicDispatcher, true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "filesetDispatcher", filesetDispatcher, true); } @Override @@ -115,6 +137,7 @@ public void testCreateRole() { Role role = buildRole("role1"); when(manager.createRole(any(), any(), any(), any())).thenReturn(role); + when(catalogDispatcher.catalogExists(any())).thenReturn(true); Response resp = target("/metalakes/metalake1/roles") @@ -155,7 +178,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()); @@ -341,4 +377,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))); + } } diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestSchemaOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestSchemaOperations.java index a711cc9351e..b4f119c87dd 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestSchemaOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestSchemaOperations.java @@ -44,6 +44,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -71,12 +72,12 @@ public HttpServletRequest get() { private final String catalog = "catalog1"; @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTableOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTableOperations.java index 40851410206..3766f69cee0 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTableOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTableOperations.java @@ -71,6 +71,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -100,12 +101,12 @@ public HttpServletRequest get() { private final String schema = "schema1"; @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTopicOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTopicOperations.java index a65b75fdf4a..21e740ba928 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTopicOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestTopicOperations.java @@ -43,6 +43,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -69,12 +70,12 @@ public HttpServletRequest get() { private final String schema = "default"; @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); } @Override diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestUserOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestUserOperations.java index 71e00d6f33a..8d39f4b44b0 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestUserOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestUserOperations.java @@ -37,6 +37,7 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.JerseyTest; @@ -60,13 +61,13 @@ public HttpServletRequest get() { } @BeforeAll - public static void setup() { + public static void setup() throws IllegalAccessException { Config config = mock(Config.class); Mockito.doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); Mockito.doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); - GravitinoEnv.getInstance().setLockManager(new LockManager(config)); - GravitinoEnv.getInstance().setAccessControlManager(manager); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlManager", manager, true); } @Override