asMap() does not conform to the Map contract #174
Description
I've been using Guava's conformance tests since Caffeine's inception as a secondary guard in addition to my own suite. I recently extended it to run across more cache configurations, similar to how the core suite runs a test scenario against every configuration that matches its specification constraint. The intent is to ensure that a particular feature does not interact poorly and break the expected behavior. That's a brute force approach, e.g. that change expanded the suite by ~350,000 cases.
I did a quick check to see if other caches passed the suite, and unfortunately I found issues here (83 failures, 13 errors). That's not surprising, e.g. another library had failures too (jhalterman/expiringmap#48).
In this library a few examples are,
- A
computeIfAbsentthat returns null indicates that the mapping should not be established. Instead this is treated as a null value, causing either an error or to be cached based on the builder configuration. equals(map)should be symmetric and therefore two maps with the same mappings will be equal to one another. This is violated wherecache.asMap().equals(other)fails if other does not implementConcurrentMapWrapper, whereasother.equals(cache.asMap())will pass. SimilarlyhashCodeviolates the basic equality contract. While a documented break, it seems like an unnecessary violation.- The key, value, and entry views have numerous problems with their iterators, causing issues such as due to the
Map.Entrynot implementing equality so thatentrySet.containsAll(entrySet)fails.
If helpful, Caffeine's test runners are defined in CaffeineMapTests and MapTestFactory.
A simple runner for cache2k
import java.util.Map;
import java.util.function.Supplier;
import org.cache2k.Cache2kBuilder;
import com.google.common.collect.testing.ConcurrentMapTestSuiteBuilder;
import com.google.common.collect.testing.TestStringMapGenerator;
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 junit.framework.Test;
import junit.framework.TestCase;
import junit.framework.TestSuite;
/**
* Guava testlib map tests for the asMap() view.
*/
public final class Cache2kMapTests extends TestCase {
public static Test suite() {
var suite = new TestSuite();
suite.addTest(suite("cache2k_disallowNull", /* permitNullValues */ false, () -> {
return new Cache2kBuilder<String, String>() {};
}));
suite.addTest(suite("cache2k_allowNull", /* permitNullValues */ true, () -> {
return new Cache2kBuilder<String, String>() {};
}));
return suite;
}
private static Test suite(String name, boolean permitNullValues,
Supplier<Cache2kBuilder<String, String>> supplier) {
var suite = ConcurrentMapTestSuiteBuilder
.using(new TestStringMapGenerator() {
@Override protected Map<String, String> create(Map.Entry<String, String>[] entries) {
var map = supplier.get().permitNullValues(permitNullValues).build().asMap();
for (var entry : entries) {
map.put(entry.getKey(), entry.getValue());
}
return map;
}
})
.named(name)
.withFeatures(
MapFeature.GENERAL_PURPOSE,
MapFeature.ALLOWS_NULL_ENTRY_QUERIES,
CollectionFeature.SUPPORTS_ITERATOR_REMOVE,
CollectionSize.ANY);
if (permitNullValues) {
suite.withFeatures(MapFeature.ALLOWS_NULL_VALUES);
}
return suite.createTestSuite();
}
}