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 887088f6a4 [#12969] refactor(core): Extract a shared
requireMetalakeByName metalake lookup (#14327)
887088f6a4 is described below
commit 887088f6a4a3a3074865fb54b289f3250dbc2207
Author: li jie <[email protected]>
AuthorDate: Wed Oct 28 22:21:42 2026 +0800
[#12969] refactor(core): Extract a shared requireMetalakeByName metalake
lookup (#14327)
### What changes were proposed in this pull request?
- Add `MetalakeMetaService.requireMetalakeByName(String)`, a shared
helper that looks up a metalake by name and throws
`NoSuchEntityException` when it is missing.
- Replace the seven verbatim copies of the "select the metalake by name,
throw if absent" block with calls to the new helper: the insert paths of
`CatalogMetaService`, `PolicyMetaService`, `RoleMetaService`,
`TagMetaService`, `UserMetaService` and `GroupMetaService`, plus the
internal usages in `MetalakeMetaService` (`getMetalakeByIdentifier`,
`updateMetalake`, `deleteMetalake`).
- Remove the `MetalakeMetaMapper` imports that became unused in
`UserMetaService` and `GroupMetaService`.
Per
https://github.com/apache/gravitino/issues/12969#issuecomment-6464513436,
the first item of the issue (backfilling
`OccWriteSupport.updateWithVersion`) is dropped from the scope, so this
PR only covers the shared metalake lookup.
### Why are the changes needed?
The same ten-line "look up the metalake by name, throw
`NoSuchEntityException` if missing" block was duplicated across seven
services. A future change to this error handling would have to be
applied seven times, and any missed copy would reintroduce exactly the
divergence the issue set out to remove. Centralising it keeps every
caller in sync with one implementation.
This is a pure refactor with no behavioural change: a metalake
identifier has no namespace, so `ident.toString()` equals `ident.name()`
and the exception messages are unchanged.
Fix: #12969
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `./gradlew :core:test -PskipITs` `--tests
org.apache.gravitino.storage.relational.service.TestMetalakeMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestCatalogMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestPolicyMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestRoleMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestTagMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestUserMetaService`
`--tests
org.apache.gravitino.storage.relational.service.TestGroupMetaService`
(embedded H2 mode, all pass)
- `./gradlew :core:spotlessCheck :core:compileJava`
- MySQL/PostgreSQL are covered by CI via the docker-test profile
Co-authored-by: lijie <[email protected]>
---
.../relational/service/CatalogMetaService.java | 10 +----
.../relational/service/GroupMetaService.java | 11 +----
.../relational/service/MetalakeMetaService.java | 47 ++++++++++------------
.../relational/service/PolicyMetaService.java | 10 +----
.../relational/service/RoleMetaService.java | 10 +----
.../storage/relational/service/TagMetaService.java | 10 +----
.../relational/service/UserMetaService.java | 11 +----
7 files changed, 27 insertions(+), 82 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java
index 45b12caf2a..14aed3a43c 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java
@@ -188,15 +188,7 @@ public class CatalogMetaService {
// This read runs before the transaction below, so it only tells us the
metalake ID and name
// we start from. The metalake may still be dropped or renamed right
after it. That is why
// lockMetalakeForCatalogCreate checks the row again inside the
transaction.
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalakeName));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalakeName);
- }
+ MetalakePO metalakePO =
MetalakeMetaService.getInstance().requireMetalakeByName(metalakeName);
SessionUtils.doMultipleWithCommit(
() -> lockMetalakeForCatalogCreate(metalakePO),
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
index df06e6e4b9..d788cf2728 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
@@ -43,7 +43,6 @@ import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.metrics.Monitored;
import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper;
import org.apache.gravitino.storage.relational.mapper.GroupRoleRelMapper;
-import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper;
import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper;
import org.apache.gravitino.storage.relational.po.ExtendedGroupPO;
import org.apache.gravitino.storage.relational.po.GroupPO;
@@ -183,15 +182,7 @@ public class GroupMetaService {
NameIdentifier metalakeIdent =
NameIdentifier.of(NameIdentifierUtil.getMetalake(groupEntity.nameIdentifier()));
MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class,
- mapper -> mapper.selectMetalakeMetaByName(metalakeIdent.name()));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalakeIdent.name());
- }
+
MetalakeMetaService.getInstance().requireMetalakeByName(metalakeIdent.name());
GroupPO.Builder builder =
GroupPO.builder().withMetalakeId(metalakePO.getMetalakeId());
GroupPO groupPO = POConverters.initializeGroupPOWithVersion(groupEntity,
builder);
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java
index 1cdbc5b854..f4280ceb8a 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java
@@ -114,21 +114,32 @@ public class MetalakeMetaService {
return metalakeId;
}
- @Monitored(
- metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
- baseMetricName = "getMetalakeByIdentifier")
- public BaseMetalake getMetalakeByIdentifier(NameIdentifier ident) {
- NameIdentifierUtil.checkMetalake(ident);
+ /**
+ * Returns the metalake PO for the given name.
+ *
+ * @param metalakeName the metalake name
+ * @return the metalake PO
+ * @throws NoSuchEntityException if the metalake does not exist
+ */
+ public MetalakePO requireMetalakeByName(String metalakeName) {
MetalakePO metalakePO =
SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(ident.name()));
+ MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalakeName));
if (metalakePO == null) {
throw new NoSuchEntityException(
NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
Entity.EntityType.METALAKE.name().toLowerCase(),
- ident.toString());
+ metalakeName);
}
- return POConverters.fromMetalakePO(metalakePO);
+ return metalakePO;
+ }
+
+ @Monitored(
+ metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
+ baseMetricName = "getMetalakeByIdentifier")
+ public BaseMetalake getMetalakeByIdentifier(NameIdentifier ident) {
+ NameIdentifierUtil.checkMetalake(ident);
+ return POConverters.fromMetalakePO(requireMetalakeByName(ident.name()));
}
@Monitored(
@@ -160,15 +171,7 @@ public class MetalakeMetaService {
public <E extends Entity & HasIdentifier> BaseMetalake updateMetalake(
NameIdentifier ident, Function<E, E> updater) throws IOException {
NameIdentifierUtil.checkMetalake(ident);
- MetalakePO oldMetalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(ident.name()));
- if (oldMetalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- ident.toString());
- }
+ MetalakePO oldMetalakePO = requireMetalakeByName(ident.name());
BaseMetalake oldMetalakeEntity =
POConverters.fromMetalakePO(oldMetalakePO);
BaseMetalake newMetalakeEntity = (BaseMetalake) updater.apply((E)
oldMetalakeEntity);
@@ -208,15 +211,7 @@ public class MetalakeMetaService {
baseMetricName = "deleteMetalake")
public boolean deleteMetalake(NameIdentifier ident, boolean cascade) {
NameIdentifierUtil.checkMetalake(ident);
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(ident.name()));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- ident.toString());
- }
+ MetalakePO metalakePO = requireMetalakeByName(ident.name());
Long metalakeId = metalakePO.getMetalakeId();
Long currentVersion = metalakePO.getCurrentVersion();
if (metalakeId != null) {
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java
index 0327b97bfb..713b55b2b5 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java
@@ -121,15 +121,7 @@ public class PolicyMetaService {
String metalakeName = ns.level(0);
try {
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalakeName));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalakeName);
- }
+ MetalakePO metalakePO =
MetalakeMetaService.getInstance().requireMetalakeByName(metalakeName);
PolicyPO.Builder builder =
PolicyPO.builder().withMetalakeId(metalakePO.getMetalakeId());
PolicyPO policyPO =
POConverters.initializePolicyPOWithVersion(policyEntity, builder);
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
index 6a720db477..d6b4528d7b 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
@@ -166,15 +166,7 @@ public class RoleMetaService {
AuthorizationUtils.checkRole(roleEntity.nameIdentifier());
String metalake =
NameIdentifierUtil.getMetalake(roleEntity.nameIdentifier());
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalake));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalake);
- }
+ MetalakePO metalakePO =
MetalakeMetaService.getInstance().requireMetalakeByName(metalake);
RolePO.Builder builder =
RolePO.builder().withMetalakeId(metalakePO.getMetalakeId());
RolePO rolePO = POConverters.initializeRolePOWithVersion(roleEntity,
builder);
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java
index 1c84f7027e..886bbb085d 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java
@@ -111,15 +111,7 @@ public class TagMetaService {
String metalakeName = ns.level(0);
try {
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalakeName));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalakeName);
- }
+ MetalakePO metalakePO =
MetalakeMetaService.getInstance().requireMetalakeByName(metalakeName);
TagPO.Builder builder =
TagPO.builder().withMetalakeId(metalakePO.getMetalakeId());
TagPO tagPO = POConverters.initializeTagPOWithVersion(tagEntity,
builder);
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java
index 344adede9b..2b230e3bc6 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java
@@ -41,7 +41,6 @@ import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.metrics.Monitored;
-import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper;
import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper;
import org.apache.gravitino.storage.relational.mapper.UserMetaMapper;
import org.apache.gravitino.storage.relational.mapper.UserRoleRelMapper;
@@ -142,15 +141,7 @@ public class UserMetaService {
AuthorizationUtils.checkUser(userEntity.nameIdentifier());
String metalakeName = userEntity.namespace().level(0);
- MetalakePO metalakePO =
- SessionUtils.getWithoutCommit(
- MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByName(metalakeName));
- if (metalakePO == null) {
- throw new NoSuchEntityException(
- NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
- Entity.EntityType.METALAKE.name().toLowerCase(),
- metalakeName);
- }
+ MetalakePO metalakePO =
MetalakeMetaService.getInstance().requireMetalakeByName(metalakeName);
UserPO.Builder builder =
UserPO.builder().withMetalakeId(metalakePO.getMetalakeId());
UserPO userPO = POConverters.initializeUserPOWithVersion(userEntity,
builder);
