From 6f9a06ce392ffe5d38eb14af9354a1c2cea78ad2 Mon Sep 17 00:00:00 2001 From: Agyei Holy Date: Thu, 23 Apr 2026 18:10:48 -0500 Subject: [PATCH 1/3] Add ConcurrentMap spliterator checks --- .../com/google/common/cache/LocalCache.java | 8 + .../ConcurrentMapTestSuiteBuilder.java | 4 +- .../ConcurrentMapSpliteratorTester.java | 67 +++++++ .../ConcurrentMapTestSuiteBuilderTest.java | 178 ++++++++++++++++++ .../com/google/common/cache/LocalCache.java | 8 + 5 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java create mode 100644 guava-testlib/test/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilderTest.java diff --git a/android/guava/src/com/google/common/cache/LocalCache.java b/android/guava/src/com/google/common/cache/LocalCache.java index 7c79c901b9c3..96c285a1c4ad 100644 --- a/android/guava/src/com/google/common/cache/LocalCache.java +++ b/android/guava/src/com/google/common/cache/LocalCache.java @@ -74,6 +74,8 @@ import java.util.NoSuchElementException; import java.util.Queue; import java.util.Set; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; @@ -4402,6 +4404,12 @@ public boolean isEmpty() { public void clear() { LocalCache.this.clear(); } + + @Override + public Spliterator spliterator() { + return Spliterators.spliteratorUnknownSize( + iterator(), Spliterator.CONCURRENT | Spliterator.DISTINCT | Spliterator.NONNULL); + } } final class KeySet extends AbstractCacheSet { diff --git a/guava-testlib/src/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilder.java b/guava-testlib/src/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilder.java index 048723e40385..c84b9c2744d2 100644 --- a/guava-testlib/src/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilder.java +++ b/guava-testlib/src/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilder.java @@ -22,6 +22,7 @@ import com.google.common.collect.testing.testers.ConcurrentMapRemoveTester; import com.google.common.collect.testing.testers.ConcurrentMapReplaceEntryTester; import com.google.common.collect.testing.testers.ConcurrentMapReplaceTester; +import com.google.common.collect.testing.testers.ConcurrentMapSpliteratorTester; import java.util.List; /** @@ -44,7 +45,8 @@ public static ConcurrentMapTestSuiteBuilder using(TestMapGenerator< ConcurrentMapPutIfAbsentTester.class, ConcurrentMapRemoveTester.class, ConcurrentMapReplaceTester.class, - ConcurrentMapReplaceEntryTester.class); + ConcurrentMapReplaceEntryTester.class, + ConcurrentMapSpliteratorTester.class); @SuppressWarnings("rawtypes") // class literals @Override diff --git a/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java b/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java new file mode 100644 index 000000000000..eee32a57e29f --- /dev/null +++ b/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2026 The Guava Authors + * + * Licensed 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 com.google.common.collect.testing.testers; + +import com.google.common.annotations.GwtCompatible; +import com.google.common.collect.testing.AbstractMapTester; +import java.util.Spliterator; +import java.util.concurrent.ConcurrentMap; +import org.jspecify.annotations.NullMarked; +import org.junit.Ignore; + +/** + * A generic JUnit test which tests spliterator characteristics on derived views of a concurrent + * map. Can't be invoked directly; please see {@link + * com.google.common.collect.testing.ConcurrentMapTestSuiteBuilder}. + * + * @author Louis Wasserman + */ +@GwtCompatible +@Ignore("test runners must not instantiate and run this directly, only via suites we build") +// @Ignore affects the Android test runner, which respects JUnit 4 annotations on JUnit 3 tests. +@SuppressWarnings("JUnit4ClassUsedInJUnit3") +@NullMarked +public class ConcurrentMapSpliteratorTester extends AbstractMapTester { + @Override + protected ConcurrentMap getMap() { + return (ConcurrentMap) super.getMap(); + } + + public void testKeySetSpliteratorHasConcurrentCharacteristic() { + assertTrue( + "keySet spliterator should report CONCURRENT", + getMap().keySet().spliterator().hasCharacteristics(Spliterator.CONCURRENT)); + } + + public void testKeySetSpliteratorDoesNotHaveSizedCharacteristic() { + assertFalse( + "keySet spliterator should not report SIZED", + getMap().keySet().spliterator().hasCharacteristics(Spliterator.SIZED)); + } + + public void testEntrySetSpliteratorHasConcurrentCharacteristic() { + assertTrue( + "entrySet spliterator should report CONCURRENT", + getMap().entrySet().spliterator().hasCharacteristics(Spliterator.CONCURRENT)); + } + + public void testEntrySetSpliteratorDoesNotHaveSizedCharacteristic() { + assertFalse( + "entrySet spliterator should not report SIZED", + getMap().entrySet().spliterator().hasCharacteristics(Spliterator.SIZED)); + } +} diff --git a/guava-testlib/test/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilderTest.java b/guava-testlib/test/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilderTest.java new file mode 100644 index 000000000000..e1facedee0d5 --- /dev/null +++ b/guava-testlib/test/com/google/common/collect/testing/ConcurrentMapTestSuiteBuilderTest.java @@ -0,0 +1,178 @@ +/* + * Copyright (C) 2026 The Guava Authors + * + * Licensed 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 com.google.common.collect.testing; + +import com.google.common.collect.ForwardingConcurrentMap; +import com.google.common.collect.testing.features.CollectionFeature; +import com.google.common.collect.testing.features.CollectionSize; +import com.google.common.collect.testing.features.MapFeature; +import java.util.AbstractSet; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestFailure; +import junit.framework.TestResult; + +/** Tests for {@link ConcurrentMapTestSuiteBuilder}. */ +@AndroidIncompatible // test-suite builders +public final class ConcurrentMapTestSuiteBuilderTest extends TestCase { + public void testConcurrentHashMapDerivedSetSpliteratorsPass() { + TestResult result = runSuite(new ConcurrentHashMapGenerator(), "ConcurrentHashMap"); + + assertEquals(0, result.errorCount()); + assertEquals(0, result.failureCount()); + } + + public void testConcurrentMapDerivedSetSpliteratorTestsCatchWrongCharacteristics() { + TestResult result = runSuite(new BadSpliteratorConcurrentMapGenerator(), "BadConcurrentMap"); + + assertEquals(0, result.errorCount()); + + List failedTests = failedTests(result); + assertTrue(failedTests.toString(), failedTests.size() >= 4); + assertContainsFailure( + failedTests, "testKeySetSpliteratorHasConcurrentCharacteristic"); + assertContainsFailure( + failedTests, "testKeySetSpliteratorDoesNotHaveSizedCharacteristic"); + assertContainsFailure( + failedTests, "testEntrySetSpliteratorHasConcurrentCharacteristic"); + assertContainsFailure( + failedTests, "testEntrySetSpliteratorDoesNotHaveSizedCharacteristic"); + } + + private static TestResult runSuite(TestStringMapGenerator generator, String name) { + Test suite = + ConcurrentMapTestSuiteBuilder.using(generator) + .named(name) + .withFeatures( + MapFeature.GENERAL_PURPOSE, + CollectionFeature.SUPPORTS_ITERATOR_REMOVE, + CollectionSize.ONE) + .suppressing(new OpenJdk6MapTests().suppressForConcurrentHashMap()) + .createTestSuite(); + TestResult result = new TestResult(); + suite.run(result); + return result; + } + + private static List failedTests(TestResult result) { + List failedTests = new ArrayList<>(); + for (java.util.Enumeration failures = result.failures(); + failures.hasMoreElements(); ) { + failedTests.add(failures.nextElement().failedTest().toString()); + } + return failedTests; + } + + private static void assertContainsFailure(List failedTests, String testName) { + for (String failedTest : failedTests) { + if (failedTest.contains(testName)) { + return; + } + } + fail("Expected failure for " + testName + " in " + failedTests); + } + + private static final class ConcurrentHashMapGenerator extends TestStringMapGenerator { + @Override + protected Map create(Entry[] entries) { + ConcurrentMap map = new ConcurrentHashMap<>(); + for (Entry entry : entries) { + map.put(entry.getKey(), entry.getValue()); + } + return map; + } + } + + private static final class BadSpliteratorConcurrentMapGenerator extends TestStringMapGenerator { + @Override + protected Map create(Entry[] entries) { + BadSpliteratorConcurrentMap map = new BadSpliteratorConcurrentMap(); + for (Entry entry : entries) { + map.put(entry.getKey(), entry.getValue()); + } + return map; + } + } + + private static final class BadSpliteratorConcurrentMap + extends ForwardingConcurrentMap { + private final ConcurrentMap delegate = new ConcurrentHashMap<>(); + + @Override + protected ConcurrentMap delegate() { + return delegate; + } + + @Override + public Set keySet() { + return new BadSpliteratorSet<>(delegate.keySet()); + } + + @Override + public Set> entrySet() { + return new BadSpliteratorSet<>(delegate.entrySet()); + } + } + + private static final class BadSpliteratorSet extends AbstractSet { + private final Set delegate; + + BadSpliteratorSet(Set delegate) { + this.delegate = delegate; + } + + @Override + public Iterator iterator() { + return delegate.iterator(); + } + + @Override + public int size() { + return delegate.size(); + } + + @Override + public boolean contains(Object o) { + return delegate.contains(o); + } + + @Override + public boolean remove(Object o) { + return delegate.remove(o); + } + + @Override + public void clear() { + delegate.clear(); + } + + @Override + public Spliterator spliterator() { + return Spliterators.spliterator(iterator(), size(), Spliterator.DISTINCT); + } + } +} diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index 4a90a7320b01..252bf3c8ec60 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -74,6 +74,8 @@ import java.util.NoSuchElementException; import java.util.Queue; import java.util.Set; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; @@ -4546,6 +4548,12 @@ public boolean isEmpty() { public void clear() { LocalCache.this.clear(); } + + @Override + public Spliterator spliterator() { + return Spliterators.spliteratorUnknownSize( + iterator(), Spliterator.CONCURRENT | Spliterator.DISTINCT | Spliterator.NONNULL); + } } boolean removeIf(BiPredicate filter) { From bcd722f9384ef843329b62f77922aa6c243fdfff Mon Sep 17 00:00:00 2001 From: Agyei Holy Date: Fri, 24 Apr 2026 12:56:32 -0500 Subject: [PATCH 2/3] Annotate android LocalCache.spliterator() with @IgnoreJRERequirement. CI's animal-sniffer check on the android variant (gummy-bears-api-23) rejected the Spliterator override added in the previous commit because java.util.Spliterator and Spliterators.spliteratorUnknownSize(...) are not part of Android API 23. Following the established pattern in ImmutableCollection.java, Ints.java, and Longs.java under android/guava/src, annotate the method with the package-local @IgnoreJRERequirement. The method is only invoked from APIs whose signatures already carry Java 8 types, so callers are already gated by the same constraint. The guava/src variant is unaffected because it uses the java18 signature, which includes Spliterator. Made-with: Cursor --- android/guava/src/com/google/common/cache/LocalCache.java | 1 + 1 file changed, 1 insertion(+) diff --git a/android/guava/src/com/google/common/cache/LocalCache.java b/android/guava/src/com/google/common/cache/LocalCache.java index 96c285a1c4ad..4fa83b7ac3bf 100644 --- a/android/guava/src/com/google/common/cache/LocalCache.java +++ b/android/guava/src/com/google/common/cache/LocalCache.java @@ -4406,6 +4406,7 @@ public void clear() { } @Override + @IgnoreJRERequirement // used only from APIs with Java 8 types in them public Spliterator spliterator() { return Spliterators.spliteratorUnknownSize( iterator(), Spliterator.CONCURRENT | Spliterator.DISTINCT | Spliterator.NONNULL); From d9e665df576fb44448c57d7339318ce4cc727626 Mon Sep 17 00:00:00 2001 From: Agyei Holy Date: Mon, 27 Apr 2026 17:28:22 -0500 Subject: [PATCH 3/3] Address PR review: LocalCache.Values spliterator + drop @author tag - Override spliterator() on LocalCache.Values (CONCURRENT | NONNULL) in both the main and android variants, mirroring the existing AbstractCacheSet override (per cgdecker review on #8371). The android variant uses @IgnoreJRERequirement to satisfy animal-sniffer's gummy-bears-api-23 signature. - Remove the inaccurate @author tag from the new ConcurrentMapSpliteratorTester; new Guava code no longer carries @author tags. Made-with: Cursor --- android/guava/src/com/google/common/cache/LocalCache.java | 7 +++++++ .../testing/testers/ConcurrentMapSpliteratorTester.java | 2 -- guava/src/com/google/common/cache/LocalCache.java | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/android/guava/src/com/google/common/cache/LocalCache.java b/android/guava/src/com/google/common/cache/LocalCache.java index 4fa83b7ac3bf..da772663c1e2 100644 --- a/android/guava/src/com/google/common/cache/LocalCache.java +++ b/android/guava/src/com/google/common/cache/LocalCache.java @@ -4456,6 +4456,13 @@ public Iterator iterator() { public boolean contains(Object o) { return LocalCache.this.containsValue(o); } + + @Override + @IgnoreJRERequirement // used only from APIs with Java 8 types in them + public Spliterator spliterator() { + return Spliterators.spliteratorUnknownSize( + iterator(), Spliterator.CONCURRENT | Spliterator.NONNULL); + } } final class EntrySet extends AbstractCacheSet> { diff --git a/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java b/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java index eee32a57e29f..78545adfd08f 100644 --- a/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java +++ b/guava-testlib/src/com/google/common/collect/testing/testers/ConcurrentMapSpliteratorTester.java @@ -27,8 +27,6 @@ * A generic JUnit test which tests spliterator characteristics on derived views of a concurrent * map. Can't be invoked directly; please see {@link * com.google.common.collect.testing.ConcurrentMapTestSuiteBuilder}. - * - * @author Louis Wasserman */ @GwtCompatible @Ignore("test runners must not instantiate and run this directly, only via suites we build") diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index 252bf3c8ec60..6378f9c2d719 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -4622,6 +4622,12 @@ public boolean removeIf(Predicate filter) { public boolean contains(Object o) { return LocalCache.this.containsValue(o); } + + @Override + public Spliterator spliterator() { + return Spliterators.spliteratorUnknownSize( + iterator(), Spliterator.CONCURRENT | Spliterator.NONNULL); + } } final class EntrySet extends AbstractCacheSet> {