Skip to content

Commit 5432b1b

Browse files
radcortezradcortez
committed
Do not include the unnamedKey in a @WithUnnamedKey Map if the Map value is the default
1 parent 0990aff commit 5432b1b

6 files changed

Lines changed: 235 additions & 12 deletions

File tree

‎implementation/src/main/java/io/smallrye/config/ConfigMappingContext.java‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public final class ConfigMappingContext {
4545

4646
private NamingStrategy namingStrategy = NamingStrategy.KEBAB_CASE;
4747
private BeanStyleGetters beanStyleGetters = BeanStyleGetters.DISABLED;
48+
private boolean resolvedNonDefault;
49+
4850
private final StringBuilder nameBuilder = new StringBuilder();
4951
private final Set<String> usedProperties = new HashSet<>();
5052
private final List<Problem> problems = new ArrayList<>();
@@ -289,15 +291,16 @@ public <K> ObjectCreator<T> map(
289291
final Class<? extends Converter<K>> keyConvertWith,
290292
final String unnamedKey,
291293
final Iterable<String> keys) {
292-
return map(keyRawType, keyConvertWith, unnamedKey, keys, null);
294+
return map(keyRawType, keyConvertWith, unnamedKey, keys, null, null);
293295
}
294296

295297
public <K, V> ObjectCreator<T> map(
296298
final Class<K> keyRawType,
297299
final Class<? extends Converter<K>> keyConvertWith,
298300
final String unnamedKey,
299301
final Iterable<String> keys,
300-
final Supplier<V> defaultValue) {
302+
final Supplier<V> defaultValue,
303+
final Supplier<V> unnamedDefaultSupplier) {
301304
Converter<K> keyConverter = keyConvertWith == null ? config.requireConverter(keyRawType)
302305
: getConverterInstance(keyConvertWith);
303306
List<Consumer<Function<String, Object>>> nestedCreators = new ArrayList<>();
@@ -315,11 +318,26 @@ public Object apply(final String path) {
315318
Map<K, V> map = defaultInstance != null ? new MapWithDefault<>(defaultInstance) : new HashMap<>();
316319

317320
if (unnamedKey != null) {
321+
V unnamedDefault = defaultInstance;
322+
if (unnamedDefault == null && unnamedDefaultSupplier != null) {
323+
int problemsBefore = problems.size();
324+
int length = nameBuilder.length();
325+
nameBuilder.append(".*");
326+
unnamedDefault = constructGroup(unnamedDefaultSupplier);
327+
nameBuilder.setLength(length);
328+
if (problems.size() > problemsBefore) {
329+
problems.subList(problemsBefore, problems.size()).clear();
330+
unnamedDefault = null;
331+
}
332+
}
333+
final V unnamedDefaultInstance = unnamedDefault;
318334
nestedCreators.add(new Consumer<>() {
319335
@Override
320336
public void accept(Function<String, Object> get) {
337+
resolvedNonDefault = false;
321338
V value = (V) get.apply(path);
322-
if (value != null) {
339+
if (value != null
340+
&& (resolvedNonDefault || !value.equals(unnamedDefaultInstance))) {
323341
map.put(unnamedKey.isEmpty() ? null : keyConverter.convert(unnamedKey), value);
324342
}
325343
}
@@ -590,7 +608,14 @@ private static <V> V convertValue(
590608
final String propertyName,
591609
final Converter<V> valueConverter) {
592610
context.usedProperties.add(propertyName);
593-
return context.config.getValue(propertyName, valueConverter);
611+
ConfigValue configValue = context.config.getConfigValue(propertyName);
612+
if (configValue.getValue() != null && !configValue.isDefault()) {
613+
context.resolvedNonDefault = true;
614+
}
615+
if (Converters.CONFIG_VALUE_CONVERTER.equals(valueConverter)) {
616+
return (V) configValue.noProblems();
617+
}
618+
return context.config.convertValue(configValue, valueConverter);
594619
}
595620

596621
public ObjectCreator<T> value(
@@ -644,7 +669,11 @@ private static <V> Optional<V> convertOptionalValue(
644669
final String propertyName,
645670
final Converter<V> valueConverter) {
646671
context.usedProperties.add(propertyName);
647-
return context.config.getOptionalValue(propertyName, valueConverter);
672+
ConfigValue configValue = context.config.getConfigValue(propertyName);
673+
if (configValue.getValue() != null && !configValue.isDefault()) {
674+
context.resolvedNonDefault = true;
675+
}
676+
return context.config.convertValue(configValue, newOptionalConverter(valueConverter));
648677
}
649678

650679
public <V> ObjectCreator<T> optionalValue(

‎implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,11 @@ private static void generateNestedProperty(final ObjectCreatorMethodVisitor ctor
594594
} else {
595595
ctor.visitInsn(ACONST_NULL);
596596
}
597+
if (mapProperty.hasKeyUnnamed()) {
598+
ctor.visitGroupSupplier(valueProperty.asGroup().getGroupType().getInterfaceType());
599+
} else {
600+
ctor.visitInsn(ACONST_NULL);
601+
}
597602
ctor.visitMethod(ObjectCreatorMapGroupInvocation.map);
598603
if (mapProperty.hasKeyProvider()) {
599604
ctor.visitGroupSupplier(valueProperty.asGroup().getGroupType().getInterfaceType());
@@ -1353,7 +1358,8 @@ public String desc() {
13531358
}
13541359

13551360
private enum ObjectCreatorMapGroupInvocation implements MethodInvocation {
1356-
map(INVOKEVIRTUAL, "(" + D_CLASS + D_CLASS + D_STRING + D_ITERABLE + D_SUPPLIER + ")" + D_OBJECT_CREATOR);
1361+
map(INVOKEVIRTUAL,
1362+
"(" + D_CLASS + D_CLASS + D_STRING + D_ITERABLE + D_SUPPLIER + D_SUPPLIER + ")" + D_OBJECT_CREATOR);
13571363

13581364
private final int opcode;
13591365
private final String desc;

‎implementation/src/test/java/io/smallrye/config/ConfigMappingFullTest.java‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,52 @@ void splitMappings() {
267267
.build();
268268

269269
OrmConfig ormConfig = config.getConfigMapping(OrmConfig.class);
270-
assertEquals(1, ormConfig.persistenceUnits().size());
270+
assertEquals(0, ormConfig.persistenceUnits().size());
271+
assertFalse(ormConfig.persistenceUnits().get("<default>").database().globallyQuotedIdentifiers());
271272
OrmRuntimeConfig ormRuntimeConfig = config.getConfigMapping(OrmRuntimeConfig.class);
272273
assertEquals(1, ormRuntimeConfig.persistenceUnits().size());
273274
assertEquals("drop-and-create",
274275
ormRuntimeConfig.persistenceUnits().get("<default>").database().generation().generation());
275276
}
276277

278+
@Test
279+
void noUnnamedKeyUnlessSetByUser() {
280+
SmallRyeConfig config;
281+
OrmRuntimeConfig mapping;
282+
283+
// Nothing set by the user, so no key, but I can query a default
284+
config = new SmallRyeConfigBuilder()
285+
.withMapping(OrmRuntimeConfig.class)
286+
.build();
287+
mapping = config.getConfigMapping(OrmRuntimeConfig.class);
288+
assertEquals(0, mapping.persistenceUnits().size());
289+
assertEquals("none", mapping.persistenceUnits().get("<default>").database().generation().generation());
290+
assertTrue(mapping.persistenceUnits().get("<default>").schemas().isEmpty());
291+
assertEquals("none", mapping.persistenceUnits().get("named").database().generation().generation());
292+
assertTrue(mapping.persistenceUnits().get("named").schemas().isEmpty());
293+
294+
// A property set by the user, so the default key must be present in the Map
295+
config = new SmallRyeConfigBuilder()
296+
.withSources(config("smallrye.config-orm.database.generation", "create"))
297+
.withMapping(OrmRuntimeConfig.class)
298+
.build();
299+
mapping = config.getConfigMapping(OrmRuntimeConfig.class);
300+
assertEquals(1, mapping.persistenceUnits().size());
301+
assertTrue(mapping.persistenceUnits().containsKey("<default>"));
302+
assertEquals("create", mapping.persistenceUnits().get("<default>").database().generation().generation());
303+
assertTrue(mapping.persistenceUnits().get("<default>").schemas().isEmpty());
304+
305+
config = new SmallRyeConfigBuilder()
306+
.withSources(config("smallrye.config-orm.schemas.version", "1"))
307+
.withMapping(OrmRuntimeConfig.class)
308+
.build();
309+
mapping = config.getConfigMapping(OrmRuntimeConfig.class);
310+
assertEquals(1, mapping.persistenceUnits().size());
311+
assertTrue(mapping.persistenceUnits().containsKey("<default>"));
312+
assertTrue(mapping.persistenceUnits().get("<default>").schemas().get(null).version().isPresent());
313+
assertEquals("1", mapping.persistenceUnits().get("<default>").schemas().get(null).version().get());
314+
}
315+
277316
@ConfigMapping(prefix = "smallrye.config-orm")
278317
interface OrmConfig {
279318
OrmConfigDatabase database();
@@ -307,14 +346,26 @@ interface OrmRuntimeConfig {
307346
interface OrmRuntimeConfigPersistenceUnit {
308347
OrmConfigPersistenceUnitDatabase database();
309348

349+
@WithUnnamedKey
350+
Map<String, OrmConfigPersistenceUnitSchema> schemas();
351+
310352
interface OrmConfigPersistenceUnitDatabase {
311353
OrmConfigPersistenceUnitDatabaseGeneration generation();
354+
355+
@WithDefault("true")
356+
boolean log();
357+
358+
OptionalInt version();
359+
360+
interface OrmConfigPersistenceUnitDatabaseGeneration {
361+
@WithParentName
362+
@WithDefault("none")
363+
String generation();
364+
}
312365
}
313366

314-
interface OrmConfigPersistenceUnitDatabaseGeneration {
315-
@WithParentName
316-
@WithDefault("none")
317-
String generation();
367+
interface OrmConfigPersistenceUnitSchema {
368+
Optional<String> version();
318369
}
319370
}
320371
}

‎implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,6 +2727,7 @@ void groupDefaults() {
27272727
GroupDefaults mapping = config.getConfigMapping(GroupDefaults.class);
27282728
assertEquals("value", mapping.group().value());
27292729
assertEquals("default", mapping.defaults().group().get("default").value());
2730+
assertEquals("default", config.getConfigValue("group-defaults.group.x.value").getValue());
27302731
}
27312732

27322733
@ConfigMapping(prefix = "group-defaults")

‎implementation/src/test/java/io/smallrye/config/ConfigMappingMapDefaultsUnnamedKeyTest.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,72 @@ <E> void otherConfigGroupKeyConfigured(ConfigGroupMapMapping<E> mapping, ConfigG
176176
}
177177
}
178178

179+
@ParameterizedTest
180+
@MethodSource("mapMappingsAndConfigGroupKeys")
181+
<E> void unnamedKeyConfiguredWithSameValueAsDefault(ConfigGroupMapMapping<E> mapping, ConfigGroupKey configGroupKey) {
182+
assumeTrue(mapping.hasWithUnnamedKey(), "Only applicable to mappings with @WithUnnamedKey");
183+
assumeTrue(mapping.hasConfigProperty1Default(), "Only applicable to mappings with @WithDefault on configProperty1");
184+
assumeTrue(configGroupKey == ConfigGroupKey.UNNAMED, "Only applicable to the unnamed key");
185+
186+
// Explicitly set the unnamed key's configProperty1 to the same value as the @WithDefault.
187+
// Even though the value matches the default, it comes from a real config source,
188+
// so the unnamed key should appear in the map.
189+
String configProperty1Key = mapping.configProperty1Key(configGroupKey);
190+
SmallRyeConfig config = mapping.buildConfig(configProperty1Key, "default-val");
191+
Map<String, E> map = mapping.getConfigGroupMap(config);
192+
193+
assertTrue(map.containsKey(configGroupKey.key),
194+
"containsKey should be true when the unnamed key is explicitly configured, even with the same value as @WithDefault");
195+
}
196+
197+
// -- Case 2: leaf properties resolved, all from defaults → unnamed key excluded --
198+
199+
@ConfigMapping(prefix = "case2")
200+
interface Case2Config {
201+
@WithDefaults
202+
@WithUnnamedKey("<default>")
203+
Map<String, ConfigGroup> map();
204+
}
205+
206+
@Test
207+
void leafPropertiesResolvedAllFromDefaults() {
208+
SmallRyeConfig config = new SmallRyeConfigBuilder()
209+
.withMapping(Case2Config.class)
210+
.build();
211+
212+
Map<String, ConfigGroup> map = config.getConfigMapping(Case2Config.class).map();
213+
214+
assertFalse(map.containsKey("<default>"),
215+
"unnamed key should not appear when all leaf properties resolve from defaults only");
216+
217+
ConfigGroup group = map.get("<default>");
218+
assertNotNull(group, "@WithDefaults should still provide a default via get()");
219+
assertEquals("default-val", group.configProperty1());
220+
assertEquals(Optional.empty(), group.configProperty2());
221+
}
222+
223+
// -- Case 3: no leaf properties resolved (nested map) → unnamed key included via equals() fallback --
224+
225+
@ConfigMapping(prefix = "case3")
226+
interface Case3Config {
227+
@WithUnnamedKey("<default>")
228+
Map<String, Map<String, String>> map();
229+
}
230+
231+
@Test
232+
void nestedMapNoLeafResolution() {
233+
SmallRyeConfig config = new SmallRyeConfigBuilder()
234+
.withMapping(Case3Config.class)
235+
.withSources(config(
236+
"case3.map.nested-key", "value"))
237+
.build();
238+
239+
Map<String, Map<String, String>> map = config.getConfigMapping(Case3Config.class).map();
240+
241+
assertTrue(map.containsKey("<default>"),
242+
"unnamed key should appear for nested maps when config is provided at the unnamed path");
243+
}
244+
179245
// ============================
180246
// Infrastructure
181247
// ============================
@@ -200,6 +266,16 @@ static abstract class ConfigGroupMapMapping<E> {
200266

201267
abstract Map<String, E> getConfigGroupMap(SmallRyeConfig config);
202268

269+
abstract String namedKeyConfigProperty1Key();
270+
271+
abstract String unnamedKeyConfigProperty1Key();
272+
273+
String configProperty1Key(ConfigGroupKey configGroupKey) {
274+
return configGroupKey == ConfigGroupKey.NAMED
275+
? namedKeyConfigProperty1Key()
276+
: unnamedKeyConfigProperty1Key();
277+
}
278+
203279
abstract String namedKeyConfigProperty2Key();
204280

205281
abstract String unnamedKeyConfigProperty2Key();
@@ -315,6 +391,16 @@ Class<?>[] mappingClasses() {
315391
return new Class<?>[] { Config.class };
316392
}
317393

394+
@Override
395+
String namedKeyConfigProperty1Key() {
396+
return "plain-map.map.mykey.config-property1";
397+
}
398+
399+
@Override
400+
String unnamedKeyConfigProperty1Key() {
401+
return null;
402+
}
403+
318404
@Override
319405
String namedKeyConfigProperty2Key() {
320406
return "plain-map.map.mykey.config-property2";
@@ -358,6 +444,16 @@ Class<?>[] mappingClasses() {
358444
return new Class<?>[] { Config.class };
359445
}
360446

447+
@Override
448+
String namedKeyConfigProperty1Key() {
449+
return "with-defaults-only.map.mykey.config-property1";
450+
}
451+
452+
@Override
453+
String unnamedKeyConfigProperty1Key() {
454+
return null;
455+
}
456+
361457
@Override
362458
String namedKeyConfigProperty2Key() {
363459
return "with-defaults-only.map.mykey.config-property2";
@@ -401,6 +497,16 @@ Class<?>[] mappingClasses() {
401497
return new Class<?>[] { Config.class };
402498
}
403499

500+
@Override
501+
String namedKeyConfigProperty1Key() {
502+
return "with-unnamed-key-only.map.mykey.config-property1";
503+
}
504+
505+
@Override
506+
String unnamedKeyConfigProperty1Key() {
507+
return "with-unnamed-key-only.map.config-property1";
508+
}
509+
404510
@Override
405511
String namedKeyConfigProperty2Key() {
406512
return "with-unnamed-key-only.map.mykey.config-property2";
@@ -445,6 +551,16 @@ Class<?>[] mappingClasses() {
445551
return new Class<?>[] { Config.class };
446552
}
447553

554+
@Override
555+
String namedKeyConfigProperty1Key() {
556+
return "with-defaults-and-unnamed-key.map.mykey.config-property1";
557+
}
558+
559+
@Override
560+
String unnamedKeyConfigProperty1Key() {
561+
return "with-defaults-and-unnamed-key.map.config-property1";
562+
}
563+
448564
@Override
449565
String namedKeyConfigProperty2Key() {
450566
return "with-defaults-and-unnamed-key.map.mykey.config-property2";
@@ -490,6 +606,16 @@ Class<?>[] mappingClasses() {
490606
return new Class<?>[] { Config.class };
491607
}
492608

609+
@Override
610+
String namedKeyConfigProperty1Key() {
611+
return "with-parent-name-defaults-and-unnamed-key.mykey.config-property1";
612+
}
613+
614+
@Override
615+
String unnamedKeyConfigProperty1Key() {
616+
return "with-parent-name-defaults-and-unnamed-key.config-property1";
617+
}
618+
493619
@Override
494620
String namedKeyConfigProperty2Key() {
495621
return "with-parent-name-defaults-and-unnamed-key.mykey.config-property2";
@@ -533,6 +659,16 @@ Class<?>[] mappingClasses() {
533659
return new Class<?>[] { Config.class };
534660
}
535661

662+
@Override
663+
String namedKeyConfigProperty1Key() {
664+
return "with-unnamed-key-no-property-defaults.map.mykey.config-property1";
665+
}
666+
667+
@Override
668+
String unnamedKeyConfigProperty1Key() {
669+
return "with-unnamed-key-no-property-defaults.map.config-property1";
670+
}
671+
536672
@Override
537673
String namedKeyConfigProperty2Key() {
538674
return "with-unnamed-key-no-property-defaults.map.mykey.config-property2";

0 commit comments

Comments
 (0)