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.
This commit is contained in:
John Ralls 2020-10-18 15:04:06 -07:00
parent e255a7bfe2
commit 94a68dca7c
6 changed files with 25 additions and 6 deletions

View File

@ -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()
{

View File

@ -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; }

View File

@ -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)
{

View File

@ -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.
*/

View File

@ -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)
{

View File

@ -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 );