Bug 796759 - --add-price-quotes <sql file> leaves a lock on the file.

First, save isn't necessary if the book is dirty, so don't... but that
means that the book has to be marked dirty after a session swap. No more
laziness.

Second, regardless of the outcome of inner_main_add_price_quotes the
session must be destroyed to remove the lock.

A couple of cleanups in QofSessionImpl::save as well: Rewrote the
descriptive comment to reflect how it really works when the backend has
gotten disconnected and removed the superfluous qof_book_set_backend
with the backend that we'd *just gotten from the book*.

gnucash/gnome-utils/gnc-file.c

@@ -1536,11 +1536,7 @@ gnc_file_do_save_as (GtkWindow *parent, const char* filename)
     /* if we got to here, then we've successfully gotten a new session */
     /* close up the old file session (if any) */
     qof_session_swap_data (session, new_session);
-
-    /* XXX At this point, we should really mark the data in the new session
-     * as being 'dirty', since we haven't saved it at all under the new
-     * session. But I'm lazy...
-     */
+    qof_book_mark_session_dirty (qof_session_get_book (new_session));
 
     qof_event_resume();
 

gnucash/gnucash-bin.c

@@ -585,8 +585,13 @@ inner_main_add_price_quotes(void *closure, int argc, char **argv)
     gnc_shutdown(0);
     return;
 fail:
-    if (session && qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
-        g_warning("Session Error: %s", qof_session_get_error_message(session));
+    if (session)
+    {
+        if (qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
+            g_warning("Session Error: %s",
+                      qof_session_get_error_message(session));
+        qof_session_destroy(session);
+    }
     qof_event_resume();
     gnc_shutdown(1);
 }

libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp

@@ -401,6 +401,7 @@ test_dbi_store_and_reload (Fixture* fixture, gconstpointer pData)
     g_assert (session_2 != NULL);
     g_assert_cmpint (qof_session_get_error (session_2), == , ERR_BACKEND_NO_ERR);
     qof_session_swap_data (fixture->session, session_2);
+    qof_book_mark_session_dirty (qof_session_get_book (session_2));
     qof_session_save (session_2, NULL);
     g_assert (session_2 != NULL);
     g_assert_cmpint (qof_session_get_error (session_2), == , ERR_BACKEND_NO_ERR);
@@ -459,6 +460,7 @@ test_dbi_safe_save (Fixture* fixture, gconstpointer pData)
         goto cleanup;
     }
     qof_session_swap_data (fixture->session, session_1);
+    qof_book_mark_session_dirty (qof_session_get_book (session_1));
     qof_session_save (session_1, NULL);
     /* Do a safe save */
     qof_session_safe_save (session_1, NULL);
@@ -532,6 +534,7 @@ test_dbi_version_control (Fixture* fixture, gconstpointer pData)
         goto cleanup;
     }
     qof_session_swap_data (fixture->session, sess);
+    qof_book_mark_session_dirty (qof_session_get_book (sess));
     qof_session_save (sess, NULL);
     sql_be = reinterpret_cast<decltype(sql_be)>(qof_session_get_backend (sess));
     book = qof_session_get_book (sess);
@@ -587,6 +590,7 @@ test_dbi_business_store_and_reload (Fixture* fixture, gconstpointer pData)
     session_2 = qof_session_new ();
     qof_session_begin (session_2, url, FALSE, TRUE, TRUE);
     qof_session_swap_data (fixture->session, session_2);
+    qof_book_mark_session_dirty (qof_session_get_book (session_2));
     qof_session_save (session_2, NULL);
 
     // Reload the session data

libgnucash/engine/qofsession.cpp

@@ -451,23 +451,21 @@ QofSessionImpl::is_saving () const noexcept
 void
 QofSessionImpl::save (QofPercentageFunc percentage_func) noexcept
 {
+    if (!qof_book_session_not_saved (m_book)) //Clean book, nothing to do.
+        return;
     m_saving = true;
     ENTER ("sess=%p book_id=%s", this, m_book_id.c_str ());
 
-    /* If there is a backend, and the backend is reachable
-    * (i.e. we can communicate with it), then synchronize with
-    * the backend.  If we cannot contact the backend (e.g.
-    * because we've gone offline, the network has crashed, etc.)
-    * then give the user the option to save to the local disk.
-    *
-    * hack alert -- FIXME -- XXX the code below no longer
-    * does what the words above say.  This needs fixing.
+    /* If there is a backend, the book is dirty, and the backend is reachable
+     * (i.e. we can communicate with it), then synchronize with the backend.  If
+     * we cannot contact the backend (e.g.  because we've gone offline, the
+     * network has crashed, etc.)  then raise an error so that the controlling
+     * dialog can offer the user a chance to save in a different way.
      */
     auto backend = qof_book_get_backend (m_book);
     if (backend)
     {
-        /* if invoked as SaveAs(), then backend not yet set */
-        qof_book_set_backend (m_book, backend);
+
         backend->set_percentage(percentage_func);
         backend->sync(m_book);
         auto err = backend->get_error();

libgnucash/engine/test/test-qofsession.cpp

@@ -192,6 +192,7 @@ TEST (QofSessionTest, save)
     qof_backend_register_provider (get_provider ());
     QofSession s;
     s.begin ("book1", false, false, false);
+    qof_book_mark_session_dirty (s.get_book ());
     s.save (nullptr);
     EXPECT_EQ (sync_called, true);
     qof_backend_unregister_all_providers ();