Obs save throws 'Cannot edit some fields on obs' due to autoflush of other db operations

Since ImmutableObsInterceptor is introduced in 2.x, it enforces obs not to be edited. We are facing issues around this. We do the following to save the encounter:

  1. Client sends a list of obs to be saved, wrapped inside an EncounterTransaction.
  2. Emrapi maps each obs from EncounterTransaction.Observation contract to Openmrs’ Obs contract.
  3. If obs is new, we create new Obs(), else we fetch the existing obs using its uuid and map the properties from client contract to existing obs. Here we don’t clone the obs and create new one, instead we expect Openmrs’ obsService to do it.
  4. During this process, if obs’ concept is of type Coded, we use the uuid to fetch concept using ConceptService.getConceptByUuid and set it as ValueCoded in Openmrs’ Obs.
  5. After mapping all the obs, we save the encounter using encounterService.saveEncounter which in turn calls the ObsService.saveObs.

During this mapping process, when ConceptService.getConceptByUuid call is made, it auto-flushes entities in the current hibernate session and all the obs are trying to get saved. But since obs is immutable and the edited obs are not cloned into a new obs yet, the interceptor throws error saying ‘Cannot edit some fields on obs’. Here getConceptByUuid has readOnly=true but still it autoflushes because the transaction propagation is REQUIRED and it is called inside a read-write transaction.

We faced this problem before and we fixed by making all the read calls first and then doing other manipulations. In this scenario, doing so will make the code ugly and we would like to know better solution to handle this. Any help?

@preethi_s do you have a unit test reproducing this?

This looks like what i have just got on uat-refapp.openmrs.org when i set the value of “referencedemodata.createDemoPatientsOnNextStartup” to a non zero value in order to have some demo patients.

WARN - ModuleUtil.refreshApplicationContext(889) |2016-10-10 11:57:36,930| Unable to invoke started() method on the module's activator
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)
        at org.hibernate.event.internal.DefaultFlushEntityEventListener.scheduleUpdate(DefaultFlushEntityEventListener.java:293)
        at org.hibernate.event.internal.DefaultFlushEntityEventListener.onFlushEntity(DefaultFlushEntityEventListener.java:160)
        at org.hibernate.event.internal.AbstractFlushingEventListener.flushEntities(AbstractFlushingEventListener.java:231)
        at org.hibernate.event.internal.AbstractFlushingEventListener.flushEverythingToExecutions(AbstractFlushingEventListener.java:102)
        at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:55)
        at org.hibernate.internal.SessionImpl.flush(SessionImpl.java:1258)
        at org.hibernate.internal.SessionImpl.managedFlush(SessionImpl.java:425)
        at org.hibernate.engine.transaction.internal.jdbc.JdbcTransaction.beforeTransactionCommit(JdbcTransaction.java:101)
        at org.hibernate.engine.transaction.spi.AbstractTransactionImpl.commit(AbstractTransactionImpl.java:177)
        at org.springframework.orm.hibernate4.HibernateTransactionManager.doCommit(HibernateTransactionManager.java:584)
        at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:757)
        at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:726)
        at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:515)
        at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:291)
        at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:207)
        at com.sun.proxy.$Proxy166.saveObs(Unknown Source)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:317)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
        at org.openmrs.aop.LoggingAdvice.invoke(LoggingAdvice.java:121)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at org.springframework.aop.framework.adapter.MethodBeforeAdviceInterceptor.invoke(MethodBeforeAdviceInterceptor.java:52)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at org.springframework.aop.framework.adapter.MethodBeforeAdviceInterceptor.invoke(MethodBeforeAdviceInterceptor.java:52)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:207)
        at com.sun.proxy.$Proxy167.saveObs(Unknown Source)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createNumericObs(ReferenceDemoDataActivator.java:550)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createDemoVitalsObs(ReferenceDemoDataActivator.java:537)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createDemoVitalsEncounter(ReferenceDemoDataActivator.java:518)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createDemoVisit(ReferenceDemoDataActivator.java:459)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createDemoPatient(ReferenceDemoDataActivator.java:397)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.createDemoPatients(ReferenceDemoDataActivator.java:365)
        at org.openmrs.module.referencedemodata.ReferenceDemoDataActivator.started(ReferenceDemoDataActivator.java:112)
        at org.openmrs.module.ModuleUtil.refreshApplicationContext(ModuleUtil.java:881)
        at org.openmrs.module.web.WebModuleUtil.refreshWAC(WebModuleUtil.java:866)
        at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:658)
        at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:637)
        at org.openmrs.web.Listener.startOpenmrs(Listener.java:268)
        at org.openmrs.web.WebDaemon$1.run(WebDaemon.java:42)

Discussed on today’s Design Forum, we came up with a couple of hacky workarounds you might try:

  1. You can write a custom DAO method like getConceptByUuidInNewTransaction(String) (or, outside a transaction)
  2. You can evict the obs from the session before you edit it (using Context.evictFromSession)

(In the big picture, we realize that hibernate magic leaking out of the API is a big problem, and we need to address this, but that’s not a quick fix, and any fundamental API changes aren’t really appropriate for a minor/maintenance release.)

For my case, this is what i had to do to locally fix the problem of the referencedemodata module failing to create some demo obs. https://github.com/openmrs/openmrs-core/commit/96f556d3408a The flushing happens after saving the obs, when the method is returning and hence committing the service saveObs method transaction. I did not merge it because it causes a couple of other tests, which expected the hibernate magic, to fail. So it is only to give some deeper understanding of some of the underlying issues.

I don’t think it’s a good thing to immediately make such a change to core just for the sake of adding demo data, a hack could possibly be done in the module and not core.

Ah, I’ve just realized you did this in a branch, nice!! So no revert is needed

FWIW, this is the reordering that i had to make the referencedemodata module work on platform 2.x: https://github.com/openmrs/openmrs-module-referencedemodata/commit/a15882c6b08a3468dd10d7e3ccbe6e9492e5b7a1

Thanks @dkayiwa!