Skip to content

Add support for configuration property fallback #7986

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

Closed
philwebb opened this issue Jan 13, 2017 · 9 comments
Closed

Add support for configuration property fallback #7986

philwebb opened this issue Jan 13, 2017 · 9 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@philwebb
Copy link
Member

Some way to specify the default value for interfaces and a different fallback property.

@philwebb philwebb added this to the 2.0.0.M1 milestone Jan 13, 2017
@snicoll
Copy link
Member

snicoll commented Jan 13, 2017

An example

@ConfigurationProperties("foo.bar")
public class BarProperties {

    private int timeout 1000;

    @ConfigurationPropertyValue(fallback = "foo.default.timeout")
    public int getTimeout() { return timeout; }
  
   public void setTimeout(int timeout) { ... }  

}

The same annotation can be used to provide default values for interface-based configuration properties, something like

@ConfigurationProperties("foo.bar")
public interface BarProperties {

    @ConfigurationPropertyValue(fallback = "foo.default.timeout", default = "1000")
    int getTimeout();

}

@afranken
Copy link

instead of (or in addition to) adding an annotation that works for interfaces for properties classes, you should add an annotation for fields of properties classes (much like the @Autowired annotation) and not force users to add public getter and setter methods to properties classes.

@wilkinsona wilkinsona added the theme: config-data Issues related to the configuration theme label Jan 24, 2017
@wilkinsona
Copy link
Member

@afranken Please see #1254

@thombergs
Copy link
Contributor

thombergs commented Feb 19, 2017

In addition to defining a fallback, I would like to define whether it's allowed for a configuration property to be missing. Most of the times, startup of the application should fail when an expected property is missing. With @ConfigurationProperties alone, I currently see no possibility to fail on startup if a property is missing (other than creating a @PostConstruct method or something similar that does manual validation). It will just be assigned NULL or if it's a primitive the default value of that primitive type.

So, essentially, it would be great to be able to define exceptionIfMissing (in the style of exceptionIfInvalid on @ConfigurationProperties) on the annotation proposed by @snicoll like this:

@ConfigurationProperties("foo.bar")
public class BarProperties {

   private int optionalTimeout;
   private int mandatoryTimeout;

   @ConfigurationPropertyValue(exceptionIfMissing=false)
   public int getOptionalTimeout() { return optionalTimeout; }
  
   @ConfigurationPropertyValue(exceptionIfMissing=true)
   public int getMandatoryTimeout() { return mandatoryTimeout; }
  
  ...
}

What are your thoughts on this?

@afranken
Copy link

@thombergs I would expect that JSR-303 validation will keep working as it does right now:
https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html#boot-features-external-config-validation

In your example, you would annotate your Integer fields with for example @Max or @Min, and @NotNull if you wanted an exception to be thrown during startup...

@thombergs
Copy link
Contributor

Ah, I didn't read the docs careful enough. Bean Validation works fine for my proposed use case. Thanks for the pointer.

@philwebb philwebb modified the milestones: 2.0.0.M1, 2.0.0.M2 May 16, 2017
@philwebb philwebb modified the milestones: 2.0.0.M2, 2.0.0.M3 Jun 3, 2017
@wilkinsona wilkinsona modified the milestones: 2.0.0.M3, 2.0.0.M4 Jul 26, 2017
@snicoll snicoll self-assigned this Aug 12, 2017
thombergs added a commit to thombergs/spring-boot that referenced this issue Aug 19, 2017
…ng-projects#7986).

Added onNull() to BindHandler to be able to react to properties that
bind to null to choose a fallback value instead.

ConfigurationPropertyValue annotations (which allow defining a fallback
value for a property) are now collected by
ConfigurationPropertyValueReader and a map of fallback properties is
passed into a FallbackBindHandler. The handler then reacts on a null
value by providing the fallback value instead of the null value.
thombergs added a commit to thombergs/spring-boot that referenced this issue Aug 20, 2017
…ng-projects#7986).

Added onNull() to BindHandler to be able to react to properties that
bind to null to choose a fallback value instead.

ConfigurationPropertyValue annotations (which allow defining a fallback
value for a property) are now collected by
ConfigurationPropertyValueReader and a map of fallback properties is
passed into a FallbackBindHandler. The handler then reacts on a null
value by providing the fallback value instead of the null value.
thombergs added a commit to thombergs/spring-boot that referenced this issue Aug 20, 2017
thombergs added a commit to thombergs/spring-boot that referenced this issue Aug 20, 2017
…ng-projects#7986).

Added onNull() to BindHandler to be able to react to properties that
bind to null to choose a fallback value instead.

ConfigurationPropertyValue annotations (which allow defining a fallback
value for a property) are now collected by
ConfigurationPropertyValueReader and a map of fallback properties is
passed into a FallbackBindHandler. The handler then reacts on a null
value by providing the fallback value instead of the null value.
thombergs added a commit to thombergs/spring-boot that referenced this issue Aug 20, 2017
The check has been removed so that fallback properties may come from a
different namespace than the original property (spring-projects#7986).
@snicoll
Copy link
Member

snicoll commented Aug 22, 2017

I gave this feature a bit more thought and it looks like we could use a clear split between two completely different features:

  1. Offering a fallback mechanism
  2. Allow a default value if no field is available (typically if/when we support interface binding).

Let's focus on 1. It feels to me the #1 use case is an object that gathers some default and then some Map of “stuff” with fallback on those defaults. As we aren't generating metadata for maps that fallback information will not end up in the metadata. I think that can be a problem.

Also if the fallback key has a fallback on its own you can easily create a cycle. It would be wise to allows only one level of fallback but given the metadata generation is completely decentralized, there is no way to prevent that to happen.

I am rephrasing the scope of this issue to be 1.

@snicoll snicoll changed the title Add support for @ConfigurationPropertyValue Add support for configuration property fallback Aug 22, 2017
@snicoll snicoll removed this from the 2.0.0.M4 milestone Aug 22, 2017
@snicoll snicoll removed their assignment Aug 25, 2017
thombergs added a commit to thombergs/spring-boot that referenced this issue Sep 12, 2017
…ng-projects#7986).

Added onNull() to BindHandler to be able to react to properties that
bind to null to choose a fallback value instead.

ConfigurationPropertyValue annotations (which allow defining a fallback
value for a property) are now collected by
ConfigurationPropertyValueReader and a map of fallback properties is
passed into a FallbackBindHandler. The handler then reacts on a null
value by providing the fallback value instead of the null value.
thombergs added a commit to thombergs/spring-boot that referenced this issue Sep 12, 2017
thombergs added a commit to thombergs/spring-boot that referenced this issue Dec 16, 2017
Added onNull() to BindHandler to be able to react to properties that
bind to null to choose a fallback value instead.

ConfigurationPropertyValue annotations (which allow defining a fallback
value for a property) are now collected by
ConfigurationPropertyValueReader and a map of fallback properties is
passed into a FallbackBindHandler. The handler then reacts on a null
value by providing the fallback value instead of the null value.

Removed checking for descendant properties, because otherwise fallback
properties must always be from the same namespace.

Fixes spring-projects#7986
@ammmze
Copy link

ammmze commented Aug 30, 2018

This is thread is pretty old now, but would it be better to have @ConfigurationPropertyValue take an expression? Such as:

@ConfigurationProperties("foo.bar")
public interface BarProperties {

    @ConfigurationPropertyValue(default = "${foo.default.timeout:1000}")
    int getTimeout();

}

@philwebb
Copy link
Member Author

philwebb commented Feb 1, 2019

We're going to be looking at immutable configuration properties so I think it's best to close this for now so we don't make that work more complicated. Once we've got immutable support in we can reconsider.

@philwebb philwebb closed this as completed Feb 1, 2019
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed theme: config-data Issues related to the configuration theme type: enhancement A general enhancement labels Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

6 participants