Skip to content

Commit

Permalink
Incorporate review feedback:
Browse files Browse the repository at this point in the history
- Use a function to remove the explicit use of MicroProfile config
- Rename ConfiguredProperties to ComputedProperties
  • Loading branch information
romain-grecourt committed May 20, 2024
1 parent b616559 commit c65872f
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">

<Match>
<Class name="io.helidon.integrations.cdi.configurable.ConfiguredProperties"/>
<Class name="io.helidon.integrations.cdi.configurable.ComputedProperties"/>
<Bug pattern="UG_SYNC_SET_UNSYNC_GET"/>
</Match>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,22 +256,22 @@ protected final Properties put(String name, Properties properties) {
* @see #addBean(BeanConfigurator, Named, Properties)
*/
protected final void initializeNamedProperties() {
Map<String, ConfiguredProperties> configuredProperties = new HashMap<>();
Map<String, ComputedProperties> computedProperties = new HashMap<>();
Set<String> allConfigPropertyNames = configPropertyNames();
if (!allConfigPropertyNames.isEmpty()) {
for (String configPropertyName : allConfigPropertyNames) {
Optional<String> propertyValue = configPropertyValue(configPropertyName);
if (propertyValue.isPresent()) {
Matcher matcher = matcher(configPropertyName);
if (matcher != null && matcher.matches()) {
configuredProperties.computeIfAbsent(name(matcher), n -> new ConfiguredProperties(config))
.configKey(propertyName(matcher), configPropertyName);
computedProperties.computeIfAbsent(name(matcher), n -> new ComputedProperties(this::configPropertyValue))
.computedKeys().put(propertyName(matcher), configPropertyName);
}
}
}
}
namedProperties.clear();
namedProperties.putAll(configuredProperties);
namedProperties.putAll(computedProperties);
namedProperties.putAll(explicitlySetProperties);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,47 +24,50 @@
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Function;

import org.eclipse.microprofile.config.Config;

/**
* A lazy read-only {@link Properties} implementation backed by a {@link Config} instance.
* A lazy read-only {@link Properties} implementation backed by a {@link Function}.
*/
final class ConfiguredProperties extends Properties {
final class ComputedProperties extends Properties {

private static final VarHandle ENTRIES;

static {
try {
ENTRIES = MethodHandles.lookup().findVarHandle(ConfiguredProperties.class, "entries", Map.class);
ENTRIES = MethodHandles.lookup().findVarHandle(ComputedProperties.class, "entries", Map.class);
} catch (ReflectiveOperationException e) {
throw new ExceptionInInitializerError(e);
}
}

@SuppressWarnings("unused")
private volatile Map<Object, Object> entries;
private final Map<Object, String> keys = new HashMap<>();
private final Config config;
private final Map<Object, Object> computedKeys = new HashMap<>();
private final Function<String, Optional<String>> function;

ConfiguredProperties(Config config) {
this.config = config;
ComputedProperties(Function<String, Optional<String>> function) {
this.function = function;
}

void configKey(String propertyName, String configKey) {
keys.put(propertyName, configKey);
Map<Object, Object> computedKeys() {
return computedKeys;
}

private Map<Object, Object> entries() {
Map<Object, Object> entries = this.entries; // volatile read
if (entries == null) {
Map<Object, Object> map = new HashMap<>();
keys.forEach((name, key) -> map.put(name, config.getOptionalValue(key, String.class).orElse(null)));
computedKeys.forEach((key, computedKey) -> {
if (computedKey instanceof String s) {
map.put(key, function.apply(s).orElse(null));
}
});
entries = Collections.unmodifiableMap(map);
if (!ENTRIES.compareAndSet(this, null, entries)) { // volatile assignment, maybe
entries = this.entries; // volatile read; will only happen because this.entries will be non-null
Expand All @@ -74,26 +77,26 @@ private Map<Object, Object> entries() {
}

@Override
public String getProperty(String name) {
if (entries().get(name) instanceof String s) {
public String getProperty(String key) {
if (entries().get(key) instanceof String s) {
return s;
}
return null;
}

@Override
public int size() {
return keys.size();
return computedKeys.size();
}

@Override
public boolean isEmpty() {
return keys.isEmpty();
return computedKeys.isEmpty();
}

@Override
public Enumeration<Object> keys() {
return Collections.enumeration(keys.keySet());
return Collections.enumeration(computedKeys.keySet());
}

@Override
Expand All @@ -113,18 +116,12 @@ public boolean containsValue(Object value) {

@Override
public boolean containsKey(Object key) {
if (key instanceof String s) {
return keys.containsKey(s);
}
return false;
return computedKeys.containsKey(key);
}

@Override
public Object get(Object key) {
if (key instanceof String s) {
return getProperty(s);
}
return null;
return entries().get(key);
}

@Override
Expand All @@ -134,7 +131,7 @@ public synchronized String toString() {

@Override
public Set<Object> keySet() {
return keys.keySet();
return computedKeys.keySet();
}

@Override
Expand Down

0 comments on commit c65872f

Please sign in to comment.