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

Allow property values that contain placeholders to be bound as-is #13202

Closed
wilkinsona opened this issue May 17, 2018 · 5 comments
Closed

Allow property values that contain placeholders to be bound as-is #13202

wilkinsona opened this issue May 17, 2018 · 5 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

In 1.5, it was possible to bind a property value that contained an unresolvable placeholder due to this change. In 2.0, that's no longer possible and the binding will fail with an exception similar to the following:

Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'placeholder' in value "Value with ${placeholder}"
    at org.springframework.util.PropertyPlaceholderHelper.parseStringValue(PropertyPlaceholderHelper.java:172) ~[spring-core-5.0.6.RELEASE.jar:5.0.6.RELEASE]
    at org.springframework.util.PropertyPlaceholderHelper.replacePlaceholders(PropertyPlaceholderHelper.java:124) ~[spring-core-5.0.6.RELEASE.jar:5.0.6.RELEASE]
    at org.springframework.boot.context.properties.bind.PropertySourcesPlaceholdersResolver.resolvePlaceholders(PropertySourcesPlaceholdersResolver.java:60) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
    at org.springframework.boot.context.properties.bind.Binder.bindProperty(Binder.java:323) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
    at org.springframework.boot.context.properties.bind.Binder.bindObject(Binder.java:267) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
    at org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:221) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
    ... 43 common frames omitted

If we adopt the approach that was taken in 1.x (ignore any placeholder resolution failures), a property value with a placeholder could be bound as-is as long as there's no property with that value. Ideally, we'd like to be able to support binding a value that contains something that looks like a placeholder but that should not be treated as one.

@wilkinsona wilkinsona added type: enhancement A general enhancement for: team-attention An issue we'd like other members of the team to review labels May 17, 2018
@philwebb
Copy link
Member

I think the old approach might be the least bad option.

@philwebb philwebb added this to the 2.0.x milestone May 24, 2018
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label May 30, 2018
@wilkinsona wilkinsona self-assigned this Jun 4, 2018
@wilkinsona
Copy link
Member Author

wilkinsona commented Jun 4, 2018

Fixed by 0df37b9 (I messed up the issue reference in the commit).

@wilkinsona wilkinsona modified the milestones: 2.0.x, 2.0.3 Jun 4, 2018
@artemyarulin
Copy link

artemyarulin commented Jun 21, 2018

Hm, after this change there is no configuration validation anymore - with 2.0.2 if certain placeholder cannot be resolved application would throw an exception on start, but after 2.0.3 it would be running just fine and example config.dbConn would have value of "${DB_CONN}" which is really confusing

Example:

app:
  db-conn: ${DB_CONN}
@SpringBootApplication
@ConfigurationProperties("app")
class Config {
    lateinit var dbConn: String
}

cc @wilkinsona

@wilkinsona
Copy link
Member Author

@artemyarulin That was the exact intent of this change. This is a situation where it isn't possible to please all of the people all of the time. We decided that aligning the behaviour of Boot 2.0 with the behaviour of 1.5 was the best option.

@artemyarulin
Copy link

Understand that - Spring has a rather long history, thanks for your prompt reply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants