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

"Unknown property" marker is created when there is a setter but no getter #1180

Open
eric-creekside opened this issue Feb 2, 2024 · 15 comments
Assignees
Labels
for: eclipse something that is specific for Eclipse for: vscode something that is specific for VSCode theme: property-editing-support type: bug
Milestone

Comments

@eric-creekside
Copy link

STS for Eclipse Version: 4.21.0.RELEASE

In application config YAML files, STS will mark a property as unknown (error or warning depending on how I have STS configured) even if there is a setter for that property in the corresponding Java @ConfigurationProperties class. If I have both getter and setter, the error/warning goes away.

For example, given this class:

@ConfigurationProperties(prefix = "foo")
public class MyProperties {
	private String envName;
	public void setEnvName(String name) { };
}

and this YAML:

foo:
  envName: bar

STS puts an error/warning marker in the YAML saying envName is an unknown property.

If I add this getter to MyProperties:

	public String getEnvName() { return ""; }

Then the YAML error/warning goes away.

It seems that a property with only a setter shouldn't trigger an error/warning, since all that matters to Spring is that it can know where to put the value from the YAML.
Before anyone starts quoting the Java Beans spec to me, I know that a property with no getter is not a strict Java Beans property. Practically speaking, however, Spring doesn't care; it still correctly sets the YAML value into the config object. So STS is not agreeing with Spring's own runtime behavior.

@BoykoAlex
Copy link
Contributor

BoykoAlex commented Feb 5, 2024

@eric-creekside I have looked at the <output folder>/classes/META-INF/spring-configuration-metadata.json file (target is the <output folder> for maven - this is the file generated by spring boot via the spring-boot-configuration-processor. This is the metadata that IDE uses to provide assistance with Boot properties. This file is generated by the Spring Boot.

If getter or setter is missing for the property then the JSON file misses the metadata for the property. This is because both the setter and the getter are required to be present for a property. Thus, it is kind of enforced this way via properties metadata. I quickly asked Boot team and the reasoning is if everything that looks like a setter would be accepted without having a getter then too many properties would be exposed.

Anyway, STS works as designed with respect to Boot properties. The Spring Boot requires both getter and setter.
(Also see docs: https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.external-config.typesafe-configuration-properties.java-bean-binding)

@eric-creekside
Copy link
Author

eric-creekside commented Feb 5, 2024

@BoykoAlex Like I said, I know what the documents and Java Beans say, but Spring's own behavior does not agree. I have several @ConfigurationProperties classes with properties that have no direct getters, and the values do get populated from my YAML config files. A simple example app can prove this; if I construct a simple project to demonstrate it, will you consider re-opening this issue?
I'm on Spring Boot 2.5.x, if that mattes.

@BoykoAlex
Copy link
Contributor

@eric-creekside The metadata file <output folder>/classes/META-INF/spring-configuration-metadata.json is generated by Spring Boot - we have no control over the contents of this file. However, we use the content of the file for the tooling to report errors and provide completion proposals. Folks from Spring Boot team told me that getter and setter are both required by design and hence metadata generation won't be changed.
I'd bring this up to Spring Boot team as an issue perhaps (via https://github.com/spring-projects/spring-boot). We cannot change this behaviour on the tooling side unless we generate the metadata ourselves which we don't plan on doing.

@martinlippert martinlippert closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2024
@martinlippert martinlippert removed this from the 4.21.1.RELEASE milestone Feb 6, 2024
@martinlippert
Copy link
Member

@eric-creekside I closed this as not planned since we should not workaround this in the tooling itself (as Alex mentioned). Please raise this issue with the boot project itself, so that we can get a clearer picture about the relationship how the framework behaves (as you described, accepting the setter-only case just fine) and the way the metadata is generated from boot for this.

If the boot project decides to change the metadata for this, the tooling will adopt this immediately (even without a new release from our side), so you would get this behavior right away... :-)

Side note: you mentioned that you are on Spring Boot 2.5.x, which is way out of its open-source support range, so changes to boot will not make it into that version of Spring Boot anymore. An upgrade to Spring Boot 3.x would be a good choice here. Thought I should mention that here since you are referring to Boot 2.5.x.

@BoykoAlex
Copy link
Contributor

BoykoAlex commented Feb 6, 2024

The warning under the property in the metadata json has a quickfix which adds this property to the custom metadata JSON file. I think the quickfix just adds the skeleton object thus you'd have to "fill out the blanks" in the json file. However, once the property in the "custom" metadata json the warning goes away and you can look at the property as other native boot properties in the IDE. Perhaps this could serve as a workaround.

@eric-creekside
Copy link
Author

Side note: you mentioned that you are on Spring Boot 2.5.x, which is way out of its open-source support range, so changes to boot will not make it into that version of Spring Boot anymore. An upgrade to Spring Boot 3.x would be a good choice here. Thought I should mention that here since you are referring to Boot 2.5.x.

Yes, I know. Updating Spring (and other critical dependencies) is on our roadmap but that kind of tech debt is sometimes hard to get approved.

@eric-creekside
Copy link
Author

eric-creekside commented Feb 6, 2024

The warning under the property in the metadata json has a quickfix which adds this property to the custom metadata JSON file. I think the quickfix just adds the skeleton object thus you'd have to "fill out the blanks" in the json file. However, once the property in the "custom" metadata json the warning goes away and you can look at the property as other native boot properties in the IDE. Perhaps this could serve as a workaround.

I'll have to see; I have already generated the metadata for our custom config POJOs, but as you say it's minimal; works fine for most properties, just not these "special" ones with no direct getter*.

*In case anyone is wondering about the practical use case, the property values are actually used, but for derived getters, not directly as-is.

@martinlippert
Copy link
Member

So we have two different action items here:

  • raise the issue and discuss directly with the Spring Boot team (I guess you will go ahead with this, right? @eric-creekside)
  • make sure users can use custom self-written metadata descriptions to override or enhance the generated metadata, so that users can educate the tooling to accept specific properties. Some of this is already in place, but we need to make sure that this is a smooth and straight forward experience.

I will re-open this issue to track work on the second part here.

@martinlippert
Copy link
Member

(side note: one complication here might be the m2e issue that causes some resources not being copied into the target folder every time, just to keep that in mind when making sure the second point mentioned above works smoothly, assuming the m2e issue is fixed by then)

@martinlippert
Copy link
Member

Side note: you mentioned that you are on Spring Boot 2.5.x, which is way out of its open-source support range, so changes to boot will not make it into that version of Spring Boot anymore. An upgrade to Spring Boot 3.x would be a good choice here. Thought I should mention that here since you are referring to Boot 2.5.x.

Yes, I know. Updating Spring (and other critical dependencies) is on our roadmap but that kind of tech debt is sometimes hard to get approved.

@eric-creekside Are you aware of the support that we have inside the tools to help users migrate to newer versions of Spring Boot? It does not do everything completely automatically, but helps a lot getting the most tedious tasks done.

@eric-creekside
Copy link
Author

  • make sure users can use custom self-written metadata descriptions to override or enhance the generated metadata, so that users can educate the tooling to accept specific properties. Some of this is already in place, but we need to make sure that this is a smooth and straight forward experience.

I was able to manually edit the additional-spring-configuration-metadata.json file that I generated with the QuickFix (from the root of my type-safe config POJO. The original generated file was like this:

{"properties": [
  {
    "name": "foo",
    "type": "com.foo.MyAppProperties",
    "description": "Properties to configure the application"
  }
]}

That satisfies STS for most properties in MyAppProperties, but not the ones without getters.

I edited it to add specifically the properties that don't have getters, like this:

{"properties": [
  {
    "name": "foo",
    "type": "com.foo.MyAppProperties",
    "description": "Properties to configure the application"
  },
  {
    "name": "foo.envName",
    "type": "java.lang.String",
    "description": "Custom property that has no direct getter."
  }
 ]}

With that, STS no longer generates a warning for the property envName that has no getter.

@eric-creekside
Copy link
Author

(side note: one complication here might be the m2e issue that causes some resources not being copied into the target folder every time, just to keep that in mind when making sure the second point mentioned above works smoothly, assuming the m2e issue is fixed by then)

I'm using Gradle (and Buildship), not Maven.

@eric-creekside
Copy link
Author

Yes, I know. Updating Spring (and other critical dependencies) is on our roadmap but that kind of tech debt is sometimes hard to get approved.

@eric-creekside Are you aware of the support that we have inside the tools to help users migrate to newer versions of Spring Boot? It does not do everything completely automatically, but helps a lot getting the most tedious tasks done.

I was not aware of that; I'll definitely check it out when we get to the work of upgrading.

@eric-creekside
Copy link
Author

eric-creekside commented Feb 9, 2024

So we have two different action items here:

  • raise the issue and discuss directly with the Spring Boot team (I guess you will go ahead with this, right? @eric-creekside)

I'm not sure what issue they need to be made aware of; is it

  • Spring Boot populates properties even when they have no getter?
    or
  • Spring is not generating the metadata correctly for POJOs that have a setter but no getter?

If you're suggesting the former. I don't actually want that "fixed" because it's a very useful behavior. :-)
If the latter, I'm not exactly sure what/when metadata is being generated, so unsure how to report the issue.

@BoykoAlex
Copy link
Contributor

@eric-creekside I'd re-confirm with the boot team at least that no getter properties will be supported going forward to the same extent as today. If they "shut this door" all of a sudden it'd a very unpleasant surprise :-\
They were very clear with me about not generating the metadata for these properties as they don't want us to promote defining properties in a non-recommended way. They might advice you perhaps how make your implicit getters more explicit for spring though... Some extra annotations for getters etc. Thus, I'd start some sort of discussion with them... share code snippets ask for an advise... I'd give this a try.

@BoykoAlex BoykoAlex modified the milestones: 4.22.0.RELEASE, Backlog Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: eclipse something that is specific for Eclipse for: vscode something that is specific for VSCode theme: property-editing-support type: bug
Projects
None yet
Development

No branches or pull requests

3 participants