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

Consider @JsonProperty-defined property name for affordance metadata #1563

Closed
talentedasian opened this issue Jun 29, 2021 · 3 comments
Closed
Assignees
Labels
in: core Core parts of the project in: mediatypes Media type related functionality type: bug
Milestone

Comments

@talentedasian
Copy link

Make the default name or value of the field name of the DefaultMetadataProperty to the value that is present in the @JsonProperty annotation else, use the default field name.

@talentedasian talentedasian changed the title Default the name of a DefaultMetadataProperty to the value of a @JsonProperty if an annotation is present else, use the field name. Default the name of a the name field in DefaultMetadataProperty to the value of a @JsonProperty if an annotation is present else, use the field name. Jun 29, 2021
@talentedasian talentedasian changed the title Default the name of a the name field in DefaultMetadataProperty to the value of a @JsonProperty if an annotation is present else, use the field name. Default the name of a the "name" field in DefaultMetadataProperty to the value of a @JsonProperty if an annotation is present else, use the field name. Jun 29, 2021
@odrotbohm
Copy link
Member

Would you mind taking a step back and describing the actual problem to be solved?

@talentedasian
Copy link
Author

talentedasian commented Jun 29, 2021

image
The image above is the desired output. Even though the field is named politicalParty, a @JsonProperty annotation is present with the value of political_party. This should be the default behavior for names of Hal+Form _templates property name value(_templates.default.properties[0].name) because the @JsonProperty values are the one that are expected in the @RequestBody.

For example, I have a DTO that is used in a @RequestBody:

public class AddPoliticianDTORequest {

        @JsonProperty("first_name")
	String firstName;

        @JsonProperty("last_name")	
	String lastName;

Spring MVC expects the body of the POST method to have json values of first_name and last_name. If I were to make an affordance for this in a Hal+Form mediatype, the resulting data would be:

"templates":{
   "default":{
      "method":"post",
      "properties":[
         {
            "name":"firstName",
             .......
         },
        {
            "name":"lastName",
             .......
         }
      ],
      "target":"http://localhost:8080/api/politician/politicians"
   }
}

The response would ultimately break your clients that use your api since you expect first_name and last_name instead of "firstName" and "lastName" all because of the @JsonProperty. The name of the property values should always be the value of a @JsonProperty annotation when it is present and if not, use the default field name.

@odrotbohm
Copy link
Member

Awesome, thanks for the additional context.

@odrotbohm odrotbohm changed the title Default the name of a the "name" field in DefaultMetadataProperty to the value of a @JsonProperty if an annotation is present else, use the field name. Consider @JsonProperty-defined property name for affordance metadata Jun 29, 2021
@odrotbohm odrotbohm added in: core Core parts of the project in: mediatypes Media type related functionality type: bug and removed process: waiting for feedback labels Jun 29, 2021
odrotbohm added a commit that referenced this issue Jun 29, 2021
We now consider Jackson's @JsonProperty annotation to rename PropertyMetadata instances accordingly.

Original pull request: #1594.
odrotbohm added a commit that referenced this issue Jun 29, 2021
This method is currently used in test cases only and the just introduced ability to rename property makes the method not reliably usable as the way it is named doesn't convey which name is to be used. It currently is the potentially renamed value. Moving to the actual property name as input creates stronger boundaries to assume a type backing the metadata, which we want to avoid.
@odrotbohm odrotbohm added this to the 1.4 M1 milestone Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Core parts of the project in: mediatypes Media type related functionality type: bug
Projects
None yet
Development

No branches or pull requests

2 participants