implement multi-user conflict resolution when deleted transactions are edited

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

src/engine/BackendP.h

@@ -50,6 +50,8 @@
 #include "gnc-book.h"
 #include "gnc-pricedb.h"
 
+#define BACKEND_ROLLBACK_DESTROY 999
+
 /*
  * The book_begin() routine gives the backend a second initialization
  *    opportunity.  It is suggested that the backend check that 
@@ -90,6 +92,24 @@
  *    the argument for the trans_begin_edit() callback.  (It doesn't
  *    have to be identical, it can be a clone).
  *
+ * The trans_rollback_edit() routine is invoked in one of two different
+ *    ways.  In one case, the user may hit 'undo' in the GUI, resulting
+ *    in xaccTransRollback() being called, which in turn calls this
+ *    routine.  In this manner, xaccTransRollback() implements a
+ *    single-level undo convenience routine for the GUI.  The other
+ *    way in which this routine gets invoked involves conflicting
+ *    edits by two users to the same transaction.  The second user
+ *    to make an edit will typically fail in trans_commit_edit(),
+ *    with trans_commit_edit() returning an error code.  This 
+ *    causes xaccTransCommitEdit() to call xaccTransRollback()
+ *    which in turn calls this routine.  Thus, this routine
+ *    gives the backend a chance to clean up failed commits.
+ *
+ *    If the second user tries to modify a transaction that
+ *    the first user deleted, then the backend should return
+ *    BACKEND_ROLLBACK_DESTROY from this routine, so that the 
+ *    engine can properly clean up.
+ *
  * The run_query() callback takes a GnuCash query object. 
  *    For an SQL backend,  the contents of the query object need to 
  *    be turned into a corresponding SQL query statement, and sent 

src/engine/Transaction.c

@@ -1673,6 +1673,19 @@ xaccTransRollbackEdit (Transaction *trans)
       int rc = 0;
       rc = (be->trans_rollback_edit) (be, trans);
 
+      if (BACKEND_ROLLBACK_DESTROY == rc)
+      {
+         /* The backend is asking us to delete this transaction.
+          * This typically happens because another (remote) user
+          * has deleted this transaction, and we haven't found
+          * out about it until this user tried to edit it.
+          */
+         trans->editlevel++;
+         xaccTransDestroy (trans);
+         xaccFreeTransaction (trans);
+         LEAVE ("deleted trans addr=%p\n", trans);
+         return;
+      }
       if (rc) {
 	PERR ("Rollback Failed.  Ouch!");
       }

src/engine/sql/PostgresBackend.c

@@ -206,8 +206,10 @@ pgendGetResults (PGBackend *be,
 static gpointer
 get_version_cb (PGBackend *be, PGresult *result, int j, gpointer data)
 {
-   if (-1 != (int) data || 0 != j) return (gpointer) -1;
-   return ((gpointer) atoi(DB_GET_VAL ("version", 0)));
+   int version = atoi(DB_GET_VAL ("version", j));
+   int incoming = (int) data;
+   if (version < incoming) version = incoming;
+   return (gpointer) version;
 }
 
 /* ============================================================= */
@@ -1949,7 +1951,10 @@ pgend_trans_commit_edit (Backend * bend,
       }
       }
 #else
-      if (0 < pgendTransactionCompareVersion (be, oldtrans)) rollback ++;
+      /* first, see if someone else has already deleted this transaction */
+      if (-1 < pgendTransactionGetDeletedVersion (be, oldtrans)) { rollback ++; }
+      else
+      if (0 < pgendTransactionCompareVersion (be, oldtrans)) { rollback ++; }
 #endif
    
       if (rollback) {
@@ -2025,16 +2030,27 @@ pgend_trans_commit_edit (Backend * bend,
  */
 
 static int
-pgend_trans_rollback_edit (Backend * bend, 
-                         Transaction * trans)
+pgend_trans_rollback_edit (Backend * bend, Transaction * trans)
 {
    PGBackend *be = (PGBackend *)bend;
    const GUID * trans_guid;
 
    if (!be || !trans) return 0;
+   ENTER ("be=%p, trans=%p", be, trans);
+
+   /* First, lets see if the other user had deleted this transaction.
+    * If so, then we want to delete it from the local cache as well.
+    */
+   if (-1 < pgendTransactionGetDeletedVersion (be, trans))
+   {
+      LEAVE ("destroyed");
+      return BACKEND_ROLLBACK_DESTROY;
+   }
 
    trans_guid = xaccTransGetGUID (trans);
    pgendCopyTransactionToEngine (be, trans_guid);
+
+   LEAVE ("rolled back");
    return 0;
 }
 
@@ -2146,6 +2162,9 @@ pgend_price_commit_edit (Backend * bend, GNCPrice *pr)
  * to the same account or transaction.  This routine does not check
  * for such conflicts or report them.  Hack alert: this is a bug that
  * should be fixed.
+ *
+ * This routine should also check for deleted transactions (that
+ * other users have deleted, but are still present in out cache).
  */
 
 

src/engine/sql/README

@@ -278,11 +278,6 @@ multi-user mode is fundamentally broken unless they are fixed.
    They're not needed until there are a lot of transactions in the 
    system.  Need to recompute checkpoints on some periodic basis.
 
--- bug: if another user deletes a transaction, or an account, we
-   need to look at the audit trail to see if the thing has been deleted.
-   Otherwise, any sort of sync will incorrectly
-   add the deleted transaction back in.  
-
 -- we should use LISTEN/NOTIFY to let others know tht things have 
    changed.  This is of limited utility, but still...
    It could be used to update the balances in the main window 
@@ -291,12 +286,20 @@ multi-user mode is fundamentally broken unless they are fixed.
 -- during sync, detect and report conflicting edits to accounts 
    and transactions.  See the notes for pgendSync() for details
    as to what this is about.  For the first pass, this is not a 
-   serious issue; its a 'nice to have' thing.
+   serious issue; its a 'nice to have' thing.  (sync is called
+   when user hits the 'save' button, and should be disabled for 
+   multi-user modes)
 
 -- implement account rollback (i.e. if other user has modified the
    account, we need to do something to merge their work into ours...)
    ditto for prices ...
 
+-- bug: if another user deletes an account, we need to look at the 
+   audit trail to see if the thing has been deleted.
+   Otherwise, any edit of this account will incorrectly
+   add the deleted account back in. (Note that from the user
+   perspective, deleting accounts is a bad idea ...)
+
 -- fix caching in the face of lost contact to the backend.  If the
    backend can't contact its server, then we should just save up caches,
    and then when contact with backend re-established, we should spit
@@ -308,7 +311,8 @@ multi-user mode is fundamentally broken unless they are fixed.
    user's permission to view/edit account by account ... (hmmm this
    done by the dbadmin... using SQL commands... which means if user
    tries to write to something they're not allowed to write to,
-   then they should be bounced back.)
+   then they should be bounced back.)  Does some user have the permission
+   to create new accounts ??
 
 -- Review versioning verification in backend. The desired semantic for 
    updates should be like CVS: multiple nearly-simultaneous writers 
@@ -316,7 +320,8 @@ multi-user mode is fundamentally broken unless they are fixed.
    The losers know themselves because they are trying to update info 
    of the wrong version. 
    -- pgend_transaction_commit does it correctly; but the GUI doesn't
-      report a rollback.
+      report a rollback.  (need to get err message out of engine, into 
+      gui).
    -- pgTransactionSync() is broken, but its not used anywhere.
    -- pgend_account_commit checks version but doesn't rollback.
       (nor does the GUI report a rollback.)

src/engine/sql/base-objects.m4

@@ -34,3 +34,7 @@ put_one_only(price)
 compare_version(account)
 compare_version(transaction)
 compare_version(price)
+
+is_deleted(account)
+is_deleted(transaction)
+is_deleted(price)

src/engine/sql/table.m4

@@ -347,6 +347,33 @@ pgend`'func_name($@)`'CompareVersion (PGBackend *be, xacc_type($@) *ptr)
 
 ')
 
+define(`is_deleted',
+`
+/* ------------------------------------------------------ */
+/* This routine looks at the audit trail to see if the
+ * indicated object has been deleted. If it has been,
+ * it returns the version number of the deleted object;
+ * otherwise it returns -1.
+ */ 
+
+static int 
+pgend`'func_name($@)`'GetDeletedVersion (PGBackend *be, xacc_type($@) *ptr)
+{
+   char *p;
+   int sql_version = -1;
+
+   p = be->buff; *p = 0;
+   p = stpcpy (p, "SELECT version FROM tablename($@)" "Trail WHERE key_fieldname($@) = ''`");
+   p = guid_to_string_buff (&(ptr->guid), p);
+   p = stpcpy (p, "''` AND change = ''`d''`;");
+   SEND_QUERY (be,be->buff, -1);
+   sql_version = (int) pgendGetResults (be, get_version_cb, (gpointer) -1);
+
+   return sql_version;
+}
+
+')
+
 define(`store_audit', 
 `
 /* ------------------------------------------------------ */