Skip to content

Commit

Permalink
Fixes for the JCache 1.1 preview, except for 1.0 TCK breaking changes
Browse files Browse the repository at this point in the history
A preview of JCache 1.1 has been made available for review until 11/18.
This update includes JavaDoc fixes, spec corrections, additional tests,
and fixes incorrect tests.

The 1.1 TCK is not compatible with 1.0's TCK. Some tests assert the
opposite behavior, such as relaxing types to not throw exceptions or
changing the statistics of putIfAbsent. Since 1.1 is in preview, we
maintain 1.0 compatibility with the acceptable fixes uncovered by the
later TCK.

In particular the specification requires that a loader, writer, expiry,
and listeners are closed if they implement Closable. This was in the
JavaDoc but not the TCK, making it easy to overlook.

https://jcp.org/aboutJava/communityprocess/maintenance/jsr107/107MR1-summary.html
  • Loading branch information
ben-manes committed Nov 1, 2017
1 parent 74a3dc3 commit 6be13d6
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@
* @author ben.manes@gmail.com (Ben Manes)
*/
final class CacheFactory {
// Avoid asynchronous executions due to the TCK's poor test practices
// https://github.com/jsr107/jsr107tck/issues/78
static final boolean USE_DIRECT_EXECUTOR =
System.getProperties().containsKey("org.jsr107.tck.management.agentId");

final Config rootConfig;
final CacheManager cacheManager;

Expand Down Expand Up @@ -143,8 +138,8 @@ private final class Builder<K, V> {
this.caffeine = Caffeine.newBuilder();
this.statistics = new JCacheStatisticsMXBean();
this.ticker = config.getTickerFactory().create();
this.executor = config.getExecutorFactory().create();
this.expiryPolicy = config.getExpiryPolicyFactory().create();
this.executor = USE_DIRECT_EXECUTOR ? Runnable::run : config.getExecutorFactory().create();
this.dispatcher = new EventDispatcher<>(executor);

caffeine.executor(executor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;

import java.io.Closeable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -57,6 +58,7 @@
import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
import com.github.benmanes.caffeine.jcache.copy.Copier;
import com.github.benmanes.caffeine.jcache.event.EventDispatcher;
import com.github.benmanes.caffeine.jcache.event.Registration;
import com.github.benmanes.caffeine.jcache.integration.DisabledCacheWriter;
import com.github.benmanes.caffeine.jcache.management.JCacheMXBean;
import com.github.benmanes.caffeine.jcache.management.JCacheStatisticsMXBean;
Expand Down Expand Up @@ -864,6 +866,11 @@ public CacheManager getCacheManager() {
return cacheManager;
}

@Override
public boolean isClosed() {
return closed;
}

@Override
public void close() {
if (isClosed()) {
Expand All @@ -875,14 +882,43 @@ public void close() {
enableStatistics(false);
cacheManager.destroyCache(name);
closed = true;

Throwable thrown = null;
thrown = tryClose(expiry, thrown);
thrown = tryClose(writer, thrown);
thrown = tryClose(cacheLoader.orElse(null), thrown);
for (Registration<K, V> registration : dispatcher.registrations()) {
thrown = tryClose(registration.getCacheEntryListener(), thrown);
}
if (thrown != null) {
logger.log(Level.WARNING, "Failure when closing cache resources", thrown);
}
}
}
cache.invalidateAll();
}

@Override
public boolean isClosed() {
return closed;
/**
* Attempts to close the resource. If an error occurs and an outermost exception is set, then adds
* the error to the suppression list.
*
* @param o the resource to close if Closeable
* @param outer the outermost error, or null if unset
* @return the outermost error, or null if unset and successful
*/
private static Throwable tryClose(Object o, @Nullable Throwable outer) {
if (o instanceof Closeable) {
try {
((Closeable) o).close();
} catch (Throwable t) {
if (outer == null) {
return t;
}
outer.addSuppressed(t);
return outer;
}
}
return null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import static java.util.Objects.requireNonNull;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -62,6 +64,11 @@ public EventDispatcher(Executor exectuor) {
this.exectuor = requireNonNull(exectuor);
}

/** Returns the cache entry listener registrations. */
public Set<Registration<K, V>> registrations() {
return Collections.unmodifiableSet(dispatchQueues.keySet());
}

/**
* Registers a cache entry listener based on the supplied configuration.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import static java.util.Objects.requireNonNull;

import java.io.Closeable;
import java.io.IOException;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -36,7 +38,7 @@
*/
final class EventTypeAwareListener<K, V> implements CacheEntryCreatedListener<K, V>,
CacheEntryUpdatedListener<K, V>, CacheEntryRemovedListener<K, V>,
CacheEntryExpiredListener<K, V> {
CacheEntryExpiredListener<K, V>, Closeable {
static final Logger logger = Logger.getLogger(EventTypeAwareListener.class.getName());

final CacheEntryListener<? super K, ? super V> listener;
Expand All @@ -63,6 +65,9 @@ public boolean isCompatible(@Nonnull EventType eventType) {
/** Processes the event and logs if an exception is thrown. */
public void dispatch(@Nonnull JCacheEntryEvent<K, V> event) {
try {
if (event.getSource().isClosed()) {
return;
}
switch (event.getEventType()) {
case CREATED:
onCreated(event);
Expand Down Expand Up @@ -116,4 +121,11 @@ public void onExpired(Iterable<CacheEntryEvent<? extends K, ? extends V>> events
((CacheEntryExpiredListener<K, V>) listener).onExpired(events);
}
}

@Override
public void close() throws IOException {
if (listener instanceof Closeable) {
((Closeable) listener).close();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*
* @author ben.manes@gmail.com (Ben Manes)
*/
final class Registration<K, V> {
public final class Registration<K, V> {
private final CacheEntryListenerConfiguration<K, V> configuration;
private final EventTypeAwareListener<K, V> listener;
private final CacheEntryEventFilter<K, V> filter;
Expand Down

0 comments on commit 6be13d6

Please sign in to comment.