(sorry about that, halfway through drafting my "clarification", gmail
deciding to send my e-mail)
Mark,
I guess I'm not getting my point across. Let me try a different way. I'm
proposing that we teach developers something like this:
*Example 1:*
Patient patient = Context.getPatientService().getPatient(2);
patient.setGender("F");
// assume gender is persisted, but don't count on it (not guaranteed)
Context.getPatientService().savePatient(patient);
// change in gender is now guaranteed because of save
*Example 2:*
Context.openSession();
Patient patient = Context.getPatientService().getPatient(2);
patient.setGender("F");
// assume gender is persisted, but don't count on it (not guaranteed)
// oops! forgot to call savePatient() … this is bad practice.
Context.closeSession();
// change in gender is now guaranteed to be persisted, because
// closeSession() will guarantee unsaved changes get saved;
// however, relying on this will ensure your code breaks when
// we go to API 2.0
By flushing in API save methods & in Context.closeSession, we guarantee
changes get saved. By recommending that people call API save methods after
changes, we promote more robust (API 2.0-friendly) code. Then our API docs
just need to communicate something like:
"Changes to domain objects retrieved from the API may be autosaved between
the time you make the change and when you invoke the API's save method, so
don't make changes to domain objects retrieved from the API if you don't
want those changes persisted. You should not rely on this autosaving, but
continue to call the API's save method both to guarantee changes are saved
and to keep your code compatible with future versions of the API.
Autosaving (before the save method is called) is a confusing 'feature' of
the API and will be removed in API 2.0."
I don't think we should be trying to convince ourselves or anyone else that
we can change domain objects retrieved from the API and reliably avoid
having those changes persisted. On the other hand, we are not in a
position to guarantee that autosaving will always happen and when/where it
will happen, so the best we can do is explain the behavior and provide a
gurantee of persistence following a save or closeSession.
Clear as mud?
-Burke
On Fri, Mar 9, 2012 at 4:26 PM, Mark Goodrich <[email protected]> wrote:
>
>> Burke,****
>>
>> ** **
>>
>> The thing is, I don’t know if we can change the API so it that doesn’t
>> behave unexpectedly (sometimes) without a major revision to the API.****
>>
>> ** **
>>
>> I don’t think we can 100% enforce that objects aren’t persisted until
>> save is called unless we do a major rework. So do we say (and make a few
>> tweaks if needed) “Assume that changes to objects retrieved from the API *
>> *are** persisted whenever a transaction ends or a session is closed,
>> regardless of whether a save method is called; please disregard the fact
>> that much of the existing code is written as if save methods need to be
>> called explicitly” :)****
>>
>> ** **
>>
>> Mark****
>>
>> ** **
>>
>> ** **
>>
>> ** **
>>
>> *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Burke
>> Mamlin
>> *Sent:* Friday, March 09, 2012 3:01 PM
>>
>> *To:* [email protected]
>> *Subject:* Re: [OPENMRS-DEV] Hibernate flush mode****
>>
>> ** **
>>
>> Rafał,****
>>
>> ** **
>>
>> I have a launchbar <http://www.obdev.at/products/launchbar/index.html> script
>> for your name (out of respect to you, of course). ;-)****
>>
>> ** **
>>
>> I'd rather have an explicit, well-explained, and reliable API that
>> doesn't work the way we would like (yet) instead of an API trying to be
>> something that it isn't and, therefore, behave unexpectedly &
>> unpredictabely. For example, "Write code that saves changes, but assume
>> changes to objects retrieved from the API *may* be persisted before you
>> call save(); this will be fixed in the next major revision of the API."**
>> **
>>
>> ** **
>>
>> Cheers,****
>>
>> ** **
>>
>> -Burke****
>>
>> ** **
>>
>> On Fri, Mar 9, 2012 at 2:09 PM, Rafal Korytkowski <[email protected]>
>> wrote:****
>>
>> Ben, I don't see a problem in clearing the session in our
>> Context.closeSession(). It'll work in Mark's case, but it won't fix
>> all the problems. If Hibernates chooses to flush the session before
>> you call Context.clearSesssion() and Context.closeSession(), the
>> changes will hit the db and when you call Context.openSession() and
>> try to retrieve the data you'll see them as if they were persisted
>> anyway as long as the transaction wasn't rolled back.
>>
>> Burke, where do you find "ł" on your keyboard ;-) Hibernate chooses to
>> flush at very specific points. That is whenever you try to get
>> something from the db and it's not available in the session (cache) or
>> it's in the session, but marked as modified. It tries not to be eager
>> with flushes and does it only in cases when you access the same table
>> as you modified, but as reported here you cannot rely on that:
>> https://hibernate.onjira.com/browse/HHH-2399
>>
>> In TRUNK-3069 I made flushes happen whenever you called save...,
>> void..., etc. and only then. At least it's when you would normally
>> expect them to happen. The drawback is still that all objects are
>> flushed not only the one you intend.
>>
>> I don't like it anymore for the reason that it won't work with the MDS
>> module that needs exclusive control over the Hibernate flushes and it
>> won't be easy to make it work with that approach, unless I'll open a
>> back gate to disable Context.flushSession() ;-)
>>
>> I also agree with Mark that we shouldn't be fighting with
>> Hibernate/JPA most of the time, but accept its pros and cons and maybe
>> consider detaching objects from Hibernate in API 2.0.
>>
>> -Rafal
>>
>> 2012/3/9 Mark Goodrich <[email protected]>:****
>>
>> > It just seems like the way our API is written and is used, the
>> assumption is that changes made to a domain object are only persisted if a
>> saveObject() service method is called. But by default Hibernate persists
>> changes to any attached objects automatically, and we are struggling
>> without complete success to stop "premature" flushes, which make the prior
>> assumption false. I haven't read the API 2.0 notes, but my suggestion
>> would be in 2.0 we give up on that assumption instead of trying to enforce
>> it.
>> >
>> > Mark
>> > ________________________________________
>> > From: [email protected] [[email protected]] On Behalf Of Burke Mamlin [
>> [email protected]]
>> > Sent: Friday, March 09, 2012 12:54 PM
>> > To: [email protected]
>> > Subject: Re: [OPENMRS-DEV] Hibernate flush mode
>> >
>> > Rafał,
>> >
>> > If calls the API are made within the "// don't call a save patient
>> here" excerpt in Mark's example, isn't it likely that the change will be
>> persisted if Hibernate chooses to flush at any point?
>> >
>> > It seems that both save() methods and Context.closeSession() should
>> perform a flush – i.e., ensure Hibernate has a chance to save any unsaved
>> changes. Then we can at least provide a guarantee that a save() method or
>> closeSession() will avoid persisting changes.
>> >
>> > -Burke
>> >
>> > On Fri, Mar 9, 2012 at 12:23 PM, Rafal Korytkowski <[email protected]
>> <mailto:[email protected]>> wrote:
>> > The problem in Mark's test is that when you call
>> > Context.closeSession() within a transaction (our unit tests are
>> > transactional by default) it only dettaches the session from the
>> > transaction manager and doesn't close or even clear the session. It's
>> > stored and attached again when you call Context.openSession(). If you
>> > call Context.clearSession() before Context.closeSession() then it'll
>> > work as you expect.
>> >
>> > Roger, Ben explained the problem with dettaching objects from
>> > Hibernate. Our domain objects are interconnected and lazily
>> > initialized and we would have to initialize them prior to dettaching
>> > from Hibernate. Sometimes a graph of objects that needs to be
>> > initialized is quite vast and it could kill performance. It would
>> > require some redesigning of our API and our business model classes,
>> > which is quite a big change.
>> >
>> > -Rafal
>> >
>> > On 9 March 2012 17:50, Friedman, Roger (CDC/CGH/DGHA) (CTR)
>> > <[email protected]<mailto:[email protected]>> wrote:
>> >> Rafa --
>> >> Thanks for the spadework.
>> >> ConceptDAO contains a wealth of not so great ideas. I think
>> working on REST has made us more aware of subclasses and helper classes and
>> how to represent them.
>> >> Is it an option to have only the DAO layer deal with
>> Hibernate-connected objects, to have it serve disconnected objects on read
>> and reconnect them on write? Wouldn't that mean our services and the API
>> would no longer be engaged with the session and its cache?
>> >> Saludos, Roger
>> >>
>> >> -----Original Message-----
>> >> From: [email protected]<mailto:[email protected]> [mailto:[email protected]
>> <mailto:[email protected]>] On Behalf Of Rafal Korytkowski
>> >> Sent: Friday, March 09, 2012 9:54 AM
>> >> To: [email protected]<mailto:
>> [email protected]>
>> >> Subject: [OPENMRS-DEV] Hibernate flush mode
>> >>
>> >> Hey,
>> >>
>> >> I have been experimenting with Hibernate flush modes recently. The
>> motivation was that we experienced premature flushes triggered by Hibernate
>> while retrieving data from the DB, which made us temporarily switch from
>> FlushMode.AUTO to FlushMode.COMMIT or MANUAL to execute some parts of code
>> like validation.
>> >>
>> >> The initial attempt for a more general solution was TRUNK-3069, which
>> switched from the AUTO to COMMIT mode for all transactions. The flush was
>> triggered by us around any method annotated with
>> @Transactional(readOnly=false), but not around
>> @Transactional(readOnly=true). It seemed like a good approach at first, but
>> then I discovered TRUNK-3103. The problem could be eventually resolved by
>> annotating with @Transactional dao methods, which we are considering.
>> >>
>> >> Anyway TRUNK-3069 disables much of Hibernate functionality to handle
>> flushes for us and now I think it is not how we should approach the problem.
>> >>
>> >> I believe we need to stay with the AUTO flush mode and tune it only
>> when it is needed. Unfortunately, we cannot change the flush mode in any
>> other place than the DAO layer where we have access to hibernate's session,
>> whereas most of the time we actually need to control it in the service
>> layer.
>> >>
>> >> So far whenever we needed to execute a service method in the manual
>> flush mode our approach was to go down to the DAO layer, which resulted in
>> such strange constructions as in getDefaultConceptMapType [0], where we put
>> in the DAO layer code that really belonged to the service layer.
>> >>
>> >> We have a few possibilities to deal with that.
>> >>
>> >> 1. We continue to handle the flush issue in DAOs the way it was before.
>> >> 2. We have something like CustomSessionFlushTask [1].
>> >> 3. We have Context.getFlushMode() and Context.setFlushMode(flushMode).
>> >> We need our own FlushMode enum so that we don't introduce a dependency
>> on Hibernate in the service layer.
>> >> 4. We have @ManualFlush annotation to annotate service methods that we
>> want to explicitly execute in the manual flush mode. It's a more elegant
>> variation of 2., but slightly less useful since it requires to create a
>> dedicated method. For instance we have the getConcept method and if we want
>> it to be executed in one place only in the manual flush mode we need to
>> create a second method getConceptInManualFlush for the purpose of
>> annotating it with @ManualFlush.
>> >>
>> >> I am really curious what do you think or if there is anyone who has
>> more experience with that.
>> >>
>> >> [0] -
>> https://source.openmrs.org/browse/~br=1.9.x/OpenMRS/branches/1.9.x/api/src/main/java/org/openmrs/api/db/hibernate/HibernateConceptDAO.java?r=26243
>> >> [1] -
>> https://source.openmrs.org/browse/~br=trunk/Modules/metadatasharing/trunk/src/org/openmrs/module/metadatasharing/api/db/hibernate/CustomSessionFlushTask.java?r=26268
>> >>
>> >> -Rafal
>> >>
>>
>> ****
>>
>> ** **
>> ------------------------------
>>
>> Click here to
>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from
>> OpenMRS Developers' mailing list
>> ****
>>
>
>
_________________________________________
To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to
[email protected] with "SIGNOFF openmrs-devel-l" in the body (not
the subject) of your e-mail.
[mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
