This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 912302110a [#13557] fix(core): Invalidate the authorizer name-id
mapping on model, tag, policy and job template rename or delete (#13558)
912302110a is described below
commit 912302110a77c330640cf1fd74ce13d3e70ca1cf
Author: Qi Yu <[email protected]>
AuthorDate: Sat Oct 10 09:04:59 2026 +0800
[#13557] fix(core): Invalidate the authorizer name-id mapping on model,
tag, policy and job template rename or delete (#13558)
### What changes were proposed in this pull request?
Invalidate the built-in authorizer's old name-to-ID mapping after a
successful model, tag, policy or job template deletion or rename,
without calling catalog authorization plugins.
- Model, tag, policy and job template hook dispatchers notify the
authorizer after a successful delete or rename. The view hook dispatcher
now also notifies after a successful drop; view rename already did.
- `JobTemplateMetaService` writes `entity_change_log` records for job
template delete and rename. Job templates are not entity-cacheable, so
`JDBCBackend` did not log them before.
- `EntityChangeLogWriter.append` is now the single change-log writer,
replacing the copies in `JDBCBackend`, `ModelMetaService`,
`SemanticModelMetaService` and `JobTemplateMetaService`.
- The JCasbin change-log listener replays TAG, POLICY, JOB and
JOB_TEMPLATE records so peer nodes drop stale mappings.
### Why are the changes needed?
Deleting or renaming these entities and recreating their old names could
leave authorization using the previous object's ID. Tag and policy
change-log records were skipped on peer nodes, and job template changes
were never logged, so a peer could keep the old ID until the cache TTL
(default 3600s).
Fix: #13557
### Does this PR introduce _any_ user-facing change?
No API changes. After a rename or delete/recreate through the public
API, authorization resolves the current object's ID immediately on the
local node. Peer nodes, and internal paths that bypass the hook
dispatchers (such as built-in job template upgrades), pick it up within
one change-log poll interval.
### How was this patch tested?
- Hook tests for model, tag, policy and job template dispatchers,
including failed renames, and for view drop.
- `TestJobTemplateMetaService`: rename and delete write one change-log
record each; comment-only updates, missing deletes, deletes rejected for
active jobs and renames to an existing name write none.
- Real-cache regression tests for model deletion/rename, similarly named
models and model change-log replay.
- Change-log replay tests for TAG, POLICY, JOB and JOB_TEMPLATE records.
- All `org.apache.gravitino.server.authorization.jcasbin` tests, the
hook tests, `TestJobManager`, `TestJobManagerMultiNode`,
`TestJobMetaService`, `TestEntityChangeLogService`,
`TestEntityStorageChangeLog`, `TestEntityChangeLogPoller`,
`TestEntityChangeLogMapper` and the Model, SemanticModel, Fileset and
Table meta service tests pass. Spotless checks pass.
---
.../apache/gravitino/hook/JobHookDispatcher.java | 22 ++-
.../apache/gravitino/hook/ModelHookDispatcher.java | 15 +-
.../gravitino/hook/PolicyHookDispatcher.java | 17 ++-
.../apache/gravitino/hook/TagHookDispatcher.java | 17 ++-
.../apache/gravitino/hook/ViewHookDispatcher.java | 8 +-
.../storage/relational/EntityChangeLogWriter.java | 51 +++++++
.../gravitino/storage/relational/JDBCBackend.java | 20 +--
.../relational/service/JobTemplateMetaService.java | 16 ++-
.../relational/service/ModelMetaService.java | 41 ++----
.../service/SemanticModelMetaService.java | 46 ++----
.../gravitino/hook/TestJobHookDispatcher.java | 71 ++++++++++
.../gravitino/hook/TestModelHookDispatcher.java | 70 +++++++++
.../gravitino/hook/TestPolicyHookDispatcher.java | 71 ++++++++++
.../gravitino/hook/TestTagHookDispatcher.java | 66 +++++++++
.../gravitino/hook/TestViewHookDispatcher.java | 23 +++
.../service/TestJobTemplateMetaService.java | 85 +++++++++++
.../jcasbin/JcasbinChangeListener.java | 29 ++--
.../jcasbin/TestJcasbinAuthorizationCacheKeys.java | 15 ++
.../jcasbin/TestJcasbinChangePoller.java | 37 +++--
.../jcasbin/TestJcasbinModelCacheInvalidation.java | 157 +++++++++++++++++++++
20 files changed, 758 insertions(+), 119 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
index d6ca24eff2..320ab0ec0e 100644
--- a/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
@@ -19,11 +19,13 @@
package org.apache.gravitino.hook;
import java.io.IOException;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.InUseException;
@@ -74,14 +76,30 @@ public class JobHookDispatcher implements
JobOperationDispatcher {
@Override
public boolean deleteJobTemplate(String metalake, String jobTemplateName)
throws InUseException {
- return jobOperationDispatcher.deleteJobTemplate(metalake, jobTemplateName);
+ boolean deleted = jobOperationDispatcher.deleteJobTemplate(metalake,
jobTemplateName);
+ if (deleted) {
+ // A job template registered later under the same name gets a new id, so
drop the cached
+ // mapping.
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofJobTemplate(metalake, jobTemplateName),
+ Entity.EntityType.JOB_TEMPLATE);
+ }
+ return deleted;
}
@Override
public JobTemplateEntity alterJobTemplate(
String metalake, String jobTemplateName, JobTemplateChange... changes)
throws NoSuchJobTemplateException, IllegalArgumentException {
- return jobOperationDispatcher.alterJobTemplate(metalake, jobTemplateName,
changes);
+ JobTemplateEntity alteredJobTemplate =
+ jobOperationDispatcher.alterJobTemplate(metalake, jobTemplateName,
changes);
+ if (Arrays.stream(changes)
+ .anyMatch(change -> change instanceof
JobTemplateChange.RenameJobTemplate)) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofJobTemplate(metalake, jobTemplateName),
+ Entity.EntityType.JOB_TEMPLATE);
+ }
+ return alteredJobTemplate;
}
@Override
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 facad41909..cb2b28f343 100644
--- a/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
@@ -19,11 +19,13 @@
package org.apache.gravitino.hook;
+import java.util.Arrays;
import java.util.Map;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
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.ModelDispatcher;
@@ -82,7 +84,12 @@ public class ModelHookDispatcher implements ModelDispatcher {
@Override
public boolean deleteModel(NameIdentifier ident) {
- return dispatcher.deleteModel(ident);
+ boolean deleted = dispatcher.deleteModel(ident);
+ if (deleted) {
+ // A model registered later under the same name gets a new id, so drop
the cached mapping.
+ AuthorizationUtils.notifyEntityNameIdMappingChange(ident,
Entity.EntityType.MODEL);
+ }
+ return deleted;
}
@Override
@@ -181,7 +188,11 @@ public class ModelHookDispatcher implements
ModelDispatcher {
@Override
public Model alterModel(NameIdentifier ident, ModelChange... changes)
throws NoSuchModelException, IllegalArgumentException {
- return dispatcher.alterModel(ident, changes);
+ Model alteredModel = dispatcher.alterModel(ident, changes);
+ if (Arrays.stream(changes).anyMatch(change -> change instanceof
ModelChange.RenameModel)) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(ident,
Entity.EntityType.MODEL);
+ }
+ return alteredModel;
}
@Override
diff --git
a/core/src/main/java/org/apache/gravitino/hook/PolicyHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/PolicyHookDispatcher.java
index 34394357b3..1d08d80f53 100644
--- a/core/src/main/java/org/apache/gravitino/hook/PolicyHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/PolicyHookDispatcher.java
@@ -17,10 +17,12 @@
package org.apache.gravitino.hook;
+import java.util.Arrays;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.RelationalEntity;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.NoSuchPolicyException;
@@ -82,7 +84,12 @@ public class PolicyHookDispatcher implements
PolicyDispatcher {
@Override
public PolicyEntity alterPolicy(String metalake, String policyName,
PolicyChange... changes) {
- return dispatcher.alterPolicy(metalake, policyName, changes);
+ PolicyEntity alteredPolicy = dispatcher.alterPolicy(metalake, policyName,
changes);
+ if (Arrays.stream(changes).anyMatch(change -> change instanceof
PolicyChange.RenamePolicy)) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofPolicy(metalake, policyName),
Entity.EntityType.POLICY);
+ }
+ return alteredPolicy;
}
@Override
@@ -97,7 +104,13 @@ public class PolicyHookDispatcher implements
PolicyDispatcher {
@Override
public boolean deletePolicy(String metalake, String policyName) {
- return dispatcher.deletePolicy(metalake, policyName);
+ boolean deleted = dispatcher.deletePolicy(metalake, policyName);
+ if (deleted) {
+ // A policy created later under the same name gets a new id, so drop the
cached mapping.
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofPolicy(metalake, policyName),
Entity.EntityType.POLICY);
+ }
+ return deleted;
}
@Override
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
index 0662767e3e..fcc470c17c 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
@@ -17,11 +17,13 @@
package org.apache.gravitino.hook;
+import java.util.Arrays;
import java.util.Map;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.RelationalEntity;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.NoSuchTagException;
@@ -90,12 +92,23 @@ public class TagHookDispatcher implements TagDispatcher {
@Override
public Tag alterTag(String metalake, String name, TagChange... changes)
throws IllegalArgumentException, TagAlreadyExistsException {
- return dispatcher.alterTag(metalake, name, changes);
+ Tag alteredTag = dispatcher.alterTag(metalake, name, changes);
+ if (Arrays.stream(changes).anyMatch(change -> change instanceof
TagChange.RenameTag)) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofTag(metalake, name), Entity.EntityType.TAG);
+ }
+ return alteredTag;
}
@Override
public boolean deleteTag(String metalake, String name) {
- return dispatcher.deleteTag(metalake, name);
+ boolean deleted = dispatcher.deleteTag(metalake, name);
+ if (deleted) {
+ // A tag created later under the same name gets a new id, so drop the
cached mapping.
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofTag(metalake, name), Entity.EntityType.TAG);
+ }
+ return deleted;
}
@Override
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 1f871682e9..c24117ef5f 100644
--- a/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/ViewHookDispatcher.java
@@ -118,6 +118,12 @@ public class ViewHookDispatcher implements ViewDispatcher {
@Override
public boolean dropView(NameIdentifier ident) {
- return dispatcher.dropView(ident);
+ boolean dropped = dispatcher.dropView(ident);
+ if (dropped) {
+ // A view created later under the same name gets a new id, so drop the
cached mapping. Views
+ // have no catalog authorization plugin privileges to remove.
+ AuthorizationUtils.notifyEntityNameIdMappingChange(ident,
Entity.EntityType.VIEW);
+ }
+ return dropped;
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogWriter.java
b/core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogWriter.java
new file mode 100644
index 0000000000..54e750b69a
--- /dev/null
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/EntityChangeLogWriter.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage.relational;
+
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
+import org.apache.gravitino.storage.relational.po.cache.OperateType;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/** Appends {@code entity_change_log} records that peer nodes replay to
invalidate their caches. */
+public final class EntityChangeLogWriter {
+
+ private EntityChangeLogWriter() {}
+
+ /**
+ * Appends a change record in the caller's transaction, so it commits or
rolls back with the
+ * change it describes.
+ *
+ * @param ident the name that peers may have cached: the pre-mutation name
for an alter and the
+ * current name for a drop
+ * @param entityType the entity type
+ * @param operateType the change operation
+ */
+ public static void append(
+ NameIdentifier ident, Entity.EntityType entityType, OperateType
operateType) {
+ String metalake = NameIdentifierUtil.getMetalake(ident);
+ String fullName = EntityChangeLogNameIdentifierCodec.encode(ident);
+ SessionUtils.doWithoutCommit(
+ EntityChangeLogMapper.class,
+ mapper -> mapper.insertEntityChange(metalake, entityType.name(),
fullName, operateType));
+ EntityChangeLogDiagnostics.logAppended(metalake, entityType.name(),
operateType, fullName);
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
index b6f184fa8c..995b385a4e 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
@@ -72,7 +72,6 @@ import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.meta.ViewEntity;
import
org.apache.gravitino.storage.relational.converters.SQLExceptionConverterFactory;
import org.apache.gravitino.storage.relational.database.H2Database;
-import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
import org.apache.gravitino.storage.relational.po.cache.OperateType;
import org.apache.gravitino.storage.relational.service.CatalogMetaService;
import org.apache.gravitino.storage.relational.service.FilesetMetaService;
@@ -100,7 +99,6 @@ import
org.apache.gravitino.storage.relational.service.ViewMetaService;
import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper;
import org.apache.gravitino.storage.relational.utils.SessionUtils;
import org.apache.gravitino.tag.TagValue;
-import org.apache.gravitino.utils.NameIdentifierUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -207,7 +205,7 @@ public class JDBCBackend implements RelationalBackend,
SupportsOrphanedRelationC
boolean committed = false;
try {
insertEntity(e, true);
- insertEntityChange(e.nameIdentifier(), e.type(), OperateType.ALTER);
+ EntityChangeLogWriter.append(e.nameIdentifier(), e.type(),
OperateType.ALTER);
if (transactionOwner) {
SessionUtils.commitTransaction();
}
@@ -234,7 +232,7 @@ public class JDBCBackend implements RelationalBackend,
SupportsOrphanedRelationC
boolean committed = false;
try {
E updatedEntity = updateEntity(ident, entityType, updater);
- insertEntityChange(ident, entityType, OperateType.ALTER);
+ EntityChangeLogWriter.append(ident, entityType, OperateType.ALTER);
if (transactionOwner) {
SessionUtils.commitTransaction();
}
@@ -399,7 +397,7 @@ public class JDBCBackend implements RelationalBackend,
SupportsOrphanedRelationC
try {
boolean deleted = deleteEntity(ident, entityType, cascade);
if (deleted) {
- insertEntityChange(ident, entityType, OperateType.DROP);
+ EntityChangeLogWriter.append(ident, entityType, OperateType.DROP);
}
if (transactionOwner) {
SessionUtils.commitTransaction();
@@ -439,7 +437,7 @@ public class JDBCBackend implements RelationalBackend,
SupportsOrphanedRelationC
// reported to the caller as "there was nothing to delete".
return Optional.empty();
}
- insertEntityChange(ident, entityType, OperateType.DROP);
+ EntityChangeLogWriter.append(ident, entityType, OperateType.DROP);
E deletedEntity = clazz.cast(deletedFileset);
// Run external cleanup while the metadata delete can still be rolled
back. The callback uses
// the same snapshot whose OCC token won above, so it cannot act on
stale locations.
@@ -1119,16 +1117,6 @@ public class JDBCBackend implements RelationalBackend,
SupportsOrphanedRelationC
}
}
- private static void insertEntityChange(
- NameIdentifier ident, Entity.EntityType entityType, OperateType
operateType) {
- String metalake = NameIdentifierUtil.getMetalake(ident);
- String fullName = EntityChangeLogNameIdentifierCodec.encode(ident);
- SessionUtils.doWithoutCommit(
- EntityChangeLogMapper.class,
- mapper -> mapper.insertEntityChange(metalake, entityType.name(),
fullName, operateType));
- EntityChangeLogDiagnostics.logAppended(metalake, entityType.name(),
operateType, fullName);
- }
-
private static boolean shouldRecordEntityDrop(Entity.EntityType entityType) {
// Functions bypass the Entity Store cache, but their drops must still
invalidate JCasbin's
// name-to-ID cache on peer nodes.
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/JobTemplateMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/JobTemplateMetaService.java
index 9f5dffbfa8..9628fa7f7e 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/JobTemplateMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/JobTemplateMetaService.java
@@ -35,9 +35,11 @@ import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.exceptions.NonEmptyEntityException;
import org.apache.gravitino.meta.JobTemplateEntity;
import org.apache.gravitino.metrics.Monitored;
+import org.apache.gravitino.storage.relational.EntityChangeLogWriter;
import org.apache.gravitino.storage.relational.mapper.JobMetaMapper;
import org.apache.gravitino.storage.relational.mapper.JobTemplateMetaMapper;
import org.apache.gravitino.storage.relational.po.JobTemplatePO;
+import org.apache.gravitino.storage.relational.po.cache.OperateType;
import org.apache.gravitino.storage.relational.utils.ExceptionUtils;
import org.apache.gravitino.storage.relational.utils.SessionUtils;
@@ -161,7 +163,15 @@ public class JobTemplateMetaService {
JobTemplateMetaMapper.class,
mapper ->
mapper.updateJobTemplateMeta(newJobTemplatePO,
oldJobTemplatePO)),
- () -> writeFailure(jobTemplateIdent, oldJobTemplatePO)));
+ () -> writeFailure(jobTemplateIdent, oldJobTemplatePO)),
+ () -> {
+ // Job templates are not entity-cacheable, so JDBCBackend does not
log their changes.
+ // Peers still cache the old name's id for authorization, so log a
rename here.
+ if (!Objects.equals(oldJobTemplateEntity.name(),
newJobTemplateEntity.name())) {
+ EntityChangeLogWriter.append(
+ jobTemplateIdent, Entity.EntityType.JOB_TEMPLATE,
OperateType.ALTER);
+ }
+ });
} catch (RuntimeException e) {
ExceptionUtils.checkSQLException(e, Entity.EntityType.JOB_TEMPLATE,
jobTemplateIdent.name());
throw e;
@@ -248,7 +258,9 @@ public class JobTemplateMetaService {
() ->
SessionUtils.doWithoutCommit(
JobMetaMapper.class,
- mapper ->
mapper.softDeleteJobsByTemplateId(observed.jobTemplateId())));
+ mapper ->
mapper.softDeleteJobsByTemplateId(observed.jobTemplateId())),
+ () ->
+ EntityChangeLogWriter.append(ident,
Entity.EntityType.JOB_TEMPLATE, OperateType.DROP));
}
/** Locks the observed template while a job is inserted in the same
transaction. */
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java
index 49f5e7d260..1b209445f1 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java
@@ -38,9 +38,7 @@ import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.meta.ModelEntity;
import org.apache.gravitino.meta.NamespacedEntityId;
import org.apache.gravitino.metrics.Monitored;
-import org.apache.gravitino.storage.relational.EntityChangeLogDiagnostics;
-import
org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
-import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
+import org.apache.gravitino.storage.relational.EntityChangeLogWriter;
import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper;
import
org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper;
import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper;
@@ -132,9 +130,8 @@ public class ModelMetaService {
String metalakeName = ident.namespace().level(0);
String catalogName = ident.namespace().level(1);
String schemaName = ident.namespace().level(2);
- String modelFullName =
- EntityChangeLogNameIdentifierCodec.encode(
- NameIdentifierUtil.ofModel(metalakeName, catalogName, schemaName,
ident.name()));
+ NameIdentifier modelIdent =
+ NameIdentifierUtil.ofModel(metalakeName, catalogName, schemaName,
ident.name());
// Delete the model row first, and only when its concurrency version still
matches the value
// read above. If another writer changed the model, stop before removing
any related data.
@@ -142,18 +139,8 @@ public class ModelMetaService {
SessionUtils.doMultipleWithCommit(
() -> deleteModelWithVersion(ident, modelPO),
() -> deleteModelDependents(modelPO),
- () -> {
- SessionUtils.doWithoutCommit(
- EntityChangeLogMapper.class,
- mapper ->
- mapper.insertEntityChange(
- metalakeName,
- Entity.EntityType.MODEL.name(),
- modelFullName,
- OperateType.DROP));
- EntityChangeLogDiagnostics.logAppended(
- metalakeName, Entity.EntityType.MODEL.name(),
OperateType.DROP, modelFullName);
- });
+ () ->
+ EntityChangeLogWriter.append(modelIdent,
Entity.EntityType.MODEL, OperateType.DROP));
} catch (NoSuchEntityException e) {
// Another writer dropped the model between the read above and this
transaction. A drop that
// finds nothing to drop is reported the same way as the read above
reports it, so that a
@@ -338,10 +325,8 @@ public class ModelMetaService {
String metalakeName = identifier.namespace().level(0);
String catalogName = identifier.namespace().level(1);
String schemaName = identifier.namespace().level(2);
- String oldFullName =
- EntityChangeLogNameIdentifierCodec.encode(
- NameIdentifierUtil.ofModel(
- metalakeName, catalogName, schemaName, oldModelEntity.name()));
+ NameIdentifier oldModelIdent =
+ NameIdentifierUtil.ofModel(metalakeName, catalogName, schemaName,
oldModelEntity.name());
boolean isRenamed = !Objects.equals(oldModelEntity.name(),
newEntity.name());
try {
@@ -368,16 +353,8 @@ public class ModelMetaService {
},
() -> {
if (isRenamed) {
- SessionUtils.doWithoutCommit(
- EntityChangeLogMapper.class,
- mapper ->
- mapper.insertEntityChange(
- metalakeName,
- Entity.EntityType.MODEL.name(),
- oldFullName,
- OperateType.ALTER));
- EntityChangeLogDiagnostics.logAppended(
- metalakeName, Entity.EntityType.MODEL.name(),
OperateType.ALTER, oldFullName);
+ EntityChangeLogWriter.append(
+ oldModelIdent, Entity.EntityType.MODEL,
OperateType.ALTER);
}
});
} catch (RuntimeException re) {
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelMetaService.java
index 4baffc78ee..624d038866 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelMetaService.java
@@ -40,8 +40,7 @@ import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.exceptions.OptimisticLockException;
import org.apache.gravitino.meta.SemanticModelEntity;
import org.apache.gravitino.metrics.Monitored;
-import
org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
-import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
+import org.apache.gravitino.storage.relational.EntityChangeLogWriter;
import org.apache.gravitino.storage.relational.mapper.SemanticModelMetaMapper;
import
org.apache.gravitino.storage.relational.mapper.SemanticModelVersionInfoMapper;
import org.apache.gravitino.storage.relational.po.SemanticModelPO;
@@ -224,13 +223,9 @@ public class SemanticModelMetaService {
String metalakeName = identifier.namespace().level(0);
String catalogName = identifier.namespace().level(1);
String schemaName = identifier.namespace().level(2);
- String oldFullName =
- EntityChangeLogNameIdentifierCodec.encode(
- NameIdentifierUtil.ofSemanticModel(
- metalakeName,
- catalogName,
- schemaName,
- oldSemanticModelPO.getSemanticModelName()));
+ NameIdentifier oldSemanticModelIdent =
+ NameIdentifierUtil.ofSemanticModel(
+ metalakeName, catalogName, schemaName,
oldSemanticModelPO.getSemanticModelName());
boolean isRenamed =
!Objects.equals(
oldSemanticModelPO.getSemanticModelName(),
newSemanticModelPO.getSemanticModelName());
@@ -263,14 +258,8 @@ public class SemanticModelMetaService {
newSemanticModelPO.getSemanticModelVersionInfoPO())),
() -> {
if (isRenamed && updateResult.get() > 0) {
- SessionUtils.doWithoutCommit(
- EntityChangeLogMapper.class,
- mapper ->
- mapper.insertEntityChange(
- metalakeName,
- Entity.EntityType.SEMANTIC_MODEL.name(),
- oldFullName,
- OperateType.ALTER));
+ EntityChangeLogWriter.append(
+ oldSemanticModelIdent, Entity.EntityType.SEMANTIC_MODEL,
OperateType.ALTER);
}
});
return newEntity;
@@ -374,13 +363,12 @@ public class SemanticModelMetaService {
NameIdentifier identifier, SemanticModelPO observedSemanticModelPO) {
Long semanticModelId = observedSemanticModelPO.getSemanticModelId();
String metalakeName = identifier.namespace().level(0);
- String semanticModelFullName =
- EntityChangeLogNameIdentifierCodec.encode(
- NameIdentifierUtil.ofSemanticModel(
- metalakeName,
- identifier.namespace().level(1),
- identifier.namespace().level(2),
- observedSemanticModelPO.getSemanticModelName()));
+ NameIdentifier semanticModelIdent =
+ NameIdentifierUtil.ofSemanticModel(
+ metalakeName,
+ identifier.namespace().level(1),
+ identifier.namespace().level(2),
+ observedSemanticModelPO.getSemanticModelName());
// TODO: Soft-delete Semantic Model owner, tag, and securable-object
relations in this
// transaction and in the metalake/catalog/schema cascade delete paths.
SessionUtils.doMultipleWithCommit(
@@ -398,14 +386,8 @@ public class SemanticModelMetaService {
SemanticModelVersionInfoMapper.class,
mapper ->
mapper.softDeleteSemanticModelVersionsBySemanticModelId(semanticModelId)),
() ->
- SessionUtils.doWithoutCommit(
- EntityChangeLogMapper.class,
- mapper ->
- mapper.insertEntityChange(
- metalakeName,
- Entity.EntityType.SEMANTIC_MODEL.name(),
- semanticModelFullName,
- OperateType.DROP)));
+ EntityChangeLogWriter.append(
+ semanticModelIdent, Entity.EntityType.SEMANTIC_MODEL,
OperateType.DROP));
}
private SemanticModelPO updateSemanticModelPO(
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestJobHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestJobHookDispatcher.java
index baea025fc4..d1f95b1a0b 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestJobHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestJobHookDispatcher.java
@@ -21,17 +21,22 @@ package org.apache.gravitino.hook;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Collections;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.job.JobOperationDispatcher;
+import org.apache.gravitino.job.JobTemplateChange;
import org.apache.gravitino.meta.JobEntity;
import org.apache.gravitino.meta.JobTemplateEntity;
+import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -46,6 +51,8 @@ public class TestJobHookDispatcher {
// Save the original ownerDispatcher before each test and restore it in
tearDown so we do not
// leak null state into the GravitinoEnv singleton across tests.
private OwnerDispatcher savedOwnerDispatcher;
+ private GravitinoAuthorizer mockAuthorizer;
+ private GravitinoAuthorizer savedAuthorizer;
@BeforeEach
public void setUp() throws IllegalAccessException {
@@ -54,6 +61,9 @@ public class TestJobHookDispatcher {
savedOwnerDispatcher =
GravitinoEnv.getInstance().internalOwnerDispatcher();
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
+ mockAuthorizer = mock(GravitinoAuthorizer.class);
+ savedAuthorizer = GravitinoEnv.getInstance().gravitinoAuthorizer();
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(mockAuthorizer);
hookDispatcher = new JobHookDispatcher(mockDispatcher);
}
@@ -61,6 +71,7 @@ public class TestJobHookDispatcher {
public void tearDown() throws IllegalAccessException {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
savedOwnerDispatcher, true);
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(savedAuthorizer);
}
@Test
@@ -104,4 +115,64 @@ public class TestJobHookDispatcher {
Assertions.assertEquals("Set owner failed", thrown.getMessage());
verify(mockDispatcher).runJob(any(), any(), any());
}
+
+ @Test
+ public void testDeleteJobTemplateInvalidatesNameIdMapping() {
+ when(mockDispatcher.deleteJobTemplate("test_metalake",
"test_template")).thenReturn(true);
+
+ Assertions.assertTrue(hookDispatcher.deleteJobTemplate("test_metalake",
"test_template"));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofJobTemplate("test_metalake", "test_template"),
+ Entity.EntityType.JOB_TEMPLATE);
+ }
+
+ @Test
+ public void testDeleteMissingJobTemplateDoesNotInvalidateNameIdMapping() {
+ when(mockDispatcher.deleteJobTemplate("test_metalake",
"test_template")).thenReturn(false);
+
+ Assertions.assertFalse(hookDispatcher.deleteJobTemplate("test_metalake",
"test_template"));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testRenameJobTemplateInvalidatesOldNameIdMapping() {
+ JobTemplateChange[] changes = {
+ JobTemplateChange.updateComment("new comment"),
JobTemplateChange.rename("new_template")
+ };
+ JobTemplateEntity mockTemplate = mock(JobTemplateEntity.class);
+ when(mockDispatcher.alterJobTemplate("test_metalake", "test_template",
changes))
+ .thenReturn(mockTemplate);
+
+ Assertions.assertSame(
+ mockTemplate, hookDispatcher.alterJobTemplate("test_metalake",
"test_template", changes));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofJobTemplate("test_metalake", "test_template"),
+ Entity.EntityType.JOB_TEMPLATE);
+ }
+
+ @Test
+ public void
testAlterJobTemplateWithoutRenameDoesNotInvalidateNameIdMapping() {
+ JobTemplateChange[] changes = {JobTemplateChange.updateComment("new
comment")};
+ when(mockDispatcher.alterJobTemplate("test_metalake", "test_template",
changes))
+ .thenReturn(mock(JobTemplateEntity.class));
+
+ hookDispatcher.alterJobTemplate("test_metalake", "test_template", changes);
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testFailedRenameJobTemplateDoesNotInvalidateNameIdMapping() {
+ JobTemplateChange[] changes = {JobTemplateChange.rename("new_template")};
+ when(mockDispatcher.alterJobTemplate("test_metalake", "test_template",
changes))
+ .thenThrow(new RuntimeException("Alter failed"));
+
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () -> hookDispatcher.alterJobTemplate("test_metalake",
"test_template", changes));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
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 25df575ca3..d2fc6b0090 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestModelHookDispatcher.java
@@ -22,14 +22,17 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Collections;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.CatalogManager;
@@ -40,6 +43,7 @@ import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
import org.apache.gravitino.model.Model;
+import org.apache.gravitino.model.ModelChange;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -57,6 +61,8 @@ public class TestModelHookDispatcher {
// state into the GravitinoEnv singleton across tests.
private OwnerDispatcher savedOwnerDispatcher;
private CatalogManager savedCatalogManager;
+ private GravitinoAuthorizer mockAuthorizer;
+ private GravitinoAuthorizer savedAuthorizer;
@BeforeEach
public void setUp() throws Exception {
@@ -75,6 +81,9 @@ public class TestModelHookDispatcher {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager",
mockCatalogManager, true);
+ mockAuthorizer = mock(GravitinoAuthorizer.class);
+ savedAuthorizer = GravitinoEnv.getInstance().gravitinoAuthorizer();
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(mockAuthorizer);
hookDispatcher =
new ModelNormalizeDispatcher(new ModelHookDispatcher(mockDispatcher),
mockCatalogManager);
}
@@ -84,6 +93,7 @@ public class TestModelHookDispatcher {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
savedOwnerDispatcher, true);
FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager",
savedCatalogManager, true);
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(savedAuthorizer);
}
@Test
@@ -166,6 +176,66 @@ public class TestModelHookDispatcher {
+ " ident.name() was used as the metalake arg");
}
+ @Test
+ public void testDeleteModelInvalidatesNormalizedNameIdMapping() {
+ when(mockCatalog.capability()).thenReturn(new CaseInsensitiveCapability());
+ NameIdentifier normalizedIdent =
+ NameIdentifier.of("test_metalake", "test_catalog", "test_schema",
"my_model");
+ when(mockDispatcher.deleteModel(normalizedIdent)).thenReturn(true);
+
+ Assertions.assertTrue(
+ hookDispatcher.deleteModel(
+ NameIdentifier.of("test_metalake", "test_catalog", "TEST_SCHEMA",
"MY_MODEL")));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange("test_metalake", normalizedIdent,
Entity.EntityType.MODEL);
+ }
+
+ @Test
+ public void testDeleteMissingModelDoesNotInvalidateNameIdMapping() {
+ NameIdentifier ident =
+ NameIdentifier.of("test_metalake", "test_catalog", "test_schema",
"test_model");
+ when(mockDispatcher.deleteModel(ident)).thenReturn(false);
+
+ Assertions.assertFalse(hookDispatcher.deleteModel(ident));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testRenameModelInvalidatesOldNameIdMapping() {
+ NameIdentifier ident =
+ NameIdentifier.of("test_metalake", "test_catalog", "test_schema",
"test_model");
+ ModelChange[] changes = {ModelChange.updateComment("new comment"),
ModelChange.rename("new")};
+ Model mockModel = mock(Model.class);
+ when(mockDispatcher.alterModel(eq(ident),
any(ModelChange[].class))).thenReturn(mockModel);
+
+ Assertions.assertSame(mockModel, hookDispatcher.alterModel(ident,
changes));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange("test_metalake", ident,
Entity.EntityType.MODEL);
+ }
+
+ @Test
+ public void testAlterModelWithoutRenameDoesNotInvalidateNameIdMapping() {
+ NameIdentifier ident =
+ NameIdentifier.of("test_metalake", "test_catalog", "test_schema",
"test_model");
+ when(mockDispatcher.alterModel(eq(ident), any(ModelChange[].class)))
+ .thenReturn(mock(Model.class));
+
+ hookDispatcher.alterModel(ident, ModelChange.updateComment("new comment"));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testFailedRenameModelDoesNotInvalidateNameIdMapping() {
+ NameIdentifier ident =
+ NameIdentifier.of("test_metalake", "test_catalog", "test_schema",
"test_model");
+ when(mockDispatcher.alterModel(eq(ident), any(ModelChange[].class)))
+ .thenThrow(new RuntimeException("Alter failed"));
+
+ Assertions.assertThrows(
+ RuntimeException.class, () -> hookDispatcher.alterModel(ident,
ModelChange.rename("new")));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
private static class CaseInsensitiveCapability implements Capability {
@Override
public CapabilityResult caseSensitiveOnName(Scope scope) {
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestPolicyHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestPolicyHookDispatcher.java
index fd0ca523fd..a99ba3cd90 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestPolicyHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestPolicyHookDispatcher.java
@@ -22,14 +22,19 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.meta.PolicyEntity;
+import org.apache.gravitino.policy.PolicyChange;
import org.apache.gravitino.policy.PolicyDispatcher;
+import org.apache.gravitino.utils.NameIdentifierUtil;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -43,6 +48,8 @@ public class TestPolicyHookDispatcher {
// Save the original ownerDispatcher before each test and restore it in
tearDown so we do not
// leak null state into the GravitinoEnv singleton across tests.
private OwnerDispatcher savedOwnerDispatcher;
+ private GravitinoAuthorizer mockAuthorizer;
+ private GravitinoAuthorizer savedAuthorizer;
@BeforeEach
public void setUp() throws IllegalAccessException {
@@ -51,6 +58,9 @@ public class TestPolicyHookDispatcher {
savedOwnerDispatcher =
GravitinoEnv.getInstance().internalOwnerDispatcher();
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
+ mockAuthorizer = mock(GravitinoAuthorizer.class);
+ savedAuthorizer = GravitinoEnv.getInstance().gravitinoAuthorizer();
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(mockAuthorizer);
hookDispatcher = new PolicyHookDispatcher(mockDispatcher);
}
@@ -58,6 +68,7 @@ public class TestPolicyHookDispatcher {
public void tearDown() throws IllegalAccessException {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
savedOwnerDispatcher, true);
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(savedAuthorizer);
}
@Test
@@ -79,4 +90,64 @@ public class TestPolicyHookDispatcher {
Assertions.assertEquals("Set owner failed", thrown.getMessage());
verify(mockDispatcher).createPolicy(any(), any(), any(), any(),
anyBoolean(), any());
}
+
+ @Test
+ public void testDeletePolicyInvalidatesNameIdMapping() {
+ when(mockDispatcher.deletePolicy("test_metalake",
"test_policy")).thenReturn(true);
+
+ Assertions.assertTrue(hookDispatcher.deletePolicy("test_metalake",
"test_policy"));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofPolicy("test_metalake", "test_policy"),
+ Entity.EntityType.POLICY);
+ }
+
+ @Test
+ public void testDeleteMissingPolicyDoesNotInvalidateNameIdMapping() {
+ when(mockDispatcher.deletePolicy("test_metalake",
"test_policy")).thenReturn(false);
+
+ Assertions.assertFalse(hookDispatcher.deletePolicy("test_metalake",
"test_policy"));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testRenamePolicyInvalidatesOldNameIdMapping() {
+ PolicyChange[] changes = {
+ PolicyChange.updateComment("new comment"),
PolicyChange.rename("new_policy")
+ };
+ PolicyEntity mockPolicy = mock(PolicyEntity.class);
+ when(mockDispatcher.alterPolicy("test_metalake", "test_policy", changes))
+ .thenReturn(mockPolicy);
+
+ Assertions.assertSame(
+ mockPolicy, hookDispatcher.alterPolicy("test_metalake", "test_policy",
changes));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofPolicy("test_metalake", "test_policy"),
+ Entity.EntityType.POLICY);
+ }
+
+ @Test
+ public void testAlterPolicyWithoutRenameDoesNotInvalidateNameIdMapping() {
+ PolicyChange[] changes = {PolicyChange.updateComment("new comment")};
+ when(mockDispatcher.alterPolicy("test_metalake", "test_policy", changes))
+ .thenReturn(mock(PolicyEntity.class));
+
+ hookDispatcher.alterPolicy("test_metalake", "test_policy", changes);
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testFailedRenamePolicyDoesNotInvalidateNameIdMapping() {
+ PolicyChange[] changes = {PolicyChange.rename("new_policy")};
+ when(mockDispatcher.alterPolicy("test_metalake", "test_policy", changes))
+ .thenThrow(new RuntimeException("Alter failed"));
+
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () -> hookDispatcher.alterPolicy("test_metalake", "test_policy",
changes));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
}
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestTagHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestTagHookDispatcher.java
index 0e92cd403b..6189a4d122 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestTagHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestTagHookDispatcher.java
@@ -21,15 +21,20 @@ package org.apache.gravitino.hook;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Collections;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.tag.Tag;
+import org.apache.gravitino.tag.TagChange;
import org.apache.gravitino.tag.TagDispatcher;
+import org.apache.gravitino.utils.NameIdentifierUtil;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -43,6 +48,8 @@ public class TestTagHookDispatcher {
// Save the original ownerDispatcher before each test and restore it in
tearDown so we do not
// leak null state into the GravitinoEnv singleton across tests.
private OwnerDispatcher savedOwnerDispatcher;
+ private GravitinoAuthorizer mockAuthorizer;
+ private GravitinoAuthorizer savedAuthorizer;
@BeforeEach
public void setUp() throws IllegalAccessException {
@@ -51,6 +58,9 @@ public class TestTagHookDispatcher {
savedOwnerDispatcher =
GravitinoEnv.getInstance().internalOwnerDispatcher();
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
mockOwnerDispatcher, true);
+ mockAuthorizer = mock(GravitinoAuthorizer.class);
+ savedAuthorizer = GravitinoEnv.getInstance().gravitinoAuthorizer();
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(mockAuthorizer);
hookDispatcher = new TagHookDispatcher(mockDispatcher);
}
@@ -58,6 +68,7 @@ public class TestTagHookDispatcher {
public void tearDown() throws IllegalAccessException {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalOwnerDispatcher",
savedOwnerDispatcher, true);
+ GravitinoEnv.getInstance().setGravitinoAuthorizer(savedAuthorizer);
}
@Test
@@ -78,4 +89,59 @@ public class TestTagHookDispatcher {
Assertions.assertEquals("Set owner failed", thrown.getMessage());
verify(mockDispatcher).createTag(any(), any(), any(), any(), any());
}
+
+ @Test
+ public void testDeleteTagInvalidatesNameIdMapping() {
+ when(mockDispatcher.deleteTag("test_metalake",
"test_tag")).thenReturn(true);
+
+ Assertions.assertTrue(hookDispatcher.deleteTag("test_metalake",
"test_tag"));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofTag("test_metalake", "test_tag"),
+ Entity.EntityType.TAG);
+ }
+
+ @Test
+ public void testDeleteMissingTagDoesNotInvalidateNameIdMapping() {
+ when(mockDispatcher.deleteTag("test_metalake",
"test_tag")).thenReturn(false);
+
+ Assertions.assertFalse(hookDispatcher.deleteTag("test_metalake",
"test_tag"));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testRenameTagInvalidatesOldNameIdMapping() {
+ TagChange[] changes = {TagChange.updateComment("new comment"),
TagChange.rename("new_tag")};
+ Tag mockTag = mock(Tag.class);
+ when(mockDispatcher.alterTag("test_metalake", "test_tag",
changes)).thenReturn(mockTag);
+
+ Assertions.assertSame(mockTag, hookDispatcher.alterTag("test_metalake",
"test_tag", changes));
+ verify(mockAuthorizer)
+ .handleEntityNameIdMappingChange(
+ "test_metalake",
+ NameIdentifierUtil.ofTag("test_metalake", "test_tag"),
+ Entity.EntityType.TAG);
+ }
+
+ @Test
+ public void testAlterTagWithoutRenameDoesNotInvalidateNameIdMapping() {
+ TagChange[] changes = {TagChange.updateComment("new comment")};
+ when(mockDispatcher.alterTag("test_metalake", "test_tag",
changes)).thenReturn(mock(Tag.class));
+
+ hookDispatcher.alterTag("test_metalake", "test_tag", changes);
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
+
+ @Test
+ public void testFailedRenameTagDoesNotInvalidateNameIdMapping() {
+ TagChange[] changes = {TagChange.rename("new_tag")};
+ when(mockDispatcher.alterTag("test_metalake", "test_tag", changes))
+ .thenThrow(new RuntimeException("Alter failed"));
+
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () -> hookDispatcher.alterTag("test_metalake", "test_tag", changes));
+ verify(mockAuthorizer, never()).handleEntityNameIdMappingChange(any(),
any(), any());
+ }
}
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 2041d9163e..d10954a151 100644
--- a/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
+++ b/core/src/test/java/org/apache/gravitino/hook/TestViewHookDispatcher.java
@@ -19,8 +19,10 @@
package org.apache.gravitino.hook;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@@ -148,6 +150,27 @@ public class TestViewHookDispatcher {
}
}
+ @Test
+ public void testDropViewInvalidatesNameIdMappingOnlyWhenDropped() {
+ ViewDispatcher dispatcher = Mockito.mock(ViewDispatcher.class);
+ ViewHookDispatcher hook = new ViewHookDispatcher(dispatcher, () -> null);
+ NameIdentifier dropped = NameIdentifier.of(METALAKE, CATALOG, "schema",
"view");
+ NameIdentifier missing = NameIdentifier.of(METALAKE, CATALOG, "schema",
"missing");
+ Mockito.when(dispatcher.dropView(dropped)).thenReturn(true);
+ Mockito.when(dispatcher.dropView(missing)).thenReturn(false);
+
+ try (MockedStatic<AuthorizationUtils> authorizationUtils =
+ Mockito.mockStatic(AuthorizationUtils.class)) {
+ assertFalse(hook.dropView(missing));
+ authorizationUtils.verifyNoInteractions();
+
+ assertTrue(hook.dropView(dropped));
+ authorizationUtils.verify(
+ () ->
+ AuthorizationUtils.notifyEntityNameIdMappingChange(dropped,
Entity.EntityType.VIEW));
+ }
+ }
+
private View createView(ViewDispatcher hook, NameIdentifier ident) {
Representation[] representations =
new Representation[] {
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobTemplateMetaService.java
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobTemplateMetaService.java
index 7d3fec6f43..31ed1f9100 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobTemplateMetaService.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobTemplateMetaService.java
@@ -21,6 +21,8 @@ package org.apache.gravitino.storage.relational.service;
import java.io.IOException;
import java.time.Instant;
import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.exceptions.NonEmptyEntityException;
@@ -30,7 +32,12 @@ import org.apache.gravitino.meta.BaseMetalake;
import org.apache.gravitino.meta.JobEntity;
import org.apache.gravitino.meta.JobTemplateEntity;
import org.apache.gravitino.storage.RandomIdGenerator;
+import
org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
import org.apache.gravitino.storage.relational.TestJDBCBackend;
+import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
+import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord;
+import org.apache.gravitino.storage.relational.po.cache.OperateType;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.junit.jupiter.api.Assertions;
@@ -197,11 +204,14 @@ public class TestJobTemplateMetaService extends
TestJDBCBackend {
newJobEntity("job_template_with_jobs", JobHandle.Status.SUCCEEDED,
METALAKE_NAME);
backend.insert(jobEntity2, false);
+ long lastChangeId = maxEntityChangeId();
Assertions.assertThrows(
NonEmptyEntityException.class,
() ->
jobTemplateMetaService.deleteJobTemplate(
NameIdentifierUtil.ofJobTemplate(METALAKE_NAME,
"job_template_with_jobs")));
+ // A delete rejected for active jobs logs no change, so peers keep a valid
mapping.
+ Assertions.assertTrue(jobTemplateChangesAfter(lastChangeId).isEmpty());
Assertions.assertEquals(
2,
JobMetaService.getInstance()
@@ -284,11 +294,86 @@ public class TestJobTemplateMetaService extends
TestJDBCBackend {
.withAuditInfo(updatedJobTemplateEntity.auditInfo())
.build();
+ long lastChangeId = maxEntityChangeId();
Assertions.assertThrows(
EntityAlreadyExistsException.class,
() ->
jobTemplateMetaService.updateJobTemplate(
updatedJobTemplateEntity.nameIdentifier(), e ->
duplicateNameJobTemplateEntity));
+ // A rename rejected for a duplicate name logs no change.
+ Assertions.assertTrue(jobTemplateChangesAfter(lastChangeId).isEmpty());
+ }
+
+ @TestTemplate
+ public void testRenameAndDeleteJobTemplateLogChanges() throws IOException {
+ BaseMetalake metalake =
+ createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), METALAKE_NAME,
AUDIT_INFO);
+ backend.insert(metalake, false);
+
+ JobTemplateMetaService jobTemplateMetaService =
JobTemplateMetaService.getInstance();
+ JobTemplateEntity original =
+ newShellJobTemplateEntity("logged_template", "A logged job template",
METALAKE_NAME);
+ jobTemplateMetaService.insertJobTemplate(original, false);
+
+ // A change that keeps the name leaves the cached name-to-id mapping valid.
+ long lastChangeId = maxEntityChangeId();
+ JobTemplateEntity commented = renamedJobTemplate(original,
original.name(), "new comment");
+ jobTemplateMetaService.updateJobTemplate(original.nameIdentifier(), e ->
commented);
+ Assertions.assertTrue(jobTemplateChangesAfter(lastChangeId).isEmpty());
+
+ lastChangeId = maxEntityChangeId();
+ JobTemplateEntity renamed = renamedJobTemplate(commented,
"renamed_template", "renamed");
+ jobTemplateMetaService.updateJobTemplate(commented.nameIdentifier(), e ->
renamed);
+ assertJobTemplateChange(lastChangeId, "logged_template",
OperateType.ALTER);
+
+ lastChangeId = maxEntityChangeId();
+
Assertions.assertTrue(jobTemplateMetaService.deleteJobTemplate(renamed.nameIdentifier()));
+ assertJobTemplateChange(lastChangeId, "renamed_template",
OperateType.DROP);
+
+ // Deleting a missing template changes nothing, so it must not log a
change.
+ lastChangeId = maxEntityChangeId();
+
Assertions.assertFalse(jobTemplateMetaService.deleteJobTemplate(renamed.nameIdentifier()));
+ Assertions.assertTrue(jobTemplateChangesAfter(lastChangeId).isEmpty());
+ }
+
+ private static JobTemplateEntity renamedJobTemplate(
+ JobTemplateEntity entity, String name, String comment) {
+ return JobTemplateEntity.builder()
+ .withId(entity.id())
+ .withName(name)
+ .withNamespace(entity.namespace())
+ .withTemplateContent(entity.templateContent())
+ .withComment(comment)
+ .withAuditInfo(entity.auditInfo())
+ .build();
+ }
+
+ private static long maxEntityChangeId() {
+ Long maxId =
+ SessionUtils.doWithCommitAndFetchResult(
+ EntityChangeLogMapper.class,
EntityChangeLogMapper::selectMaxChangeId);
+ return maxId == null ? 0L : maxId;
+ }
+
+ private static List<EntityChangeRecord> jobTemplateChangesAfter(long
lastConsumedId) {
+ return SessionUtils.doWithCommitAndFetchResult(
+ EntityChangeLogMapper.class, mapper ->
mapper.selectEntityChanges(lastConsumedId, 100))
+ .stream()
+ .filter(change ->
Entity.EntityType.JOB_TEMPLATE.name().equals(change.getEntityType()))
+ .collect(Collectors.toList());
+ }
+
+ private static void assertJobTemplateChange(
+ long lastConsumedId, String jobTemplateName, OperateType operateType) {
+ List<EntityChangeRecord> changes = jobTemplateChangesAfter(lastConsumedId);
+ Assertions.assertEquals(1, changes.size());
+ EntityChangeRecord change = changes.get(0);
+ Assertions.assertEquals(METALAKE_NAME, change.getMetalakeName());
+ Assertions.assertEquals(
+ EntityChangeLogNameIdentifierCodec.encode(
+ NameIdentifierUtil.ofJobTemplate(METALAKE_NAME, jobTemplateName)),
+ change.getFullName());
+ Assertions.assertEquals(operateType, change.getOperateType());
}
static JobEntity newJobEntity(String templateName, JobHandle.Status status,
String metalake) {
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangeListener.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangeListener.java
index 7ef37273e0..d245f3a95c 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangeListener.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangeListener.java
@@ -60,12 +60,11 @@ public class JcasbinChangeListener implements
EntityChangeLogListener, AutoClose
private static final Logger LOG =
LoggerFactory.getLogger(JcasbinChangeListener.class);
/**
- * Entity types that are cacheable in the entity store — and therefore
emitted into {@code
- * entity_change_log} — but that live in a virtual namespace ({@code
- * <metalake>.system.<kind>.<name>}) instead of under a catalog. Their
change-log identifier has
- * more levels than {@link MetadataObjects#of} accepts for the matching
type, and the JCasbin
- * {@code metadataIdCache} never keys on them, so they are dropped before
any mapping is
- * attempted.
+ * Entity types that are emitted into {@code entity_change_log} but live in
a virtual namespace
+ * ({@code <metalake>.system.<kind>.<name>}) instead of under a catalog.
Their change-log
+ * identifier has more levels than {@link MetadataObjects#of} accepts for
the matching type, so
+ * only the leaf name is mapped. Authorization resolves them by that name,
so the JCasbin {@code
+ * metadataIdCache} does key on them and peers must drop the mapping after a
delete or rename.
*/
private static final Set<MetadataObject.Type> VIRTUAL_NAMESPACE_TYPES =
ImmutableSet.of(
@@ -219,11 +218,12 @@ public class JcasbinChangeListener implements
EntityChangeLogListener, AutoClose
* Invalidates the affected {@code metadataIdCache} keys from an
entity-change batch.
*
* <p><b>Contract with the writer side:</b> {@code
entity_change_log.full_name} must be the
- * <i>pre-mutation</i> name (the name that consumers currently have cached).
{@code JDBCBackend}
- * emits the pre-mutation identifier on update and the current name on drop,
so the cacheKey we
- * build here resolves to the entry a peer node would have populated under
that name. If a future
- * change starts emitting the new post-rename name, this invalidation will
silently miss and stale
- * entries will only clear via LRU eviction.
+ * <i>pre-mutation</i> name (the name that consumers currently have cached).
Every writer ({@code
+ * JDBCBackend}, and the services that log types it skips, such as {@code
ModelMetaService} and
+ * {@code JobTemplateMetaService}) emits the pre-mutation identifier on
update and the current
+ * name on drop, so the cacheKey we build here resolves to the entry a peer
node would have
+ * populated under that name. If a future change starts emitting the new
post-rename name, this
+ * invalidation will silently miss and stale entries will only clear via LRU
eviction.
*
* <p><b>Bad rows:</b> a record this listener cannot understand is logged
and skipped instead of
* being thrown up. Such a row does not point at any cache key, so skipping
it leaves nothing
@@ -257,10 +257,6 @@ public class JcasbinChangeListener implements
EntityChangeLogListener, AutoClose
continue;
}
- if (VIRTUAL_NAMESPACE_TYPES.contains(mdType)) {
- continue;
- }
-
String cacheKey;
try {
MetadataObject mdObj = metadataObjectFromChangeLog(metalake, fullName,
mdType);
@@ -303,6 +299,9 @@ public class JcasbinChangeListener implements
EntityChangeLogListener, AutoClose
static MetadataObject metadataObjectFromChangeLog(
String metalake, String fullName, MetadataObject.Type type) {
NameIdentifier ident = EntityChangeLogNameIdentifierCodec.decode(fullName);
+ if (VIRTUAL_NAMESPACE_TYPES.contains(type)) {
+ return MetadataObjects.of(null, ident.name(), type);
+ }
List<String> names = new
ArrayList<>(Arrays.asList(ident.namespace().levels()));
names.add(ident.name());
if (type != MetadataObject.Type.METALAKE
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
index be359f2977..d6c0d2efc0 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
@@ -60,6 +60,12 @@ public class TestJcasbinAuthorizationCacheKeys {
key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "TABLE", "tbl1",
"COLUMN", "col1"),
columnKey);
+ MetadataObject model =
+ MetadataObjects.of(Arrays.asList("cat1", "sch1", "model1"),
MetadataObject.Type.MODEL);
+ String modelKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
model);
+ Assertions.assertEquals(
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "MODEL", "model1"),
modelKey);
+
MetadataObject view =
MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"),
MetadataObject.Type.VIEW);
String viewKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
view);
@@ -80,6 +86,12 @@ public class TestJcasbinAuthorizationCacheKeys {
Assertions.assertTrue(tableKey.startsWith(schemaKey));
Assertions.assertTrue(columnKey.startsWith(tableKey));
Assertions.assertFalse(viewKey.startsWith(tableKey));
+ Assertions.assertNotEquals(
+ modelKey,
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(
+ "ml1",
+ MetadataObjects.of(
+ Arrays.asList("cat1", "sch1", "model10"),
MetadataObject.Type.MODEL)));
}
@Test
@@ -93,6 +105,9 @@ public class TestJcasbinAuthorizationCacheKeys {
Assertions.assertTrue(
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.TABLE));
+ Assertions.assertFalse(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.MODEL));
+
Assertions.assertFalse(
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.VIEW));
Assertions.assertFalse(
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
index 3d4f6c1017..c8e815a926 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
@@ -24,6 +24,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.cache.GravitinoCache;
import
org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
@@ -119,28 +120,38 @@ public class TestJcasbinChangePoller {
}
@Test
- void testVirtualNamespaceTypesAreSkippedWithoutFailingTheBatch() {
+ void testVirtualNamespaceTypesInvalidateLookupKeys() {
RecordingCache<String, Long> metadataIdCache = new RecordingCache<>();
RecordingCache<Long, Optional<OwnerInfo>> ownerRelCache = new
RecordingCache<>();
JcasbinChangeListener poller = new JcasbinChangeListener(metadataIdCache,
ownerRelCache, 1);
- // TAG/POLICY/JOB live in a virtual namespace, so their change-log full
name has more levels
- // than MetadataObjects.of() accepts. They must be skipped, not blow up
the whole batch.
- Assertions.assertDoesNotThrow(
- () ->
- poller.onEntityChange(
- List.of(
- change(1L, MetadataObject.Type.TAG, "ml1.system.tag.pii"),
- change(2L, MetadataObject.Type.POLICY,
"ml1.system.policy.retention"),
- change(3L, MetadataObject.Type.JOB, "ml1.system.job.job1"),
- change(4L, MetadataObject.Type.TABLE,
"ml1.cat1.sch1.tbl1"))));
+ // TAG/POLICY/JOB/JOB_TEMPLATE live in a virtual namespace
(<metalake>.system.<kind>.<name>).
+ // Authorization caches their ids by leaf name, so a peer must drop
exactly that key.
+ poller.onEntityChange(
+ List.of(
+ change(1L, MetadataObject.Type.TAG, "ml1.system.tag.pii"),
+ change(2L, MetadataObject.Type.POLICY,
"ml1.system.policy.retention"),
+ change(3L, MetadataObject.Type.JOB, "ml1.system.job.job1"),
+ change(4L, MetadataObject.Type.JOB_TEMPLATE,
"ml1.system.job_template.tpl1"),
+ change(5L, MetadataObject.Type.TABLE, "ml1.cat1.sch1.tbl1")));
- // The one mappable record in the batch is still applied.
Assertions.assertEquals(
List.of(key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "TABLE",
"tbl1", "")),
metadataIdCache.invalidatedPrefixes);
- Assertions.assertEquals(List.of(), metadataIdCache.invalidatedKeys);
+ Assertions.assertEquals(
+ List.of(
+ lookupKey("pii", MetadataObject.Type.TAG),
+ lookupKey("retention", MetadataObject.Type.POLICY),
+ lookupKey("job1", MetadataObject.Type.JOB),
+ lookupKey("tpl1", MetadataObject.Type.JOB_TEMPLATE)),
+ metadataIdCache.invalidatedKeys);
+ }
+
+ // The key JcasbinAuthorizationLookups builds when authorizing a
metalake-level object by name.
+ private static String lookupKey(String name, MetadataObject.Type type) {
+ return JcasbinAuthorizationCacheKeys.metadataIdCacheKey(
+ "ml1", MetadataObjects.of(null, name, type));
}
@Test
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinModelCacheInvalidation.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinModelCacheInvalidation.java
new file mode 100644
index 0000000000..7a6f67b607
--- /dev/null
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinModelCacheInvalidation.java
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import static org.mockito.Answers.CALLS_REAL_METHODS;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.Optional;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.cache.CaffeineGravitinoCache;
+import org.apache.gravitino.catalog.CapabilityHelpers;
+import org.apache.gravitino.catalog.CatalogManager;
+import org.apache.gravitino.catalog.ModelDispatcher;
+import org.apache.gravitino.catalog.ModelNormalizeDispatcher;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.hook.ModelHookDispatcher;
+import org.apache.gravitino.model.Model;
+import org.apache.gravitino.model.ModelChange;
+import org.apache.gravitino.server.authorization.MetadataIdConverter;
+import
org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
+import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
+import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord;
+import org.apache.gravitino.storage.relational.po.cache.OperateType;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.MockedStatic;
+
+/** Regression tests for invalidating model IDs in the metadata id cache. */
+public class TestJcasbinModelCacheInvalidation {
+
+ private static final String METALAKE = "ml1";
+ private static final MetadataObject MODEL =
+ MetadataObjects.parse("cat1.sch1.model1", MetadataObject.Type.MODEL);
+ private static final MetadataObject OTHER_MODEL =
+ MetadataObjects.parse("cat1.sch1.model10", MetadataObject.Type.MODEL);
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testModelMutationInvalidatesModel(boolean rename) throws Exception {
+ GravitinoEnv env = mock(GravitinoEnv.class);
+ CatalogManager catalogManager = mock(CatalogManager.class);
+ when(env.catalogManager()).thenReturn(catalogManager);
+ ModelDispatcher delegate = mock(ModelDispatcher.class);
+ NameIdentifier normalizedIdent = NameIdentifier.of(METALAKE, "cat1",
"sch1", "model1");
+ when(delegate.deleteModel(normalizedIdent)).thenReturn(true);
+ Model renamedModel = mock(Model.class);
+ when(delegate.alterModel(eq(normalizedIdent), any(ModelChange[].class)))
+ .thenReturn(renamedModel);
+
+ try (CaffeineGravitinoCache<String, Long> metadataIdCache =
+ new CaffeineGravitinoCache<>(60_000L, 100L);
+ CaffeineGravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache =
+ new CaffeineGravitinoCache<>(60_000L, 100L);
+ MockedStatic<GravitinoEnv> environment =
mockStatic(GravitinoEnv.class);
+ MockedStatic<CapabilityHelpers> capabilities =
+ mockStatic(CapabilityHelpers.class, CALLS_REAL_METHODS);
+ MockedStatic<MetadataIdConverter> converter =
mockStatic(MetadataIdConverter.class)) {
+ environment.when(GravitinoEnv::getInstance).thenReturn(env);
+ capabilities
+ .when(() -> CapabilityHelpers.getCapability(any(),
eq(catalogManager)))
+ .thenReturn(Capability.DEFAULT);
+ converter
+ .when(() -> MetadataIdConverter.getID(MODEL, METALAKE))
+ .thenReturn(Optional.of(10L), Optional.of(30L));
+ // Use the real JCasbin hook and shared caches without starting its
background poller.
+ JcasbinAuthorizer authorizer = mock(JcasbinAuthorizer.class,
CALLS_REAL_METHODS);
+ FieldUtils.writeField(authorizer, "metadataIdCache", metadataIdCache,
true);
+ when(env.gravitinoAuthorizer()).thenReturn(authorizer);
+ JcasbinAuthorizationLookups lookups =
+ new JcasbinAuthorizationLookups(metadataIdCache, ownerRelCache);
+ Assertions.assertEquals(
+ Optional.of(10L),
+ lookups.resolveMetadataId(MODEL, METALAKE, new
AuthorizationRequestContext()));
+ String otherKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(METALAKE, OTHER_MODEL);
+ metadataIdCache.put(otherKey, 50L);
+
+ ModelDispatcher dispatcher =
+ new ModelNormalizeDispatcher(new ModelHookDispatcher(delegate),
catalogManager);
+ if (rename) {
+ Assertions.assertSame(
+ renamedModel, dispatcher.alterModel(normalizedIdent,
ModelChange.rename("renamed")));
+ } else {
+ Assertions.assertTrue(dispatcher.deleteModel(normalizedIdent));
+ }
+ Assertions.assertFalse(
+ metadataIdCache
+
.getIfPresent(JcasbinAuthorizationCacheKeys.metadataIdCacheKey(METALAKE, MODEL))
+ .isPresent());
+ Assertions.assertEquals(Optional.of(50L),
metadataIdCache.getIfPresent(otherKey));
+
+ // Simulate recreation under the old name with a fresh model ID.
+ Assertions.assertEquals(
+ Optional.of(30L),
+ lookups.resolveMetadataId(MODEL, METALAKE, new
AuthorizationRequestContext()));
+ converter.verify(() -> MetadataIdConverter.getID(MODEL, METALAKE),
times(2));
+ }
+ }
+
+ @ParameterizedTest
+ @EnumSource(
+ value = OperateType.class,
+ names = {"ALTER", "DROP"})
+ void testModelChangeLogInvalidatesModel(OperateType operation) {
+ try (CaffeineGravitinoCache<String, Long> metadataIdCache =
+ new CaffeineGravitinoCache<>(60_000L, 100L);
+ CaffeineGravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache =
+ new CaffeineGravitinoCache<>(60_000L, 100L);
+ JcasbinChangeListener listener =
+ new JcasbinChangeListener(metadataIdCache, ownerRelCache, 1)) {
+ String modelKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(METALAKE, MODEL);
+ String otherKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(METALAKE, OTHER_MODEL);
+ metadataIdCache.put(modelKey, 10L);
+ metadataIdCache.put(otherKey, 50L);
+ EntityChangeRecord change =
+ new EntityChangeRecord(
+ 1L,
+ METALAKE,
+ "MODEL",
+ EntityChangeLogNameIdentifierCodec.encode(
+ NameIdentifier.of(METALAKE, "cat1", "sch1", "model1")),
+ operation,
+ 1L);
+ listener.onEntityChange(List.of(change));
+
Assertions.assertFalse(metadataIdCache.getIfPresent(modelKey).isPresent());
+ Assertions.assertEquals(Optional.of(50L),
metadataIdCache.getIfPresent(otherKey));
+ }
+ }
+}
