Skip to content

Commit

Permalink
Validate mappings on creation instead of on lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
@radcortez
radcortez committed Dec 4, 2024
1 parent 95881f0 commit ee652ad
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 58 deletions.
46 changes: 32 additions & 14 deletions implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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 @@ -36,7 +37,7 @@
public final class ConfigMappingContext {
private final SmallRyeConfig config;
private final ConfigMappingNames names;
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 @@ -46,43 +47,60 @@ public final class ConfigMappingContext {
private final Set<String> usedProperties = new HashSet<>();
private final List<Problem> problems = new ArrayList<>();

public ConfigMappingContext(final SmallRyeConfig config, final Map<Class<?>, Set<String>> roots) {
public ConfigMappingContext(final SmallRyeConfig config, final Map<Class<?>, Set<String>> mappings) {
this(config, new Supplier<Map<String, Map<String, Set<String>>>>() {
@Override
public Map<String, Map<String, Set<String>>> get() {
// All mapping names must be loaded first because of split mappings
Map<String, Map<String, Set<String>>> names = new HashMap<>();
for (Map.Entry<Class<?>, Set<String>> mapping : roots.entrySet()) {
for (Map.Entry<Class<?>, Set<String>> mapping : mappings.entrySet()) {
for (Map.Entry<String, Map<String, Set<String>>> entry : configMappingNames(mapping.getKey()).entrySet()) {
names.putIfAbsent(entry.getKey(), new HashMap<>());
names.get(entry.getKey()).putAll(entry.getValue());
}
}
return names;
}
}.get(), roots);
}.get(), mappings);
}

ConfigMappingContext(
final SmallRyeConfig config,
final Map<String, Map<String, Set<String>>> names,
final Map<Class<?>, Set<String>> roots) {
final Map<Class<?>, Set<String>> mappings) {

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

@SuppressWarnings("unchecked")
<T> T constructRoot(Class<T> interfaceType) {
return constructGroup(interfaceType);
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(), rootPath, mappingObject);
} else {
config.getConfigValidator().validateMapping(interfaceType, rootPath, mappingObject);
}
} catch (ConfigValidationException e) {
problems.addAll(Arrays.asList(e.getProblems()));
}
return (T) mappingObject;
}

public <T> T constructGroup(Class<T> interfaceType) {
Expand Down Expand Up @@ -168,8 +186,8 @@ List<Problem> getProblems() {
return problems;
}

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

private void matchPropertiesWithEnv(final Map<Class<?>, Set<String>> roots) {
Expand Down Expand Up @@ -316,7 +334,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
25 changes: 11 additions & 14 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,7 +80,7 @@ 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);
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 @@ -605,7 +605,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 @@ -626,24 +630,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
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 ee652ad

Please sign in to comment.