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

Issue 36: Collection handling and deserializers for JSON-HAL #47

Closed
wants to merge 13 commits into from

Conversation

Laures
Copy link
Contributor

@Laures Laures commented Feb 14, 2013

According to the spec and the mailing list, collections of resources are supposed to be serialized in the "_embedded" property with relations.

This pull request does the following:

  • serialize instances of Resources in this fashion using the "content" relation. Currently there is no specific logic to "find" a specific relation.
  • add deserializers for JSON-HAL for ResourceSupport and Resources
  • add serialization and deserialization tests

A piece of Information about the deserializers: currently the only way for a deserializer to figure out the content type of a Resources instance is by evaluating the Jackson BeanProperty. This type resolution is not perfect as it does not handle other available sources of Type information (Jackson annotations on interfaces for example)
See: FasterXML/jackson-databind#165

Should fix #36

}

// save the relation in case the link does not contain it
relation = jp.getText();
Copy link
Member

Choose a reason for hiding this comment

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

What's the effect here? You store the value but then don't use it going forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in json hal the relation does not have to be in the link so the original idea was to set it on the read link.
but currently Link is basicly imutable. i wanted to open an issue to discuss that. thanks for reminding me.

@odrotbohm
Copy link
Member

Immutable types are a virtue, not a bug :). We can get that sorted out, I think. You could simply create a new Link instance. Regarding the issue of resolving the relation attribute on serialization. What if we had a Spring bean available exposing the following interface:

interface RelProvider {

  String getSingleResourceRel(Class<?> type);

  String getCollectionResourceRel(Class<?> type);
}

If the serializer implementations are injection aware (accepting such a RelProvider) you could lookup the rel to be used based on the property type and only fall back to content in undefined cases. In Spring Data REST we already maintain configurable relation type attributes through @RestResource which could be exposed this way, and even for plain Spring Hateoas we have @ExposesResourceFor to be used on controller methods.

I've quickly tried to adapt your code accordingly but it seems that by default the (De)Serializers are expected to have a no-arg constructor as Jackson instantiates them by reflection which get's in the way here. There seem to be options to work around that (see this blog post for example) but that seems to require the Module implementation to be rather set up in setupModule(SetupContext context) instead of by using mixins.

So what if we were be able to provide a RelProvider to the module constructors? Would you mind giving the idea a spin to setup the module using the (de)serializer factory APIs handing the RelProvider forward? I gave it a quick spin but got lost in Jackson to some degree and you seem to be into it already.

@Laures
Copy link
Contributor Author

Laures commented Feb 14, 2013

unfortunately getting spring managed beans (or any pre created instance) into jackson is a pain. I'll try to go the way with a custom handler instantiator (it works, but is a bit buggy right now). As fallback I might use a class level annotation. something like @HateoasRelation

@odrotbohm
Copy link
Member

I don't think we need to rely on direct Spring DI into the (de)serializers. Assume you get the RelProvider handed as constructor argument on the Module implementation. The thing I got stuck with was that as the (de)serializer implementations need to have no arg constructors there's no way to pass the RelProvider from the module to the (de)serializer (at least with the current implementation). If you could tweak it to achive that, we're all set.

I've started work on a general @EnableHypermediaSupport annotation (see #50) to tweak the ObjectMapper setup by registering the module manually. So we should be set in this regard, I think.

@odrotbohm
Copy link
Member

An annotation based RelProvider reading the information from the entities is not a bad idea I think. It's just that I guess that most people don't want to annotate their domain types with web related stuff in the first place. Plus, we're still stuck with the problem that there's no real way to call the setter you exposed on the serializer, as Jackson is creating instances for this class over and over again and there's no way to massage the serializer after that has happened, right?

@Laures
Copy link
Contributor Author

Laures commented Feb 15, 2013

as Jackson is creating instances for this class over and over again and there's no way to massage the serializer after that has happened, right?

not totally. Jackson checks if a HandlerInstanciator is configured for the object mapper. If it is the instanciator is asked to supply an instance for the (de)serializer class (same happens for a lot of other handler classes) if the instanciator returns null an instance is created by using the default constructor. By default there is no instanciator configured.

The blog post you mentioned created a handlerinstantiator that tries to look up instances from the application context.

I would suggest two HandlerInstantiators: one where you can simply configure the resolver and that returns a configured serializer when required and a second one that tries to look up everything from the application context.

@odrotbohm
Copy link
Member

Feel free to go ahead. I think you're more into Jackson than I am. We can discuss stuff then. Just wanted to make sure we think (and implement) in the same direction ;).

configuration possibility by using handler instantiators.
@Laures
Copy link
Contributor Author

Laures commented Feb 15, 2013

added relation resolution on a "by object" base. resolvers have to be configured on the handler instatiators (nested inside the module). The module can't do this as it has no access to the real objectmapper.
please take a look at the handler instantiators inside the modules. maybe i was a bit to generic^^

greater reusability to for example annotate methods or properties
@odrotbohm
Copy link
Member

I've tried to rebase the changes onto current master but failed unfotunately. I got an initial set of your work working again in the hal-#47 branch. Any chance you can get your latest work rebased on that one?

In the worst case, simply remove the HAL modules in the current codebase, take f5a6441 and build your latest version of that on top of that. I'll have a look at that then…

@Laures
Copy link
Contributor Author

Laures commented Apr 15, 2013

created a new pull request based on your hal-#47 branch.

#60

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 added a commit that referenced this pull request Apr 15, 2013
odrotbohm added a commit that referenced this pull request Apr 15, 2013
@Laures Laures closed this Apr 15, 2013
odrotbohm added a commit that referenced this pull request Apr 29, 2013
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
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.

Hal support is incomplete
2 participants