This is an automated email from the ASF dual-hosted git repository. jmclean pushed a commit to branch revert_group in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 7c9db012b47bad1c16ec19be4582e600fc26d913 Author: Justin Mclean <[email protected]> AuthorDate: Mon Aug 25 09:26:37 2025 +1000 Revert "Fix early return in GroupMetaService.updateGroup ignoring non-role changes (#8255)" This reverts commit bfddf5cbcac26915b85267cb5628495c12ca40c2. --- .../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()); - } }
