Improve safety of ConfigMappings. (#410)

smallrye · Oct 2, 2020 · 52d3203 · 52d3203
1 parent beae833
commit 52d3203
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 32 deletions.
diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java
@@ -598,13 +598,13 @@ KeyMap<String> getDefaultValues() {
         return defaultValues;
     }
 
-    ConfigMappings mapConfiguration(SmallRyeConfig config) throws ConfigValidationException {
-        return mapConfiguration(config, new ConfigMappings());
+    void mapConfiguration(SmallRyeConfig config) throws ConfigValidationException {
+        mapConfiguration(config, config.getConfigMappings());
     }
 
-    ConfigMappings mapConfiguration(SmallRyeConfig config, ConfigMappings mappings) throws ConfigValidationException {
+    private void mapConfiguration(SmallRyeConfig config, ConfigMappings mappings) throws ConfigValidationException {
         if (roots.isEmpty()) {
-            return new ConfigMappings(new HashMap<>());
+            return;
         }
 
         Assert.checkNotNullParam("config", config);
@@ -637,13 +637,11 @@ ConfigMappings mapConfiguration(SmallRyeConfig config, ConfigMappings mappings)
         }
         ArrayList<ConfigValidationException.Problem> problems = context.getProblems();
         if (!problems.isEmpty()) {
-            throw new ConfigValidationException(
-                    problems.toArray(ConfigValidationException.Problem.NO_PROBLEMS));
+            throw new ConfigValidationException(problems.toArray(ConfigValidationException.Problem.NO_PROBLEMS));
         }
         context.fillInOptionals();
 
         mappings.registerConfigMappings(context.getRootsMap());
-        return mappings;
     }
 
     private boolean isPropertyInRoot(String propertyName) {

diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappings.java b/implementation/src/main/java/io/smallrye/config/ConfigMappings.java
@@ -1,32 +1,28 @@
 package io.smallrye.config;
 
 import java.io.Serializable;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import org.eclipse.microprofile.config.spi.ConfigSource;
 
 public final class ConfigMappings implements Serializable {
     private static final long serialVersionUID = -7790784345796818526L;
 
-    private final Map<Class<?>, Map<String, ConfigMappingObject>> mappings;
+    private final ConcurrentMap<Class<?>, Map<String, ConfigMappingObject>> mappings;
 
     ConfigMappings() {
-        this(new HashMap<>());
+        this.mappings = new ConcurrentHashMap<>();
     }
 
-    ConfigMappings(final Map<Class<?>, Map<String, ConfigMappingObject>> mappings) {
-        this.mappings = mappings;
-    }
-
-    void registerConfigMappings(Map<Class<?>, Map<String, ConfigMappingObject>> mappings) {
+    void registerConfigMappings(final Map<Class<?>, Map<String, ConfigMappingObject>> mappings) {
         this.mappings.putAll(mappings);
     }
 
-    public void registerConfigMappings(final SmallRyeConfig config, final Set<ConfigMappingWithPrefix> mappings)
+    public static void registerConfigMappings(final SmallRyeConfig config, final Set<ConfigMappingWithPrefix> mappings)
             throws ConfigValidationException {
         final ConfigMappingProvider.Builder builder = ConfigMappingProvider.builder();
         for (ConfigMappingWithPrefix mapping : mappings) {
@@ -36,7 +32,7 @@ public void registerConfigMappings(final SmallRyeConfig config, final Set<Config
         registerConfigMappings(config, builder.build());
     }
 
-    public void registerConfigMappings(final SmallRyeConfig config, final ConfigMappingProvider mappingProvider)
+    public static void registerConfigMappings(final SmallRyeConfig config, final ConfigMappingProvider mappingProvider)
             throws ConfigValidationException {
         for (ConfigSource configSource : config.getConfigSources()) {
             if (configSource instanceof DefaultValuesConfigSource) {
@@ -45,7 +41,7 @@ public void registerConfigMappings(final SmallRyeConfig config, final ConfigMapp
             }
         }
 
-        mappingProvider.mapConfiguration(config, this);
+        mappingProvider.mapConfiguration(config);
     }
 
     <T> T getConfigMapping(Class<T> type) {
@@ -57,10 +53,16 @@ <T> T getConfigMapping(Class<T> type, String prefix) {
             return getConfigMapping(type);
         }
 
-        final ConfigMappingObject configMappingObject = mappings.getOrDefault(type, Collections.emptyMap()).get(prefix);
+        final Map<String, ConfigMappingObject> mappingsForType = mappings.get(type);
+        if (mappingsForType == null) {
+            throw ConfigMessages.msg.mappingNotFound(type.getName());
+        }
+
+        final ConfigMappingObject configMappingObject = mappingsForType.get(prefix);
         if (configMappingObject == null) {
-            throw ConfigMessages.msg.mappingNotFound(type.getName(), prefix);
+            throw ConfigMessages.msg.mappingPrefixNotFound(type.getName(), prefix);
         }
+
         return type.cast(configMappingObject);
     }
 

diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMessages.java b/implementation/src/main/java/io/smallrye/config/ConfigMessages.java
@@ -95,6 +95,9 @@ interface ConfigMessages {
     @Message(id = 26, value = "%s cannot be converted into a UUID")
     IllegalArgumentException malformedUUID(@Cause Throwable cause, String malformedUUID);
 
-    @Message(id = 27, value = "Could not find a mapping for %s with prefix %s")
-    NoSuchElementException mappingNotFound(String className, String prefix);
+    @Message(id = 27, value = "Could not find a mapping for %s")
+    NoSuchElementException mappingNotFound(String className);
+
+    @Message(id = 28, value = "Could not find a mapping for %s with prefix %s")
+    NoSuchElementException mappingPrefixNotFound(String className, String prefix);
 }
diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java
@@ -335,7 +335,7 @@ public SmallRyeConfig build() {
         try {
             ConfigMappings configMappings = new ConfigMappings();
             SmallRyeConfig config = new SmallRyeConfig(this, configMappings);
-            mappingProvider.mapConfiguration(config, configMappings);
+            mappingProvider.mapConfiguration(config);
             return config;
         } catch (ConfigValidationException e) {
             throw new IllegalStateException(e);

diff --git a/implementation/src/test/java/io/smallrye/config/ConfigMappingProviderTest.java b/implementation/src/test/java/io/smallrye/config/ConfigMappingProviderTest.java
@@ -38,16 +38,29 @@ void configMapping() {
     void noConfigMapping() {
         final SmallRyeConfig config = new SmallRyeConfigBuilder()
                 .withSources(config("server.host", "localhost", "server.port", "8080")).build();
-        assertThrows(NoSuchElementException.class, () -> config.getConfigMapping(Server.class, "server"),
-                "Could not find a mapping for " + Server.class.getName() + " with prefix server");
+        final NoSuchElementException exception = assertThrows(NoSuchElementException.class,
+                () -> config.getConfigMapping(Server.class, "server"));
+        assertEquals("SRCFG00027: Could not find a mapping for " + Server.class.getName(), exception.getMessage());
     }
 
     @Test
     void unregisteredConfigMapping() {
         final SmallRyeConfig config = new SmallRyeConfigBuilder()
                 .withSources(config("host", "localhost", "port", "8080")).build();
-        assertThrows(NoSuchElementException.class, () -> config.getConfigMapping(Server.class),
-                "Could not find a mapping for " + Server.class.getName() + "with prefix");
+        final NoSuchElementException exception = assertThrows(NoSuchElementException.class,
+                () -> config.getConfigMapping(Server.class));
+        assertEquals("SRCFG00027: Could not find a mapping for " + Server.class.getName(), exception.getMessage());
+    }
+
+    @Test
+    void unregistedPrefix() {
+        final SmallRyeConfig config = new SmallRyeConfigBuilder()
+                .withMapping(Server.class)
+                .withSources(config("host", "localhost", "port", "8080")).build();
+        final NoSuchElementException exception = assertThrows(NoSuchElementException.class,
+                () -> config.getConfigMapping(Server.class, "server"));
+        assertEquals("SRCFG00028: Could not find a mapping for " + Server.class.getName() + " with prefix server",
+                exception.getMessage());
     }
 
     @Test
@@ -67,7 +80,8 @@ void configMappingBuilder() throws Exception {
         final SmallRyeConfig config = new SmallRyeConfigBuilder().withSources(
                 config("server.host", "localhost", "server.port", "8080", "server.name", "name")).build();
 
-        final Server server = configMappingProvider.mapConfiguration(config).getConfigMapping(Server.class, "server");
+        configMappingProvider.mapConfiguration(config);
+        final Server server = config.getConfigMapping(Server.class, "server");
         assertEquals("localhost", server.host());
         assertEquals(8080, server.port());
     }
@@ -130,12 +144,13 @@ void splitRoots() throws Exception {
                 .addRoot("server", SplitRootServerName.class)
                 .build();
 
-        final ConfigMappings result = configMappingProvider.mapConfiguration(config);
-        final SplitRootServerHostAndPort server = result.getConfigMapping(SplitRootServerHostAndPort.class, "server");
+        configMappingProvider.mapConfiguration(config);
+
+        final SplitRootServerHostAndPort server = config.getConfigMapping(SplitRootServerHostAndPort.class, "server");
         assertEquals("localhost", server.host());
         assertEquals(8080, server.port());
 
-        final SplitRootServerName name = result.getConfigMapping(SplitRootServerName.class, "server");
+        final SplitRootServerName name = config.getConfigMapping(SplitRootServerName.class, "server");
         assertEquals("konoha", name.name());
     }