Great summary, Mark. I think we need to acknowledge that the API ("API
1.0") returns "magical" objects that are tied to the API and have the
tendency to auto-save any changes. I don't see any way to avoid this magic
until we get to API
2.0<https://wiki.openmrs.org/x/SwDX#TowardsAPI2.0-ServiceChangeWishlist>,
meaning I wouldn't expect the current API to support your unit test. The
question is, what guaranteed/predictable behavior for developers can we
provide in the current API? For example:
- Assume changes to domain objects retrieved from the API are persisted
immediately. In fact, changes *may* be persisted during any subsequent
API call and are only guaranteed to be persisted with a call to save() the
object or with a call to Context.closeSession().
- Always call save() after changing an object to (1) guarantee changes
are persisted and (2) ensure your code is future-proofed should the
"auto-save" behavior go away.
Can we gurantee that changes are persisted during a save() of an object?
I'd hope so.
-Burke
On Fri, Mar 9, 2012 at 11:02 AM, Mark Goodrich <[email protected]> wrote:
> This may be completely unhelpful suggestion :) but as I've learned more
> about hibernate it seems like we've been making some basic invalid
> assumptions about how hibernate works. It would take a significant
> reworking, but would it be worth considering redoing the API so that we
> don't have to worry about the flush mode?
>
> The change here would be we'd have to assume that whenever you change an
> object attached to a hibernate session, you are changing that object at the
> DB level, whether or not you call save or update.
>
> For instance, the following unit test fails, but (correct me if I'm
> wrong... maybe I'm the only one that has been confused :) haven't we
> generally been writing code until the assumption that it would pass?
>
> @Test
> public void shouldDisplayDatePropertyAccessor() throws Exception {
>
> Patient patient = Context.getPatientService().getPatient(2);
> Assert.assertEquals("M", patient.getGender());
>
> patient.setGender("F");
>
> // don't call a save patient here
>
> Context.closeSession();
> Context.openSession();
> authenticate();
>
> Patient patient2 =
> Context.getPatientService().getPatient(2);
> Assert.assertEquals("M", patient2.getGender());
>
> }
>
> Take care,
> Mark
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Rafal
> Korytkowski
> Sent: Friday, March 09, 2012 9:54 AM
> To: [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
>
> _________________________________________
>
> 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]
>
>
_________________________________________
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]
