Bug #638225: Sort when saving as XML

Patch by Jim Radford (with code beautified and re-indented by myself):

The attached patches sort the slots, lots, book accounts, bill terms,
customers, employees, entries, invoices, jobs, orders, tax tables and
vendors before saving them to the GnuCash XML file.

This is an attempt to make saves more idempotent thereby facilitating
the use of a revision control system on the GnuCash XML files.

With these patches most of the needless and seemingly random churn is
gone and I can add or remove a transaction and expect
there to be no unrelated changes to the GnuCash file.

I've been using and refining this patches for the last few years, so it has
received quite a bit of testing.

David Fraser adds: Without specific testing, I'm using this on an average-sized gnucash file
(5.7MB) without noticing any particular slowdown in saving, but a wonderful
reduction in diffs when comparing changes.

git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@20067 57a11ea4-9604-0410-9ed3-97b8803252fd

src/backend/xml/gnc-account-xml-v2.c

@@ -158,6 +158,8 @@ gnc_account_dom_tree_create(Account *act,
     {
         xmlNodePtr toaddto = xmlNewChild(ret, NULL, BAD_CAST act_lots_string, NULL);
 
+        lots = g_list_sort(lots, qof_instance_guid_compare);
+
         for (n = lots; n; n = n->next)
         {
             GNCLot * lot = n->data;

src/backend/xml/gnc-bill-term-xml-v2.c

@@ -527,7 +527,7 @@ xml_add_billterm (QofInstance *term_p, gpointer out_p)
 static gboolean
 billterm_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_billterm, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_billterm, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-book-xml-v2.c

@@ -69,6 +69,7 @@ append_account_tree (xmlNodePtr parent,
     GList *children, *node;
 
     children = gnc_account_get_children(account);
+    children = g_list_sort(children, qof_instance_guid_compare);
     for (node = children; node; node = node->next)
     {
         xmlNodePtr accnode;

src/backend/xml/gnc-customer-xml-v2.c

@@ -513,7 +513,7 @@ xml_add_customer (QofInstance * cust_p, gpointer out_p)
 static gboolean
 customer_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_customer, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_customer, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-employee-xml-v2.c

@@ -436,7 +436,7 @@ xml_add_employee (QofInstance * employee_p, gpointer out_p)
 static gboolean
 employee_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_employee, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_employee, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-entry-xml-v2.c

@@ -832,7 +832,7 @@ xml_add_entry (QofInstance * entry_p, gpointer out_p)
 static gboolean
 entry_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_entry, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_entry, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-invoice-xml-v2.c

@@ -542,7 +542,7 @@ xml_add_invoice (QofInstance * invoice_p, gpointer out_p)
 static gboolean
 invoice_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_invoice, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_invoice, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-job-xml-v2.c

@@ -329,7 +329,7 @@ xml_add_job (QofInstance * job_p, gpointer out_p)
 static gboolean
 job_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_job, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_job, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-order-xml-v2.c

@@ -372,7 +372,7 @@ xml_add_order (QofInstance * order_p, gpointer out_p)
 static gboolean
 order_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_order, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_order, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-tax-table-xml-v2.c

@@ -489,7 +489,7 @@ xml_add_taxtable (QofInstance * table_p, gpointer out_p)
 static gboolean
 taxtable_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_taxtable, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_taxtable, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/gnc-vendor-xml-v2.c

@@ -457,7 +457,7 @@ xml_add_vendor (QofInstance * vendor_p, gpointer out_p)
 static gboolean
 vendor_write (FILE *out, QofBook *book)
 {
-    qof_object_foreach (_GNC_MOD_NAME, book, xml_add_vendor, (gpointer) out);
+    qof_object_foreach_sorted (_GNC_MOD_NAME, book, xml_add_vendor, (gpointer) out);
     return ferror(out) == 0;
 }
 

src/backend/xml/sixtp-dom-generators.c

@@ -334,13 +334,15 @@ add_kvp_value_node(xmlNodePtr node, gchar *tag, kvp_value* val)
     case KVP_TYPE_GLIST:
     {
         GList *cursor;
+        GList *sorted = g_list_sort(g_list_copy(kvp_value_get_glist(val)), (GCompareFunc)strcmp);
 
         xmlSetProp(val_node, BAD_CAST "type", BAD_CAST "list");
-        for (cursor = kvp_value_get_glist(val); cursor; cursor = cursor->next)
+        for (cursor = sorted; cursor; cursor = cursor->next)
         {
             kvp_value *val = (kvp_value*)cursor->data;
             add_kvp_value_node(val_node, "slot:value", val);
         }
+        g_list_free(sorted);
     }
 
     break;
@@ -354,8 +356,8 @@ add_kvp_value_node(xmlNodePtr node, gchar *tag, kvp_value* val)
         if (!frame || !kvp_frame_get_hash (frame))
             break;
 
-        g_hash_table_foreach(kvp_frame_get_hash(frame),
-                             add_kvp_slot, val_node);
+        g_hash_table_foreach_sorted(kvp_frame_get_hash(frame),
+                                    add_kvp_slot, val_node, (GCompareFunc)strcmp);
     }
     break;
 
@@ -397,7 +399,8 @@ kvp_frame_to_dom_tree(const char *tag, const kvp_frame *frame)
 
     ret = xmlNewNode(NULL, BAD_CAST tag);
 
-    g_hash_table_foreach(kvp_frame_get_hash(frame), add_kvp_slot, ret);
+    g_hash_table_foreach_sorted(kvp_frame_get_hash(frame),
+                                add_kvp_slot, ret, (GCompareFunc)strcmp);
 
     return ret;
 }

src/libqof/qof/qofobject.c

@@ -189,6 +189,31 @@ qof_object_foreach (QofIdTypeConst type_name, QofBook *book,
     return;
 }
 
+static void
+do_append (QofInstance *qof_p, gpointer list_p)
+{
+    GList **list = list_p;
+    *list = g_list_append(*list, qof_p);
+}
+
+void
+qof_object_foreach_sorted (QofIdTypeConst type_name, QofBook *book, QofInstanceForeachCB cb, gpointer user_data)
+{
+    GList *list = NULL;
+    GList *iter;
+
+    qof_object_foreach(type_name, book, do_append, &list);
+
+    list = g_list_sort(list, qof_instance_guid_compare);
+
+    for (iter = list; iter; iter = iter->next)
+    {
+        cb(iter->data, user_data);
+    }
+
+    g_list_free(list);
+}
+
 const char *
 qof_object_printable (QofIdTypeConst type_name, gpointer obj)
 {
@@ -370,7 +395,7 @@ void qof_object_foreach_backend (const char *backend_name,
     cb_data.cb = cb;
     cb_data.user_data = user_data;
 
-    g_hash_table_foreach (ht, foreach_backend, &cb_data);
+    g_hash_table_foreach_sorted (ht, foreach_backend, &cb_data, (GCompareFunc)strcmp);
 }
 
 /* ========================= END OF FILE =================== */

src/libqof/qof/qofobject.h

@@ -162,6 +162,10 @@ void qof_object_foreach_type (QofForeachTypeCB cb, gpointer user_data);
 void qof_object_foreach (QofIdTypeConst type_name, QofBook *book,
                          QofInstanceForeachCB cb, gpointer user_data);
 
+/** Invoke callback 'cb' on each instance in guid orted order */
+void qof_object_foreach_sorted (QofIdTypeConst type_name, QofBook *book,
+                                QofInstanceForeachCB cb, gpointer user_data);
+
 /** Register and lookup backend-specific data for this particular object */
 gboolean qof_object_register_backend (QofIdTypeConst type_name,
                                       const char *backend_name,

src/libqof/qof/qofutil.c

@@ -36,6 +36,20 @@
 
 static QofLogModule log_module = QOF_MOD_UTIL;
 
+void
+g_hash_table_foreach_sorted(GHashTable *hash_table, GHFunc func, gpointer user_data, GCompareFunc compare_func)
+{
+    GList *iter;
+    GList *keys = g_list_sort(g_hash_table_get_keys(hash_table), compare_func);
+
+    for (iter = keys; iter; iter = iter->next)
+    {
+        func(iter->data, g_hash_table_lookup(hash_table, iter->data), user_data);
+    }
+
+    g_list_free(keys);
+}
+
 gboolean
 qof_utf8_substr_nocase (const gchar *haystack, const gchar *needle)
 {

src/libqof/qof/qofutil.h

@@ -166,6 +166,12 @@ void qof_close (void);
 
 /* **** Prototypes *********************************************/
 
+/** Calls the given function for each of the key/value pairs in the
+ *  GHashTable in an order determined by the GCompareFunc applied to
+ *  the keys. The function is passed the key and value of each pair,
+ *  and the given user_data parameter. */
+void g_hash_table_foreach_sorted(GHashTable *hash_table, GHFunc func, gpointer user_data, GCompareFunc compare_func);
+
 /** Search for an occurence of the substring needle in the string
  * haystack, ignoring case. Return TRUE if one is found or FALSE
  * otherwise. */