After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 87652 - KVP modification does not change "dirty" flag
KVP modification does not change "dirty" flag
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Engine
git-master
Other All
: Normal normal
: ---
Assigned To: gnucash-core-maint
gnucash-core-maint
Depends on:
Blocks:
 
 
Reported: 2002-07-08 13:59 UTC by Derek Atkins
Modified: 2018-06-29 20:13 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Derek Atkins 2002-07-08 13:59:14 UTC
When you make a change to KVP data, the kvp "owner" is not marked dirty. 
This means that users can lose data if they only make changes to KVP data
without making other changes.  This is particular true of, e.g. the Book
KVP.

A short workaround is to force 'dirty' the object when kvps are changed,
but it would be better if there were some other way to keep track of this
information.
Comment 1 Rolf Leggewie 2008-11-04 21:32:42 UTC
still an issue?
Comment 2 Derek Atkins 2008-11-05 14:42:03 UTC
I believe so, yes.
Comment 3 Phil Longstaff 2009-05-01 19:48:46 UTC
What is wrong with just marking the object dirty?  Any more thoughts on how to handle this?  Note: it's currently assigned to linas.
Comment 4 Derek Atkins 2009-05-01 19:59:25 UTC
There is no reference to the containing object in the KVP Frame.  So there's no way to "mark the object dirty" because when we're modifying the KVP there's no up-reference to the object to mark it.
Comment 5 John Ralls 2013-09-14 04:43:20 UTC
(In reply to comment #4)
> There is no reference to the containing object in the KVP Frame.  So there's no
> way to "mark the object dirty" because when we're modifying the KVP there's no
> up-reference to the object to mark it.

That's because the normal way to get at kvp is to use a function of the form xaccFooGetSlot() which returns a pointer directly into some other  object's state. That's *seriously* bad design.

As a partial remedy, some time ago I went through all of the cases I could find of accessing KVP and wrapped them with the appropriate begin/mark_dirty/commit.
I don't know if I got them all, and I don't see a good way of testing.

The correct solution IMO is to move KVP to the backend as a storage-only solution and to have the data stored as object member variables and to access them correctly via defined API.
Comment 6 John Ralls 2015-07-04 23:20:29 UTC
The normal way to set KVP is now via qof_instance_set_kvp() and qof_book_set_option(), both of which commit the change, as does the special case gnc_ab_set_book_template_list(). The special case of guid lists in Splits, where qof_instance doesn't know how to commit because we don't use vfuncs in engine are all properly wrapped in Splits.c. The case of qof_instance_swap_kvp and qof_instance_copy_kvp are part of the roll-back mechanism and committing isn't appropriate.

Fixed in kvp-cleanup, soon to be merged into master.
Comment 7 John Ralls 2018-06-29 20:13:47 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=87652. Please update any external references or bookmarks.