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

RestTemplateBuilder headers no longer get applied when using MockRestServiceServer #17885

Closed
gilinykh opened this issue Aug 16, 2019 · 8 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@gilinykh
Copy link

build method in default implementation of MockRestServiceServerBuilder overwrites request factory which makes impossible to write tests using remote resources access via RestTemplate built through RestTemplateBuilder.defaultHeader() with headers.

How to reproduce:

bean definition:

        @Bean
        public RestTemplate restTemplate(RestTemplateBuilder restTemplateBuilder) {
            return restTemplateBuilder.defaultHeader("x-api-key", "1234567890").build();
        }

using in test:

        mockApi = MockRestServiceServer.createServer(restTemplate);

And then requests sent to that mock api will come without custom x-api-key header

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 16, 2019
@philwebb
Copy link
Member

@gilinykh Creating RestTemplate as a @Bean often causes problems like this, which is one reason the RestTemplateBuilder was created. I suspect that you're using MockRestServiceServer.bindTo on that singleton bean, which will indeed replace ClientHttpRequestFactory.

Can you please share a complete sample that we can run so that we can confirm if this is a bug or expected behavior.

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Aug 16, 2019
@gilinykh
Copy link
Author

gilinykh commented Aug 19, 2019

@philwebb Here's the link to full sample gilinykh/mockrestserver-test

Your summary is correct. But then what's the intended way to write tests with MockRestServiceServer using RestTemplateBuilder and still be able to configure general request properties (e.g. authorization header) only once (at bean creation?). Oh, and I tend to use db access in my tests (e.g. @Sql) and rely on DI working too so @RestClientTest is not of much help here...

@snicoll
Copy link
Member

snicoll commented Sep 2, 2019

IMO, the problem is that since Spring Boot 2.2 there are a number of features that have been moved to wrapping the ClientHttpRequestFactory. Unfortunately, MockRestServiceServer is overriding this and the wrapping that Spring Boot applies is completely ignored.

@snicoll snicoll added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Sep 2, 2019
@snicoll snicoll added this to the 2.2.x milestone Sep 2, 2019
@philwebb
Copy link
Member

philwebb commented Sep 5, 2019

We changed this because spring-projects/spring-framework#22002 was declined and we wanted to set headers without reading the entire body. I have a feeling we might need that framework change after all.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Sep 6, 2019

Maybe we add support for RestTemplateRequestCustomizer, or some such contract, directly in the RestTemplate. That's effectively what an alternative interceptor would have to be in any case due to the way request execution works, which can start before the execute method, e.g. when the body is accessed as OutputStream.

@philwebb
Copy link
Member

philwebb commented Sep 6, 2019

I was working on a PR. I think a ClientHttpRequestInitializer might be a good compromise. We don't actually need to change ClientHttpRequest like the interceptor does, we just need initialize it in a certain way before it's used.

@philwebb philwebb self-assigned this Sep 9, 2019
@philwebb philwebb changed the title MockRestServiceServerBuilder overwrites request factory thus removing defaultHeader RestTemplateBuilder is no longer compatible with MockRestServiceServer Sep 10, 2019
@philwebb philwebb changed the title RestTemplateBuilder is no longer compatible with MockRestServiceServer RestTemplateBuilder headers no longer get applied when using MockRestServiceServer Sep 10, 2019
@philwebb
Copy link
Member

I've reopened spring-projects/spring-framework#22002 with a suggested fix. If that gets accepted to can apply https://github.com/philwebb/spring-boot/tree/gh-17885

@philwebb philwebb added the status: blocked An issue that's blocked on an external project change label Sep 10, 2019
@snicoll snicoll removed the status: blocked An issue that's blocked on an external project change label Sep 10, 2019
@snicoll snicoll modified the milestones: 2.2.x, 2.2.0.RC1 Sep 10, 2019
@snicoll
Copy link
Member

snicoll commented Sep 10, 2019

It's been accepted. Let me know if you need any help for this.

@snicoll snicoll self-assigned this Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants