Reputation: 3714
All requests and responses handled by our Spring Rest Controller has a Common section which has certain values:
{
"common": {
"requestId": "foo-bar-123",
"otherKey1": "value1",
"otherKey2": "value2",
"otherKey3": "value3"
},
...
}
Currently all my controller functions are reading the common
and copying it into the response manually. I would like to move it into an interceptor of some sort.
I tried to do this using ControllerAdvice
and ThreadLocal
:
@ControllerAdvice
public class RequestResponseAdvice extends RequestBodyAdviceAdapter
implements ResponseBodyAdvice<MyGenericPojo> {
private ThreadLocal<Common> commonThreadLocal = new ThreadLocal<>();
/* Request */
@Override
public boolean supports(
MethodParameter methodParameter, Type type, Class<? extends HttpMessageConverter<?>> aClass) {
return MyGenericPojo.class.isAssignableFrom(methodParameter.getParameterType());
}
@Override
public Object afterBodyRead(
Object body,
HttpInputMessage inputMessage,
MethodParameter parameter,
Type targetType,
Class<? extends HttpMessageConverter<?>> converterType) {
var common = (MyGenericPojo)body.getCommon();
if (common.getRequestId() == null) {
common.setRequestId(generateNewRequestId());
}
commonThreadLocal(common);
return body;
}
/* Response */
@Override
public boolean supports(
MethodParameter returnType, Class<? extends HttpMessageConverter<?>> converterType) {
return MyGenericPojo.class.isAssignableFrom(returnType.getParameterType());
}
@Override
public MyGenericPojo beforeBodyWrite(
MyGenericPojo body,
MethodParameter returnType,
MediaType selectedContentType,
Class<? extends HttpMessageConverter<?>> selectedConverterType,
ServerHttpRequest request,
ServerHttpResponse response) {
body.setCommon(commonThreadLocal.get());
commonThreadLocal.remove();
return body;
}
}
This works when I test sending one request at a time. But, is it guaranteed that afterBodyRead
and beforeBodyWrite
is called in the same thread, when multiple requests are coming?
If not, or even otherwise, what is the best way of doing this?
Upvotes: 5
Views: 3554
Reputation: 2560
I think that there is no need of your own ThreadLocal
you can use request attributes.
@Override
public Object afterBodyRead(
Object body,
HttpInputMessage inputMessage,
MethodParameter parameter,
Type targetType,
Class<? extends HttpMessageConverter<?>> converterType) {
var common = ((MyGenericPojo) body).getCommon();
if (common.getRequestId() == null) {
common.setRequestId(generateNewRequestId());
}
Optional.ofNullable((ServletRequestAttributes) RequestContextHolder.getRequestAttributes())
.map(ServletRequestAttributes::getRequest)
.ifPresent(request -> {request.setAttribute(Common.class.getName(), common);});
return body;
}
@Override
public MyGenericPojo beforeBodyWrite(
MyGenericPojo body,
MethodParameter returnType,
MediaType selectedContentType,
Class<? extends HttpMessageConverter<?>> selectedConverterType,
ServerHttpRequest request,
ServerHttpResponse response) {
Optional.ofNullable(RequestContextHolder.getRequestAttributes())
.map(rc -> rc.getAttribute(Common.class.getName(), RequestAttributes.SCOPE_REQUEST))
.ifPresent(o -> {
Common common = (Common) o;
body.setCommon(common);
});
return body;
}
EDIT
Optional
s can be replaced with
RequestContextHolder.getRequestAttributes().setAttribute(Common.class.getName(),common,RequestAttributes.SCOPE_REQUEST);
RequestContextHolder.getRequestAttributes().getAttribute(Common.class.getName(),RequestAttributes.SCOPE_REQUEST);
EDIT 2
About thread safety
1) standard servlet-based Spring web application we have thread-per-request scenario. Request is processed by one of the worker threads through all the filters and routines. The processing chain will be executed by the very same thread from start to end . So afterBodyRead
and beforeBodyWrite
guaranteed to be executed by the very same thread for a given request.
2) Your RequestResponseAdvice by itself is stateless. We used RequestContextHolder.getRequestAttributes()
which is ThreadLocal and declared as
private static final ThreadLocal<RequestAttributes> requestAttributesHolder =
new NamedThreadLocal<>("Request attributes");
And ThreadLocal javadoc states:
his class provides thread-local variables. These variables differ from their normal counterparts in that each thread that accesses one (via its get or set method) has its own, independently initialized copy of the variable.
So I don't see any thread-safety issues into this sulotion.
Upvotes: 4
Reputation: 2560
Also I have already answered this thread, but I prefer another way to solve such kind of problems
I would use Aspect-s in this scenario.
I have written included this in one file but you should create proper separate classes.
@Aspect
@Component
public class CommonEnricher {
// annotation to mark methods that should be intercepted
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface EnrichWithCommon {
}
@Configuration
@EnableAspectJAutoProxy
public static class CommonEnricherConfig {}
// Around query to select methods annotiated with @EnrichWithCommon
@Around("@annotation(com.example.CommonEnricher.EnrichWithCommon)")
public Object enrich(ProceedingJoinPoint joinPoint) throws Throwable {
MyGenericPojo myGenericPojo = (MyGenericPojo) joinPoint.getArgs()[0];
var common = myGenericPojo.getCommon();
if (common.getRequestId() == null) {
common.setRequestId(UUID.randomUUID().toString());
}
//actual rest controller method invocation
MyGenericPojo res = (MyGenericPojo) joinPoint.proceed();
//adding common to body
res.setCommon(common);
return res;
}
//example controller
@RestController
@RequestMapping("/")
public static class MyRestController {
@PostMapping("/test" )
@EnrichWithCommon // mark method to intercept
public MyGenericPojo test(@RequestBody MyGenericPojo myGenericPojo) {
return myGenericPojo;
}
}
}
We have here an annotation @EnrichWithCommon
which marks endpoints where enrichment should happen.
Upvotes: 0
Reputation: 18235
Quick answer: RequestBodyAdvice
and ResponseBodyAdvice
are invoked within the same thread for one request.
You can debug the implementation at: ServletInvocableHandlerMethod#invokeAndHandle
The way you're doing it is not safe though:
ThreadLocal
should be defined as static final
, otherwise it's similar to any other class propertyResponseBodyAdvice
(hence the threadlocal data is not removed)"More safe way": Make the request body supports any class (not just MyGenericPojo
), in the afterBodyRead
method:
ThreadLocal#remove
MyGenericPojo
then set the common data to threadlocalUpvotes: 1
Reputation: 357
If it's only a meta data that you copy from the request to the response, you can do one of the followings:
1- store the meta in the request/response header,and just use filters to do the copy :
@WebFilter(filterName="MetaDatatFilter", urlPatterns ={"/*"})
public class MyFilter implements Filter{
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
HttpServletRequest httpServletRequest = (HttpServletRequest) request;
HttpServletResponse httpServletResponse = (HttpServletResponse) response;
httpServletResponse.setHeader("metaData", httpServletRequest.getHeader("metaData"));
}
}
2- move the work into the service layer where you can do the cope through a reusable common method, or have it run through AOP
public void copyMetaData(whatEverType request,whatEverType response) {
response.setMeta(request.getMeta);
}
Upvotes: -1