Honor Collection.remove() contract in CompositeCollection

Before this commit, `second.remove(o)` was called no matter whether
`first.remove(o)` returned `true` or `false`. If the element was present
in both collections, it was removed from both, which violates the
`Collection.remove(Object)` contract of removing a single instance.

Closes gh-37332

Signed-off-by: Yanming Zhou <zhouyanming@gmail.com>
This commit is contained in:
Yanming Zhou
2026-10-02 16:26:53 +02:00
committed by GitHub
parent db4f57a54f
commit f5a7123044
2 changed files with 14 additions and 4 deletions
@@ -114,9 +114,7 @@ class CompositeCollection<E extends @Nullable Object> implements Collection<E> {
@Override
public boolean remove(Object o) {
boolean firstResult = this.first.remove(o);
boolean secondResult = this.second.remove(o);
return firstResult || secondResult;
return (this.first.remove(o) || this.second.remove(o));
}
@Override
@@ -124,8 +124,19 @@ class CompositeCollectionTests {
assertThat(composite.remove("foo")).isTrue();
assertThat(composite.contains("foo")).isFalse();
assertThat(first).containsExactly("bar");
assertThat(composite.remove("quux")).isFalse();
// test remove element existing in both first and second
first = new ArrayList<>(List.of("foo", "bar"));
second = new ArrayList<>(List.of("foo", "qux"));
composite = new CompositeCollection<>(first, second);
assertThat(composite.remove("foo")).isTrue();
assertThat(first).containsExactly("bar");
assertThat(composite.contains("foo")).isTrue();
assertThat(composite.remove("foo")).isTrue();
assertThat(second).containsExactly("qux");
assertThat(composite.contains("foo")).isFalse();
}
@Test
@@ -207,4 +218,5 @@ class CompositeCollectionTests {
assertThat(composite).containsExactly("foo", null, "bar", null);
}
}