This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new c2e69d464e [#13007] fix(core,iceberg): Restore event dispatcher
ordering (#13000)
c2e69d464e is described below
commit c2e69d464e8555446fd349977d0503b1572f1bcc
Author: roryqi <[email protected]>
AuthorDate: Wed Sep 9 13:17:20 2026 +0800
[#13007] fix(core,iceberg): Restore event dispatcher ordering (#13000)
### What changes were proposed in this pull request?
Restore the public dispatcher order to:
`Event -> Normalize -> Hook -> Manager/OperationDispatcher`
Keep internal dispatchers independent from hook and event dispatchers,
and apply the same ordering to Iceberg REST dispatchers.
Route ownership updates for bulk role creation through the internal
owner dispatcher, remove redundant normalization from hook dispatchers,
and add regression tests covering owner assignment failures.
### Why are the changes needed?
PR #10834 placed the hook dispatcher outside the event dispatcher so
that CREATE events were emitted before the SET_OWNER event.
After PR #12893, automatic ownership assignment is routed through
internal dispatchers and no longer emits a nested SET_OWNER event.
Keeping the previous order would emit a successful CREATE event before
owner assignment completes, even if the overall CREATE request
subsequently fails.
Bulk role creation was also missed when automatic ownership updates were
moved to internal dispatchers, so it could still emit nested SET_OWNER
events.
Restoring the event dispatcher as the outermost layer makes the CREATE
event reflect the result of the complete operation.
Fix: #13007
### Does this PR introduce _any_ user-facing change?
Yes. If automatic owner assignment fails, the CREATE operation now emits
a failure event instead of an earlier success event. Automatic ownership
updates, including bulk role creation, no longer emit nested SET_OWNER
events.
There are no API or property changes.
### How was this patch tested?
- `./gradlew :core:test -PskipITs`
- `./gradlew :core:test --tests
org.apache.gravitino.hook.TestAccessControlHookDispatcher --tests
org.apache.gravitino.listener.api.event.TestTableEvent -PskipITs`
- Targeted Iceberg REST dispatcher tests
- Core and Iceberg REST compile tests
- `./gradlew :core:spotlessApply
:iceberg:iceberg-rest-server:spotlessApply`
- `git diff --check`
---
.../java/org/apache/gravitino/GravitinoEnv.java | 137 +++++++++++----------
.../gravitino/catalog/CapabilityHelpers.java | 11 --
.../hook/AccessControlHookDispatcher.java | 2 +-
.../gravitino/hook/FilesetHookDispatcher.java | 13 +-
.../gravitino/hook/FunctionHookDispatcher.java | 25 +---
.../apache/gravitino/hook/ModelHookDispatcher.java | 22 +---
.../gravitino/hook/SchemaHookDispatcher.java | 19 +--
.../apache/gravitino/hook/TableHookDispatcher.java | 20 +--
.../apache/gravitino/hook/TopicHookDispatcher.java | 13 +-
.../apache/gravitino/hook/ViewHookDispatcher.java | 17 +--
.../hook/TestAccessControlHookDispatcher.java | 17 +++
.../gravitino/hook/TestFilesetHookDispatcher.java | 5 +-
.../gravitino/hook/TestFunctionHookDispatcher.java | 31 ++---
.../gravitino/hook/TestModelHookDispatcher.java | 6 +-
.../gravitino/hook/TestSchemaHookDispatcher.java | 5 +-
.../gravitino/hook/TestTableHookDispatcher.java | 17 ++-
.../gravitino/hook/TestTopicHookDispatcher.java | 5 +-
.../gravitino/hook/TestViewHookDispatcher.java | 16 +--
.../listener/api/event/TestTableEvent.java | 36 ++++++
.../org/apache/gravitino/iceberg/RESTService.java | 33 +++--
20 files changed, 205 insertions(+), 245 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
index 1734e32fd3..a635e62577 100644
--- a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
+++ b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
@@ -841,30 +841,27 @@ public class GravitinoEnv {
// Create and initialize Catalog related modules first so MetalakeManager
can force-drop
// child catalogs through CatalogManager.dropCatalog (same path as
FilesetCatalogOperations).
- // CatalogHookDispatcher -> CatalogEventDispatcher ->
CatalogNormalizeDispatcher ->
+ // CatalogEventDispatcher -> CatalogNormalizeDispatcher ->
CatalogHookDispatcher ->
// CatalogManager
// CatalogManager registers its own change-log listener with the entity
store (when the store
// supports it), so no poller wiring is needed here.
this.catalogManager = new CatalogManager(config, entityStore, idGenerator,
secretManager);
// Create and initialize metalake related modules, the operation chain is:
- // MetalakeHookDispatcher -> MetalakeEventDispatcher ->
MetalakeNormalizeDispatcher ->
+ // MetalakeEventDispatcher -> MetalakeNormalizeDispatcher ->
MetalakeHookDispatcher ->
// MetalakeManager
this.metalakeManager = new MetalakeManager(entityStore, idGenerator,
catalogManager);
+ this.internalMetalakeDispatcher = new
MetalakeNormalizeDispatcher(metalakeManager);
+ MetalakeHookDispatcher metalakeHookDispatcher = new
MetalakeHookDispatcher(metalakeManager);
MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
- new MetalakeNormalizeDispatcher(metalakeManager);
- this.internalMetalakeDispatcher = metalakeNormalizeDispatcher;
- MetalakeEventDispatcher metalakeEventDispatcher =
- new MetalakeEventDispatcher(eventBus, metalakeNormalizeDispatcher);
- this.metalakeDispatcher = new
MetalakeHookDispatcher(metalakeEventDispatcher);
+ new MetalakeNormalizeDispatcher(metalakeHookDispatcher);
+ this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
- this.internalCatalogDispatcher = catalogManager;
+ this.internalCatalogDispatcher = new
CatalogNormalizeDispatcher(catalogManager);
+ CatalogHookDispatcher catalogHookDispatcher = new
CatalogHookDispatcher(catalogManager);
CatalogNormalizeDispatcher catalogNormalizeDispatcher =
- new CatalogNormalizeDispatcher(catalogManager);
- this.internalCatalogDispatcher = catalogNormalizeDispatcher;
- CatalogEventDispatcher catalogEventDispatcher =
- new CatalogEventDispatcher(eventBus, catalogNormalizeDispatcher);
- this.catalogDispatcher = new CatalogHookDispatcher(catalogEventDispatcher);
+ new CatalogNormalizeDispatcher(catalogHookDispatcher);
+ this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
this.credentialOperationDispatcher =
new CredentialOperationDispatcher(catalogManager, entityStore,
idGenerator, secretManager);
@@ -876,29 +873,32 @@ public class GravitinoEnv {
// Fileset dispatcher is created before schema dispatcher so schema can
take it directly.
FilesetOperationDispatcher filesetOperationDispatcher =
new FilesetOperationDispatcher(catalogManager, entityStore,
idGenerator, secretManager);
- FilesetNormalizeDispatcher filesetNormalizeDispatcher =
+ FilesetNormalizeDispatcher internalFilesetNormalizeDispatcher =
new FilesetNormalizeDispatcher(filesetOperationDispatcher,
catalogManager);
- this.internalFilesetDispatcher = filesetNormalizeDispatcher;
- FilesetEventDispatcher filesetEventDispatcher =
- new FilesetEventDispatcher(eventBus, filesetNormalizeDispatcher);
- this.filesetDispatcher = new FilesetHookDispatcher(filesetEventDispatcher);
+ this.internalFilesetDispatcher = internalFilesetNormalizeDispatcher;
+ FilesetHookDispatcher filesetHookDispatcher =
+ new FilesetHookDispatcher(filesetOperationDispatcher);
+ FilesetNormalizeDispatcher filesetNormalizeDispatcher =
+ new FilesetNormalizeDispatcher(filesetHookDispatcher, catalogManager);
+ this.filesetDispatcher = new FilesetEventDispatcher(eventBus,
filesetNormalizeDispatcher);
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(
- catalogManager, entityStore, idGenerator, secretManager,
filesetNormalizeDispatcher);
- this.internalSchemaDispatcher = schemaOperationDispatcher;
- SchemaNormalizeDispatcher schemaNormalizeDispatcher =
+ catalogManager,
+ entityStore,
+ idGenerator,
+ secretManager,
+ internalFilesetNormalizeDispatcher);
+ this.internalSchemaDispatcher =
new SchemaNormalizeDispatcher(schemaOperationDispatcher,
catalogManager);
- this.internalSchemaDispatcher = schemaNormalizeDispatcher;
- SchemaEventDispatcher schemaEventDispatcher =
- new SchemaEventDispatcher(eventBus, schemaNormalizeDispatcher);
- this.schemaDispatcher = new SchemaHookDispatcher(schemaEventDispatcher);
+ SchemaHookDispatcher schemaHookDispatcher = new
SchemaHookDispatcher(schemaOperationDispatcher);
+ SchemaNormalizeDispatcher schemaNormalizeDispatcher =
+ new SchemaNormalizeDispatcher(schemaHookDispatcher, catalogManager);
+ this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
this.internalTableDispatcher = tableOperationDispatcher;
- TableNormalizeDispatcher tableNormalizeDispatcher =
- new TableNormalizeDispatcher(tableOperationDispatcher, catalogManager);
TableOperationDispatcher internalTableOperationDispatcher =
new TableOperationDispatcher(
catalogManager,
@@ -908,11 +908,11 @@ public class GravitinoEnv {
secretManager);
this.internalTableDispatcher =
new TableNormalizeDispatcher(internalTableOperationDispatcher,
catalogManager);
- TableEventDispatcher tableEventDispatcher =
- new TableEventDispatcher(eventBus, tableNormalizeDispatcher);
- this.tableDispatcher =
- new TableHookDispatcher(
- tableEventDispatcher, this::internalOwnerDispatcher,
catalogManager);
+ TableHookDispatcher tableHookDispatcher =
+ new TableHookDispatcher(tableOperationDispatcher,
this::internalOwnerDispatcher);
+ TableNormalizeDispatcher tableNormalizeDispatcher =
+ new TableNormalizeDispatcher(tableHookDispatcher, catalogManager);
+ this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
// TODO: We can install hooks when we need, we only supports ownership
post hook,
// partition doesn't have ownership, so we don't need it now.
@@ -924,44 +924,44 @@ public class GravitinoEnv {
TopicOperationDispatcher topicOperationDispatcher =
new TopicOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
- TopicNormalizeDispatcher topicNormalizeDispatcher =
+ TopicNormalizeDispatcher internalTopicNormalizeDispatcher =
new TopicNormalizeDispatcher(topicOperationDispatcher, catalogManager);
- this.internalTopicDispatcher = topicNormalizeDispatcher;
- TopicEventDispatcher topicEventDispatcher =
- new TopicEventDispatcher(eventBus, topicNormalizeDispatcher);
- this.topicDispatcher = new TopicHookDispatcher(topicEventDispatcher);
+ this.internalTopicDispatcher = internalTopicNormalizeDispatcher;
+ TopicHookDispatcher topicHookDispatcher = new
TopicHookDispatcher(topicOperationDispatcher);
+ TopicNormalizeDispatcher topicNormalizeDispatcher =
+ new TopicNormalizeDispatcher(topicHookDispatcher, catalogManager);
+ this.topicDispatcher = new TopicEventDispatcher(eventBus,
topicNormalizeDispatcher);
ModelOperationDispatcher modelOperationDispatcher =
new ModelOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
- ModelNormalizeDispatcher modelNormalizeDispatcher =
+ ModelNormalizeDispatcher internalModelNormalizeDispatcher =
new ModelNormalizeDispatcher(modelOperationDispatcher, catalogManager);
- this.internalModelDispatcher = modelNormalizeDispatcher;
- ModelEventDispatcher modelEventDispatcher =
- new ModelEventDispatcher(eventBus, modelNormalizeDispatcher);
- this.modelDispatcher = new ModelHookDispatcher(modelEventDispatcher);
+ this.internalModelDispatcher = internalModelNormalizeDispatcher;
+ ModelHookDispatcher modelHookDispatcher = new
ModelHookDispatcher(modelOperationDispatcher);
+ ModelNormalizeDispatcher modelNormalizeDispatcher =
+ new ModelNormalizeDispatcher(modelHookDispatcher, catalogManager);
+ this.modelDispatcher = new ModelEventDispatcher(eventBus,
modelNormalizeDispatcher);
// Create and initialize Function related modules, the operation chain is:
- // FunctionHookDispatcher -> FunctionEventDispatcher ->
FunctionNormalizeDispatcher ->
+ // FunctionEventDispatcher -> FunctionNormalizeDispatcher ->
FunctionHookDispatcher ->
// FunctionOperationDispatcher
FunctionOperationDispatcher functionOperationDispatcher =
new FunctionOperationDispatcher(
catalogManager, schemaOperationDispatcher, entityStore,
idGenerator, secretManager);
- FunctionNormalizeDispatcher functionNormalizeDispatcher =
+ FunctionNormalizeDispatcher internalFunctionNormalizeDispatcher =
new FunctionNormalizeDispatcher(functionOperationDispatcher,
catalogManager);
- this.internalFunctionDispatcher = functionNormalizeDispatcher;
- FunctionEventDispatcher functionEventDispatcher =
- new FunctionEventDispatcher(eventBus, functionNormalizeDispatcher);
- this.functionDispatcher =
- new FunctionHookDispatcher(
- functionEventDispatcher, this::internalOwnerDispatcher,
catalogManager);
-
- // View operation chain: ViewHookDispatcher -> ViewEventDispatcher ->
ViewNormalizeDispatcher
+ this.internalFunctionDispatcher = internalFunctionNormalizeDispatcher;
+ FunctionHookDispatcher functionHookDispatcher =
+ new FunctionHookDispatcher(functionOperationDispatcher,
this::internalOwnerDispatcher);
+ FunctionNormalizeDispatcher functionNormalizeDispatcher =
+ new FunctionNormalizeDispatcher(functionHookDispatcher,
catalogManager);
+ this.functionDispatcher = new FunctionEventDispatcher(eventBus,
functionNormalizeDispatcher);
+
+ // View operation chain: ViewEventDispatcher -> ViewNormalizeDispatcher ->
ViewHookDispatcher
// -> ViewOperationDispatcher.
ViewOperationDispatcher viewOperationDispatcher =
new ViewOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
this.internalViewDispatcher = viewOperationDispatcher;
- ViewNormalizeDispatcher viewNormalizeDispatcher =
- new ViewNormalizeDispatcher(viewOperationDispatcher, catalogManager);
ViewOperationDispatcher internalViewOperationDispatcher =
new ViewOperationDispatcher(
catalogManager,
@@ -971,10 +971,11 @@ public class GravitinoEnv {
secretManager);
this.internalViewDispatcher =
new ViewNormalizeDispatcher(internalViewOperationDispatcher,
catalogManager);
- ViewEventDispatcher viewEventDispatcher =
- new ViewEventDispatcher(eventBus, viewNormalizeDispatcher);
- this.viewDispatcher =
- new ViewHookDispatcher(viewEventDispatcher,
this::internalOwnerDispatcher, catalogManager);
+ ViewHookDispatcher viewHookDispatcher =
+ new ViewHookDispatcher(viewOperationDispatcher,
this::internalOwnerDispatcher);
+ ViewNormalizeDispatcher viewNormalizeDispatcher =
+ new ViewNormalizeDispatcher(viewHookDispatcher, catalogManager);
+ this.viewDispatcher = new ViewEventDispatcher(eventBus,
viewNormalizeDispatcher);
// Semantic Model operation chain: SemanticModelNormalizeDispatcher ->
// SemanticModelOperationDispatcher -> ManagedSemanticModelOperations.
@@ -996,9 +997,10 @@ public class GravitinoEnv {
AccessControlManager accessControlManager =
new AccessControlManager(entityStore, idGenerator, config);
this.internalAccessControlDispatcher = accessControlManager;
- AccessControlEventDispatcher accessControlEventDispatcher =
- new AccessControlEventDispatcher(eventBus, accessControlManager);
- this.accessControlDispatcher = new
AccessControlHookDispatcher(accessControlEventDispatcher);
+ AccessControlHookDispatcher accessControlHookDispatcher =
+ new AccessControlHookDispatcher(accessControlManager);
+ this.accessControlDispatcher =
+ new AccessControlEventDispatcher(eventBus,
accessControlHookDispatcher);
OwnerDispatcher ownerManager = new OwnerManager(entityStore);
this.internalOwnerDispatcher = ownerManager;
this.ownerDispatcher = new OwnerEventManager(eventBus, ownerManager);
@@ -1019,21 +1021,20 @@ public class GravitinoEnv {
// Create and initialize Tag related modules
TagManager tagManager = new TagManager(idGenerator, entityStore);
this.internalTagDispatcher = tagManager;
- TagEventDispatcher tagEventDispatcher = new TagEventDispatcher(eventBus,
tagManager);
- this.tagDispatcher = new TagHookDispatcher(tagEventDispatcher);
+ TagHookDispatcher tagHookDispatcher = new TagHookDispatcher(tagManager);
+ this.tagDispatcher = new TagEventDispatcher(eventBus, tagHookDispatcher);
PolicyManager policyManager = new PolicyManager(idGenerator, entityStore);
this.internalPolicyDispatcher = policyManager;
- PolicyEventDispatcher policyEventDispatcher =
- new PolicyEventDispatcher(eventBus, policyManager);
- this.policyDispatcher = new PolicyHookDispatcher(policyEventDispatcher);
+ PolicyHookDispatcher policyHookDispatcher = new
PolicyHookDispatcher(policyManager);
+ this.policyDispatcher = new PolicyEventDispatcher(eventBus,
policyHookDispatcher);
JobManager jobManager = new JobManager(config, entityStore, idGenerator);
JobTemplateValidationDispatcher validationDispatcher =
new JobTemplateValidationDispatcher(jobManager);
this.internalJobOperationDispatcher = validationDispatcher;
- JobEventDispatcher jobEventDispatcher = new JobEventDispatcher(eventBus,
validationDispatcher);
- this.jobOperationDispatcher = new JobHookDispatcher(jobEventDispatcher);
+ JobHookDispatcher jobHookDispatcher = new
JobHookDispatcher(validationDispatcher);
+ this.jobOperationDispatcher = new JobEventDispatcher(eventBus,
jobHookDispatcher);
// Register built-in job template event listener to automatically register
templates
// when metalakes are created
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CapabilityHelpers.java
b/core/src/main/java/org/apache/gravitino/catalog/CapabilityHelpers.java
index adc39cc62a..60c0be7754 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CapabilityHelpers.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CapabilityHelpers.java
@@ -124,17 +124,6 @@ public class CapabilityHelpers {
return NameIdentifier.of(namespace, name);
}
- /**
- * Convenience overload that loads the catalog capability for {@code ident}
and applies it to the
- * identifier. Use this from call sites (e.g. HookDispatchers) that need a
normalized identifier
- * but do not already hold a {@link Capability} instance.
- */
- public static NameIdentifier applyCapabilities(
- NameIdentifier ident, Capability.Scope scope, CatalogManager
catalogManager) {
- Capability capability = getCapability(ident, catalogManager);
- return applyCapabilities(ident, scope, capability);
- }
-
public static NameIdentifier[] applyCaseSensitive(
NameIdentifier[] idents, Capability.Scope scope, Capability
capabilities) {
return Arrays.stream(idents)
diff --git
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
index b02dbfd02f..0837190099 100644
---
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
@@ -234,7 +234,7 @@ public class AccessControlHookDispatcher implements
AccessControlDispatcher {
public List<BulkItemResult<Role>> createRoles(String metalake, List<RoleAdd>
roles)
throws NoSuchMetalakeException {
List<BulkItemResult<Role>> results = dispatcher.createRoles(metalake,
roles);
- OwnerDispatcher ownerDispatcher =
GravitinoEnv.getInstance().ownerDispatcher();
+ OwnerDispatcher ownerDispatcher =
GravitinoEnv.getInstance().internalOwnerDispatcher();
if (ownerDispatcher != null) {
results.stream()
.filter(BulkItemResult::succeeded)
diff --git
a/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
index 89557837b8..0e61cf758f 100644
--- a/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
@@ -28,9 +28,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
import org.apache.gravitino.catalog.FilesetDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchFilesetException;
import org.apache.gravitino.exceptions.NoSuchLocationNameException;
@@ -88,16 +86,9 @@ public class FilesetHookDispatcher implements
FilesetDispatcher {
// Set the creator as the owner of the fileset.
OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().internalOwnerDispatcher();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the fileset name (and its
schema namespace)
- // based on catalog capabilities, so the entity is stored under the
normalized identifier.
- // Apply the same normalization here so the owner is attached to the
same identifier the
- // manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(
- ident, Capability.Scope.FILESET,
GravitinoEnv.getInstance().catalogManager());
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.FILESET),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.FILESET),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
diff --git
a/core/src/main/java/org/apache/gravitino/hook/FunctionHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/FunctionHookDispatcher.java
index 33c70bda57..895731d106 100644
--- a/core/src/main/java/org/apache/gravitino/hook/FunctionHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/FunctionHookDispatcher.java
@@ -26,10 +26,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
-import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.FunctionDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.FunctionAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchFunctionException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
@@ -48,7 +45,6 @@ import org.apache.gravitino.utils.PrincipalUtils;
public class FunctionHookDispatcher implements FunctionDispatcher {
private final FunctionDispatcher dispatcher;
private final Supplier<OwnerDispatcher> ownerDispatcher;
- private final CatalogManager catalogManager;
/**
* Creates a function hook dispatcher.
@@ -56,15 +52,11 @@ public class FunctionHookDispatcher implements
FunctionDispatcher {
* @param dispatcher the underlying function dispatcher
* @param ownerDispatcher supplies the owner dispatcher, or {@code null}
when authorization is
* disabled
- * @param catalogManager the catalog manager used to apply catalog
capabilities
*/
public FunctionHookDispatcher(
- FunctionDispatcher dispatcher,
- Supplier<OwnerDispatcher> ownerDispatcher,
- CatalogManager catalogManager) {
+ FunctionDispatcher dispatcher, Supplier<OwnerDispatcher>
ownerDispatcher) {
this.dispatcher = dispatcher;
this.ownerDispatcher = ownerDispatcher;
- this.catalogManager = catalogManager;
}
@Override
@@ -101,15 +93,9 @@ public class FunctionHookDispatcher implements
FunctionDispatcher {
// Set the creator as the owner of the function.
OwnerDispatcher ownerManager = ownerDispatcher.get();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the function name (and its
schema namespace)
- // based on catalog capabilities, so the entity is stored under the
normalized identifier.
- // Apply the same normalization here so the owner is attached to the
same identifier the
- // manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(ident,
Capability.Scope.FUNCTION, catalogManager);
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.FUNCTION),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.FUNCTION),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
@@ -126,13 +112,10 @@ public class FunctionHookDispatcher implements
FunctionDispatcher {
public boolean dropFunction(NameIdentifier ident) {
boolean dropped = dispatcher.dropFunction(ident);
if (dropped) {
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCaseSensitive(ident,
Capability.Scope.FUNCTION, catalogManager);
// Function privileges are managed by Gravitino. Catalog authorization
plugins such as
// Ranger HadoopSQL do not support FUNCTION metadata objects, so only
invalidate the built-in
// authorizer's name-to-ID mapping here.
- AuthorizationUtils.notifyEntityNameIdMappingChange(
- normalizedIdent, Entity.EntityType.FUNCTION);
+ AuthorizationUtils.notifyEntityNameIdMappingChange(ident,
Entity.EntityType.FUNCTION);
}
return dropped;
}
diff --git
a/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
index 2143030b7a..facad41909 100644
--- a/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
@@ -26,9 +26,7 @@ import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
import org.apache.gravitino.catalog.ModelDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.ModelAlreadyExistsException;
import
org.apache.gravitino.exceptions.ModelVersionAliasesAlreadyExistException;
import org.apache.gravitino.exceptions.NoSuchModelException;
@@ -73,15 +71,9 @@ public class ModelHookDispatcher implements ModelDispatcher {
// Set the creator as owner of the model.
OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().internalOwnerDispatcher();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the model name based on
catalog capabilities,
- // so the entity is stored under the normalized identifier. Apply the
same normalization
- // here so the owner is attached to the same identifier the manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(
- ident, Capability.Scope.MODEL,
GravitinoEnv.getInstance().catalogManager());
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.MODEL),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.MODEL),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
@@ -167,15 +159,9 @@ public class ModelHookDispatcher implements
ModelDispatcher {
// Set the creator as owner of the model.
OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().internalOwnerDispatcher();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the model name based on
catalog capabilities,
- // so the entity is stored under the normalized identifier. Apply the
same normalization
- // here so the owner is attached to the same identifier the manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(
- ident, Capability.Scope.MODEL,
GravitinoEnv.getInstance().catalogManager());
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.MODEL),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.MODEL),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
diff --git
a/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
index 3abc121a1e..7a79a9d213 100644
--- a/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
@@ -32,9 +32,7 @@ import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
import org.apache.gravitino.catalog.SchemaDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
@@ -78,13 +76,6 @@ public class SchemaHookDispatcher implements
SchemaDispatcher {
Map<String, SecretBinding> secretBindings,
Map<String, SecretReference> secretReferences)
throws NoSuchCatalogException, SchemaAlreadyExistsException {
- // The inner NormalizeDispatcher case-folds the schema name based on
catalog capabilities, so
- // the entity is stored under the normalized identifier. Normalize here
too so ownership is
- // attached to the identifiers the manager sees and ancestor probing
matches stored names.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(
- ident, Capability.Scope.SCHEMA,
GravitinoEnv.getInstance().catalogManager());
-
// Serialize probe -> create -> owner-assignment on the catalog so
concurrent hierarchical
// creates cannot both claim a shared, newly-created ancestor (which would
let the later create
// overwrite the first creator's ownership). We lock the catalog node --
the same node the inner
@@ -92,8 +83,7 @@ public class SchemaHookDispatcher implements SchemaDispatcher
{
// deeper (branch-scoped) lock would hold the catalog node in READ mode
and deadlock against
// that inner WRITE acquisition.
NameIdentifier catalogIdent =
- NameIdentifierUtil.ofCatalog(
- normalizedIdent.namespace().level(0),
normalizedIdent.namespace().level(1));
+ NameIdentifierUtil.ofCatalog(ident.namespace().level(0),
ident.namespace().level(1));
return TreeLockUtils.doWithTreeLock(
catalogIdent,
LockType.WRITE,
@@ -102,7 +92,7 @@ public class SchemaHookDispatcher implements
SchemaDispatcher {
// missing ancestor ("A", "A:B"). Probe BEFORE the create which
ancestors are new, so
// ownership is assigned only to schemas this request actually
creates and a pre-existing
// ancestor's owner is never overwritten.
- List<NameIdentifier> newAncestors =
findMissingAncestors(normalizedIdent);
+ List<NameIdentifier> newAncestors = findMissingAncestors(ident);
Schema schema =
dispatcher.createSchema(ident, comment, properties,
secretBindings, secretReferences);
@@ -118,11 +108,10 @@ public class SchemaHookDispatcher implements
SchemaDispatcher {
ownedObjects.add(
NameIdentifierUtil.toMetadataObject(ancestor,
Entity.EntityType.SCHEMA));
}
- ownedObjects.add(
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.SCHEMA));
+ ownedObjects.add(NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.SCHEMA));
// All objects are SCHEMA-typed, so the batch path (single object
type) is valid.
ownerManager.setOwners(
- normalizedIdent.namespace().level(0),
+ ident.namespace().level(0),
ownedObjects,
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
index bb923afdf2..9a1037e1ca 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
@@ -27,10 +27,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
-import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.TableDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
@@ -52,7 +49,6 @@ import org.apache.gravitino.utils.PrincipalUtils;
public class TableHookDispatcher implements TableDispatcher {
private final TableDispatcher dispatcher;
private final Supplier<OwnerDispatcher> ownerDispatcher;
- private final CatalogManager catalogManager;
/**
* Creates a table hook dispatcher.
@@ -60,15 +56,11 @@ public class TableHookDispatcher implements TableDispatcher
{
* @param dispatcher the underlying table dispatcher
* @param ownerDispatcher supplies the owner dispatcher, or {@code null}
when authorization is
* disabled
- * @param catalogManager the catalog manager used to apply catalog
capabilities
*/
public TableHookDispatcher(
- TableDispatcher dispatcher,
- Supplier<OwnerDispatcher> ownerDispatcher,
- CatalogManager catalogManager) {
+ TableDispatcher dispatcher, Supplier<OwnerDispatcher> ownerDispatcher) {
this.dispatcher = dispatcher;
this.ownerDispatcher = ownerDispatcher;
- this.catalogManager = catalogManager;
}
@Override
@@ -99,15 +91,9 @@ public class TableHookDispatcher implements TableDispatcher {
// Set the creator as the owner of the table.
OwnerDispatcher ownerManager = ownerDispatcher.get();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the table name (and its
schema namespace)
- // based on catalog capabilities, so the entity is stored under the
normalized identifier.
- // Apply the same normalization here so the owner is attached to the
same identifier the
- // manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(ident, Capability.Scope.TABLE,
catalogManager);
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.TABLE),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.TABLE),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
index cae51e74fc..ad8440942b 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
@@ -27,9 +27,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
import org.apache.gravitino.catalog.TopicDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTopicException;
import org.apache.gravitino.exceptions.TopicAlreadyExistsException;
@@ -70,16 +68,9 @@ public class TopicHookDispatcher implements TopicDispatcher {
// Set the creator as the owner of the topic.
OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().internalOwnerDispatcher();
if (ownerManager != null) {
- // The inner NormalizeDispatcher case-folds the topic name (and its
schema namespace)
- // based on catalog capabilities, so the entity is stored under the
normalized identifier.
- // Apply the same normalization here so the owner is attached to the
same identifier the
- // manager sees.
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(
- ident, Capability.Scope.TOPIC,
GravitinoEnv.getInstance().catalogManager());
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.TOPIC),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.TOPIC),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
diff --git
a/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
index 2273cd3d3e..1f871682e9 100644
--- a/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
@@ -27,10 +27,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
-import org.apache.gravitino.catalog.CapabilityHelpers;
-import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.ViewDispatcher;
-import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchViewException;
import org.apache.gravitino.exceptions.ViewAlreadyExistsException;
@@ -48,7 +45,6 @@ import org.apache.gravitino.utils.PrincipalUtils;
public class ViewHookDispatcher implements ViewDispatcher {
private final ViewDispatcher dispatcher;
private final Supplier<OwnerDispatcher> ownerDispatcher;
- private final CatalogManager catalogManager;
/**
* Creates a view hook dispatcher.
@@ -56,15 +52,10 @@ public class ViewHookDispatcher implements ViewDispatcher {
* @param dispatcher the underlying view dispatcher
* @param ownerDispatcher supplies the owner dispatcher, or {@code null}
when authorization is
* disabled
- * @param catalogManager the catalog manager used to apply catalog
capabilities
*/
- public ViewHookDispatcher(
- ViewDispatcher dispatcher,
- Supplier<OwnerDispatcher> ownerDispatcher,
- CatalogManager catalogManager) {
+ public ViewHookDispatcher(ViewDispatcher dispatcher,
Supplier<OwnerDispatcher> ownerDispatcher) {
this.dispatcher = dispatcher;
this.ownerDispatcher = ownerDispatcher;
- this.catalogManager = catalogManager;
}
@Override
@@ -98,11 +89,9 @@ public class ViewHookDispatcher implements ViewDispatcher {
OwnerDispatcher ownerManager = ownerDispatcher.get();
if (ownerManager != null) {
- NameIdentifier normalizedIdent =
- CapabilityHelpers.applyCapabilities(ident, Capability.Scope.VIEW,
catalogManager);
ownerManager.setOwner(
- normalizedIdent.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(normalizedIdent,
Entity.EntityType.VIEW),
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.VIEW),
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestAccessControlHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestAccessControlHookDispatcher.java
index 308ea03444..3d587a05fe 100644
---
a/core/src/test/java/org/apache/gravitino/hook/TestAccessControlHookDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/hook/TestAccessControlHookDispatcher.java
@@ -26,6 +26,7 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Collections;
+import java.util.List;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.authorization.AccessControlDispatcher;
@@ -34,6 +35,8 @@ import org.apache.gravitino.authorization.Group;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.bulk.BulkItemResult;
+import org.apache.gravitino.bulk.RoleAdd;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -89,6 +92,20 @@ public class TestAccessControlHookDispatcher {
verify(mockDispatcher).createRole(any(), any(), any(), any());
}
+ @Test
+ public void testCreateRolesSetsOwnerWithInternalDispatcher() {
+ List<RoleAdd> roles =
+ Collections.singletonList(
+ new RoleAdd("test_role", Collections.emptyMap(),
Collections.emptyList()));
+ List<BulkItemResult<Role>> results =
+ Collections.singletonList(BulkItemResult.success(0, "test_role",
mock(Role.class)));
+ when(mockDispatcher.createRoles("test_metalake",
roles)).thenReturn(results);
+
+ Assertions.assertSame(results, hookDispatcher.createRoles("test_metalake",
roles));
+
+ verify(mockOwnerDispatcher).setOwner(any(), any(), any(), any());
+ }
+
@Test
public void testGrantRolesToUserInvalidatesUserRoleRelation() {
User mockUser = mock(User.class);
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestFilesetHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestFilesetHookDispatcher.java
index 2e95dabe13..a4431e7cbf 100644
---
a/core/src/test/java/org/apache/gravitino/hook/TestFilesetHookDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/hook/TestFilesetHookDispatcher.java
@@ -55,6 +55,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.FilesetDispatcher;
+import org.apache.gravitino.catalog.FilesetNormalizeDispatcher;
import org.apache.gravitino.catalog.TestFilesetOperationDispatcher;
import org.apache.gravitino.catalog.TestOperationDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
@@ -122,7 +123,9 @@ public class TestFilesetHookDispatcher extends
TestOperationDispatcher {
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
try {
- FilesetHookDispatcher localHook = new
FilesetHookDispatcher(mockFilesetDispatcher);
+ FilesetDispatcher localHook =
+ new FilesetNormalizeDispatcher(
+ new FilesetHookDispatcher(mockFilesetDispatcher),
mockCatalogManager);
NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"SCHEMA_NORM", "MY_FILESET");
localHook.createMultipleLocationFileset(
ident,
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestFunctionHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestFunctionHookDispatcher.java
index 6b375f2af1..e06a63a8eb 100644
---
a/core/src/test/java/org/apache/gravitino/hook/TestFunctionHookDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/hook/TestFunctionHookDispatcher.java
@@ -38,6 +38,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.FunctionDispatcher;
+import org.apache.gravitino.catalog.FunctionNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.connector.capability.Capability;
@@ -61,8 +62,6 @@ public class TestFunctionHookDispatcher {
Function registeredFunction = Mockito.mock(Function.class);
OwnerDispatcher ownerDispatcher = Mockito.mock(OwnerDispatcher.class);
- CatalogManager catalogManager = catalogManagerWith(Capability.DEFAULT);
-
Mockito.when(
dispatcher.registerFunction(
Mockito.eq(functionIdentifier),
@@ -73,7 +72,7 @@ public class TestFunctionHookDispatcher {
.thenReturn(registeredFunction);
FunctionHookDispatcher hookDispatcher =
- new FunctionHookDispatcher(dispatcher, () -> ownerDispatcher,
catalogManager);
+ new FunctionHookDispatcher(dispatcher, () -> ownerDispatcher);
Function result =
hookDispatcher.registerFunction(
functionIdentifier, "comment", FunctionType.SCALAR, true,
definitions);
@@ -109,9 +108,7 @@ public class TestFunctionHookDispatcher {
Mockito.eq(definitions)))
.thenReturn(registeredFunction);
- CatalogManager catalogManager = Mockito.mock(CatalogManager.class);
- FunctionHookDispatcher hookDispatcher =
- new FunctionHookDispatcher(dispatcher, () -> null, catalogManager);
+ FunctionHookDispatcher hookDispatcher = new
FunctionHookDispatcher(dispatcher, () -> null);
Function result =
hookDispatcher.registerFunction(
functionIdentifier, "comment", FunctionType.SCALAR, true,
definitions);
@@ -119,7 +116,6 @@ public class TestFunctionHookDispatcher {
assertSame(registeredFunction, result);
Mockito.verify(dispatcher)
.registerFunction(functionIdentifier, "comment", FunctionType.SCALAR,
true, definitions);
- Mockito.verifyNoInteractions(catalogManager);
}
@Test
@@ -137,9 +133,10 @@ public class TestFunctionHookDispatcher {
any(), any(), any(), Mockito.anyBoolean(), any()))
.thenReturn(mockFunction);
- FunctionHookDispatcher hook =
- new FunctionHookDispatcher(
- mockFunctionDispatcher, () -> mockOwnerDispatcher, catalogManager);
+ FunctionDispatcher hook =
+ new FunctionNormalizeDispatcher(
+ new FunctionHookDispatcher(mockFunctionDispatcher, () ->
mockOwnerDispatcher),
+ catalogManager);
NameIdentifier ident = NameIdentifier.of("metalake1", "catalog1",
"SCHEMA_NORM", "MY_FUNC");
hook.registerFunction(ident, "comment", FunctionType.SCALAR, true,
definitions);
@@ -157,8 +154,6 @@ public class TestFunctionHookDispatcher {
.when(mockOwnerDispatcher)
.setOwner(any(), any(), any(), any());
- CatalogManager catalogManager = catalogManagerWith(Capability.DEFAULT);
-
FunctionDispatcher mockFunctionDispatcher =
Mockito.mock(FunctionDispatcher.class);
Function mockFunction = Mockito.mock(Function.class);
FunctionDefinition[] definitions = new FunctionDefinition[] {};
@@ -168,8 +163,7 @@ public class TestFunctionHookDispatcher {
.thenReturn(mockFunction);
FunctionHookDispatcher hook =
- new FunctionHookDispatcher(
- mockFunctionDispatcher, () -> mockOwnerDispatcher, catalogManager);
+ new FunctionHookDispatcher(mockFunctionDispatcher, () ->
mockOwnerDispatcher);
NameIdentifier ident =
NameIdentifier.of("metalake1", "catalog1", "schema_owner_fail",
"func_owner_fail");
RuntimeException thrown =
@@ -187,7 +181,7 @@ public class TestFunctionHookDispatcher {
NameIdentifier normalizedIdentifier =
NameIdentifier.of("metalake1", "catalog1", "schema1", "func1");
FunctionDispatcher dispatcher = Mockito.mock(FunctionDispatcher.class);
- Mockito.when(dispatcher.dropFunction(functionIdentifier))
+ Mockito.when(dispatcher.dropFunction(normalizedIdentifier))
.thenReturn(true, false)
.thenThrow(new RuntimeException("Drop failed"));
CatalogManager catalogManager = catalogManagerWith(new
CaseInsensitiveCapability());
@@ -206,8 +200,9 @@ public class TestFunctionHookDispatcher {
try (MockedStatic<GravitinoEnv> envStatic =
Mockito.mockStatic(GravitinoEnv.class)) {
envStatic.when(GravitinoEnv::getInstance).thenReturn(env);
- FunctionHookDispatcher hookDispatcher =
- new FunctionHookDispatcher(dispatcher, () -> null, catalogManager);
+ FunctionDispatcher hookDispatcher =
+ new FunctionNormalizeDispatcher(
+ new FunctionHookDispatcher(dispatcher, () -> null),
catalogManager);
assertTrue(hookDispatcher.dropFunction(functionIdentifier));
assertFalse(hookDispatcher.dropFunction(functionIdentifier));
@@ -222,7 +217,7 @@ public class TestFunctionHookDispatcher {
// mock represents that integration boundary and must not receive a
removal callback.
Mockito.verifyNoInteractions(catalogAuthorizationPlugin);
Mockito.verify(catalogManager, Mockito.never()).loadCatalog(any());
- Mockito.verify(catalogManager, Mockito.times(1)).doWithCatalog(any(),
any());
+ Mockito.verify(catalogManager, Mockito.times(3)).doWithCatalog(any(),
any());
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
index 488cba5747..25df575ca3 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
@@ -35,6 +35,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.ModelDispatcher;
+import org.apache.gravitino.catalog.ModelNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
@@ -47,7 +48,7 @@ import org.mockito.ArgumentCaptor;
public class TestModelHookDispatcher {
- private ModelHookDispatcher hookDispatcher;
+ private ModelDispatcher hookDispatcher;
private ModelDispatcher mockDispatcher;
private OwnerDispatcher mockOwnerDispatcher;
private CatalogManager mockCatalogManager;
@@ -74,7 +75,8 @@ public class TestModelHookDispatcher {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager",
mockCatalogManager, true);
- hookDispatcher = new ModelHookDispatcher(mockDispatcher);
+ hookDispatcher =
+ new ModelNormalizeDispatcher(new ModelHookDispatcher(mockDispatcher),
mockCatalogManager);
}
@AfterEach
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestSchemaHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestSchemaHookDispatcher.java
index 66ed954e2e..fbf0f43949 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestSchemaHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestSchemaHookDispatcher.java
@@ -50,6 +50,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.SchemaDispatcher;
+import org.apache.gravitino.catalog.SchemaNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
@@ -147,7 +148,9 @@ public class TestSchemaHookDispatcher {
Schema mockSchema = mock(Schema.class);
when(mockDispatcher.createSchema(any(), any(), any(), any(),
any())).thenReturn(mockSchema);
- hookDispatcher.createSchema(ident, "comment", Collections.emptyMap());
+ SchemaDispatcher normalizedDispatcher =
+ new SchemaNormalizeDispatcher(hookDispatcher, mockCatalogManager);
+ normalizedDispatcher.createSchema(ident, "comment",
Collections.emptyMap());
List<MetadataObject> owned = captureOwnedObjects();
Assertions.assertEquals(1, owned.size(), "A flat schema only assigns
ownership to the leaf");
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java
index 6abb334569..b1a61795a7 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java
@@ -37,6 +37,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.TableDispatcher;
+import org.apache.gravitino.catalog.TableNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
@@ -60,8 +61,7 @@ public class TestTableHookDispatcher {
@Test
public void testDropAuthorizationPrivilege() {
TableDispatcher dispatcher = Mockito.mock(TableDispatcher.class);
- TableHookDispatcher hook =
- new TableHookDispatcher(dispatcher, () -> null,
Mockito.mock(CatalogManager.class));
+ TableHookDispatcher hook = new TableHookDispatcher(dispatcher, () -> null);
NameIdentifier ident = NameIdentifier.of(METALAKE, CATALOG, "schema",
"table");
List<String> locations = ImmutableList.of("/test");
Mockito.when(dispatcher.dropTable(ident)).thenReturn(true);
@@ -93,8 +93,9 @@ public class TestTableHookDispatcher {
Table createdTable = Mockito.mock(Table.class);
Mockito.when(dispatcher.createTable(any(), any(), any(), any(), any(),
any(), any(), any()))
.thenReturn(createdTable);
- TableHookDispatcher hook =
- new TableHookDispatcher(dispatcher, () -> ownerDispatcher,
catalogManager);
+ TableDispatcher hook =
+ new TableNormalizeDispatcher(
+ new TableHookDispatcher(dispatcher, () -> ownerDispatcher),
catalogManager);
NameIdentifier ident = NameIdentifier.of(METALAKE, CATALOG, "SCHEMA_NORM",
"MY_TABLE");
assertSame(
@@ -123,7 +124,7 @@ public class TestTableHookDispatcher {
Table createdTable = Mockito.mock(Table.class);
Mockito.when(dispatcher.createTable(any(), any(), any(), any(), any(),
any(), any(), any()))
.thenReturn(createdTable);
- TableHookDispatcher hook = new TableHookDispatcher(dispatcher, () -> null,
catalogManager);
+ TableHookDispatcher hook = new TableHookDispatcher(dispatcher, () -> null);
assertSame(
createdTable,
@@ -153,8 +154,7 @@ public class TestTableHookDispatcher {
BaseCatalog<?> catalog = Mockito.mock(BaseCatalog.class);
Mockito.when(catalog.capability()).thenReturn(Capability.DEFAULT);
CatalogTestUtils.mockDoWithCatalog(catalogManager, catalog);
- TableHookDispatcher hook =
- new TableHookDispatcher(dispatcher, () -> ownerDispatcher,
catalogManager);
+ TableHookDispatcher hook = new TableHookDispatcher(dispatcher, () ->
ownerDispatcher);
RuntimeException thrown =
assertThrows(
@@ -176,8 +176,7 @@ public class TestTableHookDispatcher {
@Test
public void testRenameAuthorizationPrivilege() {
TableDispatcher dispatcher = Mockito.mock(TableDispatcher.class);
- TableHookDispatcher hook =
- new TableHookDispatcher(dispatcher, () -> null,
Mockito.mock(CatalogManager.class));
+ TableHookDispatcher hook = new TableHookDispatcher(dispatcher, () -> null);
NameIdentifier ident = NameIdentifier.of(METALAKE, CATALOG, "schema",
"table");
Table alteredTable = Mockito.mock(Table.class);
TableChange setChange = TableChange.setProperty("key", "value");
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestTopicHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestTopicHookDispatcher.java
index 91da438bcf..e4294b2c17 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestTopicHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestTopicHookDispatcher.java
@@ -36,6 +36,7 @@ import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.TestOperationDispatcher;
import org.apache.gravitino.catalog.TestTopicOperationDispatcher;
import org.apache.gravitino.catalog.TopicDispatcher;
+import org.apache.gravitino.catalog.TopicNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.connector.capability.Capability;
@@ -96,7 +97,9 @@ public class TestTopicHookDispatcher extends
TestOperationDispatcher {
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
try {
- TopicHookDispatcher localHook = new
TopicHookDispatcher(mockTopicDispatcher);
+ TopicDispatcher localHook =
+ new TopicNormalizeDispatcher(
+ new TopicHookDispatcher(mockTopicDispatcher),
mockCatalogManager);
NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"SCHEMA_NORM", "MY_TOPIC");
localHook.createTopic(ident, "comment", null, ImmutableMap.of());
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
index 9bb348c4e4..2041d9163e 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
@@ -34,6 +34,7 @@ import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.catalog.ViewDispatcher;
+import org.apache.gravitino.catalog.ViewNormalizeDispatcher;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
@@ -66,8 +67,9 @@ public class TestViewHookDispatcher {
View createdView = Mockito.mock(View.class);
Mockito.when(dispatcher.createView(any(), any(), any(), any(), any(),
any(), any()))
.thenReturn(createdView);
- ViewHookDispatcher hook =
- new ViewHookDispatcher(dispatcher, () -> ownerDispatcher,
catalogManager);
+ ViewDispatcher hook =
+ new ViewNormalizeDispatcher(
+ new ViewHookDispatcher(dispatcher, () -> ownerDispatcher),
catalogManager);
NameIdentifier ident = NameIdentifier.of(METALAKE, CATALOG, "SCHEMA_NORM",
"MY_VIEW");
try (MockedStatic<PrincipalUtils> principalUtils =
Mockito.mockStatic(PrincipalUtils.class)) {
@@ -90,7 +92,7 @@ public class TestViewHookDispatcher {
View createdView = Mockito.mock(View.class);
Mockito.when(dispatcher.createView(any(), any(), any(), any(), any(),
any(), any()))
.thenReturn(createdView);
- ViewHookDispatcher hook = new ViewHookDispatcher(dispatcher, () -> null,
catalogManager);
+ ViewHookDispatcher hook = new ViewHookDispatcher(dispatcher, () -> null);
assertSame(
createdView, createView(hook, NameIdentifier.of(METALAKE, CATALOG,
"schema", "view")));
@@ -111,8 +113,7 @@ public class TestViewHookDispatcher {
ViewDispatcher dispatcher = Mockito.mock(ViewDispatcher.class);
Mockito.when(dispatcher.createView(any(), any(), any(), any(), any(),
any(), any()))
.thenReturn(Mockito.mock(View.class));
- ViewHookDispatcher hook =
- new ViewHookDispatcher(dispatcher, () -> ownerDispatcher,
catalogManager);
+ ViewHookDispatcher hook = new ViewHookDispatcher(dispatcher, () ->
ownerDispatcher);
RuntimeException thrown =
assertThrows(
@@ -126,8 +127,7 @@ public class TestViewHookDispatcher {
@Test
public void testRenameViewUpdatesAuthorizationMapping() {
ViewDispatcher dispatcher = Mockito.mock(ViewDispatcher.class);
- ViewHookDispatcher hook =
- new ViewHookDispatcher(dispatcher, () -> null,
Mockito.mock(CatalogManager.class));
+ ViewHookDispatcher hook = new ViewHookDispatcher(dispatcher, () -> null);
NameIdentifier ident = NameIdentifier.of(METALAKE, CATALOG, "schema",
"view");
View alteredView = Mockito.mock(View.class);
ViewChange setChange = ViewChange.setProperty("key", "value");
@@ -148,7 +148,7 @@ public class TestViewHookDispatcher {
}
}
- private View createView(ViewHookDispatcher hook, NameIdentifier ident) {
+ private View createView(ViewDispatcher hook, NameIdentifier ident) {
Representation[] representations =
new Representation[] {
SQLRepresentation.builder().withDialect("trino").withSql("SELECT
1").build()
diff --git
a/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
b/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
index f39c355515..b2f34a9b08 100644
---
a/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
+++
b/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
@@ -20,6 +20,7 @@
package org.apache.gravitino.listener.api.event;
import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -28,8 +29,10 @@ import java.util.Arrays;
import java.util.Map;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
+import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.TableDispatcher;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.hook.TableHookDispatcher;
import org.apache.gravitino.listener.DummyEventListener;
import org.apache.gravitino.listener.EventBus;
import org.apache.gravitino.listener.TableEventDispatcher;
@@ -110,6 +113,39 @@ public class TestTableEvent {
Assertions.assertEquals(OperationStatus.UNPROCESSED,
preEvent.operationStatus());
}
+ @Test
+ void testCreateTableOwnerFailureProducesCreateFailureEvent() {
+ DummyEventListener listener = new DummyEventListener();
+ EventBus eventBus = new EventBus(Arrays.asList(listener));
+ OwnerDispatcher ownerDispatcher = mock(OwnerDispatcher.class);
+ doThrow(new RuntimeException("Set owner failed"))
+ .when(ownerDispatcher)
+ .setOwner(any(), any(), any(), any());
+ TableEventDispatcher dispatcherWithHook =
+ new TableEventDispatcher(
+ eventBus, new TableHookDispatcher(mockTableDispatcher(), () ->
ownerDispatcher));
+ NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
"schema", table.name());
+
+ RuntimeException thrown =
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ dispatcherWithHook.createTable(
+ identifier,
+ table.columns(),
+ table.comment(),
+ table.properties(),
+ table.partitioning(),
+ table.distribution(),
+ table.sortOrder(),
+ table.index()));
+
+ Assertions.assertEquals("Set owner failed", thrown.getMessage());
+ Event event = listener.popPostEvent();
+ Assertions.assertEquals(CreateTableFailureEvent.class, event.getClass());
+ Assertions.assertTrue(listener.getPostEvents().isEmpty());
+ }
+
@Test
void testLoadTableEvent() {
NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
index 89f42ca0c2..78a58f71b7 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
@@ -150,38 +150,35 @@ public class RESTService implements
GravitinoAuxiliaryService {
IcebergNamespaceOperationDispatcher namespaceOperationDispatcher =
new IcebergNamespaceOperationExecutor(icebergCatalogWrapperManager,
cleanupManager);
- // Table: HookDispatcher -> EventDispatcher -> OperationExecutor
+ // Table: EventDispatcher -> HookDispatcher -> OperationExecutor
IcebergTableOperationDispatcher icebergTableOperationDispatcher =
new IcebergTableOperationExecutor(icebergCatalogWrapperManager,
cleanupManager);
- IcebergTableOperationDispatcher icebergTableEventDispatcher =
- new IcebergTableEventDispatcher(icebergTableOperationDispatcher,
eventBus, metalakeName);
if (authorizationContext.isAuthorizationEnabled()) {
- icebergTableEventDispatcher =
- new IcebergTableHookDispatcher(icebergTableEventDispatcher,
namespaceOperationDispatcher);
+ icebergTableOperationDispatcher =
+ new IcebergTableHookDispatcher(
+ icebergTableOperationDispatcher, namespaceOperationDispatcher);
}
- IcebergTableOperationDispatcher icebergTableDispatcher =
icebergTableEventDispatcher;
+ IcebergTableOperationDispatcher icebergTableDispatcher =
+ new IcebergTableEventDispatcher(icebergTableOperationDispatcher,
eventBus, metalakeName);
- // View: HookDispatcher -> EventDispatcher -> OperationExecutor
+ // View: EventDispatcher -> HookDispatcher -> OperationExecutor
IcebergViewOperationDispatcher icebergViewOperationDispatcher =
new IcebergViewOperationExecutor(icebergCatalogWrapperManager);
- IcebergViewOperationDispatcher icebergViewEventDispatcher =
- new IcebergViewEventDispatcher(icebergViewOperationDispatcher,
eventBus, metalakeName);
if (authorizationContext.isAuthorizationEnabled()) {
- icebergViewEventDispatcher =
+ icebergViewOperationDispatcher =
new IcebergViewHookDispatcher(
- icebergViewEventDispatcher, namespaceOperationDispatcher,
metalakeName);
+ icebergViewOperationDispatcher, namespaceOperationDispatcher,
metalakeName);
}
- IcebergViewOperationDispatcher icebergViewDispatcher =
icebergViewEventDispatcher;
+ IcebergViewOperationDispatcher icebergViewDispatcher =
+ new IcebergViewEventDispatcher(icebergViewOperationDispatcher,
eventBus, metalakeName);
- // Namespace: HookDispatcher -> EventDispatcher -> OperationExecutor
- IcebergNamespaceOperationDispatcher icebergNamespaceEventDispatcher =
- new IcebergNamespaceEventDispatcher(namespaceOperationDispatcher,
eventBus, metalakeName);
+ // Namespace: EventDispatcher -> HookDispatcher -> OperationExecutor
if (authorizationContext.isAuthorizationEnabled()) {
- icebergNamespaceEventDispatcher =
- new IcebergNamespaceHookDispatcher(icebergNamespaceEventDispatcher);
+ namespaceOperationDispatcher =
+ new IcebergNamespaceHookDispatcher(namespaceOperationDispatcher);
}
IcebergNamespaceOperationDispatcher icebergNamespaceDispatcher =
- icebergNamespaceEventDispatcher;
+ new IcebergNamespaceEventDispatcher(namespaceOperationDispatcher,
eventBus, metalakeName);
config.register(
new AbstractBinder() {
