gtg team mailing list archive

[Bertrand Rousseau <bertrand.rousseau@xxxxxxxxx>]

Bertrand Rousseau has proposed merging lp:~bertrand-rousseau/gtg/bugfix-984880-tag-relation-not-stored into lp:gtg.

Requested reviews:
  Gtg developers (gtg)
Related bugs:
  Bug #984880 in Getting Things GNOME!: "GTG doesn't store tag relations sometimes"
  https://bugs.launchpad.net/gtg/+bug/984880

For more details, see:
https://code.launchpad.net/~bertrand-rousseau/gtg/bugfix-984880-tag-relation-not-stored/+merge/120290

Here's a patch that introduces tag tree modifications monitoring from the datastore. This allows to request a save of the tag tree each time a tag is modified. However, in order to reduce the number of save requests, a timeout is added to only request a save 1 second after a node-modified signal is received. That behavior allows to filter unnecessary saves when a burst of "node-modified" signals are sent.

The good part:
 - it fixes bug #984880 (tag relations not saved on kill)
 - tag tree will now be regularly be saved when tags are modified (in the generic sense), which is the feature we want.

The bad part:
 - tag's "node-modified" signals are often sent unnecessarily (this is not related to this patch, it's a consequence of GTG current implementation). For instance, when a task is modified, it always notifies _all_ its tags as modified although most of the times, they are not (it serves as a hack to request a refresh of the tag list counters I suppose). With this patch, this behavior will cause a tag save request to be sent unnecessarily at each task modification. Not cool: performing unnecessary work always increase the risk of errors. We should figure out a way to reduce/remove those requests or to filter them out of the monitoring. No idea so far on how to do this, but I haven't though much about it too.
 - this patch changes the way tags are saved when tag attributes are edited. Before, a save was explicitly requested. Now, it catches the "node-modified" signal, so it happens a bit more indirectly. It may cause side-effects, which will require some testing to figure out.
 - obviously, if GTG is killed before 1 second after a tag is modified, the tag tree won't be updated on the disk. But now, if it does, it's more probably due to a code-related crash rather than a user-related action (indeed, killing GTG explicitly in less than 1 second after having modified a tag is really hard for a user), so we can't guarantee a safe tag tree save anyway.

Release-wise, I think this patch needs to be discussed further, and is too "experimental" to land in 0.3. But I'm interested in discussing options here so I start a merge request anyway.
-- 
https://code.launchpad.net/~bertrand-rousseau/gtg/bugfix-984880-tag-relation-not-stored/+merge/120290
Your team Gtg developers is requested to review the proposed merge of lp:~bertrand-rousseau/gtg/bugfix-984880-tag-relation-not-stored into lp:gtg.

=== modified file 'GTG/core/datastore.py'
--- GTG/core/datastore.py	2012-08-12 22:32:09 +0000
+++ GTG/core/datastore.py	2012-08-19 12:50:45 +0000
@@ -28,6 +28,7 @@
 import threading
 import uuid
 import os.path
+import gobject
 from collections import deque
 
 from GTG.core import requester
@@ -75,6 +76,10 @@
                                       self._activate_non_default_backends)
         self.filtered_datastore = FilteredDataStore(self)
         self._backend_mutex = threading.Lock()
+        # Monitor tag tree modifications
+        self.tag_mods_watch_id = None
+        self.__tagstore.get_main_view().register_cllbck('node-modified', \
+            self.on_tag_modified)
 
     ##########################################################################
     ### Helper functions (get_ methods for Datastore embedded objects)
@@ -271,6 +276,26 @@
 
         cleanxml.savexml(self.tagfile, doc, backup=True)
 
+    def on_tag_modified(self, tid, path=None): # pylint: disable-msg=W0613
+        """Callback: when a tag is modified, we save the tag tree."""
+        # ignore modifications on pseudo-tags
+        if tid == "gtg-tags-all" or \
+           tid == "gtg-tags-none":
+            return
+        # else, we must save the tag tree
+        def __tag_mods_request_save(self):
+            """ Request a tag tree save and unregister the watcher."""
+            self.tag_mods_watch_id = None
+            self.save_tagtree()
+            return False
+        # filter out change requests to reduce unnecessary saves
+        if self.tag_mods_watch_id is None:
+            # There is no watchers for tag modifications. Register one.
+            # We then wait 1 second before saving the tag tree in order to
+            # reduce saves
+            self.tag_mods_watch_id = gobject.timeout_add(1000, \
+                __tag_mods_request_save, self)
+
     ##########################################################################
     ### Tasks functions
     ##########################################################################

=== modified file 'GTG/core/tag.py'
--- GTG/core/tag.py	2012-07-23 14:43:13 +0000
+++ GTG/core/tag.py	2012-08-19 12:50:45 +0000
@@ -95,8 +95,6 @@
             # Attributes should all be strings.
             val = unicode(str(att_value), "UTF-8")
             self._attributes[att_name] = val
-            if self._save:
-                self._save()
             modified = True
         if modified:
             self.modified()