From 5a808c0ee932c8a0dd2f0d1f1515e3a67b1c49fc Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Fri, 9 Apr 2021 17:12:37 +0100 Subject: [PATCH] Apply the same NamingStrategy to all groups in a ConfigMapping root. (#545) --- .../smallrye/config/ConfigMappingContext.java | 12 ++ .../config/ConfigMappingGenerator.java | 18 ++- .../config/ConfigMappingInterface.java | 7 +- .../config/ConfigMappingProvider.java | 105 +++++++++++------- .../config/ConfigMappingInterfaceTest.java | 76 ++++++++++++- 5 files changed, 160 insertions(+), 58 deletions(-) diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java index 1f73b08e7..bf0a61527 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java @@ -26,6 +26,7 @@ import org.eclipse.microprofile.config.spi.Converter; import io.smallrye.config.ConfigMappingInterface.CollectionProperty; +import io.smallrye.config.ConfigMappingInterface.NamingStrategy; /** * A mapping context. This is used by generated classes during configuration mapping, and is released once the configuration @@ -42,6 +43,8 @@ public final class ConfigMappingContext { private final StringBuilder stringBuilder = new StringBuilder(); private final ArrayList problems = new ArrayList<>(); + private NamingStrategy namingStrategy = null; + ConfigMappingContext(final SmallRyeConfig config) { this.config = config; } @@ -68,6 +71,11 @@ public void registerEnclosedField(Class enclosingType, String key, Object enc .put(enclosingObject, value); } + public T constructRoot(Class interfaceType) { + this.namingStrategy = ConfigMappingInterface.getConfigurationInterface(interfaceType).getNamingStrategy(); + return constructGroup(interfaceType); + } + public T constructGroup(Class interfaceType) { final T mappingObject = ConfigMappingLoader.configMappingObject(interfaceType, this); allInstances.add((ConfigMappingObject) mappingObject); @@ -179,6 +187,10 @@ public Converter getConverterInstance(Class> createCollectionFactory(final Class type) { if (type == List.class) { return ArrayList::new; diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java index 6d274e787..f296a69bc 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java @@ -612,7 +612,7 @@ private static void addProperties( // register the group ctor.visitVarInsn(Opcodes.ALOAD, V_MAPPING_CONTEXT); ctor.visitLdcInsn(Type.getType(mapping.getInterfaceType())); - ctor.visitLdcInsn(property.getPropertyName(mapping.getNamingStrategy())); + ctor.visitLdcInsn(property.getPropertyName()); ctor.visitVarInsn(Opcodes.ALOAD, V_THIS); ctor.visitVarInsn(Opcodes.ALOAD, V_THIS); ctor.visitFieldInsn(Opcodes.GETFIELD, className, memberName, fieldDesc); @@ -637,7 +637,6 @@ private static void addProperties( // stack: this config key // get the converter to use ctor.visitVarInsn(Opcodes.ALOAD, V_MAPPING_CONTEXT); - // public Converter getValueConverter(Class enclosingType, String field) { ctor.visitLdcInsn(getType(mapping.getInterfaceType())); ctor.visitLdcInsn(memberName); ctor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, I_MAPPING_CONTEXT, "getValueConverter", @@ -782,14 +781,13 @@ private static boolean appendPropertyName(final MethodVisitor ctor, final Config ctor.visitVarInsn(Opcodes.ALOAD, V_STRING_BUILDER); - // stack: sb - // TODO - NammingStrategy - // The NamingStrategy comes from the current mapping interface. We don't support setting a NamingStrategy in - // the top of the config root for all the configs in that root to inherit the same NamingStrategy. This needs - // to be handled per instance (since groups may belong to different roots), so the NamingStrategy should be - // retrieved from the Context. This is just a first implementation that could move into that direction, which - // needs more work. - ctor.visitLdcInsn(property.getPropertyName(mapping.getNamingStrategy())); + ctor.visitVarInsn(Opcodes.ALOAD, V_MAPPING_CONTEXT); + + ctor.visitLdcInsn(property.getPropertyName()); + + ctor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, I_MAPPING_CONTEXT, + "applyNamingStrategy", "(L" + I_STRING + ";)L" + I_STRING + ";", false); + // stack: sb name ctor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, I_STRING_BUILDER, "append", "(L" + I_STRING + ";)L" + I_STRING_BUILDER + ';', false); diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java index a83c51de4..1f2d0890d 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java @@ -166,14 +166,10 @@ public Method getMethod() { } public String getPropertyName() { - return getPropertyName(KEBAB_CASE_NAMING_STRATEGY); - } - - public String getPropertyName(final NamingStrategy namingStrategy) { if (isParentPropertyName()) { return propertyName; } - return hasPropertyName() && !propertyName.isEmpty() ? propertyName : namingStrategy.apply(method.getName()); + return hasPropertyName() && !propertyName.isEmpty() ? propertyName : method.getName(); } public boolean hasPropertyName() { @@ -529,7 +525,6 @@ public static final class CollectionProperty extends MayBeOptionalProperty { private final Property element; CollectionProperty(final Class collectionType, final Property element) { - // TODO - Naming Strategy super(element.getMethod(), element.getPropertyName()); this.collectionRawType = collectionType; this.element = element; diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java index ada5c3d0e..b52f5f1b0 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingProvider.java @@ -24,6 +24,7 @@ import io.smallrye.common.constraint.Assert; import io.smallrye.common.function.Functions; import io.smallrye.config.ConfigMappingInterface.CollectionProperty; +import io.smallrye.config.ConfigMappingInterface.NamingStrategy; /** * @@ -67,7 +68,8 @@ final class ConfigMappingProvider implements Serializable { // construct the lazy match actions for each group BiFunction ef = new GetRootAction(root, entry.getKey()); - processEagerGroup(currentPath, matchActions, defaultValues, getConfigMappingInterface(root), ef); + ConfigMappingInterface mapping = getConfigMappingInterface(root); + processEagerGroup(currentPath, matchActions, defaultValues, mapping.getNamingStrategy(), mapping, ef); } currentPath.clear(); } @@ -194,10 +196,14 @@ public T apply(final ConfigMappingContext context, final NameIterator nameIterat } } - private void processEagerGroup(final ArrayDeque currentPath, - final KeyMap> matchActions, final KeyMap defaultValues, + private void processEagerGroup( + final ArrayDeque currentPath, + final KeyMap> matchActions, + final KeyMap defaultValues, + final NamingStrategy namingStrategy, final ConfigMappingInterface group, final BiFunction getEnclosingFunction) { + Class type = group.getInterfaceType(); int pc = group.getPropertyCount(); int pathLen = currentPath.size(); @@ -208,14 +214,14 @@ private void processEagerGroup(final ArrayDeque currentPath, if (usedProperties.add(memberName)) { // process by property type if (!property.isParentPropertyName()) { - NameIterator ni = new NameIterator(property.getPropertyName(group.getNamingStrategy())); + NameIterator ni = new NameIterator(namingStrategy.apply(property.getPropertyName())); while (ni.hasNext()) { currentPath.add(ni.getNextSegment()); ni.next(); } } - processProperty(currentPath, matchActions, defaultValues, group, getEnclosingFunction, type, memberName, - property); + processProperty(currentPath, matchActions, defaultValues, namingStrategy, group, getEnclosingFunction, type, + memberName, property); while (currentPath.size() > pathLen) { currentPath.removeLast(); } @@ -223,7 +229,8 @@ private void processEagerGroup(final ArrayDeque currentPath, } int sc = group.getSuperTypeCount(); for (int i = 0; i < sc; i++) { - processEagerGroup(currentPath, matchActions, defaultValues, group.getSuperType(i), getEnclosingFunction); + processEagerGroup(currentPath, matchActions, defaultValues, namingStrategy, group.getSuperType(i), + getEnclosingFunction); } } @@ -231,18 +238,20 @@ private void processProperty( final ArrayDeque currentPath, final KeyMap> matchActions, final KeyMap defaultValues, + final NamingStrategy namingStrategy, final ConfigMappingInterface group, final BiFunction getEnclosingFunction, final Class type, - final String memberName, final Property property) { + final String memberName, + final Property property) { if (property.isOptional()) { // switch to lazy mode Property nestedProperty = property.asOptional().getNestedProperty(); - processOptionalProperty(currentPath, matchActions, defaultValues, group, getEnclosingFunction, type, memberName, - nestedProperty); + processOptionalProperty(currentPath, matchActions, defaultValues, namingStrategy, group, getEnclosingFunction, type, + memberName, nestedProperty); } else if (property.isGroup()) { - processEagerGroup(currentPath, matchActions, defaultValues, property.asGroup().getGroupType(), + processEagerGroup(currentPath, matchActions, defaultValues, namingStrategy, property.asGroup().getGroupType(), new GetOrCreateEnclosingGroupInGroup(getEnclosingFunction, group, property.asGroup())); } else if (property.isPrimitive()) { // already processed eagerly @@ -262,12 +271,12 @@ private void processProperty( } else if (property.isMap()) { // the enclosure of the map is this group processLazyMapInGroup(currentPath, matchActions, defaultValues, property.asMap(), getEnclosingFunction, - group); + namingStrategy, group); } else if (property.isCollection()) { CollectionProperty collectionProperty = property.asCollection(); currentPath.addLast(currentPath.removeLast() + "[*]"); - processProperty(currentPath, matchActions, defaultValues, group, getEnclosingFunction, type, memberName, - collectionProperty.getElement()); + processProperty(currentPath, matchActions, defaultValues, namingStrategy, group, getEnclosingFunction, type, + memberName, collectionProperty.getElement()); } } @@ -275,9 +284,11 @@ private void processOptionalProperty( final ArrayDeque currentPath, final KeyMap> matchActions, final KeyMap defaultValues, + final NamingStrategy namingStrategy, final ConfigMappingInterface group, final BiFunction getEnclosingFunction, final Class type, final String memberName, final Property nestedProperty) { + if (nestedProperty.isGroup()) { GroupProperty nestedGroup = nestedProperty.asGroup(); // on match, always create the outermost group, which recursively creates inner groups @@ -287,7 +298,7 @@ private void processOptionalProperty( nestedGroup.isParentPropertyName() ? getEnclosingFunction : new ConsumeOneAndThenFn<>(getEnclosingFunction), type, memberName); - processLazyGroupInGroup(currentPath, matchActions, defaultValues, nestedGroup, ef, matchAction, + processLazyGroupInGroup(currentPath, matchActions, defaultValues, namingStrategy, nestedGroup, ef, matchAction, new HashSet<>()); } else if (nestedProperty.isLeaf()) { LeafProperty leafProperty = nestedProperty.asLeaf(); @@ -298,24 +309,27 @@ private void processOptionalProperty( } else if (nestedProperty.isCollection()) { CollectionProperty collectionProperty = nestedProperty.asCollection(); currentPath.addLast(currentPath.removeLast() + "[*]"); - processProperty(currentPath, matchActions, defaultValues, group, getEnclosingFunction, type, memberName, - collectionProperty.getElement()); + processProperty(currentPath, matchActions, defaultValues, namingStrategy, group, getEnclosingFunction, type, + memberName, collectionProperty.getElement()); } } - private void processLazyGroupInGroup(ArrayDeque currentPath, - KeyMap> matchActions, - KeyMap defaultValues, - GroupProperty groupProperty, - BiFunction getEnclosingFunction, - BiConsumer matchAction, HashSet usedProperties) { + private void processLazyGroupInGroup( + final ArrayDeque currentPath, + final KeyMap> matchActions, + final KeyMap defaultValues, + final NamingStrategy namingStrategy, + final GroupProperty groupProperty, + final BiFunction getEnclosingFunction, + final BiConsumer matchAction, HashSet usedProperties) { + ConfigMappingInterface group = groupProperty.getGroupType(); int pc = group.getPropertyCount(); int pathLen = currentPath.size(); for (int i = 0; i < pc; i++) { Property property = group.getProperty(i); if (!property.isParentPropertyName()) { - NameIterator ni = new NameIterator(property.getPropertyName(group.getNamingStrategy())); + NameIterator ni = new NameIterator(namingStrategy.apply(property.getPropertyName())); while (ni.hasNext()) { currentPath.add(ni.getNextSegment()); ni.next(); @@ -324,7 +338,7 @@ private void processLazyGroupInGroup(ArrayDeque currentPath, if (usedProperties.add(property.getMethod().getName())) { boolean optional = property.isOptional(); processLazyPropertyInGroup(currentPath, matchActions, defaultValues, getEnclosingFunction, matchAction, - usedProperties, group, optional, property); + usedProperties, namingStrategy, group, optional, property); } while (currentPath.size() > pathLen) { currentPath.removeLast(); @@ -332,8 +346,8 @@ private void processLazyGroupInGroup(ArrayDeque currentPath, } int sc = group.getSuperTypeCount(); for (int i = 0; i < sc; i++) { - processLazyGroupInGroup(currentPath, matchActions, defaultValues, groupProperty, getEnclosingFunction, - matchAction, usedProperties); + processLazyGroupInGroup(currentPath, matchActions, defaultValues, namingStrategy, groupProperty, + getEnclosingFunction, matchAction, usedProperties); } } @@ -344,6 +358,7 @@ private void processLazyPropertyInGroup( final BiFunction getEnclosingFunction, final BiConsumer matchAction, final HashSet usedProperties, + final NamingStrategy namingStrategy, final ConfigMappingInterface group, final boolean optional, final Property property) { @@ -354,7 +369,7 @@ private void processLazyPropertyInGroup( property.isParentPropertyName() ? getEnclosingFunction : new ConsumeOneAndThenFn<>(getEnclosingFunction), group, nestedGroup); - processLazyGroupInGroup(currentPath, matchActions, defaultValues, nestedGroup, nestedMatchAction, + processLazyGroupInGroup(currentPath, matchActions, defaultValues, namingStrategy, nestedGroup, nestedMatchAction, nestedMatchAction, new HashSet<>()); } else if (property.isGroup()) { GroupProperty asGroup = property.asGroup(); @@ -367,7 +382,7 @@ private void processLazyPropertyInGroup( if (!property.isParentPropertyName()) { nestedMatchAction = new ConsumeOneAndThen(nestedMatchAction); } - processLazyGroupInGroup(currentPath, matchActions, defaultValues, asGroup, nestedEnclosingFunction, + processLazyGroupInGroup(currentPath, matchActions, defaultValues, namingStrategy, asGroup, nestedEnclosingFunction, nestedMatchAction, usedProperties); } else if (property.isLeaf() || property.isPrimitive() || optional && property.asOptional().getNestedProperty().isLeaf()) { @@ -396,30 +411,38 @@ private void processLazyPropertyInGroup( } } else if (property.isMap()) { processLazyMapInGroup(currentPath, matchActions, defaultValues, property.asMap(), getEnclosingFunction, - group); + namingStrategy, group); } else if (property.isCollection() || optional && property.asOptional().getNestedProperty().isCollection()) { CollectionProperty collectionProperty = optional ? property.asOptional().getNestedProperty().asCollection() : property.asCollection(); currentPath.addLast(currentPath.removeLast() + "[*]"); processLazyPropertyInGroup(currentPath, matchActions, defaultValues, getEnclosingFunction, matchAction, - usedProperties, group, false, collectionProperty.getElement()); + usedProperties, namingStrategy, group, false, collectionProperty.getElement()); } } - private void processLazyMapInGroup(final ArrayDeque currentPath, - final KeyMap> matchActions, final KeyMap defaultValues, + private void processLazyMapInGroup( + final ArrayDeque currentPath, + final KeyMap> matchActions, + final KeyMap defaultValues, final MapProperty property, BiFunction getEnclosingGroup, - ConfigMappingInterface enclosingGroup) { + final NamingStrategy namingStrategy, + final ConfigMappingInterface enclosingGroup) { + GetOrCreateEnclosingMapInGroup getEnclosingMap = new GetOrCreateEnclosingMapInGroup(getEnclosingGroup, enclosingGroup, property); - processLazyMap(currentPath, matchActions, defaultValues, property, getEnclosingMap, enclosingGroup); + processLazyMap(currentPath, matchActions, defaultValues, property, getEnclosingMap, namingStrategy, enclosingGroup); } @SuppressWarnings({ "unchecked", "rawtypes" }) - private void processLazyMap(final ArrayDeque currentPath, - final KeyMap> matchActions, final KeyMap defaultValues, + private void processLazyMap( + final ArrayDeque currentPath, + final KeyMap> matchActions, + final KeyMap defaultValues, final MapProperty property, BiFunction> getEnclosingMap, - ConfigMappingInterface enclosingGroup) { + final NamingStrategy namingStrategy, + final ConfigMappingInterface enclosingGroup) { + Property valueProperty = property.getValueProperty(); Class> keyConvertWith = property.hasKeyConvertWith() ? property.getKeyConvertWith() : null; Class keyRawType = property.getKeyRawType(); @@ -468,12 +491,12 @@ private void processLazyMap(final ArrayDeque currentPath, } Object key = keyConv.convert(rawMapKey); return (Map) ((Map) enclosingMap).computeIfAbsent(key, x -> new HashMap<>()); - }, enclosingGroup); + }, namingStrategy, enclosingGroup); } else { assert valueProperty.isGroup(); final GetOrCreateEnclosingGroupInMap ef = new GetOrCreateEnclosingGroupInMap(getEnclosingMap, property, enclosingGroup, valueProperty.asGroup()); - processLazyGroupInGroup(currentPath, matchActions, defaultValues, valueProperty.asGroup(), + processLazyGroupInGroup(currentPath, matchActions, defaultValues, namingStrategy, valueProperty.asGroup(), ef, ef, new HashSet<>()); } currentPath.removeLast(); @@ -669,7 +692,7 @@ private void mapConfiguration(SmallRyeConfig config, ConfigMappings mappings) th for (Class root : roots) { StringBuilder sb = context.getStringBuilder(); sb.replace(0, sb.length(), path); - ConfigMappingObject group = (ConfigMappingObject) context.constructGroup(root); + ConfigMappingObject group = (ConfigMappingObject) context.constructRoot(root); context.registerRoot(root, path, group); } } diff --git a/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java b/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java index 3d9932405..c34f7f03a 100644 --- a/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java +++ b/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java @@ -754,7 +754,7 @@ interface Data { } @Test - void mapsInGroup() throws Exception { + void mapsInGroup() { final Map typesConfig = new HashMap() { { put("server.info.name", "naruto"); @@ -842,4 +842,78 @@ void prefixPropertyInRootUnknown() { assertEquals("cloud.server.name does not map to any root", ((ConfigValidationException) exception.getCause()).getProblem(0).getMessage()); } + + @ConfigMapping(prefix = "server", namingStrategy = ConfigMapping.NamingStrategy.SNAKE_CASE) + public interface ServerComposedSnakeNaming { + String theHost(); + + int thePort(); + + LogInheritedNaming log(); + } + + @ConfigMapping(prefix = "server", namingStrategy = ConfigMapping.NamingStrategy.VERBATIM) + public interface ServerComposedVerbatimNaming { + String theHost(); + + int thePort(); + + LogInheritedNaming log(); + } + + @ConfigMapping(prefix = "server") + public interface ServerComposedKebabNaming { + String theHost(); + + int thePort(); + + LogInheritedNaming log(); + } + + public interface LogInheritedNaming { + boolean isEnabled(); + + List logAppenders(); + + interface Appender { + String logName(); + } + } + + @Test + void composedNamingStrategy() { + SmallRyeConfig config = new SmallRyeConfigBuilder() + .withValidateUnknown(false) + .withMapping(ServerComposedSnakeNaming.class, "server") + .withMapping(ServerComposedVerbatimNaming.class, "server") + .withMapping(ServerComposedKebabNaming.class, "server") + .withSources(config("server.the_host", "localhost", "server.the_port", "8080")) + .withSources(config("server.log.is_enabled", "true", "server.log.log_appenders[0].log_name", "log")) + .withSources(config("server.theHost", "localhost", "server.thePort", "8080")) + .withSources(config("server.log.isEnabled", "true", "server.log.logAppenders[0].logName", "log")) + .withSources(config("server.the-host", "localhost", "server.the-port", "8080")) + .withSources(config("server.log.is-enabled", "true", "server.log.log-appenders[0].log-name", "log")) + .build(); + + ServerComposedSnakeNaming snake = config.getConfigMapping(ServerComposedSnakeNaming.class); + assertNotNull(snake); + assertEquals("localhost", snake.theHost()); + assertEquals(8080, Integer.valueOf(snake.thePort())); + assertTrue(snake.log().isEnabled()); + assertEquals("log", snake.log().logAppenders().get(0).logName()); + + ServerComposedVerbatimNaming verbatim = config.getConfigMapping(ServerComposedVerbatimNaming.class); + assertNotNull(verbatim); + assertEquals("localhost", verbatim.theHost()); + assertEquals(8080, Integer.valueOf(verbatim.thePort())); + assertTrue(verbatim.log().isEnabled()); + assertEquals("log", verbatim.log().logAppenders().get(0).logName()); + + ServerComposedKebabNaming kebab = config.getConfigMapping(ServerComposedKebabNaming.class); + assertNotNull(kebab); + assertEquals("localhost", kebab.theHost()); + assertEquals(8080, Integer.valueOf(kebab.thePort())); + assertTrue(kebab.log().isEnabled()); + assertEquals("log", kebab.log().logAppenders().get(0).logName()); + } }