Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix jsr107 CacheLoader adapter to handle null values (fixes #119)
This is required by the specification (per JavaDoc) but the TCK does
not validate it, so sadly I failed to handle that case.

To get CI functioning again, moving back to a jdk8-only config. I don't
know why the new config hangs with Guava's tests and think it might be
the distro.

Also fixed the misnamed method for DI factories. I'm going to argue it
doesn't break semver since JCache is the API and this is an impl hook
no one calls directly.
  • Loading branch information
ben-manes committed Sep 1, 2016
1 parent 12cd7f2 commit 54ce68d
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 23 deletions.
21 changes: 10 additions & 11 deletions .travis.yml
@@ -1,21 +1,16 @@
language: java

dist: trusty
group: edge
sudo: false

jdk:
- oraclejdk8
- oraclejdk9

env:
- TERM=dumb

before_install:
- cp gradle.properties.ci gradle.properties
- export JAVA_OPTS="-Xmx384m -XX:+UseG1GC -XX:SoftRefLRUPolicyMSPerMB=0 -noverify"

install:
- ./gradlew assemble --stacktrace

before_script:
- export MAVEN_SKIP_RC=true
script:
- ./gradlew check
- sh -c 'cd examples/write-behind-rxjava && mvn test'
Expand All @@ -24,8 +19,12 @@ after_success:
- ./gradlew coveralls uploadArchives

matrix:
allow_failures:
- jdk: oraclejdk9
fast_finish: true

addons:
apt:
packages:
- oracle-java8-installer

before_cache:
- rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
Expand Down
37 changes: 37 additions & 0 deletions .travis.yml.jdk9
@@ -0,0 +1,37 @@
language: java

dist: trusty
group: edge

jdk:
- oraclejdk8
- oraclejdk9

before_install:
- cp gradle.properties.ci gradle.properties
- export JAVA_OPTS="-Xmx384m -XX:+UseG1GC -XX:SoftRefLRUPolicyMSPerMB=0 -noverify"

install:
- ./gradlew assemble --stacktrace

before_script:
- export MAVEN_SKIP_RC=true
script:
- ./gradlew check
- sh -c 'cd examples/write-behind-rxjava && mvn test'

after_success:
- ./gradlew coveralls uploadArchives

matrix:
allow_failures:
- jdk: oraclejdk9

before_cache:
- rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
- rm -fr $HOME/.gradle/caches/*/plugin-resolution/
cache:
directories:
- $HOME/.m2
- $HOME/.gradle/caches/
- $HOME/.gradle/wrapper/
4 changes: 2 additions & 2 deletions gradle/dependencies.gradle
Expand Up @@ -29,7 +29,7 @@ ext {
commons_compress: '1.12',
commons_lang3: '3.4',
config: '1.3.0',
error_prone_annotations: '2.0.11',
error_prone_annotations: '2.0.12',
flip_tables: '1.0.2',
guava: '19.0',
javapoet: '1.7.0',
Expand All @@ -48,7 +48,7 @@ ext {
jcache_tck: '1.0.1',
jctools: '1.2.1',
junit: '4.12',
mockito: '2.0.111-beta',
mockito: '2.1.0-beta.119',
pax_exam: '4.9.1',
testng: '6.9.12',
truth: '0.24',
Expand Down
Expand Up @@ -93,7 +93,7 @@ public static <K, V> Optional<CaffeineConfiguration<K, V>> from(Config config, S
* @param factoryCreator the strategy for creating a factory
*/
@Inject
public static void setFactoryBuilder(FactoryCreator factoryCreator) {
public static void setFactoryCreator(FactoryCreator factoryCreator) {
TypesafeConfigurator.factoryCreator = requireNonNull(factoryCreator);
}

Expand Down
Expand Up @@ -72,6 +72,10 @@ public Expirable<V> load(K key) {
long start = statsEnabled ? ticker.read() : 0L;

V value = delegate.load(key);
if (value == null) {
return null;
}

dispatcher.publishCreated(cache, key, value);

if (statsEnabled) {
Expand Down
Expand Up @@ -23,7 +23,6 @@
import javax.cache.CacheManager;
import javax.cache.Caching;
import javax.cache.integration.CacheLoader;
import javax.cache.integration.CacheLoaderException;
import javax.cache.spi.CachingProvider;

import org.testng.annotations.AfterMethod;
Expand All @@ -33,7 +32,6 @@

import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
import com.github.benmanes.caffeine.jcache.spi.CaffeineCachingProvider;
import com.google.common.base.Functions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
Expand Down Expand Up @@ -108,15 +106,20 @@ protected final long currentTimeMillis() {
/** The loading configuration used by the test. */
protected CaffeineConfiguration<Integer, Integer> getLoadingConfiguration() {
CaffeineConfiguration<Integer, Integer> configuration = getConfiguration();
configuration.setCacheLoaderFactory(() -> new CacheLoader<Integer, Integer>() {
@Override public Integer load(Integer key) throws CacheLoaderException {
configuration.setCacheLoaderFactory(this::getCacheLoader);
configuration.setReadThrough(true);
return configuration;
}

/** The cache loader used by the test. */
protected CacheLoader<Integer, Integer> getCacheLoader() {
return new CacheLoader<Integer, Integer>() {
@Override public Integer load(Integer key) {
return key;
}
@Override public Map<Integer, Integer> loadAll(Iterable<? extends Integer> keys) {
return Maps.asMap(ImmutableSet.copyOf(keys), Functions.identity());
return Maps.asMap(ImmutableSet.copyOf(keys), this::load);
}
});
configuration.setReadThrough(true);
return configuration;
};
}
}
Expand Up @@ -64,7 +64,7 @@ public void beforeMethod() {

@AfterClass
public void afterClass() {
TypesafeConfigurator.setFactoryBuilder(FactoryBuilder::factoryOf);
TypesafeConfigurator.setFactoryCreator(FactoryBuilder::factoryOf);
}

@Test
Expand Down
@@ -0,0 +1,111 @@
/*
* Copyright 2016 Ben Manes. All Rights Reserved.
*
* 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.github.benmanes.caffeine.jcache.integration;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anEmptyMap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

import java.util.Collections;
import java.util.Map;
import java.util.function.Supplier;

import javax.cache.integration.CacheLoader;
import javax.cache.integration.CacheLoaderException;

import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import com.github.benmanes.caffeine.jcache.AbstractJCacheTest;
import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;

/**
* @author ben.manes@gmail.com (Ben Manes)
*/
public final class CacheLoaderTest extends AbstractJCacheTest {
private Supplier<Map<Integer, Integer>> loadAllSupplier;
private Supplier<Integer> loadSupplier;

@BeforeMethod
public void beforeMethod() {
loadSupplier = () -> { throw new UnsupportedOperationException(); };
loadAllSupplier = () -> { throw new UnsupportedOperationException(); };
}

@Test
public void load() {
loadSupplier = () -> -1;
assertThat(jcacheLoading.get(1), is(-1));
}

@Test
public void load_null() {
loadSupplier = () -> null;
assertThat(jcacheLoading.get(1), is(nullValue()));
}

@Test
public void load_failure() {
try {
loadSupplier = () -> { throw new IllegalStateException(); };
jcacheLoading.get(1);
Assert.fail();
} catch (CacheLoaderException e) {
assertThat(e.getCause(), instanceOf(IllegalStateException.class));
}
}

@Test
public void loadAll() {
loadAllSupplier = () -> ImmutableMap.of(1, -1, 2, -2, 3, -3);
Map<Integer, Integer> result = jcacheLoading.getAll(ImmutableSet.of(1, 2, 3));
assertThat(result, is(equalTo(loadAllSupplier.get())));
}

@Test(expectedExceptions = CacheLoaderException.class)
public void loadAll_null() {
loadAllSupplier = () -> null;
jcacheLoading.getAll(ImmutableSet.of(1, 2, 3));
}

@Test
public void loadAll_nullMapping() {
loadAllSupplier = () -> Collections.singletonMap(1, null);
Map<Integer, Integer> result = jcacheLoading.getAll(ImmutableSet.of(1, 2, 3));
assertThat(result, is(anEmptyMap()));
}

@Override protected CaffeineConfiguration<Integer, Integer> getConfiguration() {
return new CaffeineConfiguration<>();
}

@Override protected CacheLoader<Integer, Integer> getCacheLoader() {
return new CacheLoader<Integer, Integer>() {
@Override public Integer load(Integer key) {
return loadSupplier.get();
}
@Override public Map<Integer, Integer> loadAll(Iterable<? extends Integer> keys) {
return loadAllSupplier.get();
}
};
}
}

0 comments on commit 54ce68d

Please sign in to comment.