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.

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

Banner placeholders use default values too soon #34764

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2023 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.
Expand All @@ -19,7 +19,6 @@
import java.io.PrintStream;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -33,6 +32,7 @@
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertyResolver;
import org.springframework.core.env.PropertySource;
import org.springframework.core.env.PropertySourcesPropertyResolver;
import org.springframework.core.io.Resource;
import org.springframework.core.log.LogMessage;
Expand All @@ -45,13 +45,14 @@
* @author Phillip Webb
* @author Vedran Pavic
* @author Toshiaki Maki
* @author Krzysztof Krason
* @since 1.2.0
*/
public class ResourceBanner implements Banner {

private static final Log logger = LogFactory.getLog(ResourceBanner.class);

private Resource resource;
private final Resource resource;

public ResourceBanner(Resource resource) {
Assert.notNull(resource, "Resource must not be null");
Expand All @@ -77,18 +78,25 @@ public void printBanner(Environment environment, Class<?> sourceClass, PrintStre
}

protected List<PropertyResolver> getPropertyResolvers(Environment environment, Class<?> sourceClass) {
List<PropertyResolver> resolvers = new ArrayList<>();
resolvers.add(environment);
resolvers.add(getVersionResolver(sourceClass));
resolvers.add(getAnsiResolver());
resolvers.add(getTitleResolver(sourceClass));
return resolvers;
MutablePropertySources propertySources = new MutablePropertySources();
propertySources.addLast(getEnvironmentSource(environment));
propertySources.addLast(getTitleSource(sourceClass));
propertySources.addLast(getAnsiSource());
propertySources.addLast(getVersionSource(sourceClass));
return Collections.singletonList(new PropertySourcesPropertyResolver(propertySources));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilkinsona Using singletonList here changed the behavior of this method by making the returned list unmodifiable. Was this intentional?

For ResourceBanner subclasses, the change to use an unmodifiable list introduced a breaking change. We were able to work around it by creating a new list, adding items from this list, along with our own PropertyResolver(s).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot. I've fixed that in a707c5e

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @edwardsre. I missed that when reviewing the changes.

}

private PropertyResolver getVersionResolver(Class<?> sourceClass) {
MutablePropertySources propertySources = new MutablePropertySources();
propertySources.addLast(new MapPropertySource("version", getVersionsMap(sourceClass)));
return new PropertySourcesPropertyResolver(propertySources);
private PropertySource<Environment> getEnvironmentSource(Environment environment) {
return new PropertySource<Environment>("environment", environment) {
@Override
public Object getProperty(String name) {
return environment.getProperty(name);
}
};
}

private MapPropertySource getVersionSource(Class<?> sourceClass) {
return new MapPropertySource("version", getVersionsMap(sourceClass));
}

private Map<String, Object> getVersionsMap(Class<?> sourceClass) {
Expand Down Expand Up @@ -118,19 +126,15 @@ private String getVersionString(String version, boolean format) {
return format ? " (v" + version + ")" : version;
}

private PropertyResolver getAnsiResolver() {
MutablePropertySources sources = new MutablePropertySources();
sources.addFirst(new AnsiPropertySource("ansi", true));
return new PropertySourcesPropertyResolver(sources);
private AnsiPropertySource getAnsiSource() {
return new AnsiPropertySource("ansi", true);
}

private PropertyResolver getTitleResolver(Class<?> sourceClass) {
MutablePropertySources sources = new MutablePropertySources();
private MapPropertySource getTitleSource(Class<?> sourceClass) {
String applicationTitle = getApplicationTitle(sourceClass);
Map<String, Object> titleMap = Collections.singletonMap("application.title",
(applicationTitle != null) ? applicationTitle : "");
sources.addFirst(new MapPropertySource("title", titleMap));
return new PropertySourcesPropertyResolver(sources);
return new MapPropertySource("title", titleMap);
}

protected String getApplicationTitle(Class<?> sourceClass) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2023 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.
Expand Down Expand Up @@ -40,6 +40,7 @@
* @author Phillip Webb
* @author Vedran Pavic
* @author Toshiaki Maki
* @author Krzysztof Krason
*/
class ResourceBannerTests {

Expand All @@ -48,6 +49,21 @@ void reset() {
AnsiOutput.setEnabled(Enabled.DETECT);
}

@Test
void doNotUseDefaultsIfValueExists() {
Resource resource = new ByteArrayResource(
"banner ${a:def} ${spring-boot.version:def} ${application.version:def}".getBytes());
String banner = printBanner(resource, "10.2", "1.0", null);
assertThat(banner).startsWith("banner 1 10.2 1.0");
}

@Test
void useDefaults() {
Resource resource = new ByteArrayResource("banner ${b:def1} ${c:def2} ${d:def3}".getBytes());
String banner = printBanner(resource, null, null, null);
assertThat(banner).startsWith("banner def1 def2 def3");
}

@Test
void renderVersions() {
Resource resource = new ByteArrayResource(
Expand Down