Re: OSWF: Weekly OSWorkflow complaint list

On Monday, October 20, 2003, at 02:43 PM, Nick Dellamaggiore wrote:

> JIRA is still down. I'll post these once it comes back up... Don't take
> these personally, I'm happy to provide patches.
>
> Four Issues:
>
> 1.  HibernateWorkflowStore incomplete?
> I noticed that the hibernate WorkflowStore implementation doesn't 
> write to
> the "link" tables (i.e. OS_CURRENTSTEP_PREV, OS_HISTORYSTEP_PREV). 
> These
> tables provide linkage information between workflow steps and 
> essentially
> build the workflow "tree" (with initial step being the root).  Was 
> their
> omission just an oversight or was it intentional? I don't have a patch 
> at
> the moment, but I think the fix would be pretty easy (just adding some
> many-to-ones to the mapping files).
>
Yep, makes sense. Submit a patch!

> 2. JDBCWorkflowStore (and HibernateWorkflowStore) cannot be 
> incorporated
> into transactions
> I'm running some JDBC updates to some of my application tables along 
> with an
> osworkflow step transaction and I want to incorporate all database 
> changes
> into a single transaction. Now, I've overrided the the convenient
> getConnection() method to return a ThreadLocal Connection that my 
> other JDBC
> updates were running through.  But, since JDBCWorkflowStore always 
> calls
> cleanup( conn, stmt, rset) (which closes the connection) after EVERY 
> method
> call, not only does my transaction end, but the connection is closed 
> behind
> my back! evil.  I have a patch that will only close the connection if
> getConnection() was not overridden.  This way, my ThreadLocal 
> Connection
> (which has autoCommit == false) will be used, but not closed or 
> committed by
> JDBCWorkflowStore.  The patch also doesn't throw StoreException when 
> the
> JNDI datasource is not defined in osworkflow.xml (since getConnection()
> provides the Connections, not jndi)
>
This is more problematic. The correct solution really is to have a 
Workflow class (similar to OfbizWorkflow) which can appropriately start 
off a tx and commit it in the Workflow methods. Then again, we end up 
with new stores having to implement two classes realistically, a store 
and a workflow for tx'ness.

Due to this problem, making JDBC tx-safe is not trivial nor easy, I 
think. I personally would discourage people from using it for this 
reason. I don't see a strong argument for it, since I think that 
between them, ofbiz, ejb, and hibernate cover all possible deployment 
situations...or am I wrong?

> 3. Bug in JDBCWorkflowStore
> getEntryState() does a conn = ds.getConnection() instead of a conn =
> getConnection(). easy fix. I included it in the patch for #2.
>
Fixed, thanks.

> 4.  AbstractWorkflow uses WorkflowStore inefficiently
> I notice this when I switched over to JDBCWorkflowStore. A single call 
> to
> doAction() fires THREE calls to findEntry() (one in doAction(), one in
> getAvailableActions() and one in completeEntry().  I understand that 
> both
> EBJ, HIbernate, etc cache this object for you. But, JDBC does not.  How
> about doing findEntry() once and then passing it around in method 
> signatures
> (passing null => do lookup).  Or caching it locally?
>
Yeah, again this is another strike against JDBC workflow store. I'll 
have a look at modifying some of the methods to pass around stuff to 
reduce the number of lookups (and accept patches for that). Note though 
that there is an important restriction on the acceptance of any patches 
that do this, that nothing that alters the client facing API will be 
added. So if you change protected/private methods only, all is well.



-------------------------------------------------------
This SF.net email is sponsored by OSDN developer relations
Here's your chance to show off your extensive product knowledge
We want to know what you know. Tell us and you have a chance to win $100
http://www.zoomerang.com/survey.zgi?HRPT1X3RYQNC5V4MLNSV3E54