Commit b084998a authored by Carsten  Rose's avatar Carsten Rose
Browse files

#4185 / Detect modified record

modifiedRecord.pu: State Diagram
Dirty.php: implement $recordHashMd5 to detect modified records.
OnArray.php: new getMd5()
AbstractBuildForm.php: implemented but not working update of hidden input 'recordHashMd5'. Add hidden input 'recordHashMd5'.
BuildFormBootstrap:  Add hidden input 'recordHashMd5'.
formEditor.sql: Rename 'Dirty.recordModified' to 'Dirty.recordHashMd5'.
parent 1c65cdf7
@startuml
actor Alice #FFBBBB
actor Bob #BBBBFF
== Detect modified record: on lock ==
Alice -> form: open page
form -> Alice: form, recordHashMd5='a'
...
Bob -> form: open page
form -> Bob: form, recordHashMd5='a'
...
Alice -> dirty: edit: action=lock, recordHashMd5='a' (hash from form!)
dirty -> Alice: status=success, lock_timeout=<secs>
...
Alice -> save: POST (incl. recordHashMd5)
save -> Alice: action=success, update new recordHashMd5='b' in form.
...
Bob -> dirty: edit: action=lock, recordHashMd5='a' (hash from form!)
dirty -> Bob: status=conclict, msg=record has changed, reload form
== Detect modified record: on save ==
Alice -> form: open page
form -> Alice: form, recordHashMd5='a'
...
Alice -> dirty: edit: action=lock, recordHashMd5='a' (hash from form!)
dirty -> Alice: status=success, lock_timeout=<secs>
...
note over Alice: some background task.
...
System -> System: change record.
...
Alice -> save: POST (incl. recordHashMd5)
save -> Alice: status=error, msg=record has changed, reload form
@enduml
This diff is collapsed.
......@@ -445,8 +445,9 @@ class BuildFormBootstrap extends AbstractBuildForm {
}
$honeypot = $this->getHoneypotVars();
$md5 = $this->buildRecordHashMd5();
return '<form ' . OnArray::toString($attribute, '=', ' ', "'") . '>' . $honeypot;
return '<form ' . OnArray::toString($attribute, '=', ' ', "'") . '>' . $honeypot . $md5;
}
/**
......
......@@ -260,6 +260,7 @@ const ERROR_DIRTY_DELETE_RECORD = 2201;
const ERROR_DIRTY_UNKNOWN_ACTION = 2202;
const ERROR_DIRTY_MISSING_LOCK = 2203;
const ERROR_DIRTY_ALREADY_LOCKED = 2204;
const ERROR_DIRTY_RECORD_MODIFIED = 2205;
//
// Store Names: Identifier
......@@ -1120,7 +1121,7 @@ const DIRTY_FE_USER = 'feUser';
const DIRTY_EXPIRE = 'expire';
const DIRTY_TABLE_NAME = 'tableName';
const DIRTY_RECORD_ID = 'recordId';
const DIRTY_RECORD_MODIFIED = 'recordModified';
const DIRTY_RECORD_HASH_MD5 = 'recordHashMd5';
const DIRTY_REMOTE_ADDRESS = 'remoteAddress';
const DIRTY_API_ACTION = 'action'; // Name of parameter in API call of dirty.php?action=...&s=...
const DIRTY_API_ACTION_LOCK = 'lock';
......
......@@ -115,7 +115,7 @@ class QuickFormQuery {
* Store might throw an exception, in case the URL-passed SIP is invalid.
*
* @param array $t3data
* @param bool $phpUnit
* @param bool $phpUnit
* @throws CodeException
* @throws UserFormException
*/
......@@ -270,12 +270,13 @@ class QuickFormQuery {
// Check (and release) dirtyRecord.
if ($formMode === FORM_DELETE || $formMode === FORM_SAVE) {
$dirty = new Dirty();
// Comment as long as long as 4127 is open
$answer = $dirty->checkDirtyAndRelease($formMode, $this->formSpec[F_RECORD_LOCK_TIMEOUT_SECONDS],
$this->formSpec[F_DIRTY_MODE], $this->formSpec[F_TABLE_NAME], $recordId);
$this->formSpec[F_DIRTY_MODE], $this->formSpec[F_TABLE_NAME], $recordId, true);
// in case of a conflict, return immediately
if($answer[API_STATUS]!= API_ANSWER_STATUS_SUCCESS) {
if ($answer[API_STATUS] != API_ANSWER_STATUS_SUCCESS) {
$answer[API_STATUS] = API_ANSWER_STATUS_ERROR;
return $answer;
}
}
......@@ -398,7 +399,7 @@ class QuickFormQuery {
/**
* Iterate over all Clipboard source records and fire for each all FE.type=paste records.
*
* @param int $formId
* @param int $formId
* @param FormAction $formAction
* @throws CodeException
* @throws DbException
......@@ -445,7 +446,7 @@ class QuickFormQuery {
* used parameters. Do this by building a new SIP with the new recordId.
*
* @param array $formSpec
* @param $recordId
* @param int $recordId
* @return array
* @throws CodeException
* @throws UserFormException
......@@ -479,7 +480,7 @@ class QuickFormQuery {
* Loaded 'native' FormElements are in $this->feSpecNative
*
* @param string $mode FORM_LOAD|FORM_SAVE|FORM_UPDATE
* @param int $recordId
* @param int $recordId
* @param string $foundInStore
* @return bool|string if found the formName, else 'false'.
* @throws CodeException
......@@ -570,8 +571,8 @@ class QuickFormQuery {
* This code is dirty: the nearly same function exists in class 'Database' - the difference is only 'explodeTemplateGroupElements()'.
*
* @param string $sql SQL_FORM_ELEMENT_SPECIFIC_CONTAINER | SQL_FORM_ELEMENT_ALL_CONTAINER
* @param array $param Parameter which matches the prepared statement in $sql
* @param array $formSpec Main FormSpec to copy generic parameter to FormElements
* @param array $param Parameter which matches the prepared statement in $sql
* @param array $formSpec Main FormSpec to copy generic parameter to FormElements
* @return array|int
* @throws \qfq\CodeException
* @throws \qfq\DbException
......@@ -700,7 +701,7 @@ class QuickFormQuery {
* Depending on $mode various formSpec fields might be adjusted.
* E.g.: the form title is not important during a delete.
*
* @param $mode
* @param string $mode
* @param array $form
* @return array
*/
......@@ -816,10 +817,11 @@ class QuickFormQuery {
/**
* Check if loading of the given form is permitted. If not, throw an exception.
*
* @param $formNameFoundInStore
* @param string $formNameFoundInStore
* @param string $formMode
* @return bool 'true' if SIP exists, else 'false'
* @throws CodeException
* @throws UserFormException
* @throws \qfq\CodeException
* @throws \qfq\UserFormException
* @internal param $foundInStore
*/
private function validateForm($formNameFoundInStore, $formMode) {
......@@ -933,7 +935,10 @@ class QuickFormQuery {
}
unset($data[API_ELEMENT_UPDATE]);
}
$collect[] = $data;
if(count($data)>0){
$collect[] = $data;
}
}
return $collect;
......@@ -996,10 +1001,10 @@ class QuickFormQuery {
/**
* Based on the given SIP, create a new uniqe SIP by copying the relevant old params and taking the new recordId..
*
* @param $sipArray
* @param $recordId
* @param array $sipArray
* @param int $recordId
*/
private function newRecordCreateSip($sipArray, $recordId) {
private function newRecordCreateSip(array $sipArray, $recordId) {
$tmpParam = array();
......
......@@ -48,6 +48,9 @@ class Dirty {
$this->session = Session::getInstance($phpUnit);
$this->client = Client::getParam();
if (!isset($this->client[DIRTY_RECORD_HASH_MD5])) {
$this->client[DIRTY_RECORD_HASH_MD5] = '';
}
$this->db = new Database();
}
......@@ -81,7 +84,7 @@ class Dirty {
switch ($this->client[API_LOCK_ACTION]) {
case API_LOCK_ACTION_LOCK:
case API_LOCK_ACTION_EXTEND:
$answer = $this->acquireDirty($recordId, $tableVars);
$answer = $this->acquireDirty($recordId, $tableVars, $this->client[DIRTY_RECORD_HASH_MD5]);
break;
case API_LOCK_ACTION_RELEASE:
$answer = $this->checkDirtyAndRelease(FORM_SAVE, $tableVars[F_RECORD_LOCK_TIMEOUT_SECONDS], $tableVars[F_DIRTY_MODE], $tableVars[F_TABLE_NAME], $recordId);
......@@ -96,17 +99,23 @@ class Dirty {
/**
* Tries to get a 'DirtyRecord'. Returns an array (becomes JSON) about success or failure.
*
* @param int $recordId
* @param array $tableVars
* @param int $recordId
* @param array $tableVars
* @param string $recordHashMd5
* @return array
* @throws CodeException
* @throws DbException
* @internal param array $sipVars
* @throws \qfq\CodeException
*/
private function acquireDirty($recordId, array $tableVars) {
private function acquireDirty($recordId, array $tableVars, $recordHashMd5) {
$tableName = $tableVars[F_TABLE_NAME];
$formDirtyMode = $tableVars[F_DIRTY_MODE];
$rcMd5 = '';
// Check for changed record. Compute $rcMd5
$flagModified = $this->isRecordModified($tableName, $recordId, $recordHashMd5, $rcMd5);
if (($recordHashMd5 != '') && $flagModified) {
return [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => 'The record has been modified in the meantime. Please reload the form, edit and save again.'];
}
$feUser = $this->session->get(SESSION_FE_USER);
......@@ -124,7 +133,7 @@ class Dirty {
$answer = [API_STATUS => 'success', API_MESSAGE => ''];
} else {
// No dirty record found.
$answer = $this->writeDirty($this->client[SIP_SIP], $recordId, $tableVars, $feUser);
$answer = $this->writeDirty($this->client[SIP_SIP], $recordId, $tableVars, $feUser, $rcMd5);
}
} else {
$answer = $this->conflict($recordDirty, $formDirtyMode);
......@@ -136,8 +145,8 @@ class Dirty {
/**
* Load (if exist) a DirtyRecord (lock).
*
* @param $tableName
* @param $recordId
* @param string $tableName
* @param int $recordId
* @return array DirtyRecord or empty array.
* @throws CodeException
* @throws DbException
......@@ -152,8 +161,8 @@ class Dirty {
/**
*
* @param array $recordDirty
* @param $currentFormDirtyMode
* @param array $recordDirty
* @param string $currentFormDirtyMode
* @return array
*/
private function conflict(array $recordDirty, $currentFormDirtyMode) {
......@@ -161,7 +170,7 @@ class Dirty {
$at = "at " . $recordDirty[COLUMN_CREATED] . " from " . $recordDirty[DIRTY_REMOTE_ADDRESS];
// Compare modified timestamp
if (false == $this->checkModified($recordDirty[DIRTY_TABLE_NAME], $recordDirty[DIRTY_RECORD_ID], $recordDirty[DIRTY_RECORD_MODIFIED])) {
if ($this->isRecordModified($recordDirty[DIRTY_TABLE_NAME], $recordDirty[DIRTY_RECORD_ID], $recordDirty[DIRTY_RECORD_HASH_MD5], $dummy)) {
return [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => 'The record has been modified in the meantime. Please reload the form, edit and save again.'];
}
......@@ -194,28 +203,26 @@ class Dirty {
* Write a 'Dirty'-Record.
*
* @param string $s SIP given by URL GET
* @param int $recordId extracted from SIP
* @param array $tableVars columns: F_TABLE_NAME, F_DIRTY_MODE, F_RECORD_LOCK_TIMEOUT_SECONDS
* @param int $recordId extracted from SIP
* @param array $tableVars columns: F_TABLE_NAME, F_DIRTY_MODE, F_RECORD_LOCK_TIMEOUT_SECONDS
* @param string $feUser
* @param string $recordHashMd5
* @return array
* @throws CodeException
* @throws DbException
* @throws \qfq\CodeException
* @throws \qfq\DbException
*/
private function writeDirty($s, $recordId, array $tableVars, $feUser) {
private function writeDirty($s, $recordId, array $tableVars, $feUser, $recordHashMd5) {
$tableName = $tableVars[F_TABLE_NAME];
$formDirtyMode = $tableVars[F_DIRTY_MODE];
$record = $this->db->sql("SELECT * FROM $tableName WHERE id=?", ROW_EXPECT_1, [$recordId], "Record to lock not found.");
// Not all tables does have a modified column.
$recordModified = empty($record[COLUMN_MODIFIED]) ? 0 : $record[COLUMN_MODIFIED];
$expire = date('Y-m-d H:i:s', strtotime("+" . $tableVars[F_RECORD_LOCK_TIMEOUT_SECONDS] . " seconds"));
// Write 'dirty' record
$this->db->sql("INSERT INTO Dirty (`sip`, `tableName`, `recordId`, `expire`, `recordModified`, `feUser`, `qfqUserSessionCookie`, `dirtyMode`, `remoteAddress`, `created`) " .
$this->db->sql("INSERT INTO Dirty (`sip`, `tableName`, `recordId`, `expire`, `recordHashMd5`, `feUser`, `qfqUserSessionCookie`, `dirtyMode`, `remoteAddress`, `created`) " .
"VALUES ( ?,?,?,?,?,?,?,?,?,? )", ROW_REGULAR,
[$s, $tableName, $recordId, $expire, $recordModified, $feUser, $this->client[CLIENT_COOKIE_QFQ], $formDirtyMode,
[$s, $tableName, $recordId, $expire, $recordHashMd5, $feUser, $this->client[CLIENT_COOKIE_QFQ], $formDirtyMode,
$this->client[CLIENT_REMOTE_ADDRESS], date('YmdHis')]);
return [API_STATUS => API_ANSWER_STATUS_SUCCESS, API_MESSAGE => '',
......@@ -224,33 +231,29 @@ class Dirty {
}
/**
* If exist get the column 'modified' from tableName/recordId and compare with $modified.
* Get MD5 from tableName/recordId and compare with $recordHashMd5.
*
* @param string $tableName
* @param int $recordId
* @param string $modified - timestamp e.g. '2017-07-27 14:06:56'
* @return bool true if column 'modified' is missing or $modified==record['modified'], else false.
* @param int $recordId
* @param string $recordHashMd5 - timestamp e.g. '2017-07-27 14:06:56'
* @return bool true if $recordHashMd5 is different from current record md5 hash.
* @throws CodeException
* @throws DbException
*/
private function checkModified($tableName, $recordId, $modified) {
$status = true;
private function isRecordModified($tableName, $recordId, $recordHashMd5, &$rcMd5) {
$record = $this->db->sql("SELECT * FROM $tableName WHERE id=?", ROW_EXPECT_1, [$recordId], "Record to lock not found.");
if (isset($record[COLUMN_MODIFIED])) {
$status = ($modified == $record[COLUMN_MODIFIED]) ? true : false;
}
return $status;
$rcMd5 = OnArray::getMd5($record);
return ($recordHashMd5 != $rcMd5);
}
/**
* Check if a lock exist for the current table, recordId and session.
*
* @param string $tableName
* @param int $recordId
* @param array $recordDirty - return dirty record if one exist.
* @param int $recordId
* @param array $recordDirty - return dirty record if one exist.
* @param string $msg - return preformatted message in case of conflict
* @return int LOCK_NOT_FOUND | LOCK_FOUND_OWNER | LOCK_FOUND_CONFLICT,
*/
......@@ -289,18 +292,18 @@ class Dirty {
* In case of not owner, throws an exception and the save should break.
*
* @param string $formMode FORM_DELETE, FORM_SAVE
* @param int $lockTimeout
* @param int $lockTimeout
* @param string $dirtyMode DIRTY_MODE_EXCLUSIVE, DIRTY_MODE_ADVISORY, DIRTY_MODE_NONE
* @param string $tableName
* @param int $recordId
* @param int $recordId
* @return array
* @throws CodeException
* @throws UserFormException
* @throws \qfq\CodeException
* @throws \qfq\UserFormException
*/
public function checkDirtyAndRelease($formMode, $lockTimeout, $dirtyMode, $tableName, $recordId) {
public function checkDirtyAndRelease($formMode, $lockTimeout, $dirtyMode, $tableName, $recordId, $flagCheckModifiedFirst = false) {
$recordDirty = array();
$msg = '';
$rcRecordDirty = array();
$rcMsg = '';
$answer = [API_STATUS => API_ANSWER_STATUS_SUCCESS, API_MESSAGE => ''];
......@@ -308,9 +311,15 @@ class Dirty {
return $answer; // New records never have a recordDirty nor a conflict.
}
$lockStatus = $this->getCheckDirty($tableName, $recordId, $recordDirty, $msg);
// Check if the record has changed in the meantime.
if ($flagCheckModifiedFirst && $this->isRecordModified($tableName, $recordId, $this->client[DIRTY_RECORD_HASH_MD5], $dummy)) {
throw new UserFormException ('The record has been modified in the meantime. Please reload the form, edit and save again.', ERROR_DIRTY_RECORD_MODIFIED);
// return [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => 'The record has been modified in the meantime. Please reload the form, edit and save again.'];
}
if (empty($recordDirty)) {
$lockStatus = $this->getCheckDirty($tableName, $recordId, $rcRecordDirty, $rcMsg);
if (empty($rcRecordDirty)) {
if ($dirtyMode == DIRTY_MODE_NONE) {
return $answer; // only situation where it's ok that there is no dirtyRecord.
}
......@@ -324,25 +333,25 @@ class Dirty {
if ($formMode == FORM_DELETE) {
// Check if the record is timed out
if ( $lockTimeout > 0 && $recordDirty[DIRTY_EXPIRE] < date('Y-m-d H:i:s')) {
$this->deleteDirtyRecord($recordDirty[COLUMN_ID]);
if ($lockTimeout > 0 && $rcRecordDirty[DIRTY_EXPIRE] < date('Y-m-d H:i:s')) {
$this->deleteDirtyRecord($rcRecordDirty[COLUMN_ID]);
return $answer;
}
$answer = [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => $msg];
$answer = [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => $rcMsg];
return $answer;
}
// Is the dirtyRecord mine?
if ($lockStatus == LOCK_FOUND_OWNER) {
// Uncommentent as long as '#4127 /Record Locking: save() impliziert 'release'' is open
if (false == $this->checkModified($tableName, $recordId, $recordDirty[DIRTY_RECORD_MODIFIED])) {
// Check if the record has changed in the meantime.
if ($this->isRecordModified($tableName, $recordId, $rcRecordDirty[DIRTY_RECORD_HASH_MD5], $dummy)) {
return [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => 'The record has been modified in the meantime. Please reload the form, edit and save again.'];
}
// Clear the lock
$this->deleteDirtyRecord($recordDirty[COLUMN_ID]);
$this->deleteDirtyRecord($rcRecordDirty[COLUMN_ID]);
return $answer;
}
......@@ -350,23 +359,23 @@ class Dirty {
// From here: there is a foreign lock!
// Check if overwrite is allowed
if ($dirtyMode == DIRTY_MODE_ADVISORY && $recordDirty[F_DIRTY_MODE] == DIRTY_MODE_ADVISORY) {
if ($dirtyMode == DIRTY_MODE_ADVISORY && $rcRecordDirty[F_DIRTY_MODE] == DIRTY_MODE_ADVISORY) {
return $answer;
}
// Check if the record is timed out
if ($lockTimeout > 0 && $recordDirty[DIRTY_EXPIRE] < date('Y-m-d H:i:s')) {
$this->deleteDirtyRecord($recordDirty[COLUMN_ID]);
if ($lockTimeout > 0 && $rcRecordDirty[DIRTY_EXPIRE] < date('Y-m-d H:i:s')) {
$this->deleteDirtyRecord($rcRecordDirty[COLUMN_ID]);
return $answer;
}
throw new UserFormException($msg, ERROR_DIRTY_ALREADY_LOCKED);
throw new UserFormException($rcMsg, ERROR_DIRTY_ALREADY_LOCKED);
}
/**
* Delete the dirtyRecord with $recordDirtyId. Throw an exception if the record has not been deleted.
*
* @param $recordDirtyId
* @param int $recordDirtyId
* @throws CodeException
* @throws DbException
*/
......
......@@ -298,4 +298,13 @@ class OnArray {
return $new;
}
/**
* @param array $data
* @return string md5 of implode $data
*/
public static function getMd5(array $data) {
return md5(implode($data));
}
}
\ No newline at end of file
......@@ -137,7 +137,7 @@ CREATE TABLE IF NOT EXISTS `Dirty` (
`tableName` VARCHAR(255) NOT NULL,
`recordId` INT(11) NOT NULL,
`expire` DATETIME NOT NULL,
`recordModified` DATETIME NOT NULL,
`recordHashMd5` CHAR(32) NOT NULL,
`feUser` VARCHAR(255) NOT NULL,
`qfqUserSessionCookie` VARCHAR(255) NOT NULL,
`dirtyMode` ENUM('exclusive', 'advisory', 'none') NOT NULL DEFAULT 'exclusive',
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment