From 330d026be73efb6e6630094c34607a0feb6dd3ff Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Wed, 30 Oct 2013 19:36:29 +0100 Subject: [PATCH 01/12] fix the privatedata key value store --- db_structure.xml | 63 ++++++++++++++++++++++++++++++ lib/private/ocs.php | 32 ---------------- lib/private/ocs/privatedata.php | 68 +++++++++++++++++++++++++++------ 3 files changed, 119 insertions(+), 44 deletions(-) diff --git a/db_structure.xml b/db_structure.xml index f9470dc86b3..a7225a7661f 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1077,4 +1077,67 @@ + + + *dbprefix*privatedata + + + + + keyid + integer + 0 + true + true + 4 + 1 + + + + user + text + + true + 255 + + + + app + text + + true + 255 + + + + key + text + + true + 255 + + + + value + text + + true + 255 + + + + true + true + keyid_index + + keyid + ascending + + + + + +
+ + diff --git a/lib/private/ocs.php b/lib/private/ocs.php index 93e8931ce2e..e067196cf11 100644 --- a/lib/private/ocs.php +++ b/lib/private/ocs.php @@ -228,36 +228,4 @@ class OC_OCS { } } } - - /** - * get private data - * @param string $user - * @param string $app - * @param string $key - * @param bool $like use LIKE instead of = when comparing keys - * @return array - */ - public static function getData($user, $app="", $key="") { - if($app) { - $apps=array($app); - }else{ - $apps=OC_Preferences::getApps($user); - } - if($key) { - $keys=array($key); - }else{ - foreach($apps as $app) { - $keys=OC_Preferences::getKeys($user, $app); - } - } - $result=array(); - foreach($apps as $app) { - foreach($keys as $key) { - $value=OC_Preferences::getValue($user, $app, $key); - $result[]=array('app'=>$app, 'key'=>$key, 'value'=>$value); - } - } - return $result; - } - } diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index 4dfd0a6e66e..b489cd0fbb3 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -22,35 +22,75 @@ * */ + class OC_OCS_Privatedata { + /** + * read keys + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy/123 + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy + * @param array $parameters The OCS parameter + */ public static function get($parameters) { OC_Util::checkLoggedIn(); $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); - $result = OC_OCS::getData($user, $app, $key); - $xml = array(); - foreach($result as $i=>$log) { - $xml[$i]['key']=$log['key']; - $xml[$i]['app']=$log['app']; - $xml[$i]['value']=$log['value']; + + if(empty($key)) { + $query = \OCP\DB::prepare('SELECT `key`, `app`, `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? '); + $result = $query->execute(array($user, $app)); + } else { + $query = \OCP\DB::prepare('SELECT `key`, `app`, `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? AND `key` = ? '); + $result = $query->execute(array($user, $app, $key)); } + + $xml = array(); + while ($row = $result->fetchRow()) { + $data=array(); + $data['key']=$row['key']; + $data['app']=$row['app']; + $data['value']=$row['value']; + $xml[] = $data; + } + return new OC_OCS_Result($xml); - //TODO: replace 'privatedata' with 'attribute' once a new libattice has been released that works with it } + /** + * set a key + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/setattribute/testy/123 --data "value=foobar" + * @param array $parameters The OCS parameter + */ public static function set($parameters) { OC_Util::checkLoggedIn(); $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); $value = OC_OCS::readData('post', 'value', 'text'); - if(OC_Preferences::setValue($user, $app, $key, $value)) { - return new OC_OCS_Result(null, 100); + + // check if key is already set + $query = \OCP\DB::prepare('SELECT `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? AND `key` = ? '); + $result = $query->execute(array($user, $app, $key)); + + if ($result->numRows()==0) { + // store in DB + $query = \OCP\DB::prepare('INSERT INTO `*PREFIX*privatedata` (`user`, `app`, `key`, `value`)' . ' VALUES(?, ?, ?, ?)'); + $query->execute(array($user, $app, $key, $value)); + } else { + // update in DB + $query = \OCP\DB::prepare('UPDATE `*PREFIX*privatedata` SET `value` = ? WHERE `user` = ? AND `app` = ? AND `key` = ? '); + $query->execute(array($value, $user, $app, $key )); } + + return new OC_OCS_Result(null, 100); } + /** + * delete a key + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/deleteattribute/testy/123 --data "post=1" + * @param array $parameters The OCS parameter + */ public static function delete($parameters) { OC_Util::checkLoggedIn(); $user = OC_User::getUser(); @@ -59,8 +99,12 @@ class OC_OCS_Privatedata { if($key==="" or $app==="") { return new OC_OCS_Result(null, 101); //key and app are NOT optional here } - if(OC_Preferences::deleteKey($user, $app, $key)) { - return new OC_OCS_Result(null, 100); - } + + // delete in DB + $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? AND `key` = ? '); + $query->execute(array($user, $app, $key )); + + return new OC_OCS_Result(null, 100); } } + From edafd29630f5866f10f331e69ba34cf26681782b Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Wed, 30 Oct 2013 19:38:17 +0100 Subject: [PATCH 02/12] increase version number to trigger db migration --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 1a4672354f6..1f3897a093a 100644 --- a/version.php +++ b/version.php @@ -1,7 +1,7 @@ Date: Thu, 31 Oct 2013 09:58:18 +0100 Subject: [PATCH 03/12] no need to check if the user is logged in - this is already done in the ocs dispatcher itself adding @return --- lib/private/ocs/privatedata.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index b489cd0fbb3..e528dfbcb74 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -26,13 +26,13 @@ class OC_OCS_Privatedata { /** - * read keys - * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy/123 - * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy - * @param array $parameters The OCS parameter - */ + * read keys + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy/123 + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute/testy + * @param array $parameters The OCS parameter + * @return \OC_OCS_Result + */ public static function get($parameters) { - OC_Util::checkLoggedIn(); $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); From 7f64d858ddcf62602c40b32ec2325c70a74b8292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:01:04 +0100 Subject: [PATCH 04/12] first two tests --- tests/lib/ocs/privatedata.php | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/lib/ocs/privatedata.php diff --git a/tests/lib/ocs/privatedata.php b/tests/lib/ocs/privatedata.php new file mode 100644 index 00000000000..03129e4d0d0 --- /dev/null +++ b/tests/lib/ocs/privatedata.php @@ -0,0 +1,53 @@ +. + * + */ + +class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase +{ + + private $appKey; + + public function setUp() { + \OC::$session->set('user_id', 'user1'); + $this->appKey = uniqid('app'); + } + + public function tearDown() { + } + + public function testGetEmptyOne() { + $params = array('app' => $this->appKey, 'key' => '123'); + $result = OC_OCS_Privatedata::get($params); + $this->assertEquals(100, $result->getStatusCode()); + $data = $result->getData(); + $this->assertTrue(is_array($data)); + $this->assertEquals(0, sizeof($data)); + } + + public function testGetEmptyAll() { + $params = array('app' => $this->appKey); + $result = OC_OCS_Privatedata::get($params); + $this->assertEquals(100, $result->getStatusCode()); + $data = $result->getData(); + $this->assertTrue(is_array($data)); + $this->assertEquals(0, sizeof($data)); + } +} From 47da2ef8db27d7fd52b615e3f1e4e67d969aad06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:02:18 +0100 Subject: [PATCH 05/12] fixing php notice: Undefined index key --- lib/private/ocs/privatedata.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index e528dfbcb74..4b16268b8f4 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -35,7 +35,7 @@ class OC_OCS_Privatedata { public static function get($parameters) { $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); - $key = addslashes(strip_tags($parameters['key'])); + $key = isset($parameters['key']) ?addslashes(strip_tags($parameters['key'])) : null; if(empty($key)) { $query = \OCP\DB::prepare('SELECT `key`, `app`, `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? '); From 0cec17ba8700b0892927300b19abf3a4b5d8deaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:14:06 +0100 Subject: [PATCH 06/12] no need to check if the user is logged in --- lib/private/ocs/privatedata.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index 4b16268b8f4..0ef38f87b58 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -63,7 +63,6 @@ class OC_OCS_Privatedata { * @param array $parameters The OCS parameter */ public static function set($parameters) { - OC_Util::checkLoggedIn(); $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); From 8362afa94dbfd0c5eda66868270b4e06ca4382d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:14:31 +0100 Subject: [PATCH 07/12] unit tests for set added --- tests/lib/ocs/privatedata.php | 43 +++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/tests/lib/ocs/privatedata.php b/tests/lib/ocs/privatedata.php index 03129e4d0d0..0a242bd5f8e 100644 --- a/tests/lib/ocs/privatedata.php +++ b/tests/lib/ocs/privatedata.php @@ -36,18 +36,51 @@ class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase public function testGetEmptyOne() { $params = array('app' => $this->appKey, 'key' => '123'); $result = OC_OCS_Privatedata::get($params); - $this->assertEquals(100, $result->getStatusCode()); - $data = $result->getData(); - $this->assertTrue(is_array($data)); - $this->assertEquals(0, sizeof($data)); + $this->assertOcsResult(0, $result); } public function testGetEmptyAll() { $params = array('app' => $this->appKey); $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(0, $result); + } + + public function testSetOne() { + $_POST = array('value' => 123456789); + $params = array('app' => $this->appKey, 'key' => 'k-1'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(1, $result); + } + + public function testSetMany() { + $_POST = array('value' => 123456789); + + // set key 'k-1' + $params = array('app' => $this->appKey, 'key' => 'k-1'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + // set key 'k-2' + $params = array('app' => $this->appKey, 'key' => 'k-2'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + // query for all + $params = array('app' => $this->appKey); + $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(2, $result); + } + + /** + * @param \OC_OCS_Result $result + */ + public function assertOcsResult($expectedArraySize, $result) { $this->assertEquals(100, $result->getStatusCode()); $data = $result->getData(); $this->assertTrue(is_array($data)); - $this->assertEquals(0, sizeof($data)); + $this->assertEquals($expectedArraySize, sizeof($data)); } } From 64d0b0d18489e3cbacdfd826c84bdf16f8164268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:21:17 +0100 Subject: [PATCH 08/12] PHPDoc comment updated - checkLoggedIn() removed --- lib/private/ocs/privatedata.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index 0ef38f87b58..f2c6ec7fecd 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -58,10 +58,11 @@ class OC_OCS_Privatedata { } /** - * set a key - * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/setattribute/testy/123 --data "value=foobar" - * @param array $parameters The OCS parameter - */ + * set a key + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/setattribute/testy/123 --data "value=foobar" + * @param array $parameters The OCS parameter + * @return \OC_OCS_Result + */ public static function set($parameters) { $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); @@ -86,12 +87,12 @@ class OC_OCS_Privatedata { } /** - * delete a key - * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/deleteattribute/testy/123 --data "post=1" - * @param array $parameters The OCS parameter - */ + * delete a key + * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/deleteattribute/testy/123 --data "post=1" + * @param array $parameters The OCS parameter + * @return \OC_OCS_Result + */ public static function delete($parameters) { - OC_Util::checkLoggedIn(); $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); From bd5663bc3dc5d6901d9051a08d9ec4a8df24cca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:21:42 +0100 Subject: [PATCH 09/12] adding unit tests for delete --- tests/lib/ocs/privatedata.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/lib/ocs/privatedata.php b/tests/lib/ocs/privatedata.php index 0a242bd5f8e..423bb2c96b6 100644 --- a/tests/lib/ocs/privatedata.php +++ b/tests/lib/ocs/privatedata.php @@ -74,6 +74,22 @@ class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase $this->assertOcsResult(2, $result); } + /** + * @dataProvider deleteWithEmptyKeysProvider + */ + public function testDeleteWithEmptyKeys($params) { + $result = OC_OCS_Privatedata::delete($params); + $this->assertEquals(101, $result->getStatusCode()); + } + + public function deleteWithEmptyKeysProvider() { + return array( + array(array()), + array(array('app' => '123')), + array(array('key' => '123')), + ); + } + /** * @param \OC_OCS_Result $result */ From aae6e769266f516408d1c1c578e64985c2f41fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 10:24:28 +0100 Subject: [PATCH 10/12] fixing undefined index in delete() --- lib/private/ocs/privatedata.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index f2c6ec7fecd..2c53d576092 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -35,7 +35,7 @@ class OC_OCS_Privatedata { public static function get($parameters) { $user = OC_User::getUser(); $app = addslashes(strip_tags($parameters['app'])); - $key = isset($parameters['key']) ?addslashes(strip_tags($parameters['key'])) : null; + $key = isset($parameters['key']) ? addslashes(strip_tags($parameters['key'])) : null; if(empty($key)) { $query = \OCP\DB::prepare('SELECT `key`, `app`, `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? '); @@ -94,12 +94,14 @@ class OC_OCS_Privatedata { */ public static function delete($parameters) { $user = OC_User::getUser(); + if (!isset($parameters['app']) or !isset($parameters['key'])) { + //key and app are NOT optional here + return new OC_OCS_Result(null, 101); + } + $app = addslashes(strip_tags($parameters['app'])); $key = addslashes(strip_tags($parameters['key'])); - if($key==="" or $app==="") { - return new OC_OCS_Result(null, 101); //key and app are NOT optional here - } - + // delete in DB $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? AND `key` = ? '); $query->execute(array($user, $app, $key )); From 43d71aada89abfaa8b1989874af5f4d7773c0278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 31 Oct 2013 11:09:52 +0100 Subject: [PATCH 11/12] testing update and delete --- tests/lib/ocs/privatedata.php | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/lib/ocs/privatedata.php b/tests/lib/ocs/privatedata.php index 423bb2c96b6..ea8413734f1 100644 --- a/tests/lib/ocs/privatedata.php +++ b/tests/lib/ocs/privatedata.php @@ -55,6 +55,30 @@ class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase $this->assertOcsResult(1, $result); } + public function testSetExisting() { + $_POST = array('value' => 123456789); + $params = array('app' => $this->appKey, 'key' => 'k-10'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(1, $result); + $data = $result->getData(); + $data = $data[0]; + $this->assertEquals('123456789', $data['value']); + + $_POST = array('value' => 'updated'); + $params = array('app' => $this->appKey, 'key' => 'k-10'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(1, $result); + $data = $result->getData(); + $data = $data[0]; + $this->assertEquals('updated', $data['value']); + } + public function testSetMany() { $_POST = array('value' => 123456789); @@ -74,6 +98,21 @@ class Test_OC_OCS_Privatedata extends PHPUnit_Framework_TestCase $this->assertOcsResult(2, $result); } + public function testDelete() { + $_POST = array('value' => 123456789); + + // set key 'k-1' + $params = array('app' => $this->appKey, 'key' => 'k-3'); + $result = OC_OCS_Privatedata::set($params); + $this->assertEquals(100, $result->getStatusCode()); + + $result = OC_OCS_Privatedata::delete($params); + $this->assertEquals(100, $result->getStatusCode()); + + $result = OC_OCS_Privatedata::get($params); + $this->assertOcsResult(0, $result); + } + /** * @dataProvider deleteWithEmptyKeysProvider */ From d6ddb12c20a646204bee1d5c3d749e1b22783f05 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Thu, 31 Oct 2013 15:44:19 +0100 Subject: [PATCH 12/12] Get rid of the SELECT query. Try UPDATE, on failure INSERT. --- lib/private/ocs/privatedata.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/private/ocs/privatedata.php b/lib/private/ocs/privatedata.php index 2c53d576092..932413711b8 100644 --- a/lib/private/ocs/privatedata.php +++ b/lib/private/ocs/privatedata.php @@ -69,18 +69,14 @@ class OC_OCS_Privatedata { $key = addslashes(strip_tags($parameters['key'])); $value = OC_OCS::readData('post', 'value', 'text'); - // check if key is already set - $query = \OCP\DB::prepare('SELECT `value` FROM `*PREFIX*privatedata` WHERE `user` = ? AND `app` = ? AND `key` = ? '); - $result = $query->execute(array($user, $app, $key)); + // update in DB + $query = \OCP\DB::prepare('UPDATE `*PREFIX*privatedata` SET `value` = ? WHERE `user` = ? AND `app` = ? AND `key` = ?'); + $numRows = $query->execute(array($value, $user, $app, $key)); - if ($result->numRows()==0) { + if ($numRows === false || $numRows === 0) { // store in DB $query = \OCP\DB::prepare('INSERT INTO `*PREFIX*privatedata` (`user`, `app`, `key`, `value`)' . ' VALUES(?, ?, ?, ?)'); $query->execute(array($user, $app, $key, $value)); - } else { - // update in DB - $query = \OCP\DB::prepare('UPDATE `*PREFIX*privatedata` SET `value` = ? WHERE `user` = ? AND `app` = ? AND `key` = ? '); - $query->execute(array($value, $user, $app, $key )); } return new OC_OCS_Result(null, 100);