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

Add support for using Spring HATEOAS to add links in the Spring generator #1130

Merged
merged 5 commits into from
Oct 23, 2018

Conversation

mwoodland
Copy link
Contributor

Add support for Spring HATEOAS.
issue #1129

@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01)

@mwoodland
Copy link
Contributor Author

Turns out spring-boot doesn't actually run when the hateoas dependency is added, so I'll be investigating what's going wrong with that.

@wing328
Copy link
Member

wing328 commented Sep 27, 2018

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@mwoodland
Copy link
Contributor Author

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

Thanks for pointing that out Wing. I think I've fixed it by adding the second email address to my github account.

@mwoodland
Copy link
Contributor Author

Turns out spring-boot doesn't actually run when the hateoas dependency is added, so I'll be investigating what's going wrong with that.

This was an issue with my specific configuration, in general it will work, it's just I've been messing around with the ObjectMapper bean that Spring HATEOAS uses for my application.

@mwoodland
Copy link
Contributor Author

Any updates on whether this pull request will be accepted? Or if there's anything else you'd like me to do?

@wing328
Copy link
Member

wing328 commented Oct 6, 2018

@mwoodland I'll test it over the weekend and let you know if I've any feedback/question.

1. Make sure the @JsonPropertyOrder annotation is only used when the jackson library is being used since it's a part of the jackson library.
2. Make sure to include the Spring HATEOAS dependency in the pom file for the spring-cloud and spring-mvc generators when the hateoas option is enabled.
@wing328
Copy link
Member

wing328 commented Oct 8, 2018

@wing328 thanks for incorporating my feedback. If no more question/feedback from anyone, I'll merge it later today.

{{^parent}}
{{#hateoas}}
{{#jackson}}
@JsonPropertyOrder({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the use of this ?

Copy link
Contributor Author

@mwoodland mwoodland Oct 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to ensure that the links part of the JSON object is the last field in the object (it's a minor issue but the standard in the HATEOAS examples appears to be for the links to come last). E.g:

        {
            "field1": "value1",
            "field2": "value2",
            "field3": null,
            "field4": null,
            "_links": {
                "self": {
                    "href": "http://localhost:8080/api/v1/thingy-api/value1"
                }
            }
        }

Without this ordering the _links part appeared as the first item in the JSON object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that HATEOAS doesn't have such convention. If it were the case, spring-hateoas would deal with that. Can you remove this part ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to take another look at this. All the examples seem to show the links as being the last thing but in my case without adding explicit ordering they end up being the first thing. I do agree that this ordering shouldn't be needed though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an update on where I've got so far:
I feel like I must be doing something wrong because the Jackson library seems to always add the parent fields before the current fields when serializing and object that extends another class. So since the generated class extends the ResourceSupport class the links field on the ResourceSupport class will always come before the generated class.

However all of the Spring HATEOAS examples show the links as the last field in the JSON that's produced so I feel like I must be doing something differently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to add @EnableHypermediaSupport annotation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't seem to have made any difference. I'm going to try and work out what's different between my application and the example in https://spring.io/guides/gs/rest-hateoas/ which does have the links at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urgh... what's going on with the ordering is a bit difficult to explain:

All of the ordering logic exists with in the com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector class.

The example in the link above the serialized object is using the @JsonCreator annotation on its constructor, and for whatever reason the jackson library makes sure that any fields from the @JsonCreator constructor come before all the other fields. Since the links are coming from the super class rather than the constructor they come last in this case.

However... if you change the class so it doesn't use @JsonCreator and you ensure it doesn't use any @JsonProperty annotation then the links will be placed at the end because the jackson library ensure that any fields that are renamed using the @JsonProperty come last. In this case since the ResourceSupport class has an @JsonProperty annotation on the getLinks method the links will be renamed and so will come last.

The generated code uses getters with the @JsonProperty annotation on all them regardless of whether they're being renamed as opposed to using the @JsonCreator annotation. This means that all the fields including the links are being renamed and so they maintain the same order as before they were being renamed where the parent fields come first.

It's probably just coincidence that all of the examples have the links at the end and is probably because they examples are all relatively simple so don't tend to make use of the @JsonProperty annotation.

**The tldr; is that the ordering is based on the inner workings on the Jackson library. I think it doesn't really matter where the links appear but I think it probably looks a little bit neater to have them at the end.

I'm happy to remove this ordering if you insist, and happy to discuss further if you'd like.**

@wing328 wing328 modified the milestones: 3.3.1, 3.3.2 Oct 15, 2018
{{^parent}}
{{#hateoas}}
import org.springframework.hateoas.ResourceSupport;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed

@cbornet
Copy link
Member

cbornet commented Oct 15, 2018

LGTM

@wing328 wing328 merged commit 258de89 into OpenAPITools:master Oct 23, 2018
@wing328 wing328 changed the title Add support for using Spring HATEOAS to add links in the spring gener… Add support for using Spring HATEOAS to add links in the Spring generator Oct 31, 2018
@wing328
Copy link
Member

wing328 commented Oct 31, 2018

@mwoodland thanks for the PR, which is included in the v3.3.2 release: https://twitter.com/oas_generator/status/1057649626101112832

@mwoodland mwoodland deleted the spring-hateoas branch October 31, 2018 16:58
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
OpenAPITools#1130)

* Add support for using Spring HATEOAS to add links in the spring generator.

* Ensure that Spring HATEOAS links appear last in the JSON serialisation of objects.

* A couple of changes following code review:
1. Make sure the @JsonPropertyOrder annotation is only used when the jackson library is being used since it's a part of the jackson library.
2. Make sure to include the Spring HATEOAS dependency in the pom file for the spring-cloud and spring-mvc generators when the hateoas option is enabled.

* Don't order the json properties since there's no requirement for the links to be last.

* Remove unnecessary import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants