org.openmrs.api.APIException: Editing some fields on Obs is not allowed

Development
,
1

Hi all,

While making VDUI Platform 2.x-compatible, I am stumbling upon this when saving an existing complex obs via REST:

ERROR - BaseRestController.handleException(115) |2016-10-07 19:58:46,756| Editing some fields on Obs is not allowed
org.openmrs.api.APIException: Editing some fields on Obs is not allowed
  at org.openmrs.api.db.hibernate.ImmutableEntityInterceptor.onFlushDirty(ImmutableEntityInterceptor.java:111)
  at org.openmrs.api.db.hibernate.ChainingInterceptor.onFlushDirty(ChainingInterceptor.java:75)
  at org.hibernate.event.internal.DefaultFlushEntityEventListener.invokeInterceptor(DefaultFlushEntityEventListener.java:365)
  at org.hibernate.event.internal.DefaultFlushEntityEventListener.handleInterception(DefaultFlushEntityEventListener.java:342)
...

I have tried to debug but so far I couldn’t really figure out where exactly the thread is being interrupted. Does anyone know what the above error means?

Here is the full stack trace. Actually the only field that is edited and attempted to be saved is the obs.comment.

This used to work fine with Core 1.11.x and our custom ComplexObsResource.

2

The high-level answer is that OpenMRS never intended to allow obs to be edited, but rather you must void-and-create-new for auditing purposes. Starting in platform 2.0 we actually enforce this (though I don’t know the low-level details).

3

Thanks @darius.

So, since we already have designed our own REST resource, would it be a good idea to expose a new ‘edit’ method through it that basically does what you said: ’ void-and-create-new’?

4

That makes sense to me, yes.

(I wonder, does the core obs web service transparently handle this? If not, it should
)

5

Sorry, what do you mean exactly?

6

What you’re trying to achieve is being able to revise an order and the rest API doesn’t have a single call you can make to do so, you have to fetch the order you wish to revise, clone it, set the new field value(s), previousOrder and action on the clone and then post the clone. If you’re writing a java based client, cloning an obs for revision is a one line call to Obs.cloneForRevision() as shown in this test before you post it otherwise you’d have to replicate that code in your client.

7

Ooops! I don’t understand why my brain was thinking about Orders, seems like I should go for the weekend, :slight_smile: . Anyways, this might not be related but you need to update your resource to extend ObsResource1_11 if you’re running core 1.11+, this will continue to be a problem for you whenever we introduce a new version of the resource so you might want to figure out an alternative.

Back to your issue, in theory when you save an edited Obs, the API should automatically void and replace it but it seems like there is something wrong going on in your custom Obs handler, looking at this, the Obs has already been saved and then it sets valueComplex which am not sure if it’s setting a different value, seems like a possible cause of the issue.

8

Hi @wyclif, thanks for looking through the code.

I guess you refer to those two instructions in my custom handler’s saveObs(Obs):

ValueComplex valueComplex = saveComplexData(obs, docComplexData);
obs.setValueComplex(valueComplex.getValueComplex());

The first one doesn’t actually ever save the obs, it just saves its complex data. Meaning in this case saving a file on the disk. Then it sets the valueComplex to the obs object that shall be saved via the DAO layer. This used to work fine with Core 1.11.x and actually still works fine on Platform 2.x when creating obs.

This leads me to think that the problem might rather come from what @darius suggested (although I don’t know enough about the evolutions from Core 1.11.x to Platform 2.x to be sure) : it may not be possible to edit an obs via REST using our custom resource save(..) routine.

9

So just to confirm, editing with the same REST resource works fine with Ref App 2.3 / Core 1.11.4.

10

I meant: I think you should be able to POST to 
/obs/some-uuid and have this void-and-recreate (probably returning a redirect to the newly-created obs)

I’m not sure if we did this yet
 (and I haven’t checked just now)

11

Hi @darius,

I am a little stuck with this, I would be grateful if you could shed some more light.

Coming back to my suggestion above in this thread:

How do I do that? What should I do to my custom resource so that in the end a new .edit(..)method is exposed on the Obs JavaScript object?

Anyway even if I keep overloading the existing .save(..) method I keep running on the same exception. I have tried to customize the behaviour of Obs.save(Obs delegate) by simply saving a new copy of the delegate
 and I get the exact same exception “Editing some fields on Obs is not allowed”:

@Override
public Obs save(Obs delegate) {
  Obs editedObs = Obs.newInstance(complexify(delegate));
  editedObs.setUuid(null);
  return super.save(editedObs);
}

I was thinking that maybe it didn’t like the UUID (since that’s the one of the original obs), but in fact this line setting the UUID to null doesn’t change anything. The complexify(Obs) just fills in the complex data, removing it doesn’t change anything either anyway.

Cc: @wyclif

12

I suspect there is something not right in our custom handler, the API automatically handles voiding and replacing the edited Obs, it’s not the REST API’s job to do it

13

Thanks @wyclif

This handler works fine pre-Platform 2.x. So I will try to make a unit test reproducing the issue, and perhaps could you have a look at it?

What I was doing with the custom REST resource started to feel wrong, so thanks for confirming that.

14

So I will make two unit tests, one for ComplexObsResource1_10 and one for ComplexObsResource2_0. The tests will just show that editing works with the former resource and fails with the latter resource (by hopefully generating the same APIException as mentioned above in this thread).

Hopefully this will help me figuring out where the change from 1.11.x to 2.x kicks in that breaks the behaviour of my custom resource and handlers.

However, I’m already struggling with step 0 :unamused:, please see this post.

15

Having failing tests for this is the first step in fixing it! :slight_smile:

16

@mksd, I’m going to be offline +/- in full-day meetings all week, so I can’t say much here. But others on this thread can help you better anyway. (And, as Daniel says, sharing a failing test is the way to go!)

17

Sure, hopefully Daniel will be able to help me setting up those tests. I opened another thread about it as I ran straight away into yet another issue.

Thanks all for being on the fence, I really appreciate. I hope that I’ll be able to nail that Platform 2.x compatibility for VDUI by the end of the week. And after that I want to OWA-ize it :wink:

18

Some changes were made to Obs to make them immutable and yes it’s normal if your tests fail against 2.0, you can look at the logs after a failure to trace the line where everything bombs

19

Hi @wyclif and @dkayiwa,

Ok I have done some homework and I have a situation with two almost identical unit tests:

  1. ComplexObsResource1_10Test Showing that everything works fine prior to Platform 2.x.

  2. ComplexObsResource2_0Test Showing that the same logic is failing on Platform 2.x.

You can clone this commit: 6bb9d5a40

When running the failing test, it all dies here in AbstractHandler, and I have no idea why and the exception mentioned above in this thread gets thrown. This is the thread at that point:

Pasted image720×440 68.3 KB

Any clue as to what might be happening or how to work around this would be greatly appreciated.

Thanks again so much Daniel for the guidance on how to setup those tests!

20

@mksd i tried your branch but you had not yet committed your most recent changes and so some tests were failing for other reasons.