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

Proposal for Configurable Naming Strategy in ConfigMapping #1091

Closed
magicprinc opened this issue Jan 14, 2024 · 8 comments
Closed

Proposal for Configurable Naming Strategy in ConfigMapping #1091

magicprinc opened this issue Jan 14, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@magicprinc
Copy link

I am writing to propose a change that would allow the ConfigMapping.namingStrategy to be configured from the application's configuration.

Currently, the naming strategy for configuration properties is determined by the method namingStrategy() in the @ConfigMapping annotation interface, with the default value NamingStrategy.KEBAB_CASE.

You could add an additional ENUM constant: CONFIGURABLE and make it default.
This strategy would read config key
smallrye.config.naming.strategy = VERBATIM or KEBAB_CASE or SNAKE_CASE
and if present, use its value.
If smallrye.config.naming.strategy is absent, then NamingStrategy.KEBAB_CASE is used.

So, no backward compatibility is broken.

But one can change it in configuration file instead of tedious repeating in every annotation.

https://github.com/smallrye/smallrye-config/blob/main/implementation/src/main/java/io/smallrye/config/ConfigMapping.java

@radcortez
Copy link
Member

Thank you for the proposal.

Any chance to work on this? :)

@magicprinc
Copy link
Author

magicprinc commented Jan 15, 2024

@radcortez Thank You very much! I would be happy, but your magic is much beyond my knowledge. I have just opened source code: byte code manipulation, etc. Sorry 😥

Maybe I can send my JsonConfigSource and Spring integration? 🤣

@radcortez radcortez added the enhancement New feature or request label Jan 16, 2024
@radcortez
Copy link
Member

It shouldn't require changes in the bytecode generation parts. The naming strategy is applied by calling this method:

public NamingStrategy applyNamingStrategy(NamingStrategy namingStrategy) {
if (namingStrategy != null) {
this.namingStrategy = namingStrategy;
} else if (this.namingStrategy == null) {
this.namingStrategy = NamingStrategy.KEBAB_CASE;
}
return this.namingStrategy;
}
.

Generated code will call that method to pass the current naming strategy, but if we add a global one, I believe it just needs to be checked with some logic in that method. I can have a look into that.

Maybe I can send my JsonConfigSource and Spring integration? 🤣

The JSON support is already provided by our YAML Config Source. The Spring integration sounds interesting. Do you have the code available somewhere so I can have a look?

@magicprinc
Copy link
Author

magicprinc commented Jan 16, 2024

Theory first and it is quite simple.

  1. you declare @Bean for SmallRyeConfig
    image

Result: SmallRyeConfig can be injected:

@Autowire
SmallRyeConfig config;
  1. you wrap SmallRyeConfig into Spring's PropertySource
    image
    Result: you can use SmallRyeConfig properties in Spring
@Value("${hello.from.smallrye.properties.file:42}")
int theAnswer;
  1. you declare your own InstantiationAwareBeanPostProcessor (BeanPostProcessor) where you receive every bean (object) instantiated by Spring and can do everything you want, for example: check all annotations and set fields

image
You can read the fields' annotations and something with it.

Result: SmallRyeConfig/MicroProfileConfig annotations magically work in Spring:

@Value("${$groovy$ 1 + 2}")
private int g1;
@ConfigProperty(name = "$groovy$ Math.PI")
private double g2;
@ConfigProperty(name = "$expr$ Spring @Value: ${a} + ${b} = works")
private CharSequence betterThanValue;


@ConfigProperty(defaultValue = NULL)
private Integer int1;
@ConfigProperty(defaultValue = MIN)
private Integer int2;
@ConfigProperty(defaultValue = MAX)
private Integer int3;
@ConfigProperty(defaultValue = FIELD)
private Integer int4 = 42;

// any annotation: 
@ConfigProperties
private @NonNull SmallRyeConfig config2;

@ConfigProperties(prefix = "2nd-cfg")
private JPropertiesTest.Config2 configParams2withoutPrefix;

After reading this ^: how deep you want to go?

@radcortez
Copy link
Member

Your integration is more related to MicroProfile Config than SmallRye Config. You could probably replace the Bean creator to use Config instead of SmallRyeConfig and hide the implementation.

I recommend staying away from @ConfigProperty or @ConfigProperties. These annotations have shortcomings, and while they can work, they do not represent how we want to use a configuration system. We had many discussions about this in a new specification, Jakarta Config, where we agreed that a better way to do things is to use an interface with minimal configuration, with no relation to a dependency injection container. This is similar (if not equal) to our @ConfigMapping proposal.

A @ConfigMapping can be used standalone with any Java project. You don't need a Bean or CDI container. There are also considerable advantages over a class-based configuration representation, which we detailed here: jakartaee/config#122.

If we move to provide some sort of Spring integration (which I would definitely love to), I would prefer to just stick with @ConfigMapping and SmallRyeConfig, which is what we already do in Quarkus.

@magicprinc
Copy link
Author

magicprinc commented Jan 20, 2024

@radcortez I was busy… 🙏
The exact annotation is unimportant in the case of Spring.
One can create a new annotation or use existing ones.
As I already said, there are 3–4 areas of integration (all are optional):

  1. SmallRyeConfig bean
  2. Spring PropertySource wrapper for SmallRyeConfig ConfigSources
  3. Spring BeanPostProcessor to handle @ConfigProperty (if one wants @ConfigProperty in addition to Spring's @Value)
  4. Spring BeanPostProcessor to handle @ConfigMapping (or @ConfigProperties — I am sorry to say, but they are equal from BeanPostProcessor point of view).

1 and 2 → very easy
4 → easy (Spring Boot has alternative @ConfigurationProperties)
3 → a lot of converter calls and .getClass()/.getGenericType() for property from config file and for default value from annotation. And strictly speaking: Spring has its own alternative: @Value

But if all 4 are implemented, you can write anything you like:

@Value("${$gst$ Example ${1 + 2} <%= this.getClass().getSuperclass() %> } ")
private String g3;
@ConfigProperty(name = "$gst$ Example ${2 + 2} <%= this.getClass().getSuperclass() %> ")
private String g4;
@Autowired
private @NonNull SmallRyeConfig config1;
@ConfigMapping
private SmallRyeConfigTest.SimpleObj3 obj30;
@ConfigMapping(prefix = "obj2")
private SmallRyeConfigTest.SimpleObj3 obj40;
@ConfigProperties
private SmallRyeConfigTest.SimpleObj3 obj3;
@ConfigProperties(prefix = "obj2")
private SmallRyeConfigTest.SimpleObj3 obj4;

@radcortez
Copy link
Member

I know it is possible to support all kinds of annotations.

From the SR Config perspective, the only annotation that we should provide support for is @ConfigMapping. Other annotations like @ConfigProperty or @ConfigProperties are from MP Config, and while we do support such API, we recommend using only SR Config ones when possible. MP Config is mostly abandoned in favor of Jakarta Config, which is not complete yet.

@radcortez
Copy link
Member

This is not straightforward to implement.

The NamingStrategy is also used to generate the mapping implementations, meaning that we statically read the value to perform the generation. To support such a feature, the mapping generation would require not being static and using the SmallRyeConfig instance, which creates some additional complexity. Additionally, changing the configuration requires regenerating the mapping implementations.

While I see some value in the feature, the effort to support it is quite high, which makes it less interesting to add. For the time being, I'm closing this.

@radcortez radcortez closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants