Skip to content

Commit

Permalink
Improved generic types and checker framework conformance (#337)
Browse files Browse the repository at this point in the history 
Additional wildcards are used throughput the APIs to more flexibly
accept parameters. For example this allows a wider range of method
references to be used as load functions.

The generic types now match the Checker Framework's rules [1]. This
should improve usage of the cache apis in projects that run the checker.
This project does not and its implementation classes are not compliant
for the nullness checker.

[1] https://checkerframework.org/manual/#generics-instantiation
  • Loading branch information
@ben-manes
ben-manes committed Feb 15, 2021
1 parent 35a7151 commit c40966b
Show file tree
Hide file tree
Showing 26 changed files with 154 additions and 113 deletions.
1 change: 0 additions & 1 deletion build.gradle
View file
@@ -1,6 +1,5 @@
import net.ltgt.gradle.errorprone.CheckSeverity

apply plugin: 'com.github.mjdetullio.gradle.coverity'
apply plugin: 'com.github.ben-manes.versions'
apply plugin: 'com.github.kt3k.coveralls'
apply plugin: 'jacoco'
Expand Down
3 changes: 2 additions & 1 deletion caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/BasicCache.java
View file
Expand Up @@ -15,14 +15,15 @@
*/
package com.github.benmanes.caffeine.cache;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* A facade for benchmark implementations.
*
* @author ben.manes@gmail.com (Ben Manes)
*/
public interface BasicCache<K, V> {
public interface BasicCache<K extends @NonNull Object, V extends @NonNull Object> {

/** Returns the value stored in the cache, or null if not present. */
@Nullable V get(K key);
Expand Down
14 changes: 8 additions & 6 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java
View file
Expand Up @@ -23,6 +23,7 @@
import java.util.function.BiFunction;
import java.util.function.Function;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
Expand All @@ -37,7 +38,7 @@
* @param <K> the type of keys maintained by this cache
* @param <V> the type of mapped values
*/
public interface AsyncCache<K, V> {
public interface AsyncCache<K extends @NonNull Object, V extends @NonNull Object> {

/**
* Returns the future associated with {@code key} in this cache, or {@code null} if there is no
Expand Down Expand Up @@ -92,8 +93,8 @@ public interface AsyncCache<K, V> {
* @throws RuntimeException or Error if the mappingFunction does when constructing the future,
* in which case the mapping is left unestablished
*/
CompletableFuture<V> get(K key,
BiFunction<? super K, Executor, CompletableFuture<V>> mappingFunction);
CompletableFuture<V> get(K key, BiFunction<? super K, ? super Executor,
? extends CompletableFuture<? extends V>> mappingFunction);

/**
* Returns the future of a map of the values associated with {@code keys}, creating or retrieving
Expand All @@ -119,7 +120,7 @@ CompletableFuture<V> get(K key,
* left unestablished
*/
CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
Function<Set<? extends K>, Map<K, V>> mappingFunction);
Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction);

/**
* Returns the future of a map of the values associated with {@code keys}, creating or retrieving
Expand All @@ -145,7 +146,8 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
* left unestablished
*/
CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
BiFunction<Set<? extends K>, Executor, CompletableFuture<Map<K, V>>> mappingFunction);
BiFunction<? super Set<? extends K>, ? super Executor,
? extends CompletableFuture<? extends Map<? extends K, ? extends V>>> mappingFunction);

/**
* Associates {@code value} with {@code key} in this cache. If the cache previously contained a
Expand All @@ -159,7 +161,7 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
* @param valueFuture value to be associated with the specified key
* @throws NullPointerException if the specified key or value is null
*/
void put(K key, CompletableFuture<V> valueFuture);
void put(K key, CompletableFuture<? extends V> valueFuture);

/**
* Returns a view of the entries stored in this cache as a thread-safe map. Modifications made to
Expand Down
27 changes: 17 additions & 10 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java
View file
Expand Up @@ -24,6 +24,8 @@
import java.util.function.BiFunction;
import java.util.function.Function;

import org.checkerframework.checker.nullness.qual.NonNull;

/**
* Computes or retrieves values asynchronously, based on a key, for use in populating a
* {@link AsyncLoadingCache}.
Expand All @@ -42,7 +44,7 @@
*/
@FunctionalInterface
@SuppressWarnings("PMD.SignatureDeclareThrowsException")
public interface AsyncCacheLoader<K, V> {
public interface AsyncCacheLoader<K extends @NonNull Object, V extends @NonNull Object> {

/**
* Asynchronously computes or retrieves the value corresponding to {@code key}.
Expand All @@ -55,7 +57,7 @@ public interface AsyncCacheLoader<K, V> {
* treated like any other {@code Exception} in all respects except that, when it is
* caught, the thread's interrupt status is set
*/
CompletableFuture<V> asyncLoad(K key, Executor executor) throws Exception;
CompletableFuture<? extends V> asyncLoad(K key, Executor executor) throws Exception;

/**
* Asynchronously computes or retrieves the values corresponding to {@code keys}. This method is
Expand All @@ -79,7 +81,7 @@ public interface AsyncCacheLoader<K, V> {
* treated like any other {@code Exception} in all respects except that, when it is
* caught, the thread's interrupt status is set
*/
default CompletableFuture<Map<K, V>> asyncLoadAll(
default CompletableFuture<? extends Map<? extends K, ? extends V>> asyncLoadAll(
Set<? extends K> keys, Executor executor) throws Exception {
throw new UnsupportedOperationException();
}
Expand All @@ -102,7 +104,8 @@ default CompletableFuture<Map<K, V>> asyncLoadAll(
* treated like any other {@code Exception} in all respects except that, when it is
* caught, the thread's interrupt status is set
*/
default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) throws Exception {
default CompletableFuture<? extends V> asyncReload(
K key, V oldValue, Executor executor) throws Exception {
return asyncLoad(key, executor);
}

Expand All @@ -122,14 +125,15 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t
* @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
static <K, V> AsyncCacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>> mappingFunction) {
static <K extends Object, V extends Object> AsyncCacheLoader<K, V> bulk(
Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction) {
return CacheLoader.bulk(mappingFunction);
}

/**
* Returns an asynchronous cache loader that delegates to the supplied mapping function for
* retrieving the values. Note that {@link #asyncLoad} will discard any additional mappings
* loaded when retrieving the {@code key} prior to returning to the value to the cache.
* retrieving the values. Note that {@link #asyncLoad} will silently discard any additional
* mappings loaded when retrieving the {@code key} prior to returning to the value to the cache.
* <p>
* Usage example:
* <pre>{@code
Expand All @@ -142,8 +146,9 @@ static <K, V> AsyncCacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>>
* @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
static <K, V> AsyncCacheLoader<K, V> bulk(
BiFunction<Set<? extends K>, Executor, CompletableFuture<Map<K, V>>> mappingFunction) {
static <K extends Object, V extends Object> AsyncCacheLoader<K, V> bulk(
BiFunction<? super Set<? extends K>, ? super Executor,
? extends CompletableFuture<? extends Map<? extends K, ? extends V>>> mappingFunction) {
requireNonNull(mappingFunction);
return new AsyncCacheLoader<>() {
@Override public CompletableFuture<V> asyncLoad(K key, Executor executor) {
Expand All @@ -154,7 +159,9 @@ static <K, V> AsyncCacheLoader<K, V> bulk(
Set<? extends K> keys, Executor executor) {
requireNonNull(keys);
requireNonNull(executor);
return mappingFunction.apply(keys, executor);
@SuppressWarnings("unchecked")
var future = (CompletableFuture<Map<K, V>>) mappingFunction.apply(keys, executor);
return future;
}
};
}
Expand Down
5 changes: 4 additions & 1 deletion caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java
View file
Expand Up @@ -18,6 +18,8 @@
import java.util.Map;
import java.util.concurrent.CompletableFuture;

import org.checkerframework.checker.nullness.qual.NonNull;

/**
* A semi-persistent mapping from keys to values. Values are automatically loaded by the cache
* asynchronously, and are stored in the cache until either evicted or manually invalidated.
Expand All @@ -29,7 +31,8 @@
* @param <K> the type of keys maintained by this cache
* @param <V> the type of mapped values
*/
public interface AsyncLoadingCache<K, V> extends AsyncCache<K, V> {
public interface AsyncLoadingCache<K extends @NonNull Object, V extends @NonNull Object>
extends AsyncCache<K, V> {

/**
* Returns the future associated with {@code key} in this cache, obtaining that value from
Expand Down
4 changes: 2 additions & 2 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java
View file
Expand Up @@ -1230,7 +1230,7 @@ void refreshIfNeeded(Node<K, V> node, long now) {
&& !refreshes().containsKey(keyReference)) {
long[] startTime = new long[1];
@SuppressWarnings({"unchecked", "rawtypes"})
CompletableFuture<V>[] refreshFuture = new CompletableFuture[1];
CompletableFuture<? extends V>[] refreshFuture = new CompletableFuture[1];
refreshes().computeIfAbsent(keyReference, k -> {
try {
startTime[0] = statsTicker().read();
Expand Down Expand Up @@ -4005,7 +4005,7 @@ static final class AsyncLoader<K, V> implements CacheLoader<K, V> {
V newValue = (V) loader.asyncReload(key, oldValue, executor);
return newValue;
}
@Override public CompletableFuture<V> asyncReload(
@Override public CompletableFuture<? extends V> asyncReload(
K key, V oldValue, Executor executor) throws Exception {
return loader.asyncReload(key, oldValue, executor);
}
Expand Down
7 changes: 4 additions & 3 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java
View file
Expand Up @@ -21,6 +21,7 @@
import java.util.function.Function;

import org.checkerframework.checker.index.qual.NonNegative;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.checker.nullness.qual.PolyNull;

Expand All @@ -38,7 +39,7 @@
* @param <K> the type of keys maintained by this cache
* @param <V> the type of mapped values
*/
public interface Cache<K, V> {
public interface Cache<K extends @NonNull Object, V extends @NonNull Object> {

/**
* Returns the value associated with the {@code key} in this cache, or {@code null} if there is no
Expand Down Expand Up @@ -118,7 +119,7 @@ public interface Cache<K, V> {
* left unestablished
*/
Map<K, V> getAll(Iterable<? extends K> keys,
Function<Set<? extends K>, Map<K, V>> mappingFunction);
Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction);

/**
* Associates the {@code value} with the {@code key} in this cache. If the cache previously
Expand All @@ -144,7 +145,7 @@ Map<K, V> getAll(Iterable<? extends K> keys,
* @throws NullPointerException if the specified map is null or the specified map contains null
* keys or values
*/
void putAll(Map<? extends K,? extends V> map);
void putAll(Map<? extends K, ? extends V> map);

/**
* Discards any cached value for the {@code key}. The behavior of this operation is undefined for
Expand Down
26 changes: 15 additions & 11 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java
View file
Expand Up @@ -24,6 +24,7 @@
import java.util.concurrent.Executor;
import java.util.function.Function;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
Expand All @@ -43,7 +44,8 @@
*/
@FunctionalInterface
@SuppressWarnings({"PMD.SignatureDeclareThrowsException", "FunctionalInterfaceMethodChanged"})
public interface CacheLoader<K, V> extends AsyncCacheLoader<K, V> {
public interface CacheLoader<K extends @NonNull Object, V extends @NonNull Object>
extends AsyncCacheLoader<K, V> {

/**
* Computes or retrieves the value corresponding to {@code key}.
Expand Down Expand Up @@ -83,7 +85,7 @@ public interface CacheLoader<K, V> extends AsyncCacheLoader<K, V> {
* treated like any other {@code Exception} in all respects except that, when it is
* caught, the thread's interrupt status is set
*/
default Map<K, V> loadAll(Set<? extends K> keys) throws Exception {
default Map<? extends K, ? extends V> loadAll(Set<? extends K> keys) throws Exception {
throw new UnsupportedOperationException();
}

Expand All @@ -95,7 +97,7 @@ default Map<K, V> loadAll(Set<? extends K> keys) throws Exception {
* @return the future value associated with {@code key}
*/
@Override
default CompletableFuture<V> asyncLoad(K key, Executor executor) {
default CompletableFuture<? extends V> asyncLoad(K key, Executor executor) {
requireNonNull(key);
requireNonNull(executor);
return CompletableFuture.supplyAsync(() -> {
Expand Down Expand Up @@ -128,7 +130,8 @@ default CompletableFuture<V> asyncLoad(K key, Executor executor) {
* that key; <b>may not contain null values</b>
*/
@Override
default CompletableFuture<Map<K, V>> asyncLoadAll(Set<? extends K> keys, Executor executor) {
default CompletableFuture<? extends Map<? extends K, ? extends V>> asyncLoadAll(
Set<? extends K> keys, Executor executor) {
requireNonNull(keys);
requireNonNull(executor);
return CompletableFuture.supplyAsync(() -> {
Expand Down Expand Up @@ -159,8 +162,7 @@ default CompletableFuture<Map<K, V>> asyncLoadAll(Set<? extends K> keys, Executo
* treated like any other {@code Exception} in all respects except that, when it is
* caught, the thread's interrupt status is set
*/
@Nullable
default V reload(K key, V oldValue) throws Exception {
default @Nullable V reload(K key, V oldValue) throws Exception {
return load(key);
}

Expand All @@ -179,7 +181,8 @@ default V reload(K key, V oldValue) throws Exception {
* {@code null} if the mapping is to be removed
*/
@Override
default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) throws Exception {
default CompletableFuture<? extends V> asyncReload(
K key, V oldValue, Executor executor) throws Exception {
requireNonNull(key);
requireNonNull(executor);
return CompletableFuture.supplyAsync(() -> {
Expand All @@ -195,8 +198,8 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t

/**
* Returns a cache loader that delegates to the supplied mapping function for retrieving the
* values. Note that {@link #load(} will discard any additional mappings loaded when retrieving
* the {@code key} prior to returning to the value to the cache.
* values. Note that {@link #load} will silently discard any additional mappings loaded when
* retrieving the {@code key} prior to returning to the value to the cache.
* <p>
* Usage example:
* <pre>{@code
Expand All @@ -208,13 +211,14 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t
* @return a cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
static <K, V> CacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>> mappingFunction) {
static <K extends Object, V extends Object> CacheLoader<K, V> bulk(
Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction) {
requireNonNull(mappingFunction);
return new CacheLoader<K, V>() {
@Override public @Nullable V load(K key) {
return loadAll(Set.of(key)).get(key);
}
@Override public Map<K, V> loadAll(Set<? extends K> keys) {
@Override public Map<? extends K, ? extends V> loadAll(Set<? extends K> keys) {
return mappingFunction.apply(keys);
}
};
Expand Down
3 changes: 2 additions & 1 deletion caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheWriter.java
View file
Expand Up @@ -17,6 +17,7 @@

import java.util.Map;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
Expand All @@ -35,7 +36,7 @@
* eviction.
*/
@Deprecated
public interface CacheWriter<K, V> {
public interface CacheWriter<K extends @NonNull Object, V extends @NonNull Object> {

/***
* Writes the value corresponding to the {@code key} to the external resource. The cache will
Expand Down
7 changes: 4 additions & 3 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java
View file
Expand Up @@ -37,6 +37,7 @@
import java.util.function.Supplier;

import org.checkerframework.checker.index.qual.NonNegative;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

import com.github.benmanes.caffeine.cache.Async.AsyncExpiry;
Expand Down Expand Up @@ -135,7 +136,7 @@
* normally {@code Object} unless it is constrained by using a method like {@code
* #removalListener}
*/
public final class Caffeine<K, V> {
public final class Caffeine<K extends @NonNull Object, V extends @NonNull Object> {
static final Logger logger = System.getLogger(Caffeine.class.getName());
static final Supplier<StatsCounter> ENABLED_STATS_COUNTER_SUPPLIER = ConcurrentStatsCounter::new;

Expand Down Expand Up @@ -1150,8 +1151,8 @@ public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
@SuppressWarnings("unchecked")
Caffeine<K1, V1> self = (Caffeine<K1, V1>) this;
return isBounded() || refreshAfterWrite()
? new BoundedLocalCache.BoundedLocalAsyncLoadingCache<>(self, loader)
: new UnboundedLocalCache.UnboundedLocalAsyncLoadingCache<>(self, loader);
? new BoundedLocalCache.BoundedLocalAsyncLoadingCache<K1, V1>(self, loader)
: new UnboundedLocalCache.UnboundedLocalAsyncLoadingCache<K1, V1>(self, loader);
}

void requireNonLoadingCache() {
Expand Down
2 changes: 1 addition & 1 deletion caffeine/src/main/java/com/github/benmanes/caffeine/cache/CaffeineSpec.java
View file
Expand Up @@ -330,7 +330,7 @@ static TimeUnit parseTimeUnit(String key, @Nullable String value) {
}

@Override
public boolean equals(Object o) {
public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
} else if (!(o instanceof CaffeineSpec)) {
Expand Down

0 comments on commit c40966b

Please sign in to comment.