Skip to content

Simplify configuration setup #719

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
odrotbohm opened this issue Jun 19, 2018 · 10 comments
Closed

Simplify configuration setup #719

odrotbohm opened this issue Jun 19, 2018 · 10 comments
Assignees
Milestone

Comments

@odrotbohm
Copy link
Member

There have been quite a few tickets (#333, spring-projects/spring-boot#8174) that make obvious that the current configuration step is quite complex and creates difficulties for downstream projects to properly configure especially the Jackson ObjectMapper to use.

There are a couple of areas we can improve on:

  • Avoid registering an ObjectMapper at all. Instead, we should rather lazily refer to a unique ObjectMapper instance in the ApplicationContext and derive the ones to be used inside the media type specific HttpMessageConverters from that one.
  • Switch to JavaConfig for as many of the components we currently set up via BeanDefinition instances programmatically.
@odrotbohm odrotbohm added this to the 1.0 M1 milestone Jun 19, 2018
@odrotbohm odrotbohm self-assigned this Jun 19, 2018
odrotbohm added a commit that referenced this issue Jun 20, 2018
We now avoid to register ObjectMapper instances as Spring beans and rather use one already existing in the ApplicationContext and copying the setup before registering the individual HttpMessageConverters for the individual media types.

Replaced a lot of the programmatic component setup via BeanDefinitions with their corresponding JavaConfig alternatives.

Removed obsolete media type specific HttpMessageConverters and configuration classes registering them.

Backport of fix for #719.
odrotbohm added a commit that referenced this issue Jun 22, 2018
We now avoid to register ObjectMapper instances as Spring beans and rather use one already existing in the ApplicationContext and copying the setup before registering the individual HttpMessageConverters for the individual media types.

Replaced a lot of the programmatic component setup via BeanDefinitions with their corresponding JavaConfig alternatives.

Removed obsolete media type specific HttpMessageConverters and configuration classes registering them.
@odrotbohm
Copy link
Member Author

To summarize the changes made:

  • We now don't register any ObjectMapper instances in the ApplicationContext anymore. Instead we leniently pick up one available in the context and derive fresh instances (using ….copy()) we can augment with additional, media type specific Jackson modules and register those in the HttpMessageConverters we register.
  • Moved a lot of the infrastructure bean definitions we previously created programmatically into JavaConfig classes.

@sbley
Copy link

sbley commented Sep 10, 2018

I gave it a try (Spring Boot 2.0.4.RELEASE including Spring HATEOAS 0.25.0.RELEASE) and found that Spring HATEOAS does not work in controller tests anymore.

It seems that @EnableHypermediaSupport does not have an effect in Spring Boot tests. It works fine with Spring Boot 2.0.3.RELEASE and HATEOAS 0.24.0.RELEASE.

App

@SpringBootApplication
@EnableHypermediaSupport(type = EnableHypermediaSupport.HypermediaType.HAL)
public class DemoApplication {
    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }
}

Controller

@RestController
@RequestMapping("/")
public class IndexController {

    @GetMapping
    public ResponseEntity<ResourceSupport> index() {
        ResourceSupport index = new ResourceSupport();
        index.add(linkTo(methodOn(IndexController.class).index()).withSelfRel());
        return ResponseEntity.ok().body(index);
    }
}

Test

@RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@EnableHypermediaSupport(type = EnableHypermediaSupport.HypermediaType.HAL)
public class IndexControllerTest {

    @Autowired private TestRestTemplate clientRestTemplate;

    @Test
    public void responseShouldContainLinks() {
        ResponseEntity<ResourceSupport> response = clientRestTemplate.getForEntity("/", ResourceSupport.class);
        // ↓↓ This assertion fails
        assertThat(response.getBody().getLink("self")).isNotNull();
    }
}

Full sample project: https://github.com/sbley/spring-boot-2-hateoas

@odrotbohm
Copy link
Member Author

It looks like TestRestTemplate is not properly configured to find HAL links anymore. The server still produces proper HAL, as you can see by starting the app and just curling the resource.

Paging @wilkinsona as I am not familiar with the internals of how TestRestTemplate gets created and how it picks up RestTemplate instances which we post-process to add the HttpMessageConverters necessary.

@wilkinsona
Copy link
Member

wilkinsona commented Sep 10, 2018

The RestTemplate used by TestRestTemplate is created using the context's RestTemplateBuilder. The RestTemplateBuilder is configured to use any HttpMessageConverters registered with the context's HttpMessageConverters bean.

@gregturn
Copy link
Contributor

That would be the problem, then.

Spring HATEOAS's HateoasConfiguration has:

@Bean
ConverterRegisteringBeanPostProcessor jackson2ModuleRegisteringBeanPostProcessor(
		ObjectFactory<ConverterRegisteringWebMvcConfigurer> configurer) {
	return new ConverterRegisteringBeanPostProcessor(configurer);
}

This BPP in turn creates a ConverterRegisteringWebMvcConfigurer, which is a BeanFactoryAware WebMvcConfigurer, which directly adds converters for the appropriate mediatypes.

In its present configuration, those converters never get registered with the context.

@odrotbohm
Copy link
Member Author

I dug a bit deeper and I can't quite find the conceptual difference to 0.24.0.RELEASE as the media-type specific HttpMessageConverters we use have never been Spring beans in the first place. We've always post-processed RequestMappingHandlerAdapter and RestTemplate beans and augmented the converter setup.

@sbley
Copy link

sbley commented Sep 13, 2018

Just to let you know: Spring Boot 2.0.5.RELEASE together with downgraded Spring HATEOAS 0.24.0.RELEASE works.
https://github.com/sbley/spring-boot-2-hateoas/tree/boot-2.0.5-hateoas-0.24.0

@wilkinsona
Copy link
Member

You can work around the problem by adding the following bean to your application (and only using @EnableHypermediaSupport once):

@Bean
public RestTemplateCustomizer halCustomizer(ConverterRegisteringWebMvcConfigurer customizer) {
    return (template) -> customizer.extendMessageConverters(template.getMessageConverters());
}

@sbley
Copy link

sbley commented Sep 13, 2018

@iNikem
Copy link

iNikem commented Jun 13, 2019

Sorry for commenting on a closed ticket. But isn't the obvious problem here is that ConverterRegisteringBeanPostProcessor augments a bean of type RestTemplate. But oftentimes such bean does not exist in the ApplicationContext. Instead there is RestTemplateBuilder. So if I inject RestTemplateBuilder into my class, get a new RestTemplate from it, then it will NOT have a corresponding Jackson converter for HAL.

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

5 participants