Merge pull request #7253 from owncloud/preferences-caching

Add caching to OC\Preferences

2 files changed, 86 insertions, 89 deletions

diff --git a/lib/private/preferences.php b/lib/private/preferences.php
index 359d9a83589..7ebbf7aa970 100644
--- a/lib/private/preferences.php
+++ b/lib/private/preferences.php
@@ -43,8 +43,26 @@ use \OC\DB\Connection;
  * This class provides an easy way for storing user preferences.
  */
 class Preferences {
+	/**
+	 * @var \OC\DB\Connection
+	 */
 	protected $conn;
 
+	/**
+	 * 3 dimensional array with the following structure:
+	 * [ $userId =>
+	 *     [ $appId =>
+	 *         [ $key => $value ]
+	 *     ]
+	 * ]
+	 *
+	 * @var array $cache
+	 */
+	protected $cache = array();
+
+	/**
+	 * @param \OC\DB\Connection $conn
+	 */
 	public function __construct(Connection $conn) {
 		$this->conn = $conn;
 	}
@@ -58,10 +76,10 @@ class Preferences {
 	 */
 	public function getUsers() {
 		$query = 'SELECT DISTINCT `userid` FROM `*PREFIX*preferences`';
-		$result = $this->conn->executeQuery( $query );
+		$result = $this->conn->executeQuery($query);
 
 		$users = array();
-		while( $userid = $result->fetchColumn()) {
+		while ($userid = $result->fetchColumn()) {
 			$users[] = $userid;
 		}
 
@@ -69,6 +87,28 @@ class Preferences {
 	}
 
 	/**
+	 * @param string $user
+	 * @return array[]
+	 */
+	protected function getUserValues($user) {
+		if (isset($this->cache[$user])) {
+			return $this->cache[$user];
+		}
+		$data = array();
+		$query = 'SELECT `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?';
+		$result = $this->conn->executeQuery($query, array($user));
+		while ($row = $result->fetch()) {
+			$app = $row['appid'];
+			if (!isset($data[$app])) {
+				$data[$app] = array();
+			}
+			$data[$app][$row['configkey']] = $row['configvalue'];
+		}
+		$this->cache[$user] = $data;
+		return $data;
+	}
+
+	/**
 	 * @brief Get all apps of an user
 	 * @param string $user user
 	 * @return array with app ids
@@ -76,16 +116,9 @@ class Preferences {
 	 * This function returns a list of all apps of the user that have at least
 	 * one entry in the preferences table.
 	 */
-	public function getApps( $user ) {
-		$query = 'SELECT DISTINCT `appid` FROM `*PREFIX*preferences` WHERE `userid` = ?';
-		$result = $this->conn->executeQuery( $query, array( $user ) );
-
-		$apps = array();
-		while( $appid = $result->fetchColumn()) {
-			$apps[] = $appid;
-		}
-
-		return $apps;
+	public function getApps($user) {
+		$data = $this->getUserValues($user);
+		return array_keys($data);
 	}
 
 	/**
@@ -97,16 +130,13 @@ class Preferences {
 	 * This function gets all keys of an app of an user. Please note that the
 	 * values are not returned.
 	 */
-	public function getKeys( $user, $app ) {
-		$query = 'SELECT `configkey` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ?';
-		$result = $this->conn->executeQuery( $query, array( $user, $app ));
-
-		$keys = array();
-		while( $key = $result->fetchColumn()) {
-			$keys[] = $key;
+	public function getKeys($user, $app) {
+		$data = $this->getUserValues($user);
+		if (isset($data[$app])) {
+			return array_keys($data[$app]);
+		} else {
+			return array();
 		}
-
-		return $keys;
 	}
 
 	/**
@@ -120,13 +150,10 @@ class Preferences {
 	 * This function gets a value from the preferences table. If the key does
 	 * not exist the default value will be returned
 	 */
-	public function getValue( $user, $app, $key, $default = null ) {
-		// Try to fetch the value, return default if not exists.
-		$query = 'SELECT `configvalue` FROM `*PREFIX*preferences`'
-			.' WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?';
-		$row = $this->conn->fetchAssoc( $query, array( $user, $app, $key ));
-		if($row) {
-			return $row["configvalue"];
+	public function getValue($user, $app, $key, $default = null) {
+		$data = $this->getUserValues($user);
+		if (isset($data[$app]) and isset($data[$app][$key])) {
+			return $data[$app][$key];
 		} else {
 			return $default;
 		}
@@ -142,14 +169,14 @@ class Preferences {
 	 * Adds a value to the preferences. If the key did not exist before, it
 	 * will be added automagically.
 	 */
-	public function setValue( $user, $app, $key, $value ) {
+	public function setValue($user, $app, $key, $value) {
 		// Check if the key does exist
 		$query = 'SELECT COUNT(*) FROM `*PREFIX*preferences`'
-			.' WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?';
-		$count = $this->conn->fetchColumn( $query, array( $user, $app, $key ));
+			. ' WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?';
+		$count = $this->conn->fetchColumn($query, array($user, $app, $key));
 		$exists = $count > 0;
 
-		if( !$exists ) {
+		if (!$exists) {
 			$data = array(
 				'userid' => $user,
 				'appid' => $app,
@@ -168,6 +195,14 @@ class Preferences {
 			);
 			$this->conn->update('*PREFIX*preferences', $data, $where);
 		}
+
+		// only add to the cache if we already loaded data for the user
+		if (isset($this->cache[$user])) {
+			if (!isset($this->cache[$user][$app])) {
+				$this->cache[$user][$app] = array();
+			}
+			$this->cache[$user][$app][$key] = $value;
+		}
 	}
 
 	/**
@@ -178,13 +213,17 @@ class Preferences {
 	 *
 	 * Deletes a key.
 	 */
-	public function deleteKey( $user, $app, $key ) {
+	public function deleteKey($user, $app, $key) {
 		$where = array(
 			'userid' => $user,
 			'appid' => $app,
 			'configkey' => $key,
 		);
 		$this->conn->delete('*PREFIX*preferences', $where);
+
+		if (isset($this->cache[$user]) and isset($this->cache[$user][$app])) {
+			unset($this->cache[$user][$app][$key]);
+		}
 	}
 
 	/**
@@ -194,12 +233,16 @@ class Preferences {
 	 *
 	 * Removes all keys in preferences belonging to the app and the user.
 	 */
-	public function deleteApp( $user, $app ) {
+	public function deleteApp($user, $app) {
 		$where = array(
 			'userid' => $user,
 			'appid' => $app,
 		);
 		$this->conn->delete('*PREFIX*preferences', $where);
+
+		if (isset($this->cache[$user])) {
+			unset($this->cache[$user][$app]);
+		}
 	}
 
 	/**
@@ -208,11 +251,13 @@ class Preferences {
 	 *
 	 * Removes all keys in preferences belonging to the user.
 	 */
-	public function deleteUser( $user ) {
+	public function deleteUser($user) {
 		$where = array(
 			'userid' => $user,
 		);
 		$this->conn->delete('*PREFIX*preferences', $where);
+
+		unset($this->cache[$user]);
 	}
 
 	/**
@@ -221,12 +266,16 @@ class Preferences {
 	 *
 	 * Removes all keys in preferences belonging to the app.
 	 */
-	public function deleteAppFromAllUsers( $app ) {
+	public function deleteAppFromAllUsers($app) {
 		$where = array(
 			'appid' => $app,
 		);
 		$this->conn->delete('*PREFIX*preferences', $where);
+
+		foreach ($this->cache as &$userCache) {
+			unset($userCache[$app]);
+		}
 	}
 }
 
-require_once __DIR__.'/legacy/'.basename(__FILE__);
+require_once __DIR__ . '/legacy/' . basename(__FILE__);
diff --git a/tests/lib/preferences.php b/tests/lib/preferences.php
index a8236909ded..f1f6ed08003 100644
--- a/tests/lib/preferences.php
+++ b/tests/lib/preferences.php
@@ -144,58 +144,6 @@ class Test_Preferences_Object extends PHPUnit_Framework_TestCase {
 		$this->assertEquals(array('foo'), $apps);
 	}
 
-	public function testGetApps()
-	{
-		$statementMock = $this->getMock('\Doctrine\DBAL\Statement', array(), array(), '', false);
-		$statementMock->expects($this->exactly(2))
-			->method('fetchColumn')
-			->will($this->onConsecutiveCalls('foo', false));
-		$connectionMock = $this->getMock('\OC\DB\Connection', array(), array(), '', false);
-		$connectionMock->expects($this->once())
-			->method('executeQuery')
-			->with($this->equalTo('SELECT DISTINCT `appid` FROM `*PREFIX*preferences` WHERE `userid` = ?'),
-				$this->equalTo(array('bar')))
-			->will($this->returnValue($statementMock));
-
-		$preferences = new OC\Preferences($connectionMock);
-		$apps = $preferences->getApps('bar');
-		$this->assertEquals(array('foo'), $apps);
-	}
-
-	public function testGetKeys()
-	{
-		$statementMock = $this->getMock('\Doctrine\DBAL\Statement', array(), array(), '', false);
-		$statementMock->expects($this->exactly(2))
-			->method('fetchColumn')
-			->will($this->onConsecutiveCalls('foo', false));
-		$connectionMock = $this->getMock('\OC\DB\Connection', array(), array(), '', false);
-		$connectionMock->expects($this->once())
-			->method('executeQuery')
-			->with($this->equalTo('SELECT `configkey` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ?'),
-				$this->equalTo(array('bar', 'moo')))
-			->will($this->returnValue($statementMock));
-
-		$preferences = new OC\Preferences($connectionMock);
-		$keys = $preferences->getKeys('bar', 'moo');
-		$this->assertEquals(array('foo'), $keys);
-	}
-
-	public function testGetValue()
-	{
-		$connectionMock = $this->getMock('\OC\DB\Connection', array(), array(), '', false);
-		$connectionMock->expects($this->exactly(2))
-			->method('fetchAssoc')
-			->with($this->equalTo('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'),
-				$this->equalTo(array('grg', 'bar', 'red')))
-			->will($this->onConsecutiveCalls(array('configvalue'=>'foo'), null));
-
-		$preferences = new OC\Preferences($connectionMock);
-		$value = $preferences->getValue('grg', 'bar', 'red');
-		$this->assertEquals('foo', $value);
-		$value = $preferences->getValue('grg', 'bar', 'red', 'def');
-		$this->assertEquals('def', $value);
-	}
-
 	public function testSetValue()
 	{
 		$connectionMock = $this->getMock('\OC\DB\Connection', array(), array(), '', false);