Skip to content

Commit

Permalink
Validate mappings on creation instead of on lookup (#1264)
Browse files Browse the repository at this point in the history
  • Loading branch information
@radcortez
radcortez authored Dec 19, 2024
1 parent d2b7787 commit f49a590
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 65 deletions.
48 changes: 32 additions & 16 deletions implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

import static io.smallrye.config.ConfigValidationException.Problem;
import static io.smallrye.config.ProfileConfigSourceInterceptor.activeName;
import static io.smallrye.config.PropertyName.name;
import static io.smallrye.config.common.utils.StringUtil.unindexed;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -35,7 +35,7 @@
*/
public final class ConfigMappingContext {
private final SmallRyeConfig config;
private final Map<Class<?>, Map<String, ConfigMappingObject>> roots = new IdentityHashMap<>();
private final Map<Class<?>, Map<String, Object>> mappings = new IdentityHashMap<>();
private final Map<Class<?>, Converter<?>> converterInstances = new IdentityHashMap<>();

private NamingStrategy namingStrategy = NamingStrategy.KEBAB_CASE;
Expand All @@ -52,17 +52,33 @@ public ConfigMappingContext(

matchPropertiesWithEnv(mappings);
for (Map.Entry<Class<?>, Set<String>> mapping : mappings.entrySet()) {
Map<String, ConfigMappingObject> mappingObjects = new HashMap<>();
for (String rootPath : mapping.getValue()) {
applyRootPath(rootPath);
mappingObjects.put(rootPath, (ConfigMappingObject) constructRoot(mapping.getKey()));
Map<String, Object> mappingObjects = new HashMap<>();
for (String prefix : mapping.getValue()) {
applyPrefix(prefix);
mappingObjects.put(prefix, constructMapping(mapping.getKey(), prefix));
}
this.roots.put(mapping.getKey(), mappingObjects);
this.mappings.put(mapping.getKey(), mappingObjects);
}
}

<T> T constructRoot(Class<T> interfaceType) {
return constructGroup(interfaceType);
@SuppressWarnings("unchecked")
<T> T constructMapping(Class<T> interfaceType, String prefix) {
int problemsCount = problems.size();
Object mappingObject = constructGroup(interfaceType);
if (problemsCount != problems.size()) {
return (T) mappingObject;
}
try {
if (mappingObject instanceof ConfigMappingClassMapper) {
mappingObject = ((ConfigMappingClassMapper) mappingObject).map();
config.getConfigValidator().validateMapping(mappingObject.getClass(), prefix, mappingObject);
} else {
config.getConfigValidator().validateMapping(interfaceType, prefix, mappingObject);
}
} catch (ConfigValidationException e) {
problems.addAll(Arrays.asList(e.getProblems()));
}
return (T) mappingObject;
}

public <T> T constructGroup(Class<T> interfaceType) {
Expand Down Expand Up @@ -102,6 +118,10 @@ public <T> Converter<T> getConverterInstance(Class<? extends Converter<? extends
});
}

public void applyPrefix(final String prefix) {
this.nameBuilder.replace(0, nameBuilder.length(), prefix);
}

public void applyNamingStrategy(final NamingStrategy namingStrategy) {
if (namingStrategy != null) {
this.namingStrategy = namingStrategy;
Expand All @@ -114,10 +134,6 @@ public void applyBeanStyleGetters(final Boolean beanStyleGetters) {
}
}

public void applyRootPath(final String rootPath) {
this.nameBuilder.replace(0, nameBuilder.length(), rootPath);
}

private static final Function<String, String> BEAN_STYLE_GETTERS = new Function<String, String>() {
@Override
public String apply(final String name) {
Expand Down Expand Up @@ -147,8 +163,8 @@ List<Problem> getProblems() {
return problems;
}

Map<Class<?>, Map<String, ConfigMappingObject>> getRootsMap() {
return roots;
Map<Class<?>, Map<String, Object>> getMappings() {
return mappings;
}

// TODO - We shouldn't be mutating the EnvSource.
Expand Down Expand Up @@ -277,7 +293,7 @@ void reportUnknown(final Set<String> ignoredPaths) {
}

Set<String> prefixes = new HashSet<>();
for (Map<String, ConfigMappingObject> value : this.roots.values()) {
for (Map<String, Object> value : this.mappings.values()) {
prefixes.addAll(value.keySet());
}
if (prefixes.contains("")) {
Expand Down
4 changes: 4 additions & 0 deletions implementation/src/main/java/io/smallrye/config/ConfigValidationException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ private static String list(Problem[] problems) {
return b.toString();
}

Problem[] getProblems() {
return problems;
}

public int getProblemCount() {
return problems.length;
}
Expand Down
29 changes: 13 additions & 16 deletions implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ public class SmallRyeConfig implements Config, Serializable {
private final Map<Type, Converter<Optional<?>>> optionalConverters = new ConcurrentHashMap<>();

private final ConfigValidator configValidator;
private final Map<Class<?>, Map<String, ConfigMappingObject>> mappings;
private final Map<Class<?>, Map<String, Object>> mappings;

SmallRyeConfig(SmallRyeConfigBuilder builder) {
this.configSources = new ConfigSources(builder, this);
this.configSources = new ConfigSources(builder);
this.converters = buildConverters(builder);
this.configValidator = builder.getValidator();

Expand Down Expand Up @@ -115,7 +115,7 @@ private Map<Type, Converter<?>> buildConverters(final SmallRyeConfigBuilder buil
return converters;
}

Map<Class<?>, Map<String, ConfigMappingObject>> buildMappings(final SmallRyeConfigBuilder builder)
Map<Class<?>, Map<String, Object>> buildMappings(final SmallRyeConfigBuilder builder)
throws ConfigValidationException {
SmallRyeConfigBuilder.MappingBuilder mappingsBuilder = builder.getMappingsBuilder();
if (mappingsBuilder.getMappings().isEmpty()) {
Expand All @@ -139,7 +139,7 @@ public ConfigMappingContext get() {
throw new ConfigValidationException(problems.toArray(ConfigValidationException.Problem.NO_PROBLEMS));
}

return context.getRootsMap();
return context.getMappings();
}

private void matchPropertiesWithEnv() {
Expand Down Expand Up @@ -678,7 +678,11 @@ public <K, V, C extends Collection<V>> Optional<Map<K, C>> getOptionalValues(
return Optional.of(getMapIndexedValues(keys, keyConverter, valueConverter, mapFactory, collectionFactory));
}

Map<Class<?>, Map<String, ConfigMappingObject>> getMappings() {
ConfigValidator getConfigValidator() {
return configValidator;
}

Map<Class<?>, Map<String, Object>> getMappings() {
return mappings;
}

Expand All @@ -699,24 +703,17 @@ public <T> T getConfigMapping(Class<T> type, String prefix) {
return getConfigMapping(type);
}

Map<String, ConfigMappingObject> mappingsForType = mappings.get(getConfigMappingClass(type));
Map<String, Object> mappingsForType = mappings.get(getConfigMappingClass(type));
if (mappingsForType == null) {
throw ConfigMessages.msg.mappingNotFound(type.getName());
}

ConfigMappingObject configMappingObject = mappingsForType.get(prefix);
Object configMappingObject = mappingsForType.get(prefix);
if (configMappingObject == null) {
throw ConfigMessages.msg.mappingPrefixNotFound(type.getName(), prefix);
}

Object value = configMappingObject;
if (configMappingObject instanceof ConfigMappingClassMapper) {
value = ((ConfigMappingClassMapper) configMappingObject).map();
}

configValidator.validateMapping(type, prefix, value);

return type.cast(value);
return type.cast(configMappingObject);
}

/**
Expand Down Expand Up @@ -874,7 +871,7 @@ private static class ConfigSources implements Serializable {
* that this constructor must be used when the Config object is being initialized, because interceptors also
* require initialization.
*/
ConfigSources(final SmallRyeConfigBuilder builder, final SmallRyeConfig config) {
ConfigSources(final SmallRyeConfigBuilder builder) {
// Add all sources except for ConfigurableConfigSource types. These are initialized later
List<ConfigSource> sources = buildSources(builder);
// Add the default values sources separately, so we can keep a reference to it and add mappings defaults
Expand Down
6 changes: 3 additions & 3 deletions implementation/src/test/java/io/smallrye/config/ObjectCreatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ void unnamedKeys() {
.build();

ConfigMappingContext context = new ConfigMappingContext(config, new HashMap<>());
context.applyRootPath("unnamed");
context.applyPrefix("unnamed");

UnnamedKeys mapping = new UnnamedKeysImpl(context);
assertEquals("unnamed", mapping.map().get(null).value());
Expand Down Expand Up @@ -394,7 +394,7 @@ void mapDefaults() {
.build();

ConfigMappingContext context = new ConfigMappingContext(config, new HashMap<>());
context.applyRootPath("map");
context.applyPrefix("map");
MapDefaults mapping = new MapDefaultsImpl(context);

assertEquals("value", mapping.defaults().get("one"));
Expand Down Expand Up @@ -505,7 +505,7 @@ void namingStrategy() {
.build();

ConfigMappingContext context = new ConfigMappingContext(config, new HashMap<>());
context.applyRootPath("naming");
context.applyPrefix("naming");
Naming naming = new NamingImpl(context);

assertEquals("value", naming.nestedValue().value());
Expand Down
48 changes: 18 additions & 30 deletions validator/src/test/java/io/smallrye/config/validator/ValidateConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ static void restoreMessageLocale() {

@Test
void validateConfigMapping() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withSources(config(
"server.host", "localhost",
Expand Down Expand Up @@ -98,11 +98,9 @@ void validateConfigMapping() {
"server.info.admins.root[1].username", "admin",
"server.info.firewall.accepted[0]", "127.0.0.1",
"server.info.firewall.accepted[1]", "8.8.8"))
.withMapping(Server.class)
.build();
.withMapping(Server.class);

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(Server.class, "server"));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
List<String> validations = new ArrayList<>();
for (int i = 0; i < validationException.getProblemCount(); i++) {
validations.add(validationException.getProblem(i).getMessage());
Expand Down Expand Up @@ -134,16 +132,14 @@ void validateConfigMapping() {

@Test
void validateNamingStrategy() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withSources(config(
"server.the_host", "localhost",
"server.the_port", "8080"))
.withMapping(ServerNamingStrategy.class)
.build();
.withMapping(ServerNamingStrategy.class);

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(ServerNamingStrategy.class, "server"));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
List<String> validations = new ArrayList<>();
for (int i = 0; i < validationException.getProblemCount(); i++) {
validations.add(validationException.getProblem(i).getMessage());
Expand All @@ -154,13 +150,11 @@ void validateNamingStrategy() {

@Test
void validateConfigProperties() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withMapping(Client.class)
.build();
.withMapping(Client.class);

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(Client.class));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
assertEquals(1, validationException.getProblemCount());
List<String> validations = new ArrayList<>();
validations.add(validationException.getProblem(0).getMessage());
Expand All @@ -169,16 +163,14 @@ void validateConfigProperties() {

@Test
void validateParent() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withSources(config(
"server.host", "localhost",
"server.port", "80"))
.withMapping(ServerParent.class)
.build();
.withMapping(ServerParent.class);

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(ServerParent.class, "server"));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
assertEquals("server.port must be greater than or equal to 8000", validationException.getProblem(0).getMessage());
}

Expand Down Expand Up @@ -359,14 +351,12 @@ interface Nested {

@Test
void hierarchy() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withMapping(Child.class)
.withSources(config("validator.child.number", "1"))
.build();
.withSources(config("validator.child.number", "1"));

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(Child.class));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
List<String> validations = new ArrayList<>();
for (int i = 0; i < validationException.getProblemCount(); i++) {
validations.add(validationException.getProblem(i).getMessage());
Expand All @@ -387,13 +377,11 @@ public interface Child extends Parent {

@Test
void nestedMethodValidation() {
SmallRyeConfig config = new SmallRyeConfigBuilder()
SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder()
.withValidator(new BeanValidationConfigValidatorImpl())
.withMapping(MethodValidation.class)
.build();
.withMapping(MethodValidation.class);

ConfigValidationException validationException = assertThrows(ConfigValidationException.class,
() -> config.getConfigMapping(MethodValidation.class));
ConfigValidationException validationException = assertThrows(ConfigValidationException.class, builder::build);
List<String> validations = new ArrayList<>();
for (int i = 0; i < validationException.getProblemCount(); i++) {
validations.add(validationException.getProblem(i).getMessage());
Expand Down

0 comments on commit f49a590

Please sign in to comment.