Skip to content

Compatibility with Spring HATEOAS 0.19.0.RELEASE #3882

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
wilkinsona opened this issue Sep 2, 2015 · 5 comments
Closed

Compatibility with Spring HATEOAS 0.19.0.RELEASE #3882

wilkinsona opened this issue Sep 2, 2015 · 5 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@wilkinsona
Copy link
Member

Investigate removing direct use of HalHandlerInstantiator in favour of customisation applied by Jackson2ModuleRegisteringBeanPostProcessor

@wilkinsona wilkinsona changed the title Compatibility with Spring HATEOAS 0.19.0.RELEASE – Investigate removing direct use of HalHandlerInstantiator in favour of customisation applied by Jackson2ModuleRegisteringBeanPostProcessor Compatibility with Spring HATEOAS 0.19.0.RELEASE Sep 2, 2015
@wilkinsona wilkinsona added this to the 1.2.6 milestone Sep 2, 2015
@wilkinsona wilkinsona self-assigned this Sep 2, 2015
gregturn added a commit that referenced this issue Sep 2, 2015
Spring HATEOAS already decorates ObjectMappers via @EnableHypermediaSupport. Remove the extra bits of configuration that are unnecessary and introduce tight coupling with internal APIs to that project.

As a side effect, no longer need the HateoasProperties.

Resolves gh-3882
gregturn added a commit that referenced this issue Sep 2, 2015
Spring HATEOAS already properly decorates ObjectMappers with @EnableHypermediaSupport. This removes unnecessary calls and decouples Boot from internal Spring HATEOAS APIs at the same time.

Resolves gh-3882
@wilkinsona
Copy link
Member Author

@olivergierke described Jackson2ModuleRegisteringBeanPostProcessor as:

It tweaks all MappingJackson2HttpMessageConverter contained in RequestMappingHandlerAdapters, AnnotationMethodHandlerAdapters and RestTemplates to get the Jackson2HalModule registered with the _halObjectMapper

As far as I can tell, that's not what happens in the RequestMappingHandlerAdapter case at least. Instead of registering the module with the ObjectMapper of all existing MappingJackson2HttpMessageConverters it adds a new MappingJackson2HttpMessageConverter, with an ObjectMapper that is configured with the module, to the start of the list . However, this new converter is only configured to support application/hal+json. This means that the converter doesn't handle requests that accept application/json and one of the converters later in the list with an ObjectMapper that doesn't have the module registered is used instead.

The above means that the work done by Jackson2ModuleRegisteringBeanPostProcessor is insufficient as a fix for #2147 which requires a request that accepts application/json to get a HAL-formatted response when Spring HATEOAS is being used.

gregturn added a commit that referenced this issue Sep 2, 2015
Spring HATEOAS already properly decorates ObjectMappers with @EnableHypermediaSupport. This removes unnecessary calls and decouples Boot from internal Spring HATEOAS APIs at the same time.

Resolves gh-3882
@gregturn
Copy link
Contributor

gregturn commented Sep 2, 2015

I tweaked the order of things and pushed it to a branch at https://github.com/gregturn/spring-boot/tree/gh-3882.

mvn clean install passed all the tests, so if there is some lingering issue, its not trapped by the tests.

@wilkinsona
Copy link
Member Author

Unless I missed it, that branch doesn't contain the ordering change. Also, as I explained on Slack, I don't think it's a good solution:

That will have almost certainly broken something else. You've switched off the ObjectMapper created by JacksonAutoConfiguration as it's @ConditionalOnMissingBean(ObjectMapper.class) and HypermediaAutoConfiguration will have created the HAL one before JacksonAutoConfiguration runs. Among other things, that means there's now no @Primary ObjectMapper in the context.

@gregturn
Copy link
Contributor

gregturn commented Sep 2, 2015

I fixed the branch (had accidentally pushed the mod to spring-projects again), but feel free to shoot the patch or drop it all together.

@wilkinsona
Copy link
Member Author

@olivergierke and I have discussed this. We came to the conclusion that there's nothing that we should do in Spring Boot 1.2.x. I have opened #3891 to improve things in 1.3 where we can make the broader changes that I think may be necessary.

Users of Spring Boot 1.2.x and Spring Data REST that want to use Spring Data Gosling can safely do so if they also upgrade Spring HATEOAS to 0.19.0.RELEASE. This is because the auto-configuration of Spring Data REST switches off the auto-configuration for Spring HATEOAS that's incompatible with Spring HATEOAS 0.19.0.RELEASE

Users of Spring Boot 1.2.x that want to use Spring HATEOAS on its own should stick with the version that's provided by Spring Boot's dependency management.

@wilkinsona wilkinsona added the status: declined A suggestion or change that we don't feel we should currently apply label Sep 3, 2015
@wilkinsona wilkinsona removed this from the 1.2.6 milestone Mar 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants