gnucash maint: Bug 797983 - v4.2 report numbers change over gnucash restarts; ...

John Ralls jralls at code.gnucash.org
Sun Oct 18 19:12:32 EDT 2020


Updated	 via  https://github.com/Gnucash/gnucash/commit/94a68dca (commit)
	from  https://github.com/Gnucash/gnucash/commit/e255a7bf (commit)



commit 94a68dca7cf94bf23b808986c8df19449da4f2bb
Author: John Ralls <jralls at ceridwen.us>
Date:   Sun Oct 18 15:04:06 2020 -0700

    Bug 797983 - v4.2 report numbers change over gnucash restarts;...
    
    Price Database dropping user:price-editor entries.
    
    A wider problem: QofInstance was unmarking itself dirty as long as the
    backend raised an error and unconditionally marking itself non-infant.
    
    This matters because the SQL backend depends on infant status to decide
    whether to do an insert or update query; either will fail if the infant
    status is wrong.
    
    The price editor dialog clones a price having only its commodity set and
    GncSqlPriceBackend declines to save if the currency isn't set. Since the
    instance marked itself non-infant even though it wasn't saved subsequent
    commits tried to use an update query and since the price wasn't in the
    table that inevitably failed.
    
    Requiring that QofBackend::commit should doing the marking-clean
    required implementing it in the backends where it wasn't already.

diff --git a/libgnucash/backend/xml/gnc-xml-backend.cpp b/libgnucash/backend/xml/gnc-xml-backend.cpp
index 8f7b01070..dc3bbb19b 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.cpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.cpp
@@ -314,6 +314,13 @@ GncXmlBackend::sync(QofBook* book)
     remove_old_files();
 }
 
+void
+GncXmlBackend::commit(QofInstance* instance)
+{
+    if (qof_instance_is_dirty(instance))
+        qof_instance_mark_clean(instance);
+}
+
 bool
 GncXmlBackend::save_may_clobber_data()
 {
diff --git a/libgnucash/backend/xml/gnc-xml-backend.hpp b/libgnucash/backend/xml/gnc-xml-backend.hpp
index 362038452..fd84c5bbb 100644
--- a/libgnucash/backend/xml/gnc-xml-backend.hpp
+++ b/libgnucash/backend/xml/gnc-xml-backend.hpp
@@ -43,6 +43,7 @@ public:
     void export_coa(QofBook*) override;
     void sync(QofBook* book) override;
     void safe_sync(QofBook* book) override { sync(book); } // XML sync is inherently safe.
+    void commit(QofInstance* instance) override;
     const char * get_filename() { return m_fullpath.c_str(); }
     QofBook* get_book() { return m_book; }
 
diff --git a/libgnucash/engine/qof-backend.cpp b/libgnucash/engine/qof-backend.cpp
index 41c8df220..c18b8490e 100644
--- a/libgnucash/engine/qof-backend.cpp
+++ b/libgnucash/engine/qof-backend.cpp
@@ -48,6 +48,13 @@ G_GNUC_UNUSED static QofLogModule log_module = QOF_MOD_BACKEND;
 
 GModuleVec QofBackend::c_be_registry{};
 
+void
+QofBackend::commit(QofInstance* instance)
+{
+    if (qof_instance_is_dirty(instance))
+        qof_instance_mark_clean(instance);
+}
+
 void
 QofBackend::set_error(QofBackendError err)
 {
diff --git a/libgnucash/engine/qof-backend.hpp b/libgnucash/engine/qof-backend.hpp
index 5fd51e42d..aac960252 100644
--- a/libgnucash/engine/qof-backend.hpp
+++ b/libgnucash/engine/qof-backend.hpp
@@ -220,7 +220,7 @@ public:
 /**
  *    Commits the changes from the engine to the backend data storage.
  */
-    virtual void commit (QofInstance*) {}
+    virtual void commit (QofInstance*);
 /**
  *    Revert changes in the engine and unlock the backend.
  */
diff --git a/libgnucash/engine/qofinstance.cpp b/libgnucash/engine/qofinstance.cpp
index 9f2952f3c..980e43bf9 100644
--- a/libgnucash/engine/qofinstance.cpp
+++ b/libgnucash/engine/qofinstance.cpp
@@ -1030,10 +1030,9 @@ qof_commit_edit_part2(QofInstance *inst,
                 on_error(inst, errcode);
             return FALSE;
         }
-        /* XXX the backend commit code should clear dirty!! */
-        priv->dirty = FALSE;
+        if (!priv->dirty) //Cleared if the save was successful
+            priv->infant = FALSE;
     }
-    priv->infant = FALSE;
 
     if (priv->do_free)
     {
diff --git a/libgnucash/engine/test/test-qofinstance.cpp b/libgnucash/engine/test/test-qofinstance.cpp
index 301c7e289..1f979a161 100644
--- a/libgnucash/engine/test/test-qofinstance.cpp
+++ b/libgnucash/engine/test/test-qofinstance.cpp
@@ -76,6 +76,8 @@ public:
         g_assert( commit_test.m_inst == inst );
         g_assert( commit_test.m_be == this );
         commit_test.m_commit_called = true;
+        if (qof_instance_is_dirty(inst))
+            qof_instance_mark_clean(inst);
         set_error(m_qof_error);
 
     }
@@ -569,7 +571,7 @@ test_instance_commit_edit_part2( Fixture *fixture, gconstpointer pData )
     result = qof_commit_edit_part2( fixture->inst, NULL, NULL, NULL );
     g_assert( result );
     g_assert( qof_instance_get_dirty_flag( fixture->inst ) );
-    g_assert( !qof_instance_get_infant( fixture->inst ) );
+    g_assert( qof_instance_get_infant( fixture->inst ) );
     g_assert( !commit_test.m_commit_called );
     g_assert( !commit_test.m_commit_with_err_called );
     g_assert( !commit_test.m_on_error_called );
@@ -581,6 +583,7 @@ test_instance_commit_edit_part2( Fixture *fixture, gconstpointer pData )
     result = qof_commit_edit_part2( fixture->inst, on_error, on_done, on_free );
     g_assert( result );
     g_assert( qof_instance_get_dirty_flag( fixture->inst ) );
+    g_assert( qof_instance_get_infant( fixture->inst ) );
     g_assert( !commit_test.m_commit_called );
     g_assert( !commit_test.m_commit_with_err_called );
     g_assert( !commit_test.m_on_error_called );
@@ -593,6 +596,7 @@ test_instance_commit_edit_part2( Fixture *fixture, gconstpointer pData )
     result = qof_commit_edit_part2( fixture->inst, on_error, on_done, on_free );
     g_assert( result );
     g_assert( qof_instance_get_dirty_flag( fixture->inst ) );
+    g_assert( qof_instance_get_infant( fixture->inst ) );
     g_assert( !commit_test.m_commit_called );
     g_assert( !commit_test.m_commit_with_err_called );
     g_assert( !commit_test.m_on_error_called );
@@ -606,6 +610,7 @@ test_instance_commit_edit_part2( Fixture *fixture, gconstpointer pData )
     result = qof_commit_edit_part2( fixture->inst, on_error, on_done, on_free );
     g_assert( result );
     g_assert( !qof_instance_get_dirty_flag( fixture->inst ) );
+    g_assert( !qof_instance_get_infant( fixture->inst ) );
     g_assert( commit_test.m_commit_called );
     g_assert( !commit_test.m_commit_with_err_called );
     g_assert( !commit_test.m_on_error_called );
@@ -620,7 +625,7 @@ test_instance_commit_edit_part2( Fixture *fixture, gconstpointer pData )
     qof_instance_set_destroying( fixture->inst, TRUE );
     result = qof_commit_edit_part2( fixture->inst, on_error, on_done, on_free );
     g_assert( !result );
-    g_assert( qof_instance_get_dirty_flag( fixture->inst ) );
+    g_assert( !qof_instance_get_dirty_flag( fixture->inst ) );
     g_assert( !qof_instance_get_destroying( fixture->inst ) );
     g_assert( commit_test.m_commit_called );
     g_assert( commit_test.m_on_error_called );



Summary of changes:
 libgnucash/backend/xml/gnc-xml-backend.cpp  | 7 +++++++
 libgnucash/backend/xml/gnc-xml-backend.hpp  | 1 +
 libgnucash/engine/qof-backend.cpp           | 7 +++++++
 libgnucash/engine/qof-backend.hpp           | 2 +-
 libgnucash/engine/qofinstance.cpp           | 5 ++---
 libgnucash/engine/test/test-qofinstance.cpp | 9 +++++++--
 6 files changed, 25 insertions(+), 6 deletions(-)



More information about the gnucash-changes mailing list