From 65de023e94edf0f407b19025ab46442e0ff4539f Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 13 Nov 2014 01:20:54 +0300 Subject: [PATCH 1/3] Use a separate field for optype. No more LIKE in queries --- appinfo/database.xml | 7 +++++ appinfo/update.php | 14 ++++++++++ appinfo/version | 2 +- lib/db/op.php | 62 +++++++++++++++++++++++++++++++++----------- 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index e11d3913..86913184 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -147,6 +147,13 @@ 4 User and time specific + + optype + text + false + 64 + Operation type + opspec clob diff --git a/appinfo/update.php b/appinfo/update.php index a5a75ecb..9a7e2b03 100644 --- a/appinfo/update.php +++ b/appinfo/update.php @@ -32,4 +32,18 @@ if (version_compare($installedVersion, '0.7', '<=')) { if (version_compare($installedVersion, '0.8', '<')) { $query = \OC_DB::prepare('UPDATE `*PREFIX*documents_member` SET `is_guest`=1 WHERE `uid` LIKE \'%(guest)\' '); $query->execute(array()); +} + +if (version_compare($installedVersion, '0.9', '<')) { + $query = \OC_DB::prepare('UPDATE `*PREFIX*documents_op` SET `optype`=? WHERE `seq`=?'); + $ops = new \OCA\Documents\Db\Op(); + foreach ($ops->getCollection() as $opData){ + $opSpec = json_decode($opData['opspec'], true); + $query->execute( + array( + $opSpec['optype'], + $opData['seq'] + ) + ); + } } \ No newline at end of file diff --git a/appinfo/version b/appinfo/version index 100435be..ac39a106 100755 --- a/appinfo/version +++ b/appinfo/version @@ -1 +1 @@ -0.8.2 +0.9.0 diff --git a/lib/db/op.php b/lib/db/op.php index 4cd37e09..e54ad5aa 100644 --- a/lib/db/op.php +++ b/lib/db/op.php @@ -17,15 +17,16 @@ class Op extends \OCA\Documents\Db { protected $tableName = '`*PREFIX*documents_op`'; - protected $insertStatement = 'INSERT INTO `*PREFIX*documents_op` (`es_id`, `member`, `opspec`) VALUES (?, ?, ?)'; + protected $insertStatement = 'INSERT INTO `*PREFIX*documents_op` (`es_id`, `optype`, `member`, `opspec`) VALUES (?, ?, ?, ?)'; public static function addOpsArray($esId, $memberId, $ops){ $lastSeq = ""; $opObj = new Op(); foreach ($ops as $op) { $opObj->setData(array( - $esId, - $memberId, + $esId, + $op['optype'], + $memberId, json_encode($op) )); $opObj->insert(); @@ -84,42 +85,73 @@ class Op extends \OCA\Documents\Db { } public function addMember($esId, $memberId, $fullName, $color, $imageUrl){ - $op = '{"optype":"AddMember","memberid":"'. $memberId .'","timestamp":"'. time() .'", "setProperties":{"fullName":"'. $fullName .'","color":"'. $color .'","imageUrl":"'. $imageUrl .'"}}'; + $op = array( + 'optype' => 'AddMember', + 'memberid' => (string) $memberId, + 'timestamp' => (string) time(), + 'setProperties' => array( + 'fullName' => $fullName, + 'color' => $color, + 'imageUrl' => $imageUrl + ) + ); $this->insertOp($esId, $memberId, $op); } public function removeCursor($esId, $memberId){ + $op = array( + 'optype' => 'RemoveCursor', + 'memberid' => (string) $memberId, + 'reason' => 'server-idle', + 'timestamp' => (string) time() + ); if ($this->hasOp($esId, $memberId, 'AddCursor') && !$this->hasLastOp($esId, $memberId, 'RemoveCursor')){ - $op = '{"optype":"RemoveCursor","memberid":"'. $memberId .'","reason":"server-idle","timestamp":'. time() .'}'; $this->insertOp($esId, $memberId, $op); } } public function removeMember($esId, $memberId){ + $op = array( + 'optype' => 'RemoveMember', + 'memberid' => (string) $memberId, + 'timestamp' => (string) time() + ); if ($this->hasOp($esId, $memberId, 'AddMember') && !$this->hasLastOp($esId, $memberId, 'RemoveMember')){ - $op ='{"optype":"RemoveMember","memberid":"'. $memberId .'","timestamp":'. time() .'}'; $this->insertOp($esId, $memberId, $op); } } public function updateMember($esId, $memberId, $fullName, $color, $imageUrl){ //TODO: Follow the spec https://github.com/kogmbh/WebODF/blob/master/webodf/lib/ops/OpUpdateMember.js#L95 - $op = '{"optype":"UpdateMember","memberid":"'. $memberId .'","fullName":"'. $fullName .'","color":"'. $color .'","imageUrl":"'. $imageUrl .'","timestamp":'. time() .'}' - ; + $op = array( + 'optype' => 'UpdateMember', + 'memberid' => (string) $memberId, + 'timestamp' => (string) time(), + 'fullName' => $fullName, + 'color' => $color, + 'imageUrl' => $imageUrl + ); $this->insertOp($esId, $memberId, $op); } public function changeNick($esId, $memberId, $fullName){ - $op = '{"optype":"UpdateMember","memberid":"'. $memberId .'", "setProperties":{"fullName":"'. $fullName .'"},"timestamp":'. time() .'}' - ; + $op = array( + 'optype' => 'UpdateMember', + 'memberid' => (string) $memberId, + 'timestamp' => (string) time(), + 'setProperties' => array( + 'fullName' => $fullName, + ) + ); $this->insertOp($esId, $memberId, $op); } protected function insertOp($esId, $memberId, $op){ $op = new Op(array( - $esId, + $esId, + $op['optype'], $memberId, - $op + json_encode($op) )); $op->insert(); } @@ -130,7 +162,6 @@ class Op extends \OCA\Documents\Db { FROM ' . self::DB_TABLE . ' WHERE `es_id`=? AND `member`=? - ORDER BY `seq` DESC ', 2,0 @@ -149,8 +180,9 @@ class Op extends \OCA\Documents\Db { protected function hasOp($esId, $memberId, $opType){ $ops = $this->execute( - 'SELECT * FROM ' . $this->tableName . ' WHERE `es_id`=? AND `opspec` LIKE \'%"' . $opType . '","memberid":"' . $memberId .'"%\'', - array($esId) + 'SELECT * FROM ' . $this->tableName + . ' WHERE `es_id`=? AND `optype`=? AND `member`=?', + array($esId, $opType, $memberId) ); $result = $ops->fetchAll(); return is_array($result) && count($result)>0; From 59a095e98d8607dbc21b043320e459838b19e269 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 13 Nov 2014 01:34:44 +0300 Subject: [PATCH 2/3] updateMember is not used currently --- lib/db/op.php | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/db/op.php b/lib/db/op.php index e54ad5aa..12d5cefe 100644 --- a/lib/db/op.php +++ b/lib/db/op.php @@ -121,19 +121,7 @@ class Op extends \OCA\Documents\Db { } } - public function updateMember($esId, $memberId, $fullName, $color, $imageUrl){ - //TODO: Follow the spec https://github.com/kogmbh/WebODF/blob/master/webodf/lib/ops/OpUpdateMember.js#L95 - $op = array( - 'optype' => 'UpdateMember', - 'memberid' => (string) $memberId, - 'timestamp' => (string) time(), - 'fullName' => $fullName, - 'color' => $color, - 'imageUrl' => $imageUrl - ); - $this->insertOp($esId, $memberId, $op); - } - + //TODO: Implement https://github.com/kogmbh/WebODF/blob/master/webodf/lib/ops/OpUpdateMember.js#L95 public function changeNick($esId, $memberId, $fullName){ $op = array( 'optype' => 'UpdateMember', From 0f19d74f69bb814afb96c2efecc07d665b9b9c39 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 13 Nov 2014 20:05:58 +0300 Subject: [PATCH 3/3] Improve filtering for member/cursor operations --- lib/db/op.php | 93 +++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/lib/db/op.php b/lib/db/op.php index 12d5cefe..cef01215 100644 --- a/lib/db/op.php +++ b/lib/db/op.php @@ -20,9 +20,11 @@ class Op extends \OCA\Documents\Db { protected $insertStatement = 'INSERT INTO `*PREFIX*documents_op` (`es_id`, `optype`, `member`, `opspec`) VALUES (?, ?, ?, ?)'; public static function addOpsArray($esId, $memberId, $ops){ - $lastSeq = ""; $opObj = new Op(); foreach ($ops as $op) { + if (!$opObj->canInsertOp($esId, $memberId, $op)){ + continue; + } $opObj->setData(array( $esId, $op['optype'], @@ -30,9 +32,9 @@ class Op extends \OCA\Documents\Db { json_encode($op) )); $opObj->insert(); - $lastSeq = $opObj->getLastInsertId(); } - return $lastSeq; + + return $opObj->getHeadSeq($esId); } /** @@ -105,9 +107,7 @@ class Op extends \OCA\Documents\Db { 'reason' => 'server-idle', 'timestamp' => (string) time() ); - if ($this->hasOp($esId, $memberId, 'AddCursor') && !$this->hasLastOp($esId, $memberId, 'RemoveCursor')){ - $this->insertOp($esId, $memberId, $op); - } + $this->insertOp($esId, $memberId, $op); } public function removeMember($esId, $memberId){ @@ -116,9 +116,7 @@ class Op extends \OCA\Documents\Db { 'memberid' => (string) $memberId, 'timestamp' => (string) time() ); - if ($this->hasOp($esId, $memberId, 'AddMember') && !$this->hasLastOp($esId, $memberId, 'RemoveMember')){ - $this->insertOp($esId, $memberId, $op); - } + $this->insertOp($esId, $memberId, $op); } //TODO: Implement https://github.com/kogmbh/WebODF/blob/master/webodf/lib/ops/OpUpdateMember.js#L95 @@ -135,45 +133,54 @@ class Op extends \OCA\Documents\Db { } protected function insertOp($esId, $memberId, $op){ - $op = new Op(array( - $esId, - $op['optype'], - $memberId, - json_encode($op) - )); - $op->insert(); + if ($this->canInsertOp($esId, $memberId, $op)){ + $op = new Op(array( + $esId, + $op['optype'], + $memberId, + json_encode($op) + )); + $op->insert(); + } } - protected function hasLastOp($esId, $memberId, $opType){ - $query = \OCP\DB::prepare(' - SELECT `opspec` - FROM ' . self::DB_TABLE . ' - WHERE `es_id`=? - AND `member`=? - ORDER BY `seq` DESC - ', - 2,0 - ); + protected function canInsertOp($esId, $memberId, $op){ + $cursorOps = array('AddCursor', 'RemoveCursor'); + $memberOps = array('AddMember', 'RemoveMember'); + $result = true; - $result = $query->execute(array($esId, $memberId)); - $ops = $result->fetchAll(); - foreach ($ops as $op){ - $decoded = json_decode($op['opspec'], true); - if ($decoded['optype']==$opType){ - return true; - } + switch ($op['optype']){ + case 'AddCursor': + $ops = $this->getFilteredMemberOps($esId, $memberId, $cursorOps); + $result = !count($ops) || $ops[0]['optype'] === 'RemoveCursor'; + break; + case 'RemoveCursor': + $ops = $this->getFilteredMemberOps($esId, $memberId, $cursorOps); + $result = count($ops) && $ops[0]['optype'] === 'AddCursor'; + break; + case 'AddMember': + $ops = $this->getFilteredMemberOps($esId, $memberId, $memberOps); + $result = !count($ops) || $ops[0]['optype'] === 'RemoveMember'; + break; + case 'RemoveMember': + $ops = $this->getFilteredMemberOps($esId, $memberId, $memberOps); + $result = count($ops) && $ops[0]['optype'] === 'AddMember'; + break; } - return false; + return $result; } - - protected function hasOp($esId, $memberId, $opType){ - $ops = $this->execute( - 'SELECT * FROM ' . $this->tableName - . ' WHERE `es_id`=? AND `optype`=? AND `member`=?', - array($esId, $opType, $memberId) + + protected function getFilteredMemberOps($esId, $memberId, $targetOps){ + $stmt = $this->buildInQuery('optype', $targetOps); + $result = $this->execute(' + SELECT `optype` FROM ' . $this->tableName . ' + WHERE es_id=? AND member=? AND ' . $stmt . 'ORDER BY `seq` DESC', + array_merge(array($esId, $memberId), $targetOps) ); - $result = $ops->fetchAll(); - return is_array($result) && count($result)>0; + $ops = $result->fetchAll(); + if (!is_array($ops)){ + $ops = array(); + } + return $ops; } - }