Skip to content

Commit 025c15f

Browse files
alexsnapsalexsnaps
committed
Issue #523 Eh107CacheManager pass
1 parent b3d4332 commit 025c15f

File tree

4 files changed

+76
-24
lines changed

4 files changed

+76
-24
lines changed

‎107/src/main/java/org/ehcache/jsr107/Eh107Cache.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535

3636
import org.ehcache.Ehcache;
3737
import org.ehcache.EhcacheHackAccessor;
38+
import org.ehcache.Status;
39+
import org.ehcache.UserManagedCache;
3840
import org.ehcache.event.EventFiring;
3941
import org.ehcache.event.EventOrdering;
4042
import org.ehcache.function.BiFunction;
@@ -436,7 +438,7 @@ public void close() {
436438

437439
@Override
438440
public boolean isClosed() {
439-
return closed.get();
441+
return closed.get() || ((UserManagedCache)ehCache).getStatus() != Status.AVAILABLE;
440442
}
441443

442444
void closeInternal(MultiCacheException closeException) {

‎107/src/main/java/org/ehcache/jsr107/Eh107CacheManager.java‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,21 @@ class Eh107CacheManager implements CacheManager {
7575
this.managementRegistry = managementRegistry;
7676
this.configurationMerger = configurationMerger;
7777

78-
loadExistingEhcaches();
78+
refreshAllCaches();
7979
}
8080

8181
EhcacheManager getEhCacheManager() {
8282
return ehCacheManager;
8383
}
8484

85-
private void loadExistingEhcaches() {
85+
private void refreshAllCaches() {
8686
for (Map.Entry<String, CacheConfiguration<?, ?>> entry : ehCacheManager.getRuntimeConfiguration().getCacheConfigurations().entrySet()) {
8787
String name = entry.getKey();
8888
CacheConfiguration<?, ?> config = entry.getValue();
89-
caches.put(name, wrapEhcacheCache(name, config));
89+
caches.putIfAbsent(name, wrapEhcacheCache(name, config));
90+
}
91+
for (Map.Entry<String, Eh107Cache<?, ?>> namedCacheEntry : caches.entrySet()) {
92+
namedCacheEntry.getValue().isClosed();
9093
}
9194
}
9295

@@ -135,22 +138,26 @@ public Properties getProperties() {
135138
public <K, V, C extends Configuration<K, V>> Cache<K, V> createCache(String cacheName, C config)
136139
throws IllegalArgumentException {
137140

138-
synchronized (cachesLock) {
139-
checkClosed();
141+
checkClosed();
140142

141-
// TCK expects the "closed" check before these null checks
142-
if (cacheName == null || config == null) {
143-
throw new NullPointerException();
144-
}
143+
// TCK expects the "closed" check before these null checks
144+
if (cacheName == null || config == null) {
145+
throw new NullPointerException();
146+
}
145147

146-
if (caches.containsKey(cacheName)) {
147-
throw new CacheException("A Cache named [" + cacheName + "] already exists");
148-
}
148+
synchronized (cachesLock) {
149149

150150
if (config instanceof Eh107Configuration.Eh107ConfigurationWrapper) {
151151
Eh107Configuration.Eh107ConfigurationWrapper<K, V> configurationWrapper = (Eh107Configuration.Eh107ConfigurationWrapper<K, V>)config;
152152
CacheConfiguration<K, V> unwrap = configurationWrapper.getCacheConfiguration();
153-
Eh107Cache<K, V> cache = wrapEhcacheCache(cacheName, ehCacheManager.createCache(cacheName, unwrap));
153+
final org.ehcache.Cache<K, V> ehcache;
154+
try {
155+
ehcache = ehCacheManager.createCache(cacheName, unwrap);
156+
} catch (IllegalArgumentException e) {
157+
throw new CacheException("A Cache named [" + cacheName + "] already exists");
158+
}
159+
Eh107Cache<K, V> cache = wrapEhcacheCache(cacheName, ehcache);
160+
assert safeCacheRetrieval(cacheName) == null;
154161
caches.put(cacheName, cache);
155162

156163
return cache;
@@ -161,10 +168,12 @@ public <K, V, C extends Configuration<K, V>> Cache<K, V> createCache(String cach
161168
final org.ehcache.Cache<K, V> ehCache;
162169
try {
163170
ehCache = ehCacheManager.createCache(cacheName, configHolder.cacheConfiguration);
171+
} catch (IllegalArgumentException e) {
172+
MultiCacheException mce = new MultiCacheException(e);
173+
configHolder.cacheResources.closeResources(mce);
174+
throw new CacheException("A Cache named [" + cacheName + "] already exists", mce);
164175
} catch (Throwable t) {
165176
// something went wrong in ehcache land, make sure to clean up our stuff
166-
// NOTE: one relatively simple error path is if a pre-configured cache of the same name already exists in
167-
// ehcache
168177
MultiCacheException mce = new MultiCacheException(t);
169178
configHolder.cacheResources.closeResources(mce);
170179
throw mce;
@@ -223,7 +232,7 @@ public <K, V> Cache<K, V> getCache(String cacheName, Class<K> keyType, Class<V>
223232
throw new NullPointerException();
224233
}
225234

226-
Eh107Cache<K, V> cache = (Eh107Cache<K, V>) caches.get(cacheName);
235+
Eh107Cache<K, V> cache = safeCacheRetrieval(cacheName);
227236
if (cache == null) {
228237
return null;
229238
}
@@ -253,7 +262,7 @@ public <K, V> Cache<K, V> getCache(String cacheName) {
253262
throw new NullPointerException();
254263
}
255264

256-
Eh107Cache<K, V> cache = (Eh107Cache<K, V>) caches.get(cacheName);
265+
Eh107Cache<K, V> cache = safeCacheRetrieval(cacheName);
257266

258267
if (cache == null) {
259268
return null;
@@ -267,8 +276,18 @@ public <K, V> Cache<K, V> getCache(String cacheName) {
267276
return cache;
268277
}
269278

279+
@SuppressWarnings("unchecked")
280+
private <K, V> Eh107Cache<K, V> safeCacheRetrieval(final String cacheName) {
281+
final Eh107Cache<?, ?> eh107Cache = caches.get(cacheName);
282+
if(eh107Cache != null && eh107Cache.isClosed()) {
283+
return null;
284+
}
285+
return (Eh107Cache<K, V>) eh107Cache;
286+
}
287+
270288
@Override
271289
public Iterable<String> getCacheNames() {
290+
refreshAllCaches();
272291
return Collections.unmodifiableList(new ArrayList<String>(caches.keySet()));
273292
}
274293

@@ -321,7 +340,7 @@ public void enableManagement(String cacheName, boolean enabled) {
321340
throw new NullPointerException();
322341
}
323342

324-
Eh107Cache<?, ?> cache = caches.get(cacheName);
343+
Eh107Cache<?, ?> cache = safeCacheRetrieval(cacheName);
325344
if (cache == null) {
326345
throw new IllegalArgumentException("No such Cache named " + cacheName);
327346
}
@@ -371,7 +390,7 @@ public void enableStatistics(String cacheName, boolean enabled) {
371390
throw new NullPointerException();
372391
}
373392

374-
Eh107Cache<?, ?> cache = caches.get(cacheName);
393+
Eh107Cache<?, ?> cache = safeCacheRetrieval(cacheName);
375394
if (cache == null) {
376395
throw new IllegalArgumentException("No such Cache named " + cacheName);
377396
}

‎107/src/main/java/org/ehcache/jsr107/EhcacheCachingProvider.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,12 @@ public CacheManager getCacheManager(URI uri, ClassLoader classLoader, Properties
8585
}
8686

8787
cacheManager = byURI.get(uri);
88-
if (cacheManager == null) {
88+
if (cacheManager == null || cacheManager.isClosed()) {
89+
90+
if(cacheManager != null) {
91+
byURI.remove(uri, cacheManager);
92+
}
93+
8994
Configuration config;
9095
try {
9196
if (URI_DEFAULT.equals(uri)) {

‎107/src/test/java/org/ehcache/jsr107/Eh107CacheTypeTest.java‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import static org.hamcrest.CoreMatchers.equalTo;
2626
import static org.hamcrest.CoreMatchers.is;
27+
import static org.hamcrest.CoreMatchers.nullValue;
2728
import static org.hamcrest.MatcherAssert.assertThat;
2829
import static org.junit.Assert.fail;
2930

@@ -98,8 +99,33 @@ public void testTypeOverriding() throws Exception {
9899
assertThat((Class<String>)cache1CompleteConf.getValueType(), is(equalTo(String.class)));
99100
}
100101

102+
@Test
103+
public void testEhcacheCloseRemovesFromCacheManager() throws Exception {
104+
CachingProvider provider = Caching.getCachingProvider();
105+
javax.cache.CacheManager cacheManager =
106+
provider.getCacheManager(this.getClass()
107+
.getResource("/ehcache-107-types.xml")
108+
.toURI(), getClass().getClassLoader());
109+
MutableConfiguration<Long, String> cache1Conf = new MutableConfiguration<Long, String>();
110+
javax.cache.Cache<Long, String> cache = cacheManager.createCache("cache1", cache1Conf);
111+
cacheManager.unwrap(org.ehcache.CacheManager.class).removeCache(cache.getName());
112+
try {
113+
assertThat(cacheManager.getCache(cache.getName()), nullValue());
114+
} finally {
115+
cacheManager.close();
116+
}
117+
}
101118

102-
103-
104-
119+
@Test
120+
public void testCacheManagerCloseLenientToEhcacheClosed() throws Exception {
121+
CachingProvider provider = Caching.getCachingProvider();
122+
javax.cache.CacheManager cacheManager =
123+
provider.getCacheManager(this.getClass()
124+
.getResource("/ehcache-107-types.xml")
125+
.toURI(), getClass().getClassLoader());
126+
MutableConfiguration<Long, String> cache1Conf = new MutableConfiguration<Long, String>();
127+
javax.cache.Cache<Long, String> cache = cacheManager.createCache("cache1", cache1Conf);
128+
cacheManager.unwrap(org.ehcache.CacheManager.class).removeCache(cache.getName());
129+
cacheManager.close();
130+
}
105131
}

0 commit comments

Comments
 (0)