Merge branch 'develop' into 'master'

Develop

See merge request !233

Documentation-develop/RECORD_LOCK.md

+<!-- -*- markdown -*- -->
+
+# Record locking
+
+## Concept: Late locking
+
+* A lock is required on first modification.
+* Multiple forms might open the same record, all seems to have write access. The first one who modifies the record
+  get the lock, all following will switch to form=readonly on their first try to modify the record.
+  
+## Lock mode: Exclusive 
+
+* A lock can't be overwritten.
+
+## Lock mode: Advisory 
+
+* A lock can be ignored.
+* Last save win's.
+
+## Lock mode: None
+
+* No locking at all.
+
+# Workarounds
+
+* At least one Browser (FF 71, maybe other in the future too), do not allow to wrap the 'leave page' dialog anymore.
+  This might result in stale lock files (modified record, click on browser tab close or any link), cause the lock 
+  logic does not know that the user leaves the page. 
+  
+  * Workaround: before 'do you want to leave the page' appears, the lock is released, independent if the user answers 'no'.
+    As soon as the users modifies the record again, a new lock is acquired. This is better than a stale record lock.
+    
+* Reload a page (F5) on a modified record, opens the form in readonly mode (record lock found).
+
+  * Reason: the lock release is fired by the browser AFTER form load - than the lock-logic reports 'record is already locked'.
+  * Workaround: with the above workaround, this does not happen anymore. Nevertheless, a 'tabUniqId' has been implemented.
+    That one is saved as record lock and a page reload origin can be identified as the same tab as where the lock has been
+    acquired.  
+
+= State Diagram =
+
+See `Documentation-develop/diagram` for a state diagram.

Documentation/Manual.rst

@@ -2255,7 +2255,7 @@ If a timeout expires, the lock becomes invalid. During the next change in a form
 
 A lock is assigned to a record of a table. Multiple forms, with the same primary table, uses the same lock for a given record.
 
-If a `Form` acts on further records (e.g. via FE action), those records are not protected by this basic record locking.
+If a `Form` acts on further records (e.g. via FE action), those further records are not protected by this basic record locking.
 
 If a user tries to delete a record and another user already owns a lock on that record, the delete action is denied.
 

extension/Classes/Core/Constants.php

@@ -1372,6 +1372,7 @@ const QUERY_TYPE_SELECT = 'type: select,show,describe,explain';
 const QUERY_TYPE_INSERT = 'type: insert';
 const QUERY_TYPE_UPDATE = 'type: update,replace,delete';
 const QUERY_TYPE_CONTROL = 'type: set';
+const QUERY_TYPE_FAILED = 'type: query failed';
 
 //Regexp
 //const REGEXP_DATE_INT = '^\d{4}-\d{2}-\d{2}$';
@@ -1816,7 +1817,7 @@ const DIRTY_API_ACTION_EXTEND = 'extend';
 const LOCK_NOT_FOUND = 0;
 const LOCK_FOUND_OWNER = 1;
 const LOCK_FOUND_CONFLICT = 2;
-const TAB_UNIQ_ID = 'tabUniqId'; // Currently only only a uniq identifier: no values stored behind the identifier - might change.
+const TAB_UNIQ_ID = 'tabUniqId'; // Uniq identifier per tab: no values stored behind the identifier - might change.
 
 // AutoCron
 const AUTOCRON_MAX_AGE_MINUTES = 10;

extension/Classes/Core/Database/Database.php

@@ -181,6 +181,7 @@ class Database {
      * @param string $specificMessage
      * @param array $keys
      * @param array $stat DB_NUM_ROWS | DB_INSERT_ID | DB_AFFECTED_ROWS
+     * @param array $skipErrno
      *
      * @return array|int
      *      SELECT | SHOW | DESCRIBE | EXPLAIN: see $mode
@@ -190,7 +191,7 @@ class Database {
      * @throws \DbException
      * @throws \UserFormException
      */
-    public function sql($sql, $mode = ROW_REGULAR, array $parameterArray = array(), $specificMessage = '', array &$keys = array(), array &$stat = array()) {
+    public function sql($sql, $mode = ROW_REGULAR, array $parameterArray = array(), $specificMessage = '', array &$keys = array(), array &$stat = array(), array $skipErrno = array()) {
         $queryType = '';
         $result = array();
         $this->closeMysqliStmt();
@@ -205,7 +206,7 @@ class Database {
             $specificMessage .= " ";
         }
 
-        $count = $this->prepareExecute($sql, $parameterArray, $queryType, $stat, $specificMessage);
+        $count = $this->prepareExecute($sql, $parameterArray, $queryType, $stat, $specificMessage, $skipErrno);
 
         if ($count === false) {
             throw new \DbException($specificMessage . "No idea why this error happens - please take some time and check the problem.", ERROR_DB_GENERIC_CHECK);
@@ -339,23 +340,29 @@ class Database {
      * Returns the number of selected rows (SELECT, SHOW, ..) or the affected rows (UPDATE, INSERT). $stat contains
      * appropriate num_rows, insert_id or rows_affected.
      *
+     * In case of an error, throw an exception.
+     * mysqli error code listed in $skipErrno[] do not throw an error.
+     *
      * @param string $sql SQL statement with prepared statement variable.
      * @param array $parameterArray parameter array for prepared statement execution.
      * @param string $queryType returns QUERY_TYPE_SELECT | QUERY_TYPE_UPDATE | QUERY_TYPE_INSERT, depending on
      *                               the query.
      * @param array $stat DB_NUM_ROWS | DB_INSERT_ID | DB_AFFECTED_ROWS
-     *
      * @param string $specificMessage
+     * @param array $skipErrno
+     *
      * @return int|mixed
      * @throws \CodeException
      * @throws \DbException
      * @throws \UserFormException
      */
-    private function prepareExecute($sql, array $parameterArray, &$queryType, array &$stat, $specificMessage = '') {
+    private function prepareExecute($sql, array $parameterArray, &$queryType, array &$stat, $specificMessage = '', array $skipErrno = array()) {
 
         // Only log a modify type statement here if sqlLogMode is (at least) modifyAll
         // If sqlLogMode is modify, log the statement after it has been executed and we know if there are affected rows.
         $sqlLogMode = $this->isSqlModify($sql) ? SQL_LOG_MODE_MODIFY_ALL : SQL_LOG_MODE_ALL;
+        $errno = 0;
+
         $result = 0;
         $stat = array();
         $errorMsg[ERROR_MESSAGE_TO_USER] = empty($specificMessage) ? 'SQL error' : $specificMessage;
@@ -372,34 +379,52 @@ class Database {
         $this->dbLog($sqlLogMode, $sql, $parameterArray);
 
         if (false === ($this->mysqli_stmt = $this->mysqli->prepare($sql))) {
-            $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
-            $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
-            $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli->errno . ' ] ' . $this->mysqli->error;
 
-            throw new \DbException(json_encode($errorMsg), ERROR_DB_PREPARE);
+            if ($skipErrno === array() && false === array_search($this->mysqli->errno, $skipErrno)) {
+                $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
+                $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
+                $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli->errno . ' ] ' . $this->mysqli->error;
+
+                throw new \DbException(json_encode($errorMsg), ERROR_DB_PREPARE);
+            } else {
+                $errno = $this->mysqli->errno;
+            }
         }
 
         if (count($parameterArray) > 0) {
             if (false === $this->prepareBindParam($parameterArray)) {
-                $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
-                $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
-                $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli_stmt->errno . ' ] ' . $this->mysqli_stmt->error;
+                if ($skipErrno !== array() && false === array_search($this->mysqli->errno, $skipErrno)) {
+                    $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
+                    $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
+                    $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli_stmt->errno . ' ] ' . $this->mysqli_stmt->error;
 
-                throw new \DbException(json_encode($errorMsg), ERROR_DB_BIND);
+                    throw new \DbException(json_encode($errorMsg), ERROR_DB_BIND);
+                } else {
+                    $errno = $this->mysqli->errno;
+                }
             }
         }
 
         if (false === $this->mysqli_stmt->execute()) {
-            $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
-            $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
-            $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli_stmt->errno . ' ] ' . $this->mysqli_stmt->error;
+            if ($skipErrno !== array() && false === array_search($this->mysqli->errno, $skipErrno)) {
+                $this->dbLog(SQL_LOG_MODE_ERROR, $sql, $parameterArray);
+                $errorMsg[ERROR_MESSAGE_TO_DEVELOPER] = $this->getSqlHint($sql, $this->mysqli->error);
+                $errorMsg[ERROR_MESSAGE_OS] = '[ mysqli: ' . $this->mysqli_stmt->errno . ' ] ' . $this->mysqli_stmt->error;
 
-            throw new \DbException(json_encode($errorMsg), ERROR_DB_EXECUTE);
+                throw new \DbException(json_encode($errorMsg), ERROR_DB_EXECUTE);
+            } else {
+                $errno = $this->mysqli->errno;
+            }
         }
 
         $msg = '';
         $count = 0;
-        $command = strtoupper(explode(' ', $sql, 2)[0]);
+        if ($errno === 0) {
+            $command = strtoupper(explode(' ', $sql, 2)[0]);
+        } else {
+            $command = 'FAILED';
+        }
+
         switch ($command) {
             case 'SELECT':
             case 'SHOW':
@@ -444,6 +469,12 @@ class Database {
                 $count = $stat[DB_AFFECTED_ROWS];
                 $msg = '';
                 break;
+            case 'FAILED':
+                $queryType = QUERY_TYPE_FAILED;
+                $stat[DB_AFFECTED_ROWS] = 0;
+                $count = -1;
+                $msg = '[ mysqli: ' . $this->mysqli_stmt->errno . ' ] ' . $this->mysqli_stmt->error;
+                break;
 
             default:
                 // Unknown command: treat it as a control command

extension/Classes/Core/Database/DatabaseUpdate.php

@@ -373,6 +373,9 @@ class DatabaseUpdate {
      * @throws \UserFormException
      */
     private function dbUpdateStatements($old, $new) {
+
+        $dummy = array();
+
         if ($new == '' || $old === false || $old === null) {
             return;
         }
@@ -391,7 +394,9 @@ class DatabaseUpdate {
             if ($apply) {
                 // Play Statements
                 foreach ($sqlStatements as $sql) {
-                    $this->db->sql($sql, ROW_REGULAR, array(), "Apply updates to QFQ database. Installed version: $old. New QFQ version: $new");
+                    $this->db->sql($sql, ROW_REGULAR, array(),
+                        "Apply updates to QFQ database. Installed version: $old. New QFQ version: $new",
+                        $dummy, $dummy, [1060] /* duplicate column name */);
                 }
                 // Remember already applied updates - in case something breaks and the update has to be repeated.
                 $this->setDatabaseVersion($new);

extension/Classes/Core/Form/Dirty.php

@@ -146,7 +146,7 @@ class Dirty {
     }
 
     /**
-     * Tries to get a 'DirtyRecord'. Returns an array (becomes JSON) about success or failure.
+     * Tries to get a lock ('dirty record'). Returns an array (becomes JSON) about success or failure.
      *
      * @param int $recordId
      * @param array $tableVars

extension/Tests/Unit/Core/Database/AbstractDatabaseTest.php

@@ -63,6 +63,7 @@ abstract class AbstractDatabaseTest extends TestCase {
      * @throws \DbException
      * @throws \UserFormException
      * @throws \UserReportException
+     * @throws \Exception
      */
     protected function setUp() {
 

extension/Tests/Unit/Core/Database/DatabaseFunction.php

@@ -115,6 +115,7 @@ class DatabaseFunction extends AbstractDatabaseTest {
      * @throws \DbException
      * @throws \UserFormException
      * @throws \UserReportException
+     * @throws \Exception
      */
     protected function setUp() {
         parent::setUp();

extension/Tests/Unit/Core/Database/DatabaseTest.php

@@ -15,6 +15,9 @@ require_once(__DIR__ . '/AbstractDatabaseTest.php');
 class DatabaseTest extends AbstractDatabaseTest {
 
     /**
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testFetchAll() {
         $allRows = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id LIMIT 2');
@@ -25,7 +28,9 @@ class DatabaseTest extends AbstractDatabaseTest {
     }
 
     /**
-     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testFetchAllEmpty() {
         $allRows = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person WHERE id=0 ORDER BY id');
@@ -35,6 +40,7 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @throws \CodeException
+     * @throws \DbException
      * @throws \UserFormException
      */
     public function testQuerySimple() {
@@ -65,7 +71,7 @@ class DatabaseTest extends AbstractDatabaseTest {
         $this->assertEquals(1, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
 
         // Check read rows
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person ORDER BY id LIMIT 2');
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person ORDER BY id LIMIT 2');
 
         // Check count
         $this->assertEquals(2, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
@@ -74,7 +80,7 @@ class DatabaseTest extends AbstractDatabaseTest {
         // Check rows
 
         // Same as above, but specify 'ROW_REGULAR'
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person ORDER BY id LIMIT 2', ROW_REGULAR);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person ORDER BY id LIMIT 2', ROW_REGULAR);
         $this->assertEquals(2, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
         // Check rows
         $this->assertEquals($expected, $dataArray);
@@ -86,13 +92,13 @@ class DatabaseTest extends AbstractDatabaseTest {
         $this->assertEquals(array(), $dataArray);
 
         // ROW_EXPECT_1
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person WHERE id=1', ROW_EXPECT_1);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person WHERE id=1', ROW_EXPECT_1);
         $this->assertEquals(1, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
         // Check rows
         $this->assertEquals($expected[0], $dataArray);
 
         // ROW_EXPECT_0_1: 1
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person WHERE id=1', ROW_EXPECT_0_1);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person WHERE id=1', ROW_EXPECT_0_1);
         $this->assertEquals(1, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
         // Check rows
         $this->assertEquals($expected[0], $dataArray);
@@ -104,23 +110,23 @@ class DatabaseTest extends AbstractDatabaseTest {
         $this->assertEquals(array(), $dataArray);
 
         //ROW_EXPECT_GE_1 - one record
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person WHERE id=1', ROW_EXPECT_GE_1);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person WHERE id=1', ROW_EXPECT_GE_1);
         $this->assertEquals(1, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
         // Check rows
         $this->assertEquals([$expected[0]], $dataArray);
 
         // ROW_EXPECT_GE_1 - two records
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person ORDER BY id LIMIT 2', ROW_EXPECT_GE_1);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person ORDER BY id LIMIT 2', ROW_EXPECT_GE_1);
         $this->assertEquals(2, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
         // Check rows
         $this->assertEquals($expected, $dataArray);
 
         // Check Implode: 2 records
-        $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person ORDER BY id LIMIT 2', ROW_IMPLODE_ALL);
+        $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person ORDER BY id LIMIT 2', ROW_IMPLODE_ALL);
         $this->assertEquals(implode($expected[0]) . implode($expected[1]), $data);
 
         // Check Implode: 1 record
-        $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,groups FROM Person ORDER BY id LIMIT 1', ROW_IMPLODE_ALL);
+        $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT id,name,firstName,adrId,gender,`groups` FROM Person ORDER BY id LIMIT 1', ROW_IMPLODE_ALL);
         $this->assertEquals(implode($expected[0]), $data);
 
         // Check Implode 0 record
@@ -142,6 +148,7 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @throws \CodeException
+     * @throws \DbException
      * @throws \UserFormException
      */
     public function testQuerySimpleParameter() {
@@ -153,7 +160,7 @@ class DatabaseTest extends AbstractDatabaseTest {
         // Check count
         $this->assertEquals(1, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
 
-        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('UPDATE Person SET groups = ?', ROW_REGULAR, ['a,b,c']);
+        $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('UPDATE Person SET `groups` = ?', ROW_REGULAR, ['a,b,c']);
         $this->assertEquals(2, $this->store->getVar(SYSTEM_SQL_COUNT, STORE_SYSTEM));
 
         $dataArray = $this->dbArray[DB_INDEX_DEFAULT]->sql('INSERT INTO Person (`name`, `firstname`, `groups`) VALUES ( ?, ? ,? )', ROW_REGULAR, ['Meier', 'John', 'a'], 'fake', $dummy, $stat);
@@ -171,6 +178,10 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSqlExceptionExpect0() {
         $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id LIMIT 1', ROW_EXPECT_0);
@@ -178,6 +189,10 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSqlExceptionExpect1_0() {
         $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id LIMIT 0', ROW_EXPECT_1);
@@ -185,6 +200,10 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSqlExceptionExpect1_2() {
         $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id ', ROW_EXPECT_1);
@@ -192,6 +211,10 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSqlExceptionExpect01() {
         $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id ', ROW_EXPECT_0_1);
@@ -199,6 +222,10 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSqlExceptionExpectGE1() {
         $data = $this->dbArray[DB_INDEX_DEFAULT]->sql('SELECT * FROM Person ORDER BY id LIMIT 0', ROW_EXPECT_GE_1);
@@ -206,13 +233,34 @@ class DatabaseTest extends AbstractDatabaseTest {
 
     /**
      * @expectedException DbException
+     *
+     * @throws \CodeException
+     * @throws \DbException
+     * @throws \UserFormException
      */
     public function testSanitizeException() {