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 67de195003 [#13441] fix(core): include GroupPO fields in
ExtendedGroupPO.equals (#13442)
67de195003 is described below
commit 67de19500384ed30f59aca4fbfb7d8e5a2944021
Author: YangJie <[email protected]>
AuthorDate: Wed Sep 23 21:10:40 2026 -0400
[#13441] fix(core): include GroupPO fields in ExtendedGroupPO.equals
(#13442)
### What changes were proposed in this pull request?
`ExtendedGroupPO.equals` now calls `super.equals` (the `GroupPO`
identity fields) in addition to comparing `roleIds`/`roleNames`,
matching what `hashCode` already covers and aligning with the sibling
`ExtendedUserPO`.
### Why are the changes needed?
`equals` compared only the role fields while `hashCode` folded in
`super.hashCode()`, so two extended rows for different groups with
identical roles were equal but hashed differently, breaking the
`Object.hashCode` contract for any hash-based collection.
Fix: #13441
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added `TestExtendedGroupPO.testEqualsIncludesGroupFields` (two rows for
different groups with the same roles are not equal) and
`testEqualsAndHashCodeConsistent`. They fail against the pre-fix code.
---
.../storage/relational/po/ExtendedGroupPO.java | 3 +-
.../storage/relational/po/TestExtendedGroupPO.java | 60 ++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java
b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java
index 390a003983..8149d806f0 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java
@@ -48,7 +48,8 @@ public class ExtendedGroupPO extends GroupPO {
return false;
}
ExtendedGroupPO that = (ExtendedGroupPO) o;
- return Objects.equals(getRoleIds(), that.getRoleIds())
+ return super.equals(o)
+ && Objects.equals(getRoleIds(), that.getRoleIds())
&& Objects.equals(getRoleNames(), that.getRoleNames());
}
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/po/TestExtendedGroupPO.java
b/core/src/test/java/org/apache/gravitino/storage/relational/po/TestExtendedGroupPO.java
new file mode 100644
index 0000000000..1d564c9a58
--- /dev/null
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/po/TestExtendedGroupPO.java
@@ -0,0 +1,60 @@
+/*
+ * 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.po;
+
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests for {@link ExtendedGroupPO} equality. */
+public class TestExtendedGroupPO {
+
+ private static ExtendedGroupPO po(
+ Long groupId, Long metalakeId, String groupName, String roleNames,
String roleIds)
+ throws IllegalAccessException {
+ ExtendedGroupPO po = new ExtendedGroupPO();
+ FieldUtils.writeField(po, "groupId", groupId, true);
+ FieldUtils.writeField(po, "metalakeId", metalakeId, true);
+ FieldUtils.writeField(po, "groupName", groupName, true);
+ FieldUtils.writeField(po, "roleNames", roleNames, true);
+ FieldUtils.writeField(po, "roleIds", roleIds, true);
+ return po;
+ }
+
+ @Test
+ void testEqualsIncludesGroupFields() throws IllegalAccessException {
+ // The parent GroupPO fields are part of identity: two rows that differ
only in a
+ // GroupPO field (groupId, metalakeId, or groupName) must not be equal
even when the
+ // role strings match.
+ Assertions.assertNotEquals(
+ po(1L, 10L, "group-a", "r1,r2", "1,2"), po(2L, 10L, "group-a",
"r1,r2", "1,2"));
+ Assertions.assertNotEquals(
+ po(1L, 10L, "group-a", "r1,r2", "1,2"), po(1L, 20L, "group-a",
"r1,r2", "1,2"));
+ Assertions.assertNotEquals(
+ po(1L, 10L, "group-a", "r1,r2", "1,2"), po(1L, 10L, "group-b",
"r1,r2", "1,2"));
+ }
+
+ @Test
+ void testEqualsAndHashCodeConsistent() throws IllegalAccessException {
+ ExtendedGroupPO po1 = po(1L, 10L, "group-a", "r1,r2", "1,2");
+ ExtendedGroupPO po2 = po(1L, 10L, "group-a", "r1,r2", "1,2");
+ Assertions.assertEquals(po1, po2);
+ Assertions.assertEquals(po1.hashCode(), po2.hashCode());
+ }
+}
