Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate mappings on creation instead of on lookup #1264

Merged
merged 1 commit into from
Dec 19, 2024
Merged

Validate mappings on creation instead of on lookup #1264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading