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 config property --> object field custom mapping #37105

Open
mipo256 opened this issue Aug 25, 2023 · 13 comments
Open

Support config property --> object field custom mapping #37105

mipo256 opened this issue Aug 25, 2023 · 13 comments
Labels
status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Milestone

Comments

@mipo256
Copy link

mipo256 commented Aug 25, 2023

The following SO question illustrates the problem well.

Long story short, sometimes I want to have a property in my config file, which name is different from the object field. As far as I know, and seems it is still the case, there is no way to do it other than generating some custom setters, but that seems to be a workaround, not a solution.

It would be great to introduce a new annotation (or another way, but the annotation seems to be the smoothest) to handle this case, like:

class MyS3Config {

     /**
      * This annotation of course does not exists, but this is just an example
      * I cannot have an identifier named 'public' in Java, so I have to do custom mapping
      */
     @ConfigurationPropertiesMapping(property = "public")
     private String publicBucket;
}

Thanks in advance

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 25, 2023
@quaff
Copy link
Contributor

quaff commented Aug 28, 2023

Could you explain why you would like to use a different key in config file?
There is another solution:

public: ***
publicBucket: ${public}

@mipo256
Copy link
Author

mipo256 commented Aug 28, 2023

Sure.
Example you provided, as I pointed out specifically, is not a solution, it is a workaround to just make this work. What we have in reality is this: there is a Buckets nested config class in AWSConfig in our domain. We have a naming convention for decades of buckets, like:

company.iam.buckets.<bucket_name>

and the config from the top level looks like this:

public class AWSConfig {

    static class Buckets {
    } 
}

So now image we have a bucket named public. We cannot use public as an identifier in java. The only option now is to name it like:

company.iam.buckets.publicBucket

but we already have a bunch of them named like:

company.iam.buckets.local
company.iam.buckets.restricted
company.iam.buckets.data
company.iam.buckets.terraform

So we would break the naming convention. Or we need to change the naming convention just becasue of one bucket. It is clearly easier to just name the bucket in Java code as publicBucket, but retain the properties naming convention in config.

I hope it is clear now.

@quaff
Copy link
Contributor

quaff commented Aug 28, 2023

We cannot use public as an identifier in java.

I got the point.

@quaff
Copy link
Contributor

quaff commented Aug 28, 2023

Introducing a new annotation will make things more complicated, I think custom setter setPublic(String value) or placeholder publicBucket: ${public} is graceful enough, If the config class is maintained by yourself then adding setter is more appropriate, otherwise use placeholder instead.

@mipo256
Copy link
Author

mipo256 commented Aug 28, 2023

I do not think so. I have encoutererd this predicament multiple times, and the question itself was viewed 7k+ times, so I assume I am not alone. I can of course each time get by using custom setters, but that's again unclear for the reader of the code on why this setter exists, becuase this setter is not utilized anywhere in the code base, so people delete it and you got the idea etc.

It is a real experience, so I think providing inforamtion about mapping sometimes by annotation or by some other means will be great.

P.S: In spring-data we also can name columns with other names then the ones that are in db (eg using @Column). So I do not see any problem in here.

@philwebb
Copy link
Member

FWIW I don't think an additional annotation would make things too much more confusing. I'll flag this issue for team attention to see what others think.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Aug 28, 2023
@wilkinsona
Copy link
Member

wilkinsona commented Aug 29, 2023

I don't think it would add too much confusion either.

In addition to a "standard" Java class where a field could be annotated, we'd also need to consider things like records and Kotlin data classes so the implementation may not be completely straightforward.

@mhalbritter
Copy link
Contributor

I like it, too.

@wilkinsona wilkinsona added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Aug 30, 2023
@wilkinsona wilkinsona added this to the 3.x milestone Aug 30, 2023
@SledgeHammer01
Copy link

@quaff another example for needing this ... we have a property that we map to spring.application.name because that's a Spring requirement, but within the context of our code, it doesn't make sense to call it 'name'. Having tons of mappings in the config files will be a maintenance nightmare. It's something similar to @JsonProperty and @Column, so multiple examples of this already being done in Spring.

@BenchmarkingBuffalo
Copy link
Contributor

Hi,
I am currently working on a solution for this and I would like to reuse the @Name Annotation, which has already been used in #22492.
It seems to be working with Java and Kotlin as far as my tests go and I can create a PR, which makes a distinction in the JavaBeanBinder.
Is this an acceptable way to go?

@wilkinsona
Copy link
Member

Thanks for looking at this. We're not sure exactly how this should be implemented. Above, we'd discussed introducing a new annotation. Perhaps @Name could be reused if we expanded its targets. We'll have to do some design work on this one before we can proceed. Unfortunately, that may take a while as we have a lot of other things going on at the moment and are a bit short on bandwidth.

@wilkinsona wilkinsona added the status: pending-design-work Needs design work before any code can be developed label Feb 8, 2024
BenchmarkingBuffalo added a commit to BenchmarkingBuffalo/spring-boot that referenced this issue Feb 8, 2024
… name

Makes the Name annotation applicable on fields and takes this name when looking up the corresponding property.

See spring-projectsgh-37105
@BenchmarkingBuffalo
Copy link
Contributor

Hi @wilkinsona,
sorry, I saw your comment too late and had already prepared an idea.
Just as suggestion, this might be one possible way to go.
No need to rush things.

@philwebb
Copy link
Member

From a brief review of #39452 I think the PR might be a good starting point. We'll probably need updates to the configuration properties annotation processor to generate the correct metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

8 participants