Copilot commented on code in PR #8297:
URL: https://github.com/apache/gravitino/pull/8297#discussion_r2302695617
##########
core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCache.java:
##########
@@ -120,7 +122,116 @@ void testEnableStats() {
config.set(Configs.CACHE_STATS_ENABLED, true);
EntityCache cache = new CaffeineEntityCache(config);
- Assertions.assertDoesNotThrow(() -> cache.put(entity1));
+ Assertions.assertDoesNotThrow(() -> cache.put(entity_m1c1s1));
+ }
+
+ @Test
+ /**
+ * SCENE[0] <br>
+ * CACHE1 = Role1 -> [catalog1, catalog2] <br>
+ * ACTIVE: INVALIDATE Role1, then need to remove RECORD1 and RECORD2 <br>
+ */
+ void testRemoveCacheRelation0() {
+ EntityCache cache = getNormalCache();
+
+ UserEntity testUserEntity = TestUtil.getTestUserEntity();
+ GroupEntity testGroupEntity = TestUtil.getTestGroupEntity();
+ RoleEntity testRoleEntity = TestUtil.getTestRoleEntity();
+
+ cache.put(
+ testRoleEntity.nameIdentifier(),
+ Entity.EntityType.ROLE,
+ SupportsRelationOperations.Type.ROLE_GROUP_REL,
+ ImmutableList.of(testGroupEntity));
+ cache.put(
+ testRoleEntity.nameIdentifier(),
+ Entity.EntityType.ROLE,
+ SupportsRelationOperations.Type.ROLE_USER_REL,
+ ImmutableList.of(testUserEntity));
+
+ cache.invalidate(testRoleEntity.nameIdentifier(), Entity.EntityType.ROLE);
+
+ Assertions.assertEquals(0, cache.size());
+ }
+
+ @Test
+ /**
+ * SCENE[1] <br>
+ * CACHE1 = Role1 -> [catalog1, catalog2] <br>
+ * CACHE2 = catalog1 -> [tab1, tab2] <br>
+ * ACTIVE: INVALIDATE Role1, then need to remove RECORD1 and RECORD2 <br>
+ */
+ void testRemoveCacheRelation1() {
+ EntityCache cache = getNormalCache();
+
+ UserEntity testUserEntity = TestUtil.getTestUserEntity();
+ GroupEntity testGroupEntity = TestUtil.getTestGroupEntity();
+ RoleEntity testRoleEntity = TestUtil.getTestRoleEntity();
+
+ cache.put(testGroupEntity);
+ cache.put(testUserEntity);
+
+ cache.put(
+ testRoleEntity.nameIdentifier(),
+ Entity.EntityType.ROLE,
+ SupportsRelationOperations.Type.ROLE_GROUP_REL,
+ ImmutableList.of(testGroupEntity));
+ cache.put(
+ testRoleEntity.nameIdentifier(),
+ Entity.EntityType.ROLE,
+ SupportsRelationOperations.Type.ROLE_USER_REL,
+ ImmutableList.of(testUserEntity));
+
+ cache.invalidate(testRoleEntity.nameIdentifier(), Entity.EntityType.ROLE);
+
+ Assertions.assertEquals(0, cache.size());
+ }
+
+ @Test
+ /**
+ * SCENE[2] <br>
+ * CACHE1 = Role1 -> [catalog1, catalog2] <br>
+ * CACHE2 = catalog1 -> [tab1, tab2] <br>
+ * ACTIVE: INVALIDATE catalog1, then need to remove RECORD1 and RECORD2
+ */
+ void testRemoveCacheRelation2() {
+ EntityCache cache = getNormalCache();
+
+ GroupEntity testGroupEntity = TestUtil.getTestGroupEntity();
+ RoleEntity testRoleEntity = TestUtil.getTestRoleEntity();
+
+ cache.put(testGroupEntity);
+
+ cache.put(
+ testRoleEntity.nameIdentifier(),
+ Entity.EntityType.ROLE,
+ SupportsRelationOperations.Type.ROLE_GROUP_REL,
+ ImmutableList.of(testGroupEntity));
+
+ cache.invalidate(testGroupEntity.nameIdentifier(),
Entity.EntityType.GROUP);
+
+ Assertions.assertEquals(0, cache.size());
+ }
+
+ @Test
+ /**
+ * SCENE[3]<br>
+ * CACHE1 = Metadata1 -> []<br>
+ * CACHE2 = Metadata1.Catalog1.tab1 -> []<br>
+ * ACTIVE: INVALIDATE Metadata1, then need to remove RECORD1 and RECORD2<br>
+ */
+ void testRemoveCacheRelation3() {
+ EntityCache cache = getNormalCache();
+
+ BaseMetalake testMetalake = TestUtil.getTestMetalake();
+ TableEntity testTableEntity = TestUtil.getTestTableEntity();
+
+ cache.put(testMetalake);
+ cache.put(testTableEntity);
+
+ cache.invalidate(testMetalake.nameIdentifier(), Entity.EntityType.CATALOG);
Review Comment:
The test is invalidating a metalake entity but using
`Entity.EntityType.CATALOG` instead of `Entity.EntityType.METALAKE`.
```suggestion
cache.invalidate(testMetalake.nameIdentifier(),
Entity.EntityType.METALAKE);
```
##########
core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java:
##########
@@ -272,6 +286,7 @@ public <E, T extends Exception> E
withCacheLock(ThrowingSupplier<E, T> action) t
protected void invalidateExpiredItem(EntityCacheKey key) {
withLock(
() -> {
+ reverseIndex.remove(key);
cacheIndex.remove(key.toString());
});
}
Review Comment:
The method parameter is `EntityCacheKey` but should be
`EntityCacheRelationKey` to match the cache structure and reverseIndex
expectations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
