Copilot commented on code in PR #3011:
URL: https://github.com/apache/hugegraph/pull/3011#discussion_r3152836731
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedSchemaTransactionTest.java:
##########
@@ -165,6 +169,41 @@ public void testGetSchema() throws Exception {
cache.getPropertyKey(IdGenerator.of(1)).name());
}
+ @Test
+ public void testClearV2SchemaCacheByGraphName() {
+ String graphName = "DEFAULT-unit-test-v2";
+ String otherGraphName = "DEFAULT-other-v2";
+
+ Cache<Id, Object> idCache = CacheManager.instance()
+ .cache("schema-id-" +
+ graphName, 10L);
+ Cache<Id, Object> nameCache = CacheManager.instance()
+ .cache("schema-name-" +
+ graphName, 10L);
+ Cache<Id, Object> otherIdCache = CacheManager.instance()
+ .cache("schema-id-" +
+ otherGraphName,
+ 10L);
+
+ idCache.update(IdGenerator.of(1), "fake-pk-by-id");
+ nameCache.update(IdGenerator.of("fake-pk"), "fake-pk-by-name");
+ otherIdCache.update(IdGenerator.of(2), "other-pk-by-id");
+
+ Assert.assertEquals(1L, idCache.size());
+ Assert.assertEquals(1L, nameCache.size());
+ Assert.assertEquals(1L, otherIdCache.size());
+
+ Whitebox.invokeStatic(CachedSchemaTransactionV2.class,
+ new Class<?>[]{String.class},
+ "clearSchemaCache", graphName);
+
+ Assert.assertEquals(0L, idCache.size());
+ Assert.assertEquals(0L, nameCache.size());
+ Assert.assertEquals(1L, otherIdCache.size());
Review Comment:
This test only asserts that the two top-level caches are cleared, but it
doesn’t cover the new behavior in
`CachedSchemaTransactionV2.clearSchemaCache()` that also clears the `idCache`
attachment (the optimized array cache). Add an assertion that the attached
`SchemaCaches` is non-empty before the call and empty afterwards (can be done
via reflection since `SchemaCaches` is private) to prevent regressions in the
attachment-clearing logic.
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedSchemaTransactionTest.java:
##########
@@ -165,6 +169,41 @@ public void testGetSchema() throws Exception {
cache.getPropertyKey(IdGenerator.of(1)).name());
}
+ @Test
+ public void testClearV2SchemaCacheByGraphName() {
+ String graphName = "DEFAULT-unit-test-v2";
+ String otherGraphName = "DEFAULT-other-v2";
+
+ Cache<Id, Object> idCache = CacheManager.instance()
+ .cache("schema-id-" +
+ graphName, 10L);
+ Cache<Id, Object> nameCache = CacheManager.instance()
+ .cache("schema-name-" +
+ graphName, 10L);
+ Cache<Id, Object> otherIdCache = CacheManager.instance()
+ .cache("schema-id-" +
+ otherGraphName,
+ 10L);
+
+ idCache.update(IdGenerator.of(1), "fake-pk-by-id");
+ nameCache.update(IdGenerator.of("fake-pk"), "fake-pk-by-name");
+ otherIdCache.update(IdGenerator.of(2), "other-pk-by-id");
+
+ Assert.assertEquals(1L, idCache.size());
+ Assert.assertEquals(1L, nameCache.size());
+ Assert.assertEquals(1L, otherIdCache.size());
+
+ Whitebox.invokeStatic(CachedSchemaTransactionV2.class,
+ new Class<?>[]{String.class},
+ "clearSchemaCache", graphName);
+
+ Assert.assertEquals(0L, idCache.size());
+ Assert.assertEquals(0L, nameCache.size());
+ Assert.assertEquals(1L, otherIdCache.size());
+
+ otherIdCache.clear();
Review Comment:
`testClearV2SchemaCacheByGraphName()` creates global `CacheManager` caches
and mutates them, but cleanup isn’t protected if an assertion fails or an
exception is thrown. Wrap the cache population/assertions in a try/finally and
clear all caches created in this test (`idCache`, `nameCache`, `otherIdCache`)
in the finally block to keep tests isolated and avoid leaking state to
subsequent tests.
```suggestion
try {
idCache.update(IdGenerator.of(1), "fake-pk-by-id");
nameCache.update(IdGenerator.of("fake-pk"), "fake-pk-by-name");
otherIdCache.update(IdGenerator.of(2), "other-pk-by-id");
Assert.assertEquals(1L, idCache.size());
Assert.assertEquals(1L, nameCache.size());
Assert.assertEquals(1L, otherIdCache.size());
Whitebox.invokeStatic(CachedSchemaTransactionV2.class,
new Class<?>[]{String.class},
"clearSchemaCache", graphName);
Assert.assertEquals(0L, idCache.size());
Assert.assertEquals(0L, nameCache.size());
Assert.assertEquals(1L, otherIdCache.size());
} finally {
idCache.clear();
nameCache.clear();
otherIdCache.clear();
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
