[Gnucash-changes] r13351 - gnucash/trunk/lib/libqof/qof - Mark the QofCollection as dirty when a new QofEntity is added or removed.

[Chris Shoemaker]

On Tue, Feb 21, 2006 at 04:55:44PM -0500, Derek Atkins wrote:
> I'm not convinced this patch is correct..  If the QofInstance is instantly
> saved (ala SQL) then it's unclear that the collection dirty flag is
> properly cleared.  

Well, it _should_ be properly cleared.

> That's why the qof collection dirty flag is usually
> inherited from the objects in the collection.

Technically, QofCollections actually can't inherit dirty state from
the objects in the collection, because the objects in the collection
are QofEntitys, not QofInstances, and the Instances have the dirty
flag but Entities do not.  That's why the QofCollections can't
 query the dirty state of their entities.  Instead, they can only
reflect their own dirty state:

gboolean 
qof_collection_is_dirty (QofCollection *col)
{
   return col ? col->is_dirty : FALSE;
}

However, in practice, the QofCollections do tend to reflect the dirty
state of the contained QofInstances, because dirtying a QofInstance
looks like:

void
qof_instance_set_dirty(QofInstance* inst)
{
        QofCollection *coll;

        inst->dirty = TRUE;
        coll = inst->entity.collection;
        qof_collection_mark_dirty(coll);
}

However, there must be _some_ way to represent the state where all the
QofInstances in the Collection are clean while the QofCollection is
dirty (for when Instances are removed from the Collection).  I
understand that an SQL backend shouldn't ever be in this state, but we
still have to be able to represent it.  That seems to be exactly what
the Collection's dirty flag is for, and that's how this patch uses it.

>From our discussion on IRC, I understand you'd prefer that
QOF_COMMIT_EDIT call qof_collection_mark_dirty and only when it
detects that it has to because the backend didn't instantly commit.  I
see a couple disadvantages there.  1) I don't know how to tell whether
the backend really committed or not.  2) It seems that the
QofCollection dirty state should be meaningful when used with
QofEntitys that aren't QofInstances, but the QOF_COMMIT_EDIT macros
are Instance-level idioms.

I think the patch is semantically correct, and I think it should be
the responsibility of whatever peice of code actually commits the
dirtied state to call qof_collection_mark_clean().  If that means that
the SQL backend is marking the collection clean after every commit
that's okay.  Afterall, every call to qof_instance_set_dirty is
marking the collection dirty, and in the very short time between the
dirtying event and the actual commit, I think the flag really should
say 'dirty'.

-chris

> 
> -derek
> 
> Quoting Chris Shoemaker <chris at cvs.gnucash.org>:
> 
> >Author: chris
> >Date: 2006-02-21 16:46:07 -0500 (Tue, 21 Feb 2006)
> >New Revision: 13351
> >Trac: http://svn.gnucash.org/trac/changeset/13351
> >
> >Modified:
> >  gnucash/trunk/lib/libqof/qof/qofid.c
> >Log:
> >  Mark the QofCollection as dirty when a new QofEntity is added or removed.
> >
> >
> >Modified: gnucash/trunk/lib/libqof/qof/qofid.c
> >===================================================================
> >--- gnucash/trunk/lib/libqof/qof/qofid.c	2006-02-21 21:44:42 UTC (rev 
> >13350)
> >+++ gnucash/trunk/lib/libqof/qof/qofid.c	2006-02-21 21:46:07 UTC (rev 
> >13351)
> >@@ -182,6 +182,7 @@
> >  col = ent->collection;
> >  if (!col) return;
> >  g_hash_table_remove (col->hash_of_entities, &ent->guid);
> >+  qof_collection_mark_dirty(col);
> >  ent->collection = NULL;
> >}
> >
> >@@ -193,6 +194,7 @@
> >  g_return_if_fail (col->e_type == ent->e_type);
> >  qof_collection_remove_entity (ent);
> >  g_hash_table_insert (col->hash_of_entities, &ent->guid, ent);
> >+  qof_collection_mark_dirty(col);
> >  ent->collection = col;
> >}
> >
> >@@ -208,6 +210,7 @@
> >	e = qof_collection_lookup_entity(coll, &ent->guid);
> >	if ( e != NULL ) { return FALSE; }
> >	g_hash_table_insert (coll->hash_of_entities, &ent->guid, ent);
> >+	qof_collection_mark_dirty(coll);
> >	return TRUE;
> >}
> >
> >
> >_______________________________________________
> >gnucash-changes mailing list
> >gnucash-changes at gnucash.org
> >https://lists.gnucash.org/mailman/listinfo/gnucash-changes
> >
> 
> 
> 
> -- 
>       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
>       Member, MIT Student Information Processing Board  (SIPB)
>       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
>       warlord at MIT.EDU                        PGP key available
>