From f45708682fde4fc0d6b1ae4e8280310e2ad17633 Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 15:07:13 -0500 Subject: [PATCH 1/7] CC-5555 : Implement simple caching for preferences using apc to save/fetch preferences to/from a cache. --- airtime_mvc/application/models/Cache.php | 26 +++++++++++ airtime_mvc/application/models/Preference.php | 44 ++++++++++++++----- install_full/ubuntu/airtime-full-install | 2 +- 3 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 airtime_mvc/application/models/Cache.php diff --git a/airtime_mvc/application/models/Cache.php b/airtime_mvc/application/models/Cache.php new file mode 100644 index 000000000..3d89b0649 --- /dev/null +++ b/airtime_mvc/application/models/Cache.php @@ -0,0 +1,26 @@ +beginTransaction(); @@ -103,10 +107,29 @@ class Application_Model_Preference exit; } + $cache->store($key, $value, $userId); + Logging::info("SAVING {$key} {$userId} into cache. = {$value}"); } private static function getValue($key, $isUserValue = false) { + $cache = new Cache(); + + //For user specific preference, check if id matches as well + $userId = null; + if ($isUserValue) { + $auth = Zend_Auth::getInstance(); + if ($auth->hasIdentity()) { + $userId = $auth->getIdentity()->id; + } + } + + $res = $cache->fetch($key, $userId); + if ($res !== false) { + Logging::info("returning {$key} {$userId} from cache. = {$res}"); + return $res; + } + try { //Check if key already exists @@ -117,14 +140,10 @@ class Application_Model_Preference $paramMap[':key'] = $key; //For user specific preference, check if id matches as well - if ($isUserValue) { - $auth = Zend_Auth::getInstance(); - if ($auth->hasIdentity()) { - $id = $auth->getIdentity()->id; - - $sql .= " AND subjid = :id"; - $paramMap[':id'] = $id; - } + if (isset($userId)) { + + $sql .= " AND subjid = :id"; + $paramMap[':id'] = $userId; } $result = Application_Common_Database::prepareAndExecute($sql, $paramMap, Application_Common_Database::COLUMN); @@ -140,14 +159,17 @@ class Application_Model_Preference $paramMap[':key'] = $key; //For user specific preference, check if id matches as well - if ($isUserValue && $auth->hasIdentity()) { + if (isset($userId)) { $sql .= " AND subjid = :id"; - $paramMap[':id'] = $id; + $paramMap[':id'] = $userId; } $result = Application_Common_Database::prepareAndExecute($sql, $paramMap, Application_Common_Database::COLUMN); - return ($result !== false) ? $result : ""; + $res = ($result !== false) ? $result : ""; + $cache->store($key, $res, $userId); + + return $res; } } catch (Exception $e) { header('HTTP/1.0 503 Service Unavailable'); diff --git a/install_full/ubuntu/airtime-full-install b/install_full/ubuntu/airtime-full-install index 13d714407..066f4ab3a 100755 --- a/install_full/ubuntu/airtime-full-install +++ b/install_full/ubuntu/airtime-full-install @@ -59,7 +59,7 @@ libtaglib-ocaml libao-ocaml libmad-ocaml ecasound \ libesd0 libportaudio2 libsamplerate0 rabbitmq-server patch \ php5-curl mpg123 monit python-virtualenv multitail libcamomile-ocaml-data \ libpulse0 vorbis-tools lsb-release lsof sudo mp3gain vorbisgain flac vorbis-tools \ -pwgen libfaad2 +pwgen libfaad2 php-apc #install packages with --force-yes option (this is useful in the case From 06323a40fdb74c7db76a659b351d9da710dad312 Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 16:20:51 -0500 Subject: [PATCH 2/7] CC-5555 : Implement simple caching for preferences fixing up code so user id is never passed in manually to a preference getter. --- .../controllers/UserController.php | 8 +- airtime_mvc/application/models/Preference.php | 156 +++++++++--------- 2 files changed, 84 insertions(+), 80 deletions(-) diff --git a/airtime_mvc/application/controllers/UserController.php b/airtime_mvc/application/controllers/UserController.php index ddfd15c9c..fad0277db 100644 --- a/airtime_mvc/application/controllers/UserController.php +++ b/airtime_mvc/application/controllers/UserController.php @@ -72,8 +72,8 @@ class UserController extends Zend_Controller_Action // Language and timezone settings are saved on a per-user basis // By default, the default language, and timezone setting on // preferences page is what gets assigned. - Application_Model_Preference::SetUserLocale($user->getId()); - Application_Model_Preference::SetUserTimezone($user->getId()); + Application_Model_Preference::SetUserLocale(); + Application_Model_Preference::SetUserTimezone(); $form->reset(); $this->view->form = $form; @@ -143,8 +143,8 @@ class UserController extends Zend_Controller_Action $user->setJabber($formData['cu_jabber']); $user->save(); - Application_Model_Preference::SetUserLocale($user->getId(), $formData['cu_locale']); - Application_Model_Preference::SetUserTimezone($user->getId(), $formData['cu_timezone']); + Application_Model_Preference::SetUserLocale($formData['cu_locale']); + Application_Model_Preference::SetUserTimezone($formData['cu_timezone']); //configure localization with new locale setting Application_Model_Locale::configureLocalization($formData['cu_locale']); diff --git a/airtime_mvc/application/models/Preference.php b/airtime_mvc/application/models/Preference.php index df3455f2d..882fd9a08 100644 --- a/airtime_mvc/application/models/Preference.php +++ b/airtime_mvc/application/models/Preference.php @@ -4,13 +4,26 @@ require_once 'Cache.php'; class Application_Model_Preference { - + + private static function getUserId() + { + //called from a daemon process + if (!class_exists("Zend_Auth", false) || !Zend_Auth::getInstance()->hasIdentity()) { + $userId = null; + } + else { + $auth = Zend_Auth::getInstance(); + $userId = $auth->getIdentity()->id; + } + + return $userId; + } + /** * - * @param integer $userId is not null when we are setting a locale for a specific user * @param boolean $isUserValue is true when we are setting a value for the current user */ - private static function setValue($key, $value, $isUserValue = false, $userId = null) + private static function setValue($key, $value, $isUserValue = false) { $cache = new Cache(); @@ -18,13 +31,13 @@ class Application_Model_Preference $con = Propel::getConnection(CcPrefPeer::DATABASE_NAME); $con->beginTransaction(); - //called from a daemon process - if (!class_exists("Zend_Auth", false) || !Zend_Auth::getInstance()->hasIdentity()) { - $id = NULL; - } else { - $auth = Zend_Auth::getInstance(); - $id = $auth->getIdentity()->id; + $userId = self::getUserId(); + + if ($isUserValue && is_null($userId)) { + throw new Exception("User id can't be null for a user preference."); } + + Application_Common_Database::prepareAndExecute("LOCK TABLE cc_pref"); //Check if key already exists $sql = "SELECT COUNT(*) FROM cc_pref" @@ -34,15 +47,10 @@ class Application_Model_Preference $paramMap[':key'] = $key; //For user specific preference, check if id matches as well - if ($isUserValue && is_null($userId)) { + if ($isUserValue) { $sql .= " AND subjid = :id"; - $paramMap[':id'] = $id; - } else if (!is_null($userId)) { - $sql .= " AND subjid= :id"; $paramMap[':id'] = $userId; - } - - Application_Common_Database::prepareAndExecute("LOCK TABLE cc_pref"); + } $result = Application_Common_Database::prepareAndExecute($sql, $paramMap, @@ -55,39 +63,39 @@ class Application_Model_Preference //this case should not happen. throw new Exception("Invalid number of results returned. Should be ". "0 or 1, but is '$result' instead"); - } elseif ($result == 1) { + } + elseif ($result == 1) { + // result found - if (is_null($id) || !$isUserValue) { + if (is_null($userId)) { // system pref $sql = "UPDATE cc_pref" ." SET subjid = NULL, valstr = :value" ." WHERE keystr = :key"; - } else { + } + else { // user pref $sql = "UPDATE cc_pref" . " SET valstr = :value" . " WHERE keystr = :key AND subjid = :id"; - if (is_null($userId)) { - $paramMap[':id'] = $id; - } else { - $paramMap[':id'] = $userId; - } + + $paramMap[':id'] = $userId; } - } else { + } + else { + // result not found - if (is_null($id) || !$isUserValue) { + if (is_null($userId)) { // system pref $sql = "INSERT INTO cc_pref (keystr, valstr)" ." VALUES (:key, :value)"; - } else { + } + else { // user pref $sql = "INSERT INTO cc_pref (subjid, keystr, valstr)" ." VALUES (:id, :key, :value)"; - if (is_null($userId)) { - $paramMap[':id'] = $id; - } else { - $paramMap[':id'] = $userId; - } + + $paramMap[':id'] = $userId; } } $paramMap[':key'] = $key; @@ -100,7 +108,8 @@ class Application_Model_Preference $con); $con->commit(); - } catch (Exception $e) { + } + catch (Exception $e) { $con->rollback(); header('HTTP/1.0 503 Service Unavailable'); Logging::info("Database error: ".$e->getMessage()); @@ -115,22 +124,19 @@ class Application_Model_Preference { $cache = new Cache(); - //For user specific preference, check if id matches as well - $userId = null; - if ($isUserValue) { - $auth = Zend_Auth::getInstance(); - if ($auth->hasIdentity()) { - $userId = $auth->getIdentity()->id; - } - } - - $res = $cache->fetch($key, $userId); - if ($res !== false) { - Logging::info("returning {$key} {$userId} from cache. = {$res}"); - return $res; - } - try { + + $userId = self::getUserId(); + + if ($isUserValue && is_null($userId)) { + throw new Exception("User id can't be null for a user preference."); + } + + $res = $cache->fetch($key, $userId); + if ($res !== false) { + Logging::info("returning {$key} {$userId} from cache. = {$res}"); + return $res; + } //Check if key already exists $sql = "SELECT COUNT(*) FROM cc_pref" @@ -148,8 +154,9 @@ class Application_Model_Preference $result = Application_Common_Database::prepareAndExecute($sql, $paramMap, Application_Common_Database::COLUMN); + //return an empty string if the result doesn't exist. if ($result == 0) { - return ""; + $res = ""; } else { $sql = "SELECT valstr FROM cc_pref" @@ -167,11 +174,12 @@ class Application_Model_Preference $result = Application_Common_Database::prepareAndExecute($sql, $paramMap, Application_Common_Database::COLUMN); $res = ($result !== false) ? $result : ""; - $cache->store($key, $res, $userId); - - return $res; } - } catch (Exception $e) { + + $cache->store($key, $res, $userId); + return $res; + } + catch (Exception $e) { header('HTTP/1.0 503 Service Unavailable'); Logging::info("Could not connect to database: ".$e->getMessage()); exit; @@ -531,6 +539,7 @@ class Application_Model_Preference public static function SetDefaultTimezone($timezone) { self::setValue("timezone", $timezone); + //TODO check this if setting value failes. date_default_timezone_set($timezone); } @@ -540,14 +549,14 @@ class Application_Model_Preference return self::getValue("timezone"); } - public static function SetUserTimezone($userId, $timezone = null) + public static function SetUserTimezone($timezone = null) { // When a new user is created they will get the default timezone // setting which the admin sets on preferences page if (is_null($timezone)) { $timezone = self::GetDefaultTimezone(); } - self::setValue("user_timezone", $timezone, true, $userId); + self::setValue("user_timezone", $timezone, true); } public static function GetUserTimezone($id) @@ -563,11 +572,12 @@ class Application_Model_Preference // Always attempts to returns the current user's personal timezone setting public static function GetTimezone() { - $auth = Zend_Auth::getInstance(); - if ($auth->hasIdentity()) { - $id = $auth->getIdentity()->id; - return self::GetUserTimezone($id); - } else { + $userId = self::getUserId(); + + if (!is_null($userId)) { + return self::GetUserTimezone(); + } + else { return self::GetDefaultTimezone(); } } @@ -583,7 +593,7 @@ class Application_Model_Preference return self::getValue("locale"); } - public static function GetUserLocale($id) + public static function GetUserLocale() { $locale = self::getValue("user_locale", true); if (!$locale) { @@ -593,23 +603,24 @@ class Application_Model_Preference } } - public static function SetUserLocale($userId, $locale = null) + public static function SetUserLocale($locale = null) { // When a new user is created they will get the default locale // setting which the admin sets on preferences page if (is_null($locale)) { $locale = self::GetDefaultLocale(); } - self::setValue("user_locale", $locale, true, $userId); + self::setValue("user_locale", $locale, true); } public static function GetLocale() { - $auth = Zend_Auth::getInstance(); - if ($auth->hasIdentity()) { - $id = $auth->getIdentity()->id; - return self::GetUserLocale($id); - } else { + $userId = self::getUserId(); + + if (!is_null($userId)) { + return self::GetUserLocale($userId); + } + else { return self::GetDefaultLocale(); } } @@ -1317,15 +1328,8 @@ class Application_Model_Preference public static function setCurrentLibraryTableSetting($settings) { - $num_columns = count(Application_Model_StoredFile::getLibraryColumns()); - $new_columns_num = count($settings['abVisCols']); - - /*if ($num_columns != $new_columns_num) { - throw new Exception("Trying to write a user column preference with incorrect number of columns!"); - }*/ - $data = serialize($settings); - $v = self::setValue("library_datatable", $data, true); + self::setValue("library_datatable", $data, true); } public static function getCurrentLibraryTableSetting() From f69dd123ca2a6b6b2764b4c4c8fb1bb3773bf3fa Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 16:32:43 -0500 Subject: [PATCH 3/7] CC-5555 : Implement simple caching for preferences making sure locale/timezone is returned properly in generic calls. --- airtime_mvc/application/models/Preference.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/airtime_mvc/application/models/Preference.php b/airtime_mvc/application/models/Preference.php index 882fd9a08..956db1c0d 100644 --- a/airtime_mvc/application/models/Preference.php +++ b/airtime_mvc/application/models/Preference.php @@ -559,12 +559,13 @@ class Application_Model_Preference self::setValue("user_timezone", $timezone, true); } - public static function GetUserTimezone($id) + public static function GetUserTimezone() { $timezone = self::getValue("user_timezone", true); if (!$timezone) { return self::GetDefaultTimezone(); - } else { + } + else { return $timezone; } } @@ -598,7 +599,8 @@ class Application_Model_Preference $locale = self::getValue("user_locale", true); if (!$locale) { return self::GetDefaultLocale(); - } else { + } + else { return $locale; } } @@ -618,7 +620,7 @@ class Application_Model_Preference $userId = self::getUserId(); if (!is_null($userId)) { - return self::GetUserLocale($userId); + return self::GetUserLocale(); } else { return self::GetDefaultLocale(); From f615e176e26ca78ebaa2f81758481ecc6051b3ca Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 16:36:21 -0500 Subject: [PATCH 4/7] CC-5555 : Implement simple caching for preferences better to include the key in the error message --- airtime_mvc/application/models/Preference.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airtime_mvc/application/models/Preference.php b/airtime_mvc/application/models/Preference.php index 956db1c0d..7f1656341 100644 --- a/airtime_mvc/application/models/Preference.php +++ b/airtime_mvc/application/models/Preference.php @@ -34,7 +34,7 @@ class Application_Model_Preference $userId = self::getUserId(); if ($isUserValue && is_null($userId)) { - throw new Exception("User id can't be null for a user preference."); + throw new Exception("User id can't be null for a user preference {$key}."); } Application_Common_Database::prepareAndExecute("LOCK TABLE cc_pref"); From 0bae60138f6458dbe0592c049d66d1cc88bb47b3 Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 16:50:35 -0500 Subject: [PATCH 5/7] CC-5555 : Implement simple caching for preferences forgot to get rid of user id here. --- airtime_mvc/application/controllers/LoginController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airtime_mvc/application/controllers/LoginController.php b/airtime_mvc/application/controllers/LoginController.php index edb8222d7..4c58a6b57 100644 --- a/airtime_mvc/application/controllers/LoginController.php +++ b/airtime_mvc/application/controllers/LoginController.php @@ -70,7 +70,7 @@ class LoginController extends Zend_Controller_Action $tempSess->referrer = 'login'; //set the user locale in case user changed it in when logging in - Application_Model_Preference::SetUserLocale($auth->getIdentity()->id, $locale); + Application_Model_Preference::SetUserLocale($locale); $this->_redirect('Showbuilder'); } else { From b8a084998e27b3ea59bf6cac8dc14ffa86012151 Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 17:08:15 -0500 Subject: [PATCH 6/7] CC-5555 : Implement simple caching for preferences cache key creation change. --- airtime_mvc/application/models/Cache.php | 16 +++++++++++----- airtime_mvc/application/models/Preference.php | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/airtime_mvc/application/models/Cache.php b/airtime_mvc/application/models/Cache.php index 3d89b0649..fc473395a 100644 --- a/airtime_mvc/application/models/Cache.php +++ b/airtime_mvc/application/models/Cache.php @@ -3,24 +3,30 @@ class Cache { - private function createCacheKey($key, $userId = null) { + private function createCacheKey($key, $isUserValue, $userId = null) { $CC_CONFIG = Config::getConfig(); $a = $CC_CONFIG["apiKey"][0]; - $cacheKey = "{$key}{$userId}{$a}"; + if ($isUserValue) { + $cacheKey = "{$key}{$userId}{$a}"; + } + else { + $cacheKey = "{$key}{$a}"; + } + return $cacheKey; } - public function store($key, $value, $userId = null) { + public function store($key, $value, $isUserValue, $userId = null) { $cacheKey = self::createCacheKey($key, $userId); return apc_store($cacheKey, $value); } - public function fetch($key, $userId = null) { + public function fetch($key, $isUserValue, $userId = null) { - $cacheKey = self::createCacheKey($key, $userId); + $cacheKey = self::createCacheKey($key, $isUserValue, $userId); return apc_fetch($cacheKey); } } \ No newline at end of file diff --git a/airtime_mvc/application/models/Preference.php b/airtime_mvc/application/models/Preference.php index 7f1656341..660eaf825 100644 --- a/airtime_mvc/application/models/Preference.php +++ b/airtime_mvc/application/models/Preference.php @@ -116,7 +116,7 @@ class Application_Model_Preference exit; } - $cache->store($key, $value, $userId); + $cache->store($key, $value, $isUserValue, $userId); Logging::info("SAVING {$key} {$userId} into cache. = {$value}"); } @@ -132,7 +132,7 @@ class Application_Model_Preference throw new Exception("User id can't be null for a user preference."); } - $res = $cache->fetch($key, $userId); + $res = $cache->fetch($key, $isUserValue, $userId); if ($res !== false) { Logging::info("returning {$key} {$userId} from cache. = {$res}"); return $res; @@ -176,7 +176,7 @@ class Application_Model_Preference $res = ($result !== false) ? $result : ""; } - $cache->store($key, $res, $userId); + $cache->store($key, $res, $isUserValue, $userId); return $res; } catch (Exception $e) { From 757e35aaf942989618376869340a44f4780f21fa Mon Sep 17 00:00:00 2001 From: Naomi Date: Mon, 11 Nov 2013 17:22:55 -0500 Subject: [PATCH 7/7] CC-5555 : Implement simple caching for preferences comment out logging to not flood logs. --- airtime_mvc/application/models/Preference.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airtime_mvc/application/models/Preference.php b/airtime_mvc/application/models/Preference.php index 660eaf825..33269acba 100644 --- a/airtime_mvc/application/models/Preference.php +++ b/airtime_mvc/application/models/Preference.php @@ -117,7 +117,7 @@ class Application_Model_Preference } $cache->store($key, $value, $isUserValue, $userId); - Logging::info("SAVING {$key} {$userId} into cache. = {$value}"); + //Logging::info("SAVING {$key} {$userId} into cache. = {$value}"); } private static function getValue($key, $isUserValue = false) @@ -134,7 +134,7 @@ class Application_Model_Preference $res = $cache->fetch($key, $isUserValue, $userId); if ($res !== false) { - Logging::info("returning {$key} {$userId} from cache. = {$res}"); + //Logging::info("returning {$key} {$userId} from cache. = {$res}"); return $res; }