-
Notifications
You must be signed in to change notification settings - Fork 1
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
Interceptors mechanism (fixes #6) #17
Conversation
Changes Unknown when pulling 977f854 on nurkiewicz:interceptors into * on Codearte:master*. |
977f854
to
e59eb57
Compare
@@ -138,7 +140,8 @@ private boolean checkQualifier(BeanDefinition endpointBeanDefinition, Annotation | |||
public void setBeanFactory(BeanFactory beanFactory) throws BeansException { | |||
this.beanFactory = (ConfigurableListableBeanFactory) beanFactory; | |||
RestTemplate restTemplate = provideRestTemplate(this.beanFactory); | |||
beanProxyCreator = new BeanProxyCreator(restTemplate); | |||
final Collection<RestInvocationHandler> handlers = ((ConfigurableListableBeanFactory) beanFactory).getBeansOfType(RestInvocationHandler.class).values(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure all RestInvocationHandlers are available at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this can be an issue. I'll try to make it lazier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to look at @DependsOn
internals :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very helpful: AbstractBeanFactory#L286
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the walk around for this is to invoke getBean for bean we need: https://github.com/spring-projects/spring-framework/blob/4.0.x/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionValueResolver.java#L272
<artifactId>objenesis</artifactId> | ||
<version>2.1</version> | ||
<scope>test</scope> | ||
</dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need those dependencies defined explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mocking of some classes wasn't working without it. Try removing these dependencies, maybe it became obsolete in the meantime.
… handlers Implements chain-of-responsibility pattern
e59eb57
to
1fee956
Compare
Changes REST invocation flow from simple
RestClientMethodInterceptor
->RestTemplateInvoker
to much more flexible and extensible interceptor/handler architecture. Basically each REST call is wrapped inRestInvocation
value object and passed to firstRestInvocationHandler
(with highest priority). Each handler can either process invocation or pass is further (invocation.proceed()
, seeLoggingHandler
). Last handler must handle the request, typically this will beRestTemplateInvoker
(added automatically to the end of handlers chain).Handlers are auto-discovered in application context and sorted using
Ordered
interface. This architecture makes it very easy to implement a lot of custom functionality:UserAgentHandler
), (see Add support for custom headers #8)@Async
(see Add support for async calls #7)And a lot more. It's basically specialized AOP inside the library.