From 69de06ac2dd4020f2dcaa3aabba532107a7cc729 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 10 Sep 2024 09:26:42 +0100 Subject: [PATCH] Remove origin support for empty YAML maps Adding origin support caused an unexpected and unwanted change in behavior where configuration property binding would fail. The failure would occur because there was no way to convert from the entry in the environment that represents the empty map to the target type. The commit changes the YAML loader to drop empty maps, effectively reverting the map portion of 3bcbb0e59491ee7fbdc8de67e43d696bb4d2e074 and gh-21704. This aligns the behavior with the decision we made in gh-19095. Origin support for an empty list has been retained as it does not have a negative effect on configuration property binding. Prior to these changes, an empty YAML list was mapped to an origin tracked value that contains an empty list. Fully reverting 3bcbb0e59491ee7fbdc8de67e43d696bb4d2e074 would have resulted in an empty YAML list being mapped to an empty string. To avoid adding a collection type to the environment, we now map an empty YAML list to an origin tracked value that contains an empty string. Closes gh-35403 --- .../boot/env/OriginTrackedYamlLoader.java | 6 +++--- .../env/OriginTrackedYamlLoaderTests.java | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java index 0eef7864da..9082c5034f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java @@ -29,11 +29,11 @@ import org.yaml.snakeyaml.constructor.BaseConstructor; import org.yaml.snakeyaml.constructor.Constructor; import org.yaml.snakeyaml.constructor.SafeConstructor; import org.yaml.snakeyaml.error.Mark; -import org.yaml.snakeyaml.nodes.CollectionNode; import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.nodes.NodeTuple; import org.yaml.snakeyaml.nodes.ScalarNode; +import org.yaml.snakeyaml.nodes.SequenceNode; import org.yaml.snakeyaml.nodes.Tag; import org.yaml.snakeyaml.representer.Representer; import org.yaml.snakeyaml.resolver.Resolver; @@ -105,8 +105,8 @@ class OriginTrackedYamlLoader extends YamlProcessor { @Override protected Object constructObject(Node node) { - if (node instanceof CollectionNode && ((CollectionNode) node).getValue().isEmpty()) { - return constructTrackedObject(node, super.constructObject(node)); + if (node instanceof SequenceNode sequenceNode && sequenceNode.getValue().isEmpty()) { + return constructTrackedObject(node, ""); } if (node instanceof ScalarNode) { if (!(node instanceof KeyScalarNode)) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java index ce5fa86434..58720b0023 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,6 @@ package org.springframework.boot.env; import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -115,18 +114,19 @@ class OriginTrackedYamlLoaderTests { void processEmptyAndNullValues() { OriginTrackedValue empty = getValue("empty"); OriginTrackedValue nullValue = getValue("null-value"); + OriginTrackedValue emptyList = getValue("emptylist"); assertThat(empty.getValue()).isEqualTo(""); assertThat(getLocation(empty)).isEqualTo("27:8"); assertThat(nullValue.getValue()).isEqualTo(""); assertThat(getLocation(nullValue)).isEqualTo("28:13"); + assertThat(emptyList.getValue()).isEqualTo(""); + assertThat(getLocation(emptyList)).isEqualTo("29:12"); } @Test - void processEmptyListAndMap() { - OriginTrackedValue emptymap = getValue("emptymap"); - OriginTrackedValue emptylist = getValue("emptylist"); - assertThat(emptymap.getValue()).isEqualTo(Collections.emptyMap()); - assertThat(emptylist.getValue()).isEqualTo(Collections.emptyList()); + void emptyMapsAreDropped() { + Object emptyMap = getValue("emptymap"); + assertThat(emptyMap).isNull(); } @Test @@ -194,11 +194,12 @@ class OriginTrackedYamlLoaderTests { assertThat(loaded).isNotEmpty(); } - private OriginTrackedValue getValue(String name) { + @SuppressWarnings("unchecked") + private T getValue(String name) { if (this.result == null) { this.result = this.loader.load(); } - return (OriginTrackedValue) this.result.get(0).get(name); + return (T) this.result.get(0).get(name); } private String getLocation(OriginTrackedValue value) {