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

Introduce interceptors for RestTemplate [SPR-7494] #12152

Closed
spring-projects-issues opened this issue Aug 25, 2010 · 9 comments
Closed

Introduce interceptors for RestTemplate [SPR-7494] #12152

spring-projects-issues opened this issue Aug 25, 2010 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Aug 25, 2010

Arjen Poutsma opened SPR-7494 and commented

The RestTemplate will benefit from having interceptor-style delegates, similar to the Spring-WS ClientInterceptors (see http://static.springsource.org/spring-ws/sites/1.5/apidocs/org/springframework/ws/client/support/interceptor/ClientInterceptor.html)


Attachments:

Issue Links:

1 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

Duane Zamrok commented

I've attached a maven project that contains code to implement this feature. The project contains interfaces and classes to describe a HttpContext and extends the existing Spring 3.0.3 org.springframework.web.client.RestTemplate class.

The major shortcoming, so far as I can tell, of this code base is that the org.springframework.web.client.ExtendedRestTemplate does not account for the fact that the org.springframework.http.client.SimpleClientHttpResponse and the org.springframework.http.client.CommonsClientHttpResponse both maintain a single java.io.InputStream as their body. This is a shortcoming because it means that if a HttpClientInterceptor reads the body of the org.springframework.web.client.context.HttpMessageContext#getResponse then the input stream will be closed when the org.springframework.http.client.ExtendedRestTemplate attempts to extract the response. I've implemented a workaround to this problem with the org.springframework.http.client.RepeatableClientHttpResponse, and demonstrated its use with the org.springframework.web.client.interceptor.LoggingHttpClientInterceptor, but it remains a problem that must be adressed by the interceptor.

Note:
This could be resolved by using a org.springframework.http.client.RepeatableClientHttpResponse directly in the org.springframework.http.client.ExtendedRestTemplate#doExecute method, but doing so would add unecessary overhead to the call for cases where there are no interceptors and/or the interceptors do not need to access the response body.

@spring-projects-issues
Copy link
Collaborator Author

Duane Zamrok commented

I think the ClientHttpResponse, at least, could benefit from having a method along the lines of "isBodyRepeatable" to denote whether or not it is safe to read the InputStream returned by "getBody" more than once.

I've been thinking about this problem a little bit more, and noticed a problem with the LoggingHttpClientInterceptor. The interceptor will wrap the response without testing to see if it is a RepeatableClientHttpResponse. This means that if a RepeatableClientHttpResponse is received by the LoggingHttpClientInterceptor it will be wrapped inside of yet another RepeatableClientHttpResponse. To avoid this I would need to specifically test to see if the resposne I recieved is a RepeatableClientHttpResponse; which feels rather tightly coupled to a specific implementation. Without the fix however I will waste both memory and CPU on a task that is unnecessary.

Please don't take offense to the following, because I think the foundations of Spring 3.0 are way better than anything I could have done myself. However I think the ClientHttpResponse, ClientHttpRequest, HttpInputMessage, and HttpOutputMessage interfaces are all a little bulky compared to the spring-ws implementations. I'm sure it is because the interfaces were not written with multiple consumers in mind, but even so it makes working with them an exercise in extreme caution. I used my debugger extensivly to ensure that the stream states were not violated, and the LoggingInterceptor only works when the body of the request is a ByteArrayOutputStream.

@spring-projects-issues
Copy link
Collaborator Author

Arjen Poutsma commented

Fixed by adding a ClientHttpRequestInterceptor interface, which can be set on the RestTemplate

@spring-projects-issues
Copy link
Collaborator Author

Brian Relph commented

What about adding response interceptors? Even if the issues around the response body inputstream remain unresolved, I see value in being able to inspect the headers ...

@spring-projects-issues
Copy link
Collaborator Author

Arjen Poutsma commented

@Brian: you can actually change the response with this ClientHttpRequestInterceptor as well, see

https://fisheye.springsource.org/browse/~raw,r=3930/spring-framework/trunk/org.springframework.web/src/main/java/org/springframework/http/client/ClientHttpRequestInterceptor.java

Soon, Keith will write a blog entry about these interceptors and how they are used in Spring Social to provide OAuth authentication to the RestTemplate.

@spring-projects-issues
Copy link
Collaborator Author

Francisco Lozano commented

PLS back-port this to Spring Android's rest-template

@spring-projects-issues
Copy link
Collaborator Author

Arjen Poutsma commented

@Francisco: feel free to report an issue against the ANDROID project at https://jira.springsource.org/browse/ANDROID

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants