trying to add an aop proxy factory bean configuration to moduleApplicationContext.xml such as below doesn’t register the advise.
<!-- AOP before advice that calls the LoginAdvice methods -->
<bean id="userContext" class="org.openmrs.api.context.UserContext"/>
<bean id="loginInterceptor" class="org.openmrs.module.msfcore.aop.LoginAdvice"/>
<bean id="userContextProxy" class="org.springframework.aop.framework.ProxyFactoryBean">
<property name="target" ref="userContext" />
<property name="interceptorNames">
<list>
<value>loginInterceptor</value>
</list>
</property>
</bean>
Should we not support the second option for non services?
Suggestion: We have a use-case to log user activity such as login, i suggest either supporting AOP to extend to non services, otherwise to support this use-case, i suggest.
Add UserActivityService#authenticationAttempt(User) to referenceapplication module such as ApplicationEventService
Fire the authenticationAttempt event after Context.authenticate for either a successful or failed login
Create an advise in my module wrapped around UserActivityService#authenticationAttempt so as to log authentication outcomes.
what do you folks think, would this be helpful to OpenMRS?
That’s not how you register AOP advice from a module, please see this page for how to do it.
With that said, I think it’s not exactly correct to wrap the advice around UserContext, one it’s not a spring bean and secondly it’s just a holder of the details about the authenticated user and does no actions, instead you should wrap the advice around the Context object specifically around the authenticate method in your case.
Then I guess you’re right, looks like adding advice via the config.xml file and Context.addAdvice only supports services.
As I said to make it work you need to wrap the AOP around the Context class and not UserContext because I don’t think you can ‘springfy’ the UserContext objects given that they are thread locals that are created outside of spring.
I doubt if you’re getting what am trying to say, what am saying is that what you’re doing from your original post should work, just change the target you are wrapping the AOP around to be the bean with id context, and that should work.
The OpenMRS way is meant to simplify things for module developers by trying to hide the complexities, the point is that spring will still pick it up anyways.
The configuration below in the snippet you provided with a small change of the target bean are what instruct spring to set up the AOP around the context class
I’d expect spring to wrap the AOP around the context class but it doesn’t, I guess it’s because the bean is already defined in core and you can’t do it from a module. I think the only option you’re left with is to use aspectj integration with spring, spring’s documentation describes how you can do it.
Those errors look deceptive, I see no relationship between the AllUserApps bean and AOP advice you’re setting up, I think it would be more useful to say exactly what you have done, you’re saying you added the necessary dependencies, I think you need to say exactly what dependencies you added.