gnucash maint: Don't create 2 new books for every new session.
John Ralls
jralls at code.gnucash.org
Sun Sep 23 19:45:22 EDT 2018
Updated via https://github.com/Gnucash/gnucash/commit/26a17987 (commit)
from https://github.com/Gnucash/gnucash/commit/1c5eb86d (commit)
commit 26a179872ddce7fc490a30e0c6bd041d09e2b8c7
Author: John Ralls <jralls at ceridwen.us>
Date: Sun Sep 23 16:30:30 2018 -0700
Don't create 2 new books for every new session.
And don't ask to save a not-dirty or empty book, fixing
Bug 794870 - If no book is opened, gnucash still asks if the user wants
to save changes when opening a file
diff --git a/gnucash/gnome-utils/gnc-main-window.c b/gnucash/gnome-utils/gnc-main-window.c
index 330e02b..d3ac8d1 100644
--- a/gnucash/gnome-utils/gnc-main-window.c
+++ b/gnucash/gnome-utils/gnc-main-window.c
@@ -1238,6 +1238,8 @@ gnc_main_window_prompt_for_save (GtkWidget *window)
return FALSE;
session = gnc_get_current_session();
book = qof_session_get_book(session);
+ if (!qof_book_session_not_saved(book))
+ return FALSE;
filename = qof_session_get_url(session);
if (!strlen (filename))
filename = _("<unknown>");
diff --git a/libgnucash/backend/sql/test/utest-gnc-backend-sql.cpp b/libgnucash/backend/sql/test/utest-gnc-backend-sql.cpp
index 81e95e1..253bd1c 100644
--- a/libgnucash/backend/sql/test/utest-gnc-backend-sql.cpp
+++ b/libgnucash/backend/sql/test/utest-gnc-backend-sql.cpp
@@ -298,6 +298,7 @@ test_gnc_sql_commit_edit (void)
qof_instance_init_data (inst, QOF_ID_NULL, book);
qof_book_set_dirty_cb (book, test_dirty_cb, &dirty_called);
qof_instance_set_dirty_flag (inst, TRUE);
+ gnc_account_create_root (book);
qof_book_mark_session_dirty (book);
g_assert (qof_instance_get_dirty_flag (inst));
diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
index db7bfd2..d306f87 100644
--- a/libgnucash/engine/qofbook.cpp
+++ b/libgnucash/engine/qofbook.cpp
@@ -58,6 +58,8 @@ extern "C"
#include "qofobject-p.h"
#include "qofbookslots.h"
#include "kvp-frame.hpp"
+// For GNC_ID_ROOT_ACCOUNT:
+#include "AccountP.h"
static QofLogModule log_module = QOF_MOD_ENGINE;
#define AB_KEY "hbci"
@@ -456,7 +458,7 @@ gboolean
qof_book_session_not_saved (const QofBook *book)
{
if (!book) return FALSE;
- return book->session_dirty;
+ return !qof_book_empty(book) && book->session_dirty;
}
@@ -585,6 +587,15 @@ qof_book_mark_readonly(QofBook *book)
g_return_if_fail( book != NULL );
book->read_only = TRUE;
}
+
+gboolean
+qof_book_empty(const QofBook *book)
+{
+ if (!book) return TRUE;
+ auto root_acct_col = qof_book_get_collection (book, GNC_ID_ROOT_ACCOUNT);
+ return qof_collection_get_data(root_acct_col) == nullptr;
+}
+
/* ====================================================================== */
QofCollection *
diff --git a/libgnucash/engine/qofbook.h b/libgnucash/engine/qofbook.h
index 272ec93..67f41c2 100644
--- a/libgnucash/engine/qofbook.h
+++ b/libgnucash/engine/qofbook.h
@@ -258,6 +258,9 @@ gboolean qof_book_is_readonly(const QofBook *book);
/** Mark the book as read only. */
void qof_book_mark_readonly(QofBook *book);
+/** Check if the book has had anything loaded into it. */
+gboolean qof_book_empty(const QofBook *book);
+
#endif /* SWIG */
/** Returns flag indicating whether this book uses trading accounts */
diff --git a/libgnucash/engine/qofsession.cpp b/libgnucash/engine/qofsession.cpp
index 2898d5d..d15752a 100644
--- a/libgnucash/engine/qofsession.cpp
+++ b/libgnucash/engine/qofsession.cpp
@@ -125,7 +125,6 @@ QofSessionImpl::QofSessionImpl () noexcept
m_last_err {},
m_error_message {}
{
- clear_error ();
}
QofSessionImpl::~QofSessionImpl () noexcept
@@ -201,44 +200,23 @@ QofSessionImpl::load_backend (std::string access_method) noexcept
void
QofSessionImpl::load (QofPercentageFunc percentage_func) noexcept
{
+ /* We must have an empty book to load into or bad things will happen. */
+ g_return_if_fail(m_book && qof_book_empty(m_book));
+
if (!m_book_id.size ()) return;
ENTER ("sess=%p book_id=%s", this, m_book_id.c_str ());
/* At this point, we should are supposed to have a valid book
* id and a lock on the file. */
- QofBook * oldbook {m_book};
-
- QofBook * newbook {qof_book_new ()};
- m_book = newbook;
- PINFO ("new book=%p", newbook);
clear_error ();
-
- /* This code should be sufficient to initialize *any* backend,
- * whether http, postgres, or anything else that might come along.
- * Basically, the idea is that by now, a backend has already been
- * created & set up. At this point, we only need to get the
- * top-level account group out of the backend, and that is a
- * generic, backend-independent operation.
- */
- auto be (qof_book_get_backend (oldbook));
- qof_book_set_backend (newbook, be);
-
- /* Starting the session should result in a bunch of accounts
- * and currencies being downloaded, but probably no transactions;
- * The GUI will need to do a query for that.
- */
+ auto be (qof_book_get_backend(m_book));
if (be)
{
be->set_percentage(percentage_func);
- be->load (newbook, LOAD_TYPE_INITIAL_LOAD);
+ be->load (m_book, LOAD_TYPE_INITIAL_LOAD);
push_error (be->get_error(), {});
}
- /* XXX if the load fails, then we try to restore the old set of books;
- * however, we don't undo the session id (the URL). Thus if the
- * user attempts to save after a failed load, they weill be trying to
- * save to some bogus URL. This is wrong. XXX FIXME.
- */
auto err = get_error ();
if ((err != ERR_BACKEND_NO_ERR) &&
(err != ERR_FILEIO_FILE_TOO_OLD) &&
@@ -247,16 +225,11 @@ QofSessionImpl::load (QofPercentageFunc percentage_func) noexcept
(err != ERR_SQL_DB_TOO_OLD) &&
(err != ERR_SQL_DB_TOO_NEW))
{
- /* Something broke, put back the old stuff */
- qof_book_set_backend (newbook, NULL);
- qof_book_destroy (newbook);
- m_book = oldbook;
+ qof_book_destroy(m_book);
+ m_book = qof_book_new();
LEAVE ("error from backend %d", get_error ());
return;
}
- qof_book_set_backend (oldbook, NULL);
- qof_book_destroy (oldbook);
-
LEAVE ("sess = %p, book_id=%s", this, m_book_id.c_str ());
}
diff --git a/libgnucash/engine/test/test-qofbook.c b/libgnucash/engine/test/test-qofbook.c
index c568c95..794dc52 100644
--- a/libgnucash/engine/test/test-qofbook.c
+++ b/libgnucash/engine/test/test-qofbook.c
@@ -36,6 +36,8 @@ extern "C"
#include "../qof.h"
#include "../qofbook-p.h"
#include "../qofbookslots.h"
+/* For gnc_account_create_root() */
+#include "../Account.h"
static const gchar *suitename = "/qof/qofbook";
void test_suite_qofbook ( void );
@@ -285,6 +287,7 @@ test_book_session_not_saved( Fixture *fixture, gconstpointer pData )
g_assert( !qof_book_session_not_saved( fixture->book ) );
qof_book_mark_session_saved( fixture->book );
g_assert( !qof_book_session_not_saved( fixture->book ) );
+ gnc_account_create_root (fixture->book);
qof_book_mark_session_dirty( fixture-> book );
g_assert( qof_book_session_not_saved( fixture->book ) );
}
@@ -294,6 +297,7 @@ test_book_mark_session_saved( Fixture *fixture, gconstpointer pData )
{
time64 dirty_time, clean_time;
+ gnc_account_create_root (fixture->book);
qof_book_mark_session_dirty( fixture-> book );
g_assert( qof_book_session_not_saved( fixture->book ) );
dirty_time = qof_book_get_session_dirty_time( fixture->book );
@@ -626,6 +630,7 @@ test_book_mark_session_dirty( Fixture *fixture, gconstpointer pData )
g_assert( fixture->book->dirty_cb == NULL );
g_assert( qof_book_session_not_saved( fixture->book ) == FALSE );
before = gnc_time (NULL);
+ gnc_account_create_root (fixture->book);
qof_book_mark_session_dirty( fixture->book );
after = gnc_time (NULL);
g_assert_cmpint( qof_book_get_session_dirty_time( fixture->book ), >= , before);
diff --git a/libgnucash/engine/test/test-qofsession.cpp b/libgnucash/engine/test/test-qofsession.cpp
index 595eb62..bdb448c 100644
--- a/libgnucash/engine/test/test-qofsession.cpp
+++ b/libgnucash/engine/test/test-qofsession.cpp
@@ -28,6 +28,7 @@
#include <qof-backend.hpp>
#include <cstdlib>
#include "../gnc-backend-prov.hpp"
+#include "../Account.h"
static QofBook * exported_book {nullptr};
static bool safe_sync_called {false};
@@ -57,9 +58,12 @@ example_hook (QofSession & session)
hook_called = true;
}
-void QofSessionMockBackend::load (QofBook *, QofBackendLoadType)
+void QofSessionMockBackend::load (QofBook *book, QofBackendLoadType)
{
- if (load_error) set_error(ERR_BACKEND_NO_BACKEND);
+ if (load_error)
+ set_error(ERR_BACKEND_NO_BACKEND);
+ else
+ auto root = gnc_account_create_root (book);
data_loaded = true;
}
@@ -178,10 +182,13 @@ TEST (QofSessionTest, load)
EXPECT_EQ (book, s.get_book ());
// Now we'll do the load without returning an error from the backend,
- // and ensure that the book changed to a new book.
+ // and ensure that it's the original book and that it's not empty.
load_error = false;
s.load (nullptr);
- EXPECT_NE (book, s.get_book ());
+ EXPECT_EQ (book, s.get_book ());
+ EXPECT_EQ (s.get_error(), ERR_BACKEND_NO_ERR);
+ //But it's still empty, to the book shouldn't need saving
+ EXPECT_FALSE(qof_book_session_not_saved (s.get_book ()));
// I'll put load_error back just to be tidy.
load_error = true;
qof_backend_unregister_all_providers ();
@@ -192,11 +199,14 @@ TEST (QofSessionTest, save)
qof_backend_register_provider (get_provider ());
QofSession s;
s.begin ("book1", false, false, false);
+ load_error = false;
+ s.load (nullptr);
qof_book_mark_session_dirty (s.get_book ());
s.save (nullptr);
EXPECT_EQ (sync_called, true);
qof_backend_unregister_all_providers ();
sync_called = false;
+ load_error = true;
}
TEST (QofSessionTest, safe_save)
Summary of changes:
gnucash/gnome-utils/gnc-main-window.c | 2 ++
.../backend/sql/test/utest-gnc-backend-sql.cpp | 1 +
libgnucash/engine/qofbook.cpp | 13 ++++++-
libgnucash/engine/qofbook.h | 3 ++
libgnucash/engine/qofsession.cpp | 41 ++++------------------
libgnucash/engine/test/test-qofbook.c | 5 +++
libgnucash/engine/test/test-qofsession.cpp | 18 +++++++---
7 files changed, 44 insertions(+), 39 deletions(-)
More information about the gnucash-changes
mailing list