REST WS: BaseDelegatingResourceTest.getResource() returns resource's parent instance in unit test

Hi all,

I’m trying to setup a unit test for a custom REST resource that extends ObsResource1_9:

public class ComplexObsResource1_10 extends ObsResource1_9 { ...

Then, on the model of the obs resources tests found in the REST WS module, I did this as a starting point for my unit test:

public class ComplexObsResource1_10Test extends BaseDelegatingResourceTest<ComplexObsResource1_10, Obs> {
  ...   
  @Test
  public void shouldRunTest() throws Exception {
    ComplexObsResource1_10 resource = getResource();
  }
}

And I’m getting a

java.lang.ClassCastException: org.openmrs.module.webservices.rest.web.v1_0.resource.openmrs1_9.ObsResource1_9 cannot be cast to ComplexObsResource1_10

However getResource() should return the template type R that is ComplexObsResource1_10 in my case, why is it returning the parent’s type ObsResource1_9?

This looks strange! How does your resource class look like? Or do you have a branch to reproduce this?

Hi @dkayiwa,

Sorry about the delay, the weekend got in the way!

Yes you can clone this tmp branch. The unit test in question is here: ComplexObsResource1_10Test.

Let me know if you need anything else from me.

The ComplexObsResource1_10Test.shouldRunTest() test passes on my computer.

@mksd can you cross check the platform versions supported by ObsResource1_9 vs ComplexObsResource1_10 Ensure that you do not have an overlap of any platform version supporting more than one resource.

As I said on IRC, it should be possible to override an existing resource through the order= argument specified in the annotation of the resource definition. See point 2.d in ‘Adding a Web Service Step by Step Guide for Core Developers’.

I looked a bit more closely and the code that initialises the resources fills in two maps: (1) resourcesBySupportedClasses and (2) resourceDefinitionsByNames. Here ‘names’ actually mean URI, so v1/obs, v1/form, v1/person … etc etc.

At first when ComplexObsResource is processed and ObsResource* are not yet processed, (1) and (2) look like this

{org.openmrs.Obs = ComplexObsResource1_10@25371277, ...}

{v1/complexobs = ResourceDefinition@1a71edf1, ...}

And eventually when both are processed they look like this:

{org.openmrs.Obs = ObsResource1_9@15c32192, ...}

{v1/complexobs = ResourceDefinition@1a71edf1,
 v1/obs        = ResourceDefinition@22c16f5f, ...}

As we can see the custom resource gets kicked out of the class-indexed map, despite the order, while the custom name/URI remains, just because it is a unique key in that map. The order is only checked on the name-indexed map anyway (see here), this means that the class-indexed map retains the last one that has been handed for processing. That’s why we have observed inconsistent results in different environments.

This would also explain why accessing the resource through the URI (name) works anyway, while accessing it through class loading (unit tests) produces erratic results.

This code has been heavily refactored recently by @teleivo, perhaps could he confirm that the above situation is now handled more consistently? But it seems that the former state of things was such until version 2.16 :unamused:

I added tests before refactoring, and didn’t change its behavior.

Just to make sure, the resource you wrote ComplexObsResource1_10 is of order=1 ? Because resourcesBySupportedClasses keeps the resource with lowest order.

Hey @teleivo,

Thanks for responding so quick.

Actually order=0 would normally be necessary since the competing resource is of order=1. But it doesn’t matter, it gets overridden anyway (see my previous post.)

@mksd i have edited the documentation to reflect the current state of code. That is, the order argument will work only if you are using the same uri to access the resource you are overwriting.

That being said, do you mind sharing some details as to why you would like to access complex obs using v1/complexobs instead of the existing v1/obs? If you only want to do some special or extra processing for complex obs, i would think that you could still use the existing v1/obs uri and provide an order argument to override the default.

Thanks for updating the wiki.


If you try to edit or delete a complex obs via v1/obs you will hit a wall.

Let’s look at the edition for the example. The thread will eventually land in ObsServiceImpl.saveObs(Obs, String) and enter this method handling complex obs: handleExistingObsWithComplexConcept(Obs).

The issue I was facing comes from the initial check being made in it:

if (null != obs.getConcept() && obs.getConcept().isComplex()
  && null != obs.getComplexData().getData()) { ...

If you use v1/obs, the obs’s complex data is never filled up in the thread. And the 3rd check produces a NullPointerException since the complex data is null:

obs.getComplexData().getData()

Hence the need of complexifying the obs before sending it down the thread. By “complexifying” I mean filling up its complex data. However this leads to a further difficulty: in order to obtain the complex obs out of an obs, one must provide a ‘view’. This is the method that does it and invokes the relevant complex obs handler:

public Obs getComplexObs(Integer obsId, String view) { ...

When editing or - worse - deleting a complex obs, it doesn’t make any sense to provide a view to do so. But nevertheless a view must be passed on to move forward. We can get away with it by passing the raw view or something, but that’s not 100% safe in all cases, and shouldn’t be the case by design anyway.

@mksd i thought in that case you would use the order argument to process it your way instead of the default??? :slight_smile:

You mean that I should not bother changing the URI right? I should keep using v1/obs with a lower order (I think it’s got to be 0 then) so that my “complexifying” version kicks in.

I would be curious to have your opinion on this NullPointerException issue though.

Since ObsResource1_9 has order 2, then 1 should be fine without changing the uri. What about the NullPointerException? Where is it?

Please read my former message, I have detailed why a NullPointerException may occur when using v1/obs with complex obs. It comes down to the way ObsServiceImpl handles complex obs.

@mksd on today’s design call, we discussed about getting complex obs supported in the rest webservices module. When done, complex data will be set on the obs and hence obs.getComplexData() will no longer return null. @wyclif or @burke will share the notes.

As for ObsService.getComplexObs(), there is a TODO about removing this method because it is confusing, since you may have a complexObs, but not want the complexData attached at the time. Nevertheless, you may think that this method is necessary to use, when you could just call getObs(). This will attach the complexData onto the obs.

Thanks for bringing that one up! We are looking forward to seeing those changes in REST WS, is there already a JIRA issue for it?

As for ObsService.getComplexObs(String), I’d better get ready and already work with a compatibility class around it for when it goes.

@mksd are you looking for this? RESTWS-297