diff --git a/inject/src/main/java/io/micronaut/context/env/PropertySourcePropertyResolver.java b/inject/src/main/java/io/micronaut/context/env/PropertySourcePropertyResolver.java index 4b9b8d7b5c..77fe817aa6 100644 --- a/inject/src/main/java/io/micronaut/context/env/PropertySourcePropertyResolver.java +++ b/inject/src/main/java/io/micronaut/context/env/PropertySourcePropertyResolver.java @@ -99,6 +99,8 @@ public class PropertySourcePropertyResolver implements PropertyResolver, AutoClo private final Map resolvedValueCache = new ConcurrentHashMap<>(20); private final EnvironmentProperties environmentProperties = EnvironmentProperties.fork(CURRENT_ENV); + private volatile boolean refreshing = false; + /** * Creates a new, initially empty, {@link PropertySourcePropertyResolver} for the given {@link ConversionService}. * @@ -143,8 +145,15 @@ public PropertySourcePropertyResolver(PropertySource... propertySources) { void reset() { synchronized (catalog) { - Arrays.fill(catalog, null); - resetCaches(); + refreshing = true; + try { + Arrays.fill(nonGenerated, null); + Arrays.fill(rawCatalog, null); + Arrays.fill(catalog, null); + resetCaches(); + } finally { + refreshing = false; + } } } @@ -283,7 +292,9 @@ public boolean containsProperty(@Nullable String name) { if (result == null) { result = false; } - containsCache.put(name, result); + if (!refreshing) { + containsCache.put(name, result); + } } return result; } @@ -488,12 +499,15 @@ public Optional getProperty(@NonNull String name, @NonNull ArgumentConver } } + boolean inRefresh = refreshing; if (value != null) { Optional converted; if (entries != null) { // iff entries is null, the value is from placeholderResolutionCache and doesn't need this step value = resolvePlaceHoldersIfNecessary(value); - placeholderResolutionCache.put(name, value); + if (!inRefresh) { + placeholderResolutionCache.put(name, value); + } } if (requiredType.isInstance(value) && !CollectionUtils.isIterableOrMap(requiredType)) { converted = (Optional) Optional.of(value); @@ -509,12 +523,14 @@ public Optional getProperty(@NonNull String name, @NonNull ArgumentConver } } - if (cacheableType) { + if (cacheableType && !inRefresh) { resolvedValueCache.put(cacheKey, converted.orElse((T) NO_VALUE)); } return converted; } else if (cacheableType) { - resolvedValueCache.put(cacheKey, NO_VALUE); + if (!inRefresh) { + resolvedValueCache.put(cacheKey, NO_VALUE); + } return Optional.empty(); } else if (Properties.class.isAssignableFrom(requiredType)) { Properties properties = resolveSubProperties(name, entries, conversionContext); diff --git a/inject/src/test/groovy/io/micronaut/context/env/DefaultEnvironmentSpec.groovy b/inject/src/test/groovy/io/micronaut/context/env/DefaultEnvironmentSpec.groovy index 06be96bb53..42cbda6993 100644 --- a/inject/src/test/groovy/io/micronaut/context/env/DefaultEnvironmentSpec.groovy +++ b/inject/src/test/groovy/io/micronaut/context/env/DefaultEnvironmentSpec.groovy @@ -22,10 +22,13 @@ import io.micronaut.context.exceptions.ConfigurationException import io.micronaut.core.naming.NameUtils import io.micronaut.core.order.OrderUtil import io.micronaut.core.util.StringUtils +import io.micronaut.core.value.PropertyCatalog +import io.micronaut.core.value.PropertyNotFoundException import spock.lang.Issue import spock.lang.Specification import spock.util.environment.RestoreSystemProperties +import java.util.concurrent.Executors import java.util.function.Function /** @@ -95,6 +98,66 @@ class DefaultEnvironmentSpec extends Specification { diff.isEmpty() } + void "test cache related issue when refresh method is executed"() { + given: + def propertyMap = ['testPropKey': 'testPropValueOld'] + def propertySource = new MapPropertySource('CustomPS', propertyMap) + def env = new DefaultEnvironment({['test']}) + env.addPropertySource(propertySource) + env.start() + + expect: + env.getRequiredProperty("testPropKey", String.class) == 'testPropValueOld' + + and: + def testFinished = false + def executor = Executors.newSingleThreadExecutor() + executor.submit(new Runnable() { + @Override + void run() { + while (!testFinished) { + env.getRequiredProperty("testPropKey", String.class) + } + } + }) + + when: + propertyMap.put('testPropKey', 'testPropValueNew') + def diff = env.refreshAndDiff() + testFinished = true + + then: + env.getRequiredProperty("testPropKey", String.class) == 'testPropValueNew' + diff.get('test-prop-key') == 'testPropValueOld' + } + + void "test all catalogs are cleared when properties are dropped"() { + given: + def propertyMap = ['testPropKey': 'testPropValueOld'] + def propertySource = new MapPropertySource('CustomPS', propertyMap) + def env = new DefaultEnvironment({['test']}) + env.addPropertySource(propertySource) + env.start() + + expect: + env.getProperty("testPropKey", String.class).get() == 'testPropValueOld' + env.getProperty("test-prop-key", String.class).get() == 'testPropValueOld' + + when: + env.stop() // Using this to invoke dropProperties() -> reset() method since they are private + + then: + env.getProperty("testPropKey", String.class).isEmpty() // Retrieves from rawCatalog + env.getProperty("test-prop-key", String.class).isEmpty() // Retrieves from catalog + + when: + env.start() + + then: + env.getProperty("testPropKey", String.class).get() == 'testPropValueOld' + env.getProperty("test-prop-key", String.class).get() == 'testPropValueOld' + } + void "test environment system property refresh"() { when: System.setProperty("test.foo.bar", "10")