Why @Autowired doesn’t work in REST WS’s REST resource?

Tags: #<Tag:0x00007f88c5828a48>

(chine zoheir) #3

Yes, i confirmed that it’s a spring bean, and it’s already autowired in the controller here

(Nicholas Ingosi) #4

What is that alternative way suggested by @mksd

(Dimitri R) #5

@wyclif if you look in REST WS, using @Autowired is carefully avoided, for example all OpenMRS services are always gotten through a direct call to Context. This from EncounterResource would be an example among dozens.

@ningosi autowiring is only a best guess by Spring (unless you actually point to the exact bean with @Qualifier like here). A direct bean fetching can always be done through the context, for instance with ComplexObsSaver:

ComplexObsSaver complexObsSaver =
 Context.getRegisteredComponent(AttachmentsConstants.COMPONENT_COMPLEXOBS_SAVER, ComplexObsSaver.class);

(Darius Jazayeri) #6

The reason is that our rest resources are not spring managed beans, so there is nothing to process the autowired annotation.

(Dimitri R) #7

@darius do you remember what the design rationale was behind this?

(Willa Mhawila) #8

Yeah they seem to be good candidates for spring beans. I really don’t like doing Context.getService() because it is very difficult to unit test without having to make the test class application context aware (i.e. extending XContextSensitive classes) which in turn make the tests too slow due to long setup time!

(Dimitri R) #9

Yes of course it would be better if REST resources could be Spring beans, but I would assume that if it isn’t the case then there must be a good reason. Does anyone recall why that is?

(Rafal Korytkowski) #10

@mksd, I suppose it was because we did not have @OpenmrsProfile when they were originally written to load Spring beans conditionally based on OpenMRS version.

I wanted to make them Spring beans some time ago when I introduced @OpenmrsProfile, but it turned out to be a bigger task that I didn’t have enough time to complete it.

I’d be happy to see it happen though. It would improve module startup time (no need to scan through classpath for @Resource) and response time since Context.getService() is slow (especially in high concurrency scenario) due to synchronization. The ease of testing of Spring beans brought up by Willa would be another reason to do it.

(Dimitri R) #11

@raff do you remember why?

(Darius Jazayeri) #12

I imagine it’s doing any potential refactoring of individual resources, testing, and addressing backwards compatibility that makes it large.

The historical reason for not using spring beans is probably my own inexperience, but I don’t remember for sure.

(Dimitri R) #13

Thanks @darius. I was just wondering if @raff faced an unexpected blocker while undertaking this.

The “Springification” of REST resources could perhaps become a story of the RESTWS project in JIRA? And turning each resource into a Spring bean could be a entry task for newcomers. It’s going to take a while, but who knows, maybe not.

(tendo kiiza Martyn) #14

@mksd does this apply for All Resource Beans in the web.rest.services module as well ? In other words can i use Context to pull a resource and utilise it in a Test Class ?

I did something like

OrderResource1_10 resource = (OrderResource1_10)Context.getRegisteredComponents(OrderResource1_10.class);

SimpleObject result = (SimpleObject) resource.doSearch(requestContext);

but i get a java.lang.ClassCastException: java.util.ArrayList cannot be cast to org.openmrs.module.webservices.rest.web.v1_0.resource.openmrs1_10.OrderResource1_10

cc @dkayiwa

(tendo kiiza Martyn) #15

@mksd is the "Springification " of The Resource Beans as @raff had imagined still on ?

I cannot also find the appropriate constant from the RestConstants.Class bean to act as String parameter to the ``` **OrderResource1_10 resource = Context.getRegisteredComponent("", OrderResource1_10.class); ** **** instance. Can i introduce it ? and which other classes would be affected ?

(Dimitri R) #16

I don’t think it ever happened, it was just discussed here and didn’t move beyond that.

Therefore you won’t be able to fetch the bean as you did attempt to since no REST resources are registered as Spring beans (yet). However why exactly would you want to do that in a test? There’s a specific test framework for REST controllers, why can’t you use it? (Example on ConceptResource here).

(tendo kiiza Martyn) #17

@mksd Thanks alot for the response , i was not actually testing a Controller but rather OrderResource1_10Test because of a new method i introduced , but may be it was a wrong course.The method was to Mock an Http Request to the OrderResource1_10.doSearch() method is it wrong that i skipped the controller ?

the method should return an Exception instead of a null result ,incase "status " only param is specified instead of “status” and "patient uuid " Parameters.

(Dimitri R) #18

To test doSearch() you will need a new OrderController1_10Test class, again see here a search test case example (but in the context of concepts).

Something like:

public class OrderController1_10Test extends MainResourceControllerTest {

(tendo kiiza Martyn) #19

Thanks alot , had ignored that thinking the MainResourceControllerTest was for some select Resources.Let me try this out.Saved me a chunk of time Resource :grinning:

And i checked ,it looks like ``` OrderController1_10Test

(Ivange Larry Ndumbe) #20

@mksd, @darius, @wyclif, @raff do you think this will make a good GSoC project? Is it worth the effort to look into this?

GSoC 2019 is coming up and OpenMRS is on the look out for projects. Maybe we could through this in and see what proposals we get?

(Dimitri R) #21

Thanks for suggesting this @ivange94, it’s a nice to have really. My opinion is that there would be more yielding projects to push first. Even in the terms of backend refactoring (which is what this is) there might be more urgent items.

Do you intend to mentor?

(Ivange Larry Ndumbe) #22

Well we could always submit this as a project idea and prioritize later.

I might. Not sure yet.