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

Support of nested classes for bulk properties #702

Open
kshpak opened this issue Jun 4, 2021 · 4 comments
Open

Support of nested classes for bulk properties #702

kshpak opened this issue Jun 4, 2021 · 4 comments
Labels
impl. proposal ⚙️ An issue or PR which proposes an implementation of use case(s) (link use case(s) in comments)

Comments

@kshpak
Copy link

kshpak commented Jun 4, 2021

Description
Add support of nested classes for java class annotated with @ConfigProperties (like it's done for nested interfaces)

E.x.

@ConfigProperties(prefix = "user")
public class UserConfiguration {
    public String message;
    public String name;

    public UserConfigurationAdditions request;
     
       public static class UserConfigurationAdditions {
         public String min;
         public String max;
     }
}
@kshpak kshpak added the impl. proposal ⚙️ An issue or PR which proposes an implementation of use case(s) (link use case(s) in comments) label Jun 4, 2021
@radcortez
Copy link
Contributor

Hi @kshpak, thanks for reaching out.

When you refer to nested interfaces, I guess you mean the SmallRye Config @ConfigMapping with interfaces?

What do you need to achieve with classes that you are unable to achieve with interfaces?

@kshpak
Copy link
Author

kshpak commented Jun 7, 2021

Hi @radcortez,
here is an example of mentioned nested interfaces

@ConfigProperties(prefix = "user")
public interface UserConfiguration {
    String name();
    String message();

    UserConfigurationAdditions request();

    interface UserConfigurationAdditions {
        String min();
        String max();
    }
}

Pretty everything can be achieved through interfaces, it's just another option of configuring properties through classes. Mentioned in the description option was present in quarkus.arc library. Now deprecated, because @ConfigProperties annotation was introduced in MicroProfile Config 2.0.

As @ConfigProperties can still be used with classes, the nested configuration is just a good complement

@radcortez
Copy link
Contributor

Actually, Quarkus @ConfigProperties was deprecated in favour of SR Config @ConfigMapping which uses interfaces instead, not for MP Config @ConfigProperties.

In my opinion, the MP Config @ConfigProperties class support was a mistake. From the experience we had, supporting classes adds way more complexity for little gain (fields visibility, reflection, types defaults values). Please check the additional discussion here: smallrye/smallrye-config#310.

I believe that MP Config specification should review this, move to interfaces and of course support nested elements. I also believe the the specification shouldn't offer both flavours (less is more :)), but of course implementations are free to do it if they want.

@Emily-Jiang
Copy link
Member

The class support for @ConfigProperties was to cater for the migration issues from Spring Config, which uses @ConfigProprties on classes. However, we can review this to see whether we should support on interfaces as well as classes or deprecate classes with prefering interfaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impl. proposal ⚙️ An issue or PR which proposes an implementation of use case(s) (link use case(s) in comments)
Projects
None yet
Development

No branches or pull requests

3 participants