Skip to content

Commit 094f7aa

Browse files
snicollsnicoll
committed
Fix Hazelcast Cache auto-configuration ordering
Spring Boot supports the automatic configuration of an additional HazelcastInstance if one already exists and an explicit property has been set to use a different configuration for caching. So three cases are supported really: no `HazelcastInstance` exists so we need to create one anyway or an `HazelcastInstance` already exists; in that latter case, we should either reuse it or create a new one. Unfortunately, the conditions that checked those three use cases were not ordered consistently and we could easily get in a situation where both conditions were evaluated. This commit makes sure that we first check if an `HazelcastInstance` exists and then (and only then) we create the missing `HazelcastInstance` used for caching. The tests have also been improved to validate the proper `HazelcastInstance` is used for caching. Closes gh-5181
1 parent 14a5fea commit 094f7aa

File tree

3 files changed

+142
-97
lines changed

3 files changed

+142
-97
lines changed

‎spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/HazelcastCacheConfiguration.java

Lines changed: 4 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2015 the original author or authors.
2+
* Copyright 2012-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,25 +16,17 @@
1616

1717
package org.springframework.boot.autoconfigure.cache;
1818

19-
import java.io.Closeable;
20-
import java.io.IOException;
21-
22-
import com.hazelcast.core.Hazelcast;
2319
import com.hazelcast.core.HazelcastInstance;
2420
import com.hazelcast.spring.cache.HazelcastCacheManager;
2521

26-
import org.springframework.beans.factory.annotation.Autowired;
2722
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
2823
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
29-
import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate;
3024
import org.springframework.boot.autoconfigure.hazelcast.HazelcastAutoConfiguration;
3125
import org.springframework.boot.autoconfigure.hazelcast.HazelcastConfigResourceCondition;
32-
import org.springframework.boot.autoconfigure.hazelcast.HazelcastInstanceFactory;
3326
import org.springframework.cache.CacheManager;
34-
import org.springframework.context.annotation.Bean;
3527
import org.springframework.context.annotation.Conditional;
3628
import org.springframework.context.annotation.Configuration;
37-
import org.springframework.core.io.Resource;
29+
import org.springframework.context.annotation.Import;
3830

3931
/**
4032
* Hazelcast cache configuration. Can either reuse the {@link HazelcastInstance} that has
@@ -49,84 +41,10 @@
4941
* @see HazelcastConfigResourceCondition
5042
*/
5143
@Configuration
52-
@ConditionalOnClass({ HazelcastInstance.class, HazelcastCacheManager.class })
44+
@ConditionalOnClass({HazelcastInstance.class, HazelcastCacheManager.class})
5345
@ConditionalOnMissingBean(CacheManager.class)
5446
@Conditional(CacheCondition.class)
47+
@Import({HazelcastInstanceConfiguration.Existing.class, HazelcastInstanceConfiguration.Specific.class})
5548
class HazelcastCacheConfiguration {
5649

57-
@Configuration
58-
@ConditionalOnSingleCandidate(HazelcastInstance.class)
59-
static class ExistingHazelcastInstanceConfiguration {
60-
61-
@Autowired
62-
private CacheProperties cacheProperties;
63-
64-
@Bean
65-
public HazelcastCacheManager cacheManager(
66-
HazelcastInstance existingHazelcastInstance) throws IOException {
67-
Resource config = this.cacheProperties.getHazelcast().getConfig();
68-
Resource location = this.cacheProperties.resolveConfigLocation(config);
69-
if (location != null) {
70-
HazelcastInstance cacheHazelcastInstance = new HazelcastInstanceFactory(
71-
location).getHazelcastInstance();
72-
return new CloseableHazelcastCacheManager(cacheHazelcastInstance);
73-
}
74-
return new HazelcastCacheManager(existingHazelcastInstance);
75-
}
76-
}
77-
78-
@Configuration
79-
@ConditionalOnMissingBean(HazelcastInstance.class)
80-
@Conditional(ConfigAvailableCondition.class)
81-
static class DefaultHazelcastInstanceConfiguration {
82-
83-
@Autowired
84-
private CacheProperties cacheProperties;
85-
86-
@Bean
87-
public HazelcastInstance hazelcastInstance() throws IOException {
88-
Resource config = this.cacheProperties.getHazelcast().getConfig();
89-
Resource location = this.cacheProperties.resolveConfigLocation(config);
90-
if (location != null) {
91-
new HazelcastInstanceFactory(location).getHazelcastInstance();
92-
}
93-
return Hazelcast.newHazelcastInstance();
94-
}
95-
96-
@Bean
97-
public HazelcastCacheManager cacheManager() throws IOException {
98-
return new HazelcastCacheManager(hazelcastInstance());
99-
}
100-
101-
}
102-
103-
/**
104-
* {@link HazelcastConfigResourceCondition} that checks if the
105-
* {@code spring.cache.hazelcast.config} configuration key is defined.
106-
*/
107-
static class ConfigAvailableCondition extends HazelcastConfigResourceCondition {
108-
109-
ConfigAvailableCondition() {
110-
super("spring.cache.hazelcast", "config");
111-
}
112-
113-
}
114-
115-
private static class CloseableHazelcastCacheManager extends HazelcastCacheManager
116-
implements Closeable {
117-
118-
private final HazelcastInstance hazelcastInstance;
119-
120-
CloseableHazelcastCacheManager(HazelcastInstance hazelcastInstance) {
121-
super(hazelcastInstance);
122-
this.hazelcastInstance = hazelcastInstance;
123-
}
124-
125-
@Override
126-
public void close() throws IOException {
127-
this.hazelcastInstance.shutdown();
128-
}
129-
130-
}
131-
13250
}

‎spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/HazelcastInstanceConfiguration.java

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/*
2+
* Copyright 2012-2016 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.autoconfigure.cache;
18+
19+
import java.io.Closeable;
20+
import java.io.IOException;
21+
22+
import com.hazelcast.core.Hazelcast;
23+
import com.hazelcast.core.HazelcastInstance;
24+
import com.hazelcast.spring.cache.HazelcastCacheManager;
25+
26+
import org.springframework.beans.factory.annotation.Autowired;
27+
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
28+
import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate;
29+
import org.springframework.boot.autoconfigure.hazelcast.HazelcastConfigResourceCondition;
30+
import org.springframework.boot.autoconfigure.hazelcast.HazelcastInstanceFactory;
31+
import org.springframework.context.annotation.Bean;
32+
import org.springframework.context.annotation.Conditional;
33+
import org.springframework.context.annotation.Configuration;
34+
import org.springframework.core.io.Resource;
35+
36+
/**
37+
* Actual {@link HazelcastInstance} configurations imported by
38+
* {@link HazelcastCacheConfiguration}.
39+
*
40+
* @author Stephane Nicoll
41+
*/
42+
abstract class HazelcastInstanceConfiguration {
43+
44+
@Configuration
45+
@ConditionalOnSingleCandidate(HazelcastInstance.class)
46+
static class Existing {
47+
48+
@Autowired
49+
private CacheProperties cacheProperties;
50+
51+
@Bean
52+
public HazelcastCacheManager cacheManager(
53+
HazelcastInstance existingHazelcastInstance) throws IOException {
54+
Resource config = this.cacheProperties.getHazelcast().getConfig();
55+
Resource location = this.cacheProperties.resolveConfigLocation(config);
56+
if (location != null) {
57+
HazelcastInstance cacheHazelcastInstance = new HazelcastInstanceFactory(
58+
location).getHazelcastInstance();
59+
return new CloseableHazelcastCacheManager(cacheHazelcastInstance);
60+
}
61+
return new HazelcastCacheManager(existingHazelcastInstance);
62+
}
63+
}
64+
65+
@Configuration
66+
@ConditionalOnMissingBean(HazelcastInstance.class)
67+
@Conditional(ConfigAvailableCondition.class)
68+
static class Specific {
69+
70+
@Autowired
71+
private CacheProperties cacheProperties;
72+
73+
@Bean
74+
public HazelcastInstance hazelcastInstance() throws IOException {
75+
Resource config = this.cacheProperties.getHazelcast().getConfig();
76+
Resource location = this.cacheProperties.resolveConfigLocation(config);
77+
if (location != null) {
78+
return new HazelcastInstanceFactory(location).getHazelcastInstance();
79+
}
80+
return Hazelcast.newHazelcastInstance();
81+
}
82+
83+
@Bean
84+
public HazelcastCacheManager cacheManager() throws IOException {
85+
return new HazelcastCacheManager(hazelcastInstance());
86+
}
87+
88+
}
89+
90+
91+
/**
92+
* {@link HazelcastConfigResourceCondition} that checks if the
93+
* {@code spring.cache.hazelcast.config} configuration key is defined.
94+
*/
95+
static class ConfigAvailableCondition extends HazelcastConfigResourceCondition {
96+
97+
ConfigAvailableCondition() {
98+
super("spring.cache.hazelcast", "config");
99+
}
100+
101+
}
102+
103+
private static class CloseableHazelcastCacheManager extends HazelcastCacheManager
104+
implements Closeable {
105+
106+
private final HazelcastInstance hazelcastInstance;
107+
108+
CloseableHazelcastCacheManager(HazelcastInstance hazelcastInstance) {
109+
super(hazelcastInstance);
110+
this.hazelcastInstance = hazelcastInstance;
111+
}
112+
113+
@Override
114+
public void close() throws IOException {
115+
this.hazelcastInstance.shutdown();
116+
}
117+
118+
}
119+
120+
}

‎spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfigurationTests.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2015 the original author or authors.
2+
* Copyright 2012-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -70,6 +70,7 @@
7070

7171
import static org.hamcrest.CoreMatchers.equalTo;
7272
import static org.hamcrest.CoreMatchers.instanceOf;
73+
import static org.hamcrest.CoreMatchers.sameInstance;
7374
import static org.hamcrest.Matchers.contains;
7475
import static org.hamcrest.Matchers.containsInAnyOrder;
7576
import static org.hamcrest.Matchers.empty;
@@ -344,16 +345,21 @@ public void hazelcastCacheExplicit() {
344345
assertThat(cacheManager.getCacheNames(), containsInAnyOrder("defaultCache"));
345346
assertThat(cacheManager.getCacheNames(), hasSize(1));
346347
assertThat(this.context.getBean(HazelcastInstance.class),
347-
equalTo(new DirectFieldAccessor(cacheManager)
348-
.getPropertyValue("hazelcastInstance")));
348+
equalTo(getHazelcastInstance(cacheManager)));
349349
}
350350

351351
@Test
352-
public void hazelcastCacheWithConfig() {
352+
public void hazelcastCacheWithConfig() throws IOException {
353353
load(DefaultCacheConfiguration.class, "spring.cache.type=hazelcast",
354354
"spring.cache.hazelcast.config=org/springframework/boot/autoconfigure/cache/hazelcast-specific.xml");
355+
HazelcastInstance hazelcastInstance = this.context
356+
.getBean(HazelcastInstance.class);
355357
HazelcastCacheManager cacheManager = validateCacheManager(
356358
HazelcastCacheManager.class);
359+
HazelcastInstance actual = getHazelcastInstance(cacheManager);
360+
assertThat(actual, sameInstance(hazelcastInstance));
361+
assertThat(actual.getConfig().getConfigurationUrl(), equalTo(new ClassPathResource(
362+
"org/springframework/boot/autoconfigure/cache/hazelcast-specific.xml").getURL()));
357363
cacheManager.getCache("foobar");
358364
assertThat(cacheManager.getCacheNames(), containsInAnyOrder("foobar"));
359365
assertThat(cacheManager.getCacheNames(), hasSize(1));
@@ -372,9 +378,7 @@ public void hazelcastCacheWithExistingHazelcastInstance() {
372378
load(HazelcastCustomHazelcastInstance.class, "spring.cache.type=hazelcast");
373379
HazelcastCacheManager cacheManager = validateCacheManager(
374380
HazelcastCacheManager.class);
375-
assertThat(
376-
new DirectFieldAccessor(cacheManager)
377-
.getPropertyValue("hazelcastInstance"),
381+
assertThat(getHazelcastInstance(cacheManager),
378382
equalTo(this.context.getBean("customHazelcastInstance")));
379383
}
380384

@@ -392,8 +396,7 @@ public void hazelcastCacheWithMainHazelcastAutoConfiguration() throws IOExceptio
392396
HazelcastCacheManager.class);
393397
HazelcastInstance hazelcastInstance = this.context
394398
.getBean(HazelcastInstance.class);
395-
assertThat(new DirectFieldAccessor(cacheManager).getPropertyValue(
396-
"hazelcastInstance"), equalTo((Object) hazelcastInstance));
399+
assertThat(getHazelcastInstance(cacheManager), equalTo((Object) hazelcastInstance));
397400
assertThat(hazelcastInstance.getConfig().getConfigurationFile(),
398401
equalTo(new ClassPathResource(mainConfig).getFile()));
399402
}
@@ -416,8 +419,7 @@ public void hazelcastCacheWithMainHazelcastAutoConfigurationAndSeparateCacheConf
416419
.getBean(HazelcastInstance.class);
417420
HazelcastCacheManager cacheManager = validateCacheManager(
418421
HazelcastCacheManager.class);
419-
HazelcastInstance cacheHazelcastInstance = (HazelcastInstance) new DirectFieldAccessor(
420-
cacheManager).getPropertyValue("hazelcastInstance");
422+
HazelcastInstance cacheHazelcastInstance = getHazelcastInstance(cacheManager);
421423
assertThat(cacheHazelcastInstance, not(hazelcastInstance)); // Our custom
422424
assertThat(hazelcastInstance.getConfig().getConfigurationFile(),
423425
equalTo(new ClassPathResource(mainConfig).getFile()));
@@ -570,6 +572,11 @@ private void load(Class<?> config, String... environment) {
570572
this.context = applicationContext;
571573
}
572574

575+
private static HazelcastInstance getHazelcastInstance(HazelcastCacheManager cacheManager) {
576+
return (HazelcastInstance) new DirectFieldAccessor(cacheManager)
577+
.getPropertyValue("hazelcastInstance");
578+
}
579+
573580
@Configuration
574581
static class EmptyConfiguration {
575582

0 commit comments

Comments
 (0)