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

Hal 47: additional hal-support for spring-hateoas #60

Closed
wants to merge 4 commits into from

Conversation

Laures
Copy link
Contributor

@Laures Laures commented Apr 15, 2013

  • changed rel provider to handle objects, not classes. otherwise the wrapper classes (Resource...) could not be easyly handled
  • added new annotation based relprovider
  • changed way to configure a rel provider. serializers.addserializer did not work
  • new tests for rel provider

odrotbohm and others added 4 commits April 11, 2013 14:10
according to hal collections are supposed to be serialized as _embedded
with a topical relation (see
https://groups.google.com/forum/?fromgroups=#!topic/hal-discuss/bCzydTlHB3M
)

fixed formating

fixed contextual implementation

initial version of deserializers

final cleanup before deserializers

added new link deserializer

need to find a way to handle "optional" relation property in hal

working towards _embedded deserialization

deserializers done

some more tests and they should be ready

final cleanup and tests

fix formating aggain
- changed rel provider to handle objects, not classes. otherwise the
wrapper classes (Resource<T>...) could not be easyly handled
- added new annotation based relprovider
- changed way to configure a rel provider. serializers.addserializer did
not work

Tests
- tests for annotationrelprovider
odrotbohm added a commit that referenced this pull request Apr 15, 2013
Reverter RelProvider API to work with classes instead of objects so that it becomes general purpose again. Moved Resource extraction logic into helper class.

Polished AnnotationRelProvider (added annotation caching), added DefaultRelProvider to simply default to the simply class name and corrected ControllerRelProvider to lookup relation types from Controller classes. Added DelegatingRelProvider to delegate to RelProvider implementations in a PluginRegistry. Added BeanDefinition setup to HypermediaSupportBeanDefinitionRegistrar to setup a PluginRegistry of RelProviders and the wrapping DelegatingRelProvider to become the primary auto wiring candidate.

Made the BeanPostProcessors for Jackson ObjectMapper customization aware of the BeanFactory to lookup the DelegatingRelProvider instance to hand them into the Jackson HandlerInstantiators.

TODOs:
- More tests
- Correctly register ControllerRelProviders
- Add RelProvider implementation based on the Evo inflector to build plurals (http://www.csse.monash.edu.au/~damian/papers/HTML/Plurals.html)
@odrotbohm
Copy link
Member

I've pushed a few polishing and extending commits onto hal-#47, adding a few more RelProvider implementations as well as the necessary Spring glue to automatically wire the RelProvider into the HandlerInstantiator. I'll add a few more tests tomorrow and polish stuff.

I've removed the ability to use @HalRelation on fields and methods for now as the serializers/deserializers do now honor them on fields currently. Would be cool if you could override the plain type based rel mapping on the property. Any chance you can have a look at that? If not I'd be fine simply releasing 0.8 the way we have it right now. We could add the advanced stuff later.

@Laures
Copy link
Contributor Author

Laures commented Apr 17, 2013

could you describe what kind of behavior you mean with: "Would be cool if you could override the plain type based rel mapping on the property"? should the serializer ignore the class based annotation if the content attribute of Resources (for example) has an annotation? I don't quite understand it.

@Laures
Copy link
Contributor Author

Laures commented Apr 29, 2013

Hi oliver, given how long this pullrequest is running already i would like to do further changes to the hal (de)serialization in a new one and merge this asap. would that be ok with you?

@odrotbohm
Copy link
Member

I was just about to have a closer look at what we currently have. As mentioned before, I'd like to back the current code with a few more tests before digressing into more features. From what I tested in the Spring RESTBucks sample the HAL stuff seems to work fine for now. What exactly is it you want to add beyond that?

odrotbohm pushed a commit that referenced this pull request Apr 29, 2013
Collections are now rendered as _embedded in combination with a topic relation (https://groups.google.com/forum/?fromgroups=#!topic/hal-discuss/bCzydTlHB3M
). Adapted deserializers accordingly.
odrotbohm added a commit that referenced this pull request Apr 29, 2013
Added @relation annotation to be able to define the relations that shall be exposed. The annotation can be used on domain classes or ResourceSupport subtypes to define the relations that shall be used to build links pointing to them.

Introduced RelProvider implementations that use the plain domain class name plus an appended List by default (DefaultRelProvider, fallback) as well as an AnnotationRelProvider that will inspect @relation on the domain type. Added a DelegatingRelProvider that is based on a Spring Plugin PluginRegistry to allow adding RelProvider implementations transparently. Polished AnnotationRelProvider (added annotation caching) and corrected ControllerRelProvider to lookup relation types from Controller classes. Added BeanDefinition setup to HypermediaSupportBeanDefinitionRegistrar to setup a PluginRegistry of RelProviders and the wrapping DelegatingRelProvider to become the primary auto wiring candidate.

Moved RelProvider registration into Jackson HandlerInstantiator implementations. Made the BeanPostProcessors for Jackson ObjectMapper customization aware of the BeanFactory to lookup the DelegatingRelProvider instance to hand them into the Jackson HandlerInstantiators.

TODOs:
- More tests
- Correctly register ControllerRelProviders
odrotbohm added a commit that referenced this pull request Apr 29, 2013
@odrotbohm
Copy link
Member

I've polished and merged what we had in the feature branch into master.

@odrotbohm odrotbohm closed this Apr 29, 2013
@odrotbohm
Copy link
Member

@Laures - I was thinking about doing a 0.5 release in the next couple of days. What extensions to the HAL support were you thinking of? Anything urgent or potential candidates for refinements in upcoming versions?

@Laures
Copy link
Contributor Author

Laures commented Apr 30, 2013

@olivergierke i was thinking about you comment: : "Would be cool if you could override the plain type based rel mapping on the property". i still don't understand what you mean with that.

when i do, it falls probably into the refinement category. nothing really urgent.

@odrotbohm
Copy link
Member

I think I've been struck by the some false usage scenario I imagined, so don't worry about that one. Wouldn't mind if you could check whether the current codebase works fine for you. I played with it a little in the Spring RESTBucks implementation and it seems to work fine for me.

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

Successfully merging this pull request may close these issues.

2 participants