This is an automated email from the ASF dual-hosted git repository.
fanng 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 755f0caf5e Revert #8255 update group changes (#8265)
755f0caf5e is described below
commit 755f0caf5e64b97b129195a67e60be054033ab9b
Author: Justin Mclean <[email protected]>
AuthorDate: Mon Aug 25 11:36:34 2025 +1000
Revert #8255 update group changes (#8265)
### What changes were proposed in this pull request?
"Fix early return in GroupMetaService.updateGroup ignoring non-role
changes (#8255)"
This reverts commit bfddf5cbcac26915b85267cb5628495c12ca40c2.
### Why are the changes needed?
As this broke a test.
Fix: #N/A
### Does this PR introduce _any_ user-facing change?
N/A
### How was this patch tested?
Tested locally.
---
.../relational/service/GroupMetaService.java | 31 ++++++++++-------
.../relational/service/TestGroupMetaService.java | 40 ----------------------
2 files changed, 18 insertions(+), 53 deletions(-)
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 14fec721e1..4329b3a0a1 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
@@ -210,6 +210,9 @@ public class GroupMetaService {
Set<Long> insertRoleIds = Sets.difference(newRoleIds, oldRoleIds);
Set<Long> deleteRoleIds = Sets.difference(oldRoleIds, newRoleIds);
+ if (insertRoleIds.isEmpty() && deleteRoleIds.isEmpty()) {
+ return newEntity;
+ }
try {
SessionUtils.doMultipleWithCommit(
() ->
@@ -220,23 +223,25 @@ public class GroupMetaService {
POConverters.updateGroupPOWithVersion(oldGroupPO,
newEntity),
oldGroupPO)),
() -> {
- if (!insertRoleIds.isEmpty()) {
- SessionUtils.doWithoutCommit(
- GroupRoleRelMapper.class,
- mapper ->
- mapper.batchInsertGroupRoleRel(
- POConverters.initializeGroupRoleRelsPOWithVersion(
- newEntity, Lists.newArrayList(insertRoleIds))));
+ if (insertRoleIds.isEmpty()) {
+ return;
}
+ SessionUtils.doWithoutCommit(
+ GroupRoleRelMapper.class,
+ mapper ->
+ mapper.batchInsertGroupRoleRel(
+ POConverters.initializeGroupRoleRelsPOWithVersion(
+ newEntity, Lists.newArrayList(insertRoleIds))));
},
() -> {
- if (!deleteRoleIds.isEmpty()) {
- SessionUtils.doWithoutCommit(
- GroupRoleRelMapper.class,
- mapper ->
- mapper.softDeleteGroupRoleRelByGroupAndRoles(
- newEntity.id(), Lists.newArrayList(deleteRoleIds)));
+ if (deleteRoleIds.isEmpty()) {
+ return;
}
+ SessionUtils.doWithoutCommit(
+ GroupRoleRelMapper.class,
+ mapper ->
+ mapper.softDeleteGroupRoleRelByGroupAndRoles(
+ newEntity.id(), Lists.newArrayList(deleteRoleIds)));
});
} catch (RuntimeException re) {
ExceptionUtils.checkSQLException(
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java
index a5ab960693..5e90f0eb89 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java
@@ -1012,44 +1012,4 @@ class TestGroupMetaService extends TestJDBCBackend {
}
return count;
}
-
- @Test
- void updateGroupWithoutRoleChange() throws IOException {
- AuditInfo auditInfo =
-
AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build();
- BaseMetalake metalake =
- createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), metalakeName,
auditInfo);
- backend.insert(metalake, false);
-
- GroupMetaService groupMetaService = GroupMetaService.getInstance();
-
- GroupEntity group1 =
- createGroupEntity(
- RandomIdGenerator.INSTANCE.nextId(),
- AuthorizationUtils.ofGroupNamespace(metalakeName),
- "group1",
- auditInfo);
- groupMetaService.insertGroup(group1, false);
-
- Function<GroupEntity, GroupEntity> renameUpdater =
- group ->
- GroupEntity.builder()
- .withNamespace(group.namespace())
- .withId(group.id())
- .withName("group_renamed")
- .withRoleNames(group.roleNames())
- .withRoleIds(group.roleIds())
- .withAuditInfo(group.auditInfo())
- .build();
- groupMetaService.updateGroup(group1.nameIdentifier(), renameUpdater);
-
- Assertions.assertThrows(
- NoSuchEntityException.class,
- () -> groupMetaService.getGroupByIdentifier(group1.nameIdentifier()));
-
- GroupEntity updated =
- groupMetaService.getGroupByIdentifier(
- AuthorizationUtils.ofGroup(metalakeName, "group_renamed"));
- Assertions.assertEquals("group_renamed", updated.name());
- }
}
