Skip to content

HttpMessageConverters not prepended with HAL Message Converter #134

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

Closed
SirMaster opened this issue Jan 8, 2014 · 28 comments
Closed

HttpMessageConverters not prepended with HAL Message Converter #134

SirMaster opened this issue Jan 8, 2014 · 28 comments

Comments

@SirMaster
Copy link

Hi,

On the startup of our application Spring instantiates certain processors like HttpEntityMethodProcessor and RequestResponseBodyMethodProcessor. These will be created with a reference to a list of HttpMessageConverters.

The list instance is set in the abstract class AbstractMessageConverterMethodArgumentResolver#messageConverters. But because some of the processors get created before HypermediaSupportBeanDefinitionRegistrar#registerBeanDefinitions is called it occurs that the replacing of the HttpMessageConverters will not have the proper effect.

I.e. HypermediaSupportBeanDefinitionRegistrar#registerBeanDefinitions does :

public void registerBeanDefinitions(AnnotationMetadata metadata, BeanDefinitionRegistry registry) {
    ....
    if (JACKSON2_PRESENT) {
        ...
        BeanDefinitionBuilder builder = rootBeanDefinition(Jackson2ModuleRegisteringBeanPostProcessor.class);
        ....
    }
    ....
}

Jackson2ModuleRegisteringBeanPostProcessor#postProcessAfterInitialization does

public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
    ....
    if (bean instanceof RequestMappingHandlerAdapter) {
        RequestMappingHandlerAdapter adapter = (RequestMappingHandlerAdapter) bean;
        adapter.setMessageConverters(potentiallyRegisterModule(adapter.getMessageConverters()));
    }
    ....
}

Jackson2ModuleRegisteringBeanPostProcessor#potentiallyRegisterModule does

private List<HttpMessageConverter<?>> potentiallyRegisterModule(List<HttpMessageConverter<?>> converters) {
    ....
    List<HttpMessageConverter<?>> result = new ArrayList<HttpMessageConverter<?>>(converters.size());
    result.add(halConverter);
    result.addAll(converters);
    return result;
}

The problem seems to be here in potentiallyRegisterModule, it returns a new List of HttpMessageConverts while there are still objects looking at the original list.

I tested this with :
Spring 3.2.5
Spring Hateoas 0.9.0.BUILD-SNAPSHOT

Many thx in advance for any input on this.

M.

@steventhomson
Copy link

I am experiencing this issue on Spring 4.0.1 as well. Spring Hateoas 0.9.0 RELEASE

@SirMaster
Copy link
Author

Ok great thx!

@jiwhiz
Copy link

jiwhiz commented Feb 17, 2014

Any workaround for Spring Hateoas 0.9.0.RELEASE?

@SirMaster
Copy link
Author

In short yes. What we did is the following (let me know if you need specific code details)

We have already were extending the MappingJackson2HttpMessageConverter in a class called MappingJsonJackson2HttpMessageConverter. In a @PostContruct method of that class we call the method MappingJackson2HttpMessageConverter#setObjectMapper.

We pass it a custom HalJacksonObjectMapper which looks like :

public class HalJacksonObjectMapper extends JacksonObjectMapper implements BeanFactoryAware
{
    private static final long serialVersionUID = -1120363614492817224L;

    private CurieProvider curieProvider;

    public HalJacksonObjectMapper()
    {
        registerModule(new Jackson2HalModule());
        setHandlerInstantiator(new Jackson2HalModule.HalHandlerInstantiator(new AnnotationRelProvider(), curieProvider));
    }

    @Override
    public void setBeanFactory(BeanFactory beanFactory) throws BeansException
    {
        this.curieProvider = getCurieProvider(beanFactory);
    }

    private static CurieProvider getCurieProvider(BeanFactory factory)
    {
        try
        {
            return factory.getBean(CurieProvider.class);
        }
        catch (NoSuchBeanDefinitionException e)
        {
            return null;
        }
    }

Hope that helps.

@jiwhiz
Copy link

jiwhiz commented Mar 1, 2014

Thank you @SirMaster . So do I just use the MappingJsonJackson2HttpMessageConverter inside Jackson2ModuleRegisteringBeanPostProcessor#potentiallyRegisterModule? What else do I need to change?

private List<HttpMessageConverter<?>> potentiallyRegisterModule(List<HttpMessageConverter<?>> converters) {
    ...
    MappingJackson2HttpMessageConverter halConverter = new   MappingJsonJackson2HttpMessageConverter();
    ...

Could you post the complete MappingJsonJackson2HttpMessageConverter code?

Thanks.

@jiwhiz
Copy link

jiwhiz commented Mar 1, 2014

I just directly updated Jackson2ModuleRegisteringBeanPostProcessor#potentiallyRegisterModule, and it can return HAL now. Not sure if it is the right fix:

        private List<HttpMessageConverter<?>> potentiallyRegisterModule(List<HttpMessageConverter<?>> converters) {

            for (HttpMessageConverter<?> converter : converters) {
                if (converter instanceof MappingJackson2HttpMessageConverter) {
                    MappingJackson2HttpMessageConverter halConverterCandidate = (MappingJackson2HttpMessageConverter) converter;
                    ObjectMapper objectMapper = halConverterCandidate.getObjectMapper();
                    if (!Jackson2HalModule.isAlreadyRegisteredIn(objectMapper)) {
                        registerHalModule(halConverterCandidate);
                    }
                    return converters;
                }
            }

            MappingJackson2HttpMessageConverter halConverter = new MappingJackson2HttpMessageConverter();
            registerHalModule(halConverter);

            List<HttpMessageConverter<?>> result = new ArrayList<HttpMessageConverter<?>>(converters.size());
            result.add(halConverter);
            result.addAll(converters);
            return result;
        }

        private void registerHalModule(MappingJackson2HttpMessageConverter halConverter) {
            halObjectMapper.registerModule(new Jackson2HalModule());
            halObjectMapper.setHandlerInstantiator(new Jackson2HalModule.HalHandlerInstantiator(relProvider, curieProvider));
            halConverter.setSupportedMediaTypes(Arrays.asList(HAL_JSON));
            halConverter.setObjectMapper(halObjectMapper);
        }

@SirMaster
Copy link
Author

@jiwhiz, I think that could work yes. We did it a little different, but from what I am seeing of your code I think in essence it's the same, see :

MappingJsonJackson2HttpMessageConverter.java

@Component
public class MappingJsonJackson2HttpMessageConverter extends MappingJackson2HttpMessageConverter
{
    ....
    @PostConstruct
    private void postConstruct()
    {
        setObjectMapper(new HalJacksonObjectMapper());
    }
    ....
}

And in our config class we tell it to add it to the HttpMessageConverters, see ApplicationConfig.java

@Configuration
@EnableWebMvc
@EnableHypermediaSupport(type = HAL)
@ComponentScan(basePackageClasses = {
        MappingJsonJackson2HttpMessageConverter.class
 })
public class ApplicationConfig extends WebMvcConfigurerAdapter
{
    @Autowired
    private MappingJsonJackson2HttpMessageConverter mappingJackson2HttpMessageConverter;

    @Override
    public void configureMessageConverters(List<HttpMessageConverter<?>> converters)
    {
        converters.add(mappingJackson2HttpMessageConverter);
    }
}

@jiwhiz
Copy link

jiwhiz commented Mar 2, 2014

I missed the configuration change, no wonder couldn't get your workaround work. Since I don't need a customized MappingJackson2HttpMessageConverter, my hack will be fine to me. Thanks a lot!

@SirMaster
Copy link
Author

Sorry about that, I should have just posted the whole thing and save you some time.

@jiwhiz
Copy link

jiwhiz commented Mar 2, 2014

No problem at all. I was so frustrated when I couldn't get HAL message returned, until saw your issue report. You pointed out where the problem is, and that saved me lots of time.

@jiwhiz
Copy link

jiwhiz commented Mar 3, 2014

Another issue I found is to add HAL_JSON to supportedMediaTypes, not to set it as the supportedMediaTypes. Otherwise I got "415 (Unsupported Media Type)".

The updated code is

        private void registerHalModule(MappingJackson2HttpMessageConverter halConverter) {
            halObjectMapper.registerModule(new Jackson2HalModule());
            halObjectMapper.setHandlerInstantiator(new Jackson2HalModule.HalHandlerInstantiator(relProvider, curieProvider));
            List<MediaType> supportedMediaTypes = new ArrayList<MediaType>(halConverter.getSupportedMediaTypes());
            supportedMediaTypes.add(HAL_JSON);
            halConverter.setSupportedMediaTypes(supportedMediaTypes);
            halConverter.setObjectMapper(halObjectMapper);
        }

@odrotbohm
Copy link
Member

Is anyone able to come up with a test case that shows, this issue really exists? I don't necessarily doubt it, but I've seen the current code working flawlessly in standard Spring MVC applications. We also use it heavily in the Spring Data REST test code. So I am interested in finding out what is actually causing the erroneous behavior.

I can't see anything obviously wrong here, as we're simply re-setting the messageConverters property of RequestMappingHandlerAdapter. Maybe @rstoyanchev can share his view as well?

@jiwhiz
Copy link

jiwhiz commented Mar 14, 2014

Hi Oliver, I created a very simple application to demonstrate the issue, see my CIExample project in GitHub https://github.com/jiwhiz/CIExample. This example was built and deployed by CloudBee and you can access it at http://ciexample.jiwhiz.cloudbees.net/

The backend will expose a simple RESTful API "http://ciexample.jiwhiz.cloudbees.net/api/user", if you use any rest client, like Advanced Rest Client from Chrome shop, you can see the return json object is like:

{"name":"Yuan","email":"[email protected]","links":[{"rel":"self","href":"http://ciexample.jiwhiz.cloudbees.net/api/user"}]}

Definitely it is not HAL. If I change the code as my previous comments, it works, the output json is:

{"name":"Yuan","email":"[email protected]","_links":{"self":{"href":"http://localhost:8080/example/api/user"}}}

Please see the config file and let me know what the problem is:

package com.jiwhiz.example.config;

import java.util.Properties;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.hateoas.MediaTypes;
import org.springframework.hateoas.config.EnableHypermediaSupport;
import org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType;
import org.springframework.web.servlet.config.annotation.ContentNegotiationConfigurer;
import org.springframework.web.servlet.config.annotation.DefaultServletHandlerConfigurer;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
import org.springframework.web.servlet.handler.SimpleMappingExceptionResolver;

@Configuration
@EnableWebMvc
@EnableHypermediaSupport(type = { HypermediaType.HAL })
@ComponentScan(basePackages = {
        "com.jiwhiz.example.rest"
})
public class WebConfig extends WebMvcConfigurerAdapter {
    public void addResourceHandlers(ResourceHandlerRegistry registry) {
        registry.addResourceHandler("/resources/**").addResourceLocations("/resources/");
    }

    @Override
    public void configureDefaultServletHandling(DefaultServletHandlerConfigurer configurer) {
        configurer.enable();
    }

    @Bean
    public SimpleMappingExceptionResolver exceptionResolver() {
        SimpleMappingExceptionResolver exceptionResolver = new SimpleMappingExceptionResolver();

        Properties exceptionMappings = new Properties();

        exceptionMappings.put("java.lang.Exception", "error/error");
        exceptionMappings.put("java.lang.RuntimeException", "error/error");

        exceptionResolver.setExceptionMappings(exceptionMappings);

        Properties statusCodes = new Properties();

        statusCodes.put("error/404", "404");
        statusCodes.put("error/error", "500");

        exceptionResolver.setStatusCodes(statusCodes);

        return exceptionResolver;
    }

    @Override
    public void configureContentNegotiation(ContentNegotiationConfigurer c) {
        c.defaultContentType(MediaTypes.HAL_JSON);
    }

}

Thanks a lot!

Yuan

@stefanbickel
Copy link

Hi @olivergierke,

i use spring-hateoas without Spring Data REST and run into the same problem.

spring-hateoas with @EnableHypermediaSupport re-setting the messageConverters property after the RequestMappingHandlerAdapter.afterPropertiesSet() was called.

After @EnableHypermediaSupport configuration applied, the RequestMappingHandlerAdapter instance holds the modified messageConverters but the earlier created instances of HttpEntityMethodProcessor (created after RMHA.afterPropertiesSet() was called) still contains the old list of messageConverters without the new HAL Jackson Converter.

@odrotbohm
Copy link
Member

That's very helpful observation, @stefanbickel! Seems like we need to re-invoke afterPropertiesSet() on the RequestMappingHandlerAdapter. Not sure I'll get to it today. So if you want to give it a quick spin, simply copy HypermediaSupportBeanDefinitionRegistrar into a package org.springframework.hateoas.config in your local project and add the additional call to the block working with the RequestMappingHandlerAdapter in Jackson2ModuleRegisteringBeanPostProcessor.postProcessAfterInitialization(…).

odrotbohm added a commit that referenced this issue Mar 19, 2014
The Jackson(2)ModuleRegisteringBeanPostProcessor now uses postProcess*Before*Initialization to make sure, afterPropertiesSet() has not been called on the target components which might have propagated the original HttpMessageConverters into other internal components.

With this fix, adding the HttpMessageConverter for HAL is added before the call and thus will be propagated to downstream components.
@odrotbohm
Copy link
Member

Would you guys mind giving the current snapshots a spin? We're simply using the correct callback method (BeanPostProcessor.postProcess*Before*Initialization(…)) now.

@jiwhiz
Copy link

jiwhiz commented Mar 19, 2014

@olivergierke, yes, this issue was fixed, and I can get HAL messages now. Excellent!

@steventhomson
Copy link

@olivergierke nice work! It seems that while the HAL module is doing its job, global message conversion settings are lost. In my project we extend WebMvcConfigurerAdapter and override configureMessageConverters to allow mapping of some complex types, use special date formatting and to exclude nulls. Is this to be expected or should the mappers be blended? FYI, if I turn off HAL support, all my custom mapper settings work as expected with the standard hateoas link support.

@odrotbohm
Copy link
Member

What exactly does "are lost" mean? What we effectively do is registering an additional HttpMessageConverter using an independent ObjectMapper bean that gets our HAL module configured. In case you want to further customize that, get hold of the bean named _halObjectMapper and perform additional customization.

If you happen to work with Spring Boot, all (global) beans of type Module (Jackson 2) available in the ApplicationContext will be auto-registered to all (global) beans of type ObjectMapper. As we register the mapper used for the HAL converter globally, it's sufficient to simply declare a Jackson 2 module as bean and boot will take care of the registration.

Does that help?

@steventhomson
Copy link

@olivergierke
I would rather not customize the _halObjectMapper if possible and we are not using Spring Boot.

Leaving out all other configurations, this is what we are doing:

@Configuration
@EnableWebMvc
@EnableHypermediaSupport(type = HypermediaType.HAL)
@EnableSpringDataWebSupport
@ComponentScan(basePackages = { ... })
@PropertySource({ ... })
public class MvcConfig extends WebMvcConfigurerAdapter {
    @Override
    public void configureMessageConverters(List<HttpMessageConverter<?>> converters) {
        final MappingJackson2HttpMessageConverter converter = new MappingJackson2HttpMessageConverter();
        converter.setObjectMapper(MvcObjectMapper.getInstance().getObjectMapper());
        converters.add(converter);
    }
}

So, we also want to register an additional HttpMessageConverter with an independent ObjectMapper. Shouldn't they both be applied during serialization?

@steventhomson
Copy link

@olivergierke

So i tried your solution and customized _halObjectMapper just to see if it would work:

... added to previous comment ...

/**
 * @see org.springframework.hateoas.config.HypermediaSupportBeanDefinitionRegistrar
 */
private static final String HAL_OBJECT_MAPPER_BEAN_NAME = "_halObjectMapper";

@Resource(name = HAL_OBJECT_MAPPER_BEAN_NAME)
public void customizeHalObjectMapper(ObjectMapper mapper) {

    mapper.registerModule(new MyCustomModule1());
    mapper.registerModule(new MyCustomModule2());
    mapper.registerModule(new MyCustomModule3());

    mapper.setDateFormat(MY_CUSTOM_DATE_FORMAT);
    mapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false);
    mapper.setSerializationInclusion(Include.NON_EMPTY);

}

It does work for Response serialization as expected.

I still have to override configureMessageConverters to have custom Request deserialization (necessary since application/hal+json is the required content type for the halObjectMapper, but our RESTful service is happy with just application/json for incoming Requests). I don't want to force users to change ContentType, if possible, just so I can use the HAL mapper by itself. Having this hybrid is OK for now since the application works as expected, but we have to maintain two mappers and hope _halObjectMapper never gets renamed. :)

Is it the expectation that both Requests and Responses should have a content type of application/hal+json in order for everything to work properly?

Am I missing something? What would you suggest?

@drdamour
Copy link

drdamour commented May 6, 2014

this is fixed in 0.10, correct?

@meyertee
Copy link

Not sure if it's the same issue, but my HAL output is also broken in 0.16.0 and I narrowed it down to the @EnableWebMvc annotation being present. It only happens on collection resources.

I've added a commit to the spring-guides/gs-rest-hateoas sample project to demonstrate the problem: meyertee/gs-rest-hateoas@16dfe8d

The single resource output is fine:

{
    "content":"Hello, World!",
    "_links":{"self":{"href":"http://localhost:8080/greeting?name=World"}}
}

But the collection output is wrong:

[{
    "content":"Hello, World!",
    "links":[{"rel":"self","href":"http://localhost:8080/greeting?name=World"}]
}]

Adding produces = MediaType.APPLICATION_JSON_VALUE to the @RequestMapping annotation however also breaks the single resource.. see this commit: meyertee/gs-rest-hateoas@60e333e
Resulting in this output:

{
    "content":"Hello, World!",
    "links":[{"rel":"self","href":"http://localhost:8080/greeting?name=World"}]
}

@odrotbohm
Copy link
Member

That's expected behavior. According to the HAL specification the root document needs to be a Resource Object, which it isn't in case you're returning a List to be rendered. The latter observation is expected as well as application/json != application/hal+json. There's no such thing as media type inheritance.

The behavior is pretty straight-forward. You need to return something extending ResourceSupport and the client has to send a Accept header matching application/hal+json. If you restrict your controller method to produce a different media type, you get exactly that media type.

@meyertee
Copy link

Thanks, that's very helpful information, changing the media type to application/hal+json works on the single resource.

Regarding the collection resources I'm a bit confused though - in all the samples and in ResourceAssemblerSupport.toResources(Iterable<? extends T> entities) Lists are used. If the List is expected to be ResourceSupport, what is the recommended approach/return-type for collections?
The items in the list are ResourceSupport instances, I expected that to be enough.
And why is the behaviour changing when @EnableWebMvc is present or not?

@odrotbohm
Copy link
Member

The easiest way is to return an instances of Resources as it takes a list of objects to embed. There shouldn't be a difference in using @EnableWebMvc, it's totally not needed when using Spring Boot. If you see differences in the behavior a reproducing example regarding that would be great :).

@meyertee
Copy link

Simplest way to reproduce the behaviour would be to check out my fork of the spring-guides/gs-rest-hateoas project (it's a very simple hello world app) at https://github.com/meyertee/gs-rest-hateoas. Files are under the complete directory:

git clone https://github.com/meyertee/gs-rest-hateoas.git
cd gs-rest-hateoas/complete/
./gradlew bootRun

Access http://localhost:8080/greetings to see the output.
Then comment out @EnableWebMvc in Application.java and re-run.
Controller code is in GreetingController.java

Thanks again, maybe it'd be good to mention the recommended usage of Resources in the docs somewhere.

@gregturn
Copy link
Contributor

gregturn commented Mar 5, 2019

See #833 for how to create a custom media type.

See #416 and #833 such that RepresentationModelAssembler.toCollectionModel() (formerly ResourceSupportAssembler.toResources()) now returns a Spring HATEOAS collection-type instead of a Java Collection, and will thus render properly from a Spring MVC/WebFlux endpoint.

@gregturn gregturn closed this as completed Mar 5, 2019
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

No branches or pull requests

8 participants