Skip to content

Commit

Permalink
Restore support of exact match property
Browse files Browse the repository at this point in the history
This commit fixes a regression in property placeholder resolution where
the original key was no longer considered for an exact match before
processing the placeholder itself.

By default, property resolution uses ':' as the separator between the
key and the fallback value.

Consider a request to resolve ${prefix://service}. Previously,
placeholder resolution would first attempt to resolve the raw text, that
is 'prefix://service', before attempting to resolve the 'prefix' key and
then use '//service' if the key did not resolve.

This commit restores that behaviour purely for backward compatible
reason.

Closes gh-34124
  • Loading branch information
snicoll committed Dec 23, 2024
1 parent ebae02a commit 5ce5647
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ public SimplePlaceholderPart(String text,String key, @Nullable String fallback)

@Override
public String resolve(PartResolutionContext resolutionContext) {
String value = resolveRecursively(resolutionContext, this.key);
String value = resolveRecursively(resolutionContext);
if (value != null) {
return value;
}
Expand All @@ -528,6 +528,17 @@ else if (this.fallback != null) {
}
return resolutionContext.handleUnresolvablePlaceholder(this.key, text());
}

@Nullable
private String resolveRecursively(PartResolutionContext resolutionContext) {
if (!this.text().equals(this.key)) {
String value = resolveRecursively(resolutionContext, this.text());
if (value != null) {
return value;
}
}
return resolveRecursively(resolutionContext, this.key);
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.InOrder;

import org.springframework.util.PlaceholderParser.ParsedValue;
import org.springframework.util.PlaceholderParser.TextPart;
Expand All @@ -33,6 +34,7 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -220,20 +222,45 @@ static Stream<Arguments> nestedPlaceholders() {
);
}

@ParameterizedTest(name = "{0} -> {1}")
@MethodSource("exactMatchPlaceholders")
void placeholdersWithExactMatchAreConsidered(String text, String expected) {
Properties properties = new Properties();
properties.setProperty("prefix://my-service", "example-service");
properties.setProperty("px", "prefix");
properties.setProperty("p1", "${prefix://my-service}");
assertThat(this.parser.replacePlaceholders(text, properties::getProperty)).isEqualTo(expected);
}

static Stream<Arguments> exactMatchPlaceholders() {
return Stream.of(
Arguments.of("${prefix://my-service}", "example-service"),
Arguments.of("${p1}", "example-service")
);
}

@Test
void parseWithKeyEqualsToText() {
PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "Steve");
assertThat(this.parser.replacePlaceholders("${firstName}", resolver))
.isEqualTo("Steve");
verifyPlaceholderResolutions(resolver, "firstName");
}

@Test
void parseWithHardcodedFallback() {
PlaceholderResolver resolver = mockPlaceholderResolver();
assertThat(this.parser.replacePlaceholders("${firstName:Steve}", resolver))
.isEqualTo("Steve");
verifyPlaceholderResolutions(resolver, "firstName");
verifyPlaceholderResolutions(resolver, "firstName:Steve", "firstName");
}

@Test
void parseWithNestedPlaceholderInKeyUsingFallback() {
PlaceholderResolver resolver = mockPlaceholderResolver("firstName", "John");
assertThat(this.parser.replacePlaceholders("${first${invalid:Name}}", resolver))
.isEqualTo("John");
verifyPlaceholderResolutions(resolver, "invalid", "firstName");
verifyPlaceholderResolutions(resolver, "invalid:Name", "invalid", "firstName");
}

@Test
Expand Down Expand Up @@ -348,8 +375,9 @@ PlaceholderResolver mockPlaceholderResolver(String... pairs) {
}

void verifyPlaceholderResolutions(PlaceholderResolver mock, String... placeholders) {
InOrder ordered = inOrder(mock);
for (String placeholder : placeholders) {
verify(mock).resolvePlaceholder(placeholder);
ordered.verify(mock).resolvePlaceholder(placeholder);
}
verifyNoMoreInteractions(mock);
}
Expand Down

0 comments on commit 5ce5647

Please sign in to comment.