QOF Book Merge design issues

[Neil Williams]

On Sunday 05 December 2004 1:30 am, Derek Atkins wrote:
> Neil,
>
> In my quick attempt to debug the book merge test, I started taking
> a closer look into the book merge code.  I see a major (critical)
> design issue:  you've got static variables holding the merge state!
> BAD idea.

OK, . . . I never envisaged any process having >1 instance of 
qof_book_mergeData though - I couldn't see the need.

> You really need to have a "merge context" that gets created when you
> initialize the merge process and contains all the state of the merge.

Can that be global or does it have to be passed to and fro with every single 
function? Do I have to return a qof_book_mergeData* from Init, pass it to 
qof_book_mergeUpdateResult and redefine the entire 
qof_book_mergeRuleForeachCB callback then pass mergeData back to Commit?

i.e. would just making the current pointers non-static work?

> Otherwise, what happens if the user initiates two merge processes at
> the same time?  Your current code will blow up if the user does that.

Is there a real situation where two instances would be necessary AND 
desirable? 

Couldn't I add code to make the second mergeInit just abort if it can detect 
that another instance already exists? That could be simple as 
g_return_val_if_fail(mergeData != NULL, -1); in qof_book_mergeInit, before 
any of the other code executes or modifies the current statics.

e.g. there would be more trauma if two instances tried to operate on the same 
book. Imagine the nightmare of the importBook of instance A being the 
targetBook of instance B! qof_object_foreach <=> qof_book_mergeForeach may 
descend into an infinite loop / race condition. (Or have I got that mixed 
up?) The only way to detect that is for each instance to know about the other 
and compare QofBook pointers. How would that be achieved?

I can't see it ever being possible to have more than one instance where each 
points to the same targetBook - updating the same entities twice is certain 
to cause data loss. IMHO, merges must be sequential, not simultaneous.

I'm improving the target matching code now and this is one of the bigger 
problems - making absolutely sure that it is never possible for more than one 
import entity to be matched with the same target entity, whilst retaining the 
ability for each import object to be matched with the best possible target.

Equally, I can't see a situation where it would be desirable to have at least 
four QofBooks in memory and then running simultaneous merges on independent 
pairs of QofBooks. 
i.e. 
Merge1: importbook = bookA, targetbook = bookB
Merge2: importbook = bookC, targetbook = bookD

The user would have to intervene in two separate sets of dialogs, and 
presumably run a third merge once the first two complete to merge bookB and 
bookD.

Can users be expected to cope with resolving conflicts in two different but 
simultaneous merges?

> Just my $2.02, but you really need to pass around your context instead
> of making it a static.

That complicates the API considerably. e.g. using existing qof_object_foreach 
and other callbacks becomes tricky, all kinds of extra data gets put into 
qof_book_mergeData just so that it can be passed around as a gpointer 
user_data. 

> -derek

-- 

Neil Williams
=============
http://www.codehelp.co.uk/
http://www.dclug.org.uk/
http://www.isbn.org.uk/
http://sourceforge.net/projects/isbnsearch/

http://www.biglumber.com/x/web?qs=0x8801094A28BCB3E3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20041205/13dae1bf/attachment.bin