Copilot commented on code in PR #7062:
URL: https://github.com/apache/shenyu/pull/7062#discussion_r4044249130
##########
shenyu-loadbalancer/src/test/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManagerTest.java:
##########
@@ -80,6 +80,25 @@ public void findUpstreamListBySelectorIdTest() {
Assertions.assertNull(upstreamCacheManager.findUpstreamListBySelectorId(SELECTOR_ID));
}
+ @Test
+ public void findUpstreamListBySelectorIdReturnsSnapshotTest() {
+ final UpstreamCacheManager upstreamCacheManager =
UpstreamCacheManager.getInstance();
+ final String selectorId = "SNAPSHOT_TEST";
+ final Upstream upstream =
Upstream.builder().url("snapshot-url:8080").status(true).build();
+ List<Upstream> upstreamList = new ArrayList<>(1);
+ upstreamList.add(upstream);
+ upstreamCacheManager.submit(selectorId, upstreamList);
+
+ List<Upstream> snapshot =
upstreamCacheManager.findUpstreamListBySelectorId(selectorId);
+ Upstream added =
Upstream.builder().url("added-url:8080").status(true).build();
+ getUpstreamCheckTask(upstreamCacheManager).triggerAddOne(selectorId,
added);
+
+ Assertions.assertEquals(1, snapshot.size());
+ Assertions.assertSame(upstream, snapshot.get(0));
+ Assertions.assertEquals(2,
upstreamCacheManager.findUpstreamListBySelectorId(selectorId).size());
Review Comment:
This test never observes the internal collection:
`findUpstreamListBySelectorId` always returns a new `ArrayList`, and the
mutation finishes before the assertions. It would therefore still pass if
`putToMap` reverted to `ArrayList`, reintroducing the concurrent-copy race.
Assert the internal value is a `CopyOnWriteArrayList` and verify an iterator
taken before the add retains its snapshot.
--
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]
