diff --git a/db/migrations/1.191_add_dedicated_admins_role.php b/db/migrations/1.191_add_dedicated_admins_role.php index 3e637a86881c99a533f5a5a7d6f9da8b0a7c9864..9a0c5eec1d9b1d057fe35dd04032b103191ef822 100644 --- a/db/migrations/1.191_add_dedicated_admins_role.php +++ b/db/migrations/1.191_add_dedicated_admins_role.php @@ -14,7 +14,7 @@ class AddDedicatedAdminsRole extends Migration INSERT INTO `roles` (`rolename`, `system`) VALUES ('DedicatedAdmin', 'n'); "); - RolePersistence::expireCaches(); + RolePersistence::expireRolesCache(); } public function down() @@ -22,7 +22,7 @@ class AddDedicatedAdminsRole extends Migration DBManager::get()->execute(" DELETE FROM `roles` WHERE `rolename` = 'DedicatedAdmin' AND `system` = 'n' "); - RolePersistence::expireCaches(); + RolePersistence::expireRolesCache(); } } diff --git a/db/migrations/1.199_step_00302_modulverwaltung.php b/db/migrations/1.199_step_00302_modulverwaltung.php index f6c453e219a09b66a22890aff856ae923e53bba7..8bdc80dc41e49bb576aa5bcf17f89943fb6188b4 100644 --- a/db/migrations/1.199_step_00302_modulverwaltung.php +++ b/db/migrations/1.199_step_00302_modulverwaltung.php @@ -553,7 +553,7 @@ class Step00302Modulverwaltung extends Migration RolePersistence::assignPluginRoles($plugin_id, $role_ids); } - RolePersistence::expireCaches(); + RolePersistence::expireRolesCache(); /** * Logging diff --git a/lib/classes/AutoInsert.class.php b/lib/classes/AutoInsert.class.php index ee2197e7721d570f825f03d0f8a96cc19c420988..2ee009fa39f089a00067eb8caee58bf4472df215 100644 --- a/lib/classes/AutoInsert.class.php +++ b/lib/classes/AutoInsert.class.php @@ -150,7 +150,7 @@ class AutoInsert private function addUser($user_id, $seminar) { - $query = "INSERT IGNORE INTO seminar_user (Seminar_id, user_id, status, gruppe, mkdate) + $query = "INSERT IGNORE INTO seminar_user (Seminar_id, user_id, status, gruppe, mkdate) VALUES (?, ?, 'autor', ?, UNIX_TIMESTAMP())"; $statement = DBManager::get()->prepare($query); $statement->execute([$seminar['Seminar_id'], $user_id, select_group($seminar['start_time'])]); @@ -353,12 +353,11 @@ class AutoInsert /** * Returns the cache for seminars. - * @return StudipCachedArray */ - protected static function getSeminarCache() + protected static function getSeminarCache(): StudipCachedArray { if (self::$seminar_cache === null) { - self::$seminar_cache = new StudipCachedArray('/AutoInsertSeminars'); + self::$seminar_cache = new StudipCachedArray('AutoInsertSeminars'); } return self::$seminar_cache; } diff --git a/lib/classes/StudipCachedArray.php b/lib/classes/StudipCachedArray.php index 1d07ecf61bd324c56a01f1006afbf86b1200b36d..221b76c3aafd1ac0b18c7efb23f6138d447b54d1 100644 --- a/lib/classes/StudipCachedArray.php +++ b/lib/classes/StudipCachedArray.php @@ -3,60 +3,43 @@ * This class represents an array in cache and removes the neccessity to * encode/decode and store the data after every change. * - * Caching is handled by splitting the data into partitions based on the key. - * * @author Jan-Hendrik Willms <tleilax+studip@gmail.com> * @license GPL2 or any later version * @since Stud.IP 5.0 - * - * @todo Automagic partition size? */ -class StudipCachedArray implements ArrayAccess, Countable +class StudipCachedArray implements ArrayAccess { - const ENCODE_JSON = 0; - const ENCODE_SERIALIZE = 1; - protected $key; protected $cache; - protected $partitions; - protected $partition_by; - protected $encoding; + protected $duration; protected $data = []; /** * Constructs the cached array * - * @param string $key Cache key where the array is/should be stored - * @param mixed $partition_by Defines the partitioning, this may either be - * an int which will be length of the substring - * of the given chache offset or a callable which - * will return the partition key. + * @param string $key Cache key where the array is/should be stored + * an int which will be length of the substring + * of the given chache offset or a callable which + * will return the partition key. + * @param int $duration Duration in seconds for which the item shall be + * stored */ - public function __construct($key, $partition_by = 1, $encoding = self::ENCODE_JSON, StudipCache $cache = null) + public function __construct(string $key, int $duration = StudipCache::DEFAULT_EXPIRATION) { - if (!is_callable($partition_by) && !is_int($partition_by)) { - throw new Exception('Parameter $partition_by may only be a number or a callable if set'); - } - if (ctype_digit($partition_by) && $partition_by <= 1) { - throw new Exception('Parameter $partition_by must be positive and not zero'); - } + $this->key = self::class . "/{$key}"; + $this->cache = StudipCacheFactory::getCache(); + $this->duration = $duration; - $this->key = $key; - $this->partition_by = $partition_by; - $this->encoding = $encoding; - $this->setCache($cache ?? StudipCacheFactory::getCache()); + $this->reset(); } /** - * Sets the cache for this array and resets internal states. - * - * @param StudipCache $cache + * Clears cached values from memory, but does not remove them from the cache. */ - public function setCache(StudipCache $cache) + public function reset(): void { - $this->cache = $cache; - $this->reset(); + $this->data = []; } /** @@ -65,12 +48,10 @@ class StudipCachedArray implements ArrayAccess, Countable * @param string $offset Offset * @return bool */ - public function offsetExists($offset) + public function offsetExists($offset): bool { - return array_key_exists( - $offset, - $this->loadData($offset) - ); + $this->loadData($offset); + return isset($this->data[$offset]); } /** @@ -81,7 +62,8 @@ class StudipCachedArray implements ArrayAccess, Countable */ public function offsetGet($offset) { - return $this->loadData($offset)[$offset] ?? null; + $this->loadData($offset); + return $this->data[$offset]; } /** @@ -90,192 +72,90 @@ class StudipCachedArray implements ArrayAccess, Countable * @param string $offset Offset * @param mixed $value Value */ - public function offsetSet($offset, $value) - { - $this->loadData($offset)[$offset] = $value; - - $this->storeData($offset); - } - - /** - * Unsets the value at a given offset - * - * @param string $offset Offset - */ - public function offsetUnset($offset) + public function offsetSet($offset, $value): void { - $data = &$this->loadData($offset); - - if (array_key_exists($offset, $data)) { - unset($data[$offset]); - $this->storeData($offset); + if ($offset === null) { + throw new Exception('Cannot push to cached array, use StudipCachedArray instead'); } - } - /** - * Counts all values in chache. - * @return int - */ - public function count() - { - return array_sum($this->partitions); - } + if (!isset($this->data[$offset]) || $this->data[$offset] !== $value) { + $this->data[$offset] = $value; - /** - * Clears all data. - */ - public function clear() - { - foreach (array_keys($this->partitions) as $partition) { - $this->cache->expire($this->getPartitionKey($partition)); - } - - $this->partitions = []; - $this->data = []; - - $this->storeData(); - } - - /** - * Returns all items in cache as an array. - * @return array - */ - public function getArrayCopy() - { - $result = []; - foreach (array_keys($this->partitions) as $partition) { - foreach ($this->loadData($partition) as $key => $value) { - $result[$key] = $value; - } + $this->storeData($offset); } - return $result; } /** - * Loads all partitions of this cache and resets the data array - * @return array + * Unsets the value at a given offset + * + * @param string $offset Offset */ - public function reset() + public function offsetUnset($offset): void { - $this->data = []; - $this->partitions = []; - - $cached = $this->cache->read($this->key); - if ($cached) { - $this->partitions = $this->decode($cached); - } + $this->cache->expire($this->getCacheKey($offset)); + unset($this->data[$offset]); } /** * Loads the data from cache. + * + * @param string $offset Offset to load */ - protected function &loadData($offset) + protected function loadData(string $offset) { - $partition = $this->getPartition($offset); - if (!isset($this->data[$partition])) { - $cached = $this->cache->read($this->getPartitionKey($offset)); - if ($cached) { - $this->data[$partition] = $this->decode($cached); - } else { - $this->data[$partition] = []; - } + if (!array_key_exists($offset, $this->data)) { + $cached = $this->cache->read($this->getCacheKey($offset)); + $this->data[$offset] = $this->swapNullAndFalse($cached); } - return $this->data[$partition]; + return $this->data[$offset]; } /** * Stores the data back to the cache. + * Data needs to be wrapped in another array so that we can correctly read + * back a value of "false". + * + * @param string $offset Offset to store */ - protected function storeData($offset = null) + protected function storeData(string $offset): void { - $partition = false; - - if ($offset !== null) { - $partition = $this->getPartition($offset); - if (!array_key_exists($partition, $this->partitions) || count($this->data[$partition]) > 0) { - $this->partitions[$partition] = count($this->data[$partition]); - } elseif (array_key_exists($partition, $this->partitions) && count($this->data[$partition]) === 0) { - unset($this->partitions[$partition]); - } - } - - foreach ($this->data as $p => $data) { - if ($partition === false || $p == $partition) { - $key = $this->getPartitionKey($p); - - if (count($data) === 0) { - $this->cache->expire($key); - } else { - $this->cache->write($key, $this->encode($data)); - } - } - } - - if (count($this->partitions) === 0) { - $this->cache->expire($this->key); - } else { - $this->cache->write($this->key, $this->encode($this->partitions)); - } + $this->cache->write( + $this->getCacheKey($offset), + $this->swapNullAndFalse($this->data[$offset]), + $this->duration + ); } /** - * Encodes the given data based on the set encoding mechanism. - * @param mixed $data + * Returns the cache key for a specific offset. + * + * @param string $offset Offset of the cached item * @return string */ - protected function encode($data) + private function getCacheKey(string $offset): string { - if ($this->encoding === self::ENCODE_JSON) { - return json_encode($data); - } - - if ($this->encoding === self::ENCODE_SERIALIZE) { - return serialize($data); - } - - throw new Exception('Unknown encoding type'); + return rtrim($this->key, '/') . "/{$offset}"; } /** - * Decodes the given data based on the set encoding mechanism. - * @param string $data + * Swaps null and false for a value because the Stud.IP cache will return + * false if a cached item is not found instead of null. + * + * @param mixed $value Value to swap if appropriate + * * @return mixed */ - protected function decode($data) + private function swapNullAndFalse($value) { - if ($this->encoding === self::ENCODE_JSON) { - return json_decode($data, true); + if ($value === null) { + return false; } - if ($this->encoding === self::ENCODE_SERIALIZE) { - return unserialize($data); - } - - throw new Exception('Unknown decoding type'); - } - - /** - * Extracts the partition from the key. - * @param string $offset - * @return string partition - */ - protected function getPartition($offset) - { - if (is_callable($this->partition_by)) { - return call_user_func($this->partition_by, $offset); + if ($value === false) { + return null; } - return mb_substr($offset, 0, $this->partition_by); - } - /** - * Returns the partition key for storage. - * - * @param string $offset - * @return string - */ - protected function getPartitionKey($offset) - { - return rtrim($this->key, '/') . '/' . $this->getPartition($offset); + return $value; } } diff --git a/lib/functions.php b/lib/functions.php index e2cc1d7f3b0152be307bec88d70dac2003ec1a35..8a0e4036cfb0b2193dfaa5c1d4a5eed289207ded 100644 --- a/lib/functions.php +++ b/lib/functions.php @@ -197,7 +197,7 @@ function get_object_type($id, $check_only = []) // Initialize cache array if ($cache === null) { - $cache = new StudipCachedArray('/Studip/ObjectTypes'); + $cache = new StudipCachedArray('Studip/ObjectTypes'); } // No cached entry available? Go ahead and determine type diff --git a/lib/plugins/db/RolePersistence.class.php b/lib/plugins/db/RolePersistence.class.php index 358e8f1149492376afaaea722fc6df26c2bcc825..a66b9a0be2393894a4d719874263bc84189a686a 100644 --- a/lib/plugins/db/RolePersistence.class.php +++ b/lib/plugins/db/RolePersistence.class.php @@ -13,32 +13,36 @@ */ class RolePersistence { + const ROLES_CACHE_KEY = 'plugins/rolepersistence/roles'; + /** * Returns all available roles. * * @return array Roles */ - public static function getAllRoles() + public static function getAllRoles(): array { // read cache - $cache = self::getRolesCache(); + $cache = StudipCacheFactory::getCache(); // cache miss, retrieve from database - if (count($cache) === 0) { + $roles = $cache->read(self::ROLES_CACHE_KEY); + if (!$roles) { $query = "SELECT `roleid`, `rolename`, `system` = 'y' AS `is_system` FROM `roles` ORDER BY `rolename`"; $statement = DBManager::get()->query($query); $statement->setFetchMode(PDO::FETCH_ASSOC); + $roles = []; foreach ($statement as $row) { - extract($row); - - $cache[$roleid] = new Role($roleid, $rolename, $is_system); + $roles[$row['roleid']] = new Role($row['roleid'], $row['rolename'], $row['is_system']); } + + $cache->write(self::ROLES_CACHE_KEY, $roles); } - return $cache->getArrayCopy(); + return $roles; } public static function getRoleIdByName($name) @@ -55,7 +59,7 @@ class RolePersistence * Inserts the role into the database or does an update, if it's already there * * @param Role $role - * @return the role id + * @return int the role id */ public static function saveRole($role) { @@ -469,28 +473,13 @@ class RolePersistence } // Cache operations - private static $roles_cache = null; private static $user_roles_cache = null; private static $plugin_roles_cache = null; - private static function getRolesCache() - { - if (self::$roles_cache === null) { - self::$roles_cache = new StudipCachedArray( - '/Roles', - function () { - return 'All'; - }, - StudipCachedArray::ENCODE_SERIALIZE - ); - } - return self::$roles_cache; - } - private static function getUserRolesCache() { if (self::$user_roles_cache === null) { - self::$user_roles_cache = new StudipCachedArray('/UserRoles'); + self::$user_roles_cache = new StudipCachedArray('UserRoles'); } return self::$user_roles_cache; } @@ -498,20 +487,18 @@ class RolePersistence private static function getPluginRolesCache() { if (self::$plugin_roles_cache === null) { - self::$plugin_roles_cache = new StudipCachedArray('/PluginRoles'); + self::$plugin_roles_cache = new StudipCachedArray('PluginRoles'); } return self::$plugin_roles_cache; } - public static function expireCaches() + public static function expireRolesCache() { - self::getRolesCache()->clear(); - self::getUserRolesCache()->clear(); - self::getPluginRolesCache()->clear(); + StudipCacheFactory::getCache()->expire(self::ROLES_CACHE_KEY); } public static function expireUserCache($user_id) { unset(self::getUserRolesCache()[$user_id]); } -} \ No newline at end of file +} diff --git a/lib/plugins/engine/PluginManager.class.php b/lib/plugins/engine/PluginManager.class.php index 5f67c40aad264653e56b0113d2ea6cf81d88bf92..b7ecb0dfff806b382d59c8c98c0c82af127da6ce 100644 --- a/lib/plugins/engine/PluginManager.class.php +++ b/lib/plugins/engine/PluginManager.class.php @@ -28,11 +28,6 @@ class PluginManager */ private $plugins_activated_cache; - /** - * cache of plugin default activations - */ - private $plugins_default_activations_cache; - /** * Returns the PluginManager singleton instance. */ @@ -56,7 +51,6 @@ class PluginManager $this->plugin_cache = []; $this->plugins_activated_cache = new StudipCachedArray('/PluginActivations'); - $this->plugins_default_activations_cache = new StudipCachedArray('/PluginDefaultActivations'); } /** @@ -324,8 +318,8 @@ class PluginManager * Set the list of institutes for which a specific plugin should * be enabled by default. * - * @param $id id of the plugin - * @param $institutes array of institute ids + * @param int $id id of the plugin + * @param array $institutes array of institute ids */ public function setDefaultActivations ($id, $institutes) { @@ -339,8 +333,6 @@ class PluginManager foreach ($institutes as $instid) { $stmt->execute([$id, $instid]); } - - $this->plugins_default_activations_cache->clear(); } /** @@ -493,7 +485,7 @@ class PluginManager /** * Remove registration for the given plugin from the data base. * - * @param $id id of the plugin + * @param int $id id of the plugin */ public function unregisterPlugin ($id) { @@ -509,8 +501,7 @@ class PluginManager unset($this->plugins[$id]); - $this->plugins_default_activations_cache->clear(); - $this->plugins_activated_cache->clear(); + unset($this->plugins_activated_cache[$id]); } } diff --git a/tests/unit/_bootstrap.php b/tests/unit/_bootstrap.php index a02f73ba6cee5f60c4d77dc6ad22f2a1a18e7d91..7ea48011c90b50b32e70d70f587ae958c7eef79c 100644 --- a/tests/unit/_bootstrap.php +++ b/tests/unit/_bootstrap.php @@ -80,32 +80,8 @@ if (isset($config['modules']['config']['Db'])) { //DBManager::getInstance()->setConnection('studip', 'sqlite://'. $GLOBALS ,'', ''); } -// create "fake" cache class -if (!class_exists('StudipArrayCache')) { - class StudipArrayCache implements StudipCache { - public $data = []; - - function expire($key) - { - unset($this->data); - } - - function flush() - { - $this->data = []; - } - - function read($key) - { - return $this->data[$key]; - } - - function write($name, $content, $expire = 43200) - { - return ($this->data[$name] = $content); - } - } -} +// Disable caching to fallback to memory cache +$GLOBALS['CACHING_ENABLE'] = false; // SimpleORMapFake if (!class_exists('StudipTestHelper')) { @@ -114,12 +90,6 @@ if (!class_exists('StudipTestHelper')) { static function set_up_tables($tables) { // first step, set fake cache - $testconfig = new Config(['SYSTEMCACHE' => ['type' => 'StudipArrayCache']]); - Config::set($testconfig); - StudipCacheFactory::setConfig($testconfig); - - $GLOBALS['CACHING_ENABLE'] = true; - $cache = StudipCacheFactory::getCache(false); // second step, expire table scheme @@ -152,10 +122,6 @@ if (!class_exists('StudipTestHelper')) { static function tear_down_tables() { SimpleORMap::expireTableScheme(); - Config::set(null); - - StudipCacheFactory::setConfig(null); - $GLOBALS['CACHING_ENABLE'] = false; } } } diff --git a/tests/unit/lib/classes/StudipCachedArrayTest.php b/tests/unit/lib/classes/StudipCachedArrayTest.php index 24018af1153d651e975fbd4d4cedcedea06cb993..e473dc631439a19a549f0c34f5834cb7fa069e6d 100644 --- a/tests/unit/lib/classes/StudipCachedArrayTest.php +++ b/tests/unit/lib/classes/StudipCachedArrayTest.php @@ -11,35 +11,15 @@ class StudipCachedArrayTest extends \Codeception\Test\Unit { - private $cache; - - public function setUp(): void - { - $this->cache = new TestCache(); - } - - private function getCachedArray($partition_by = 1, $encoding = StudipCachedArray::ENCODE_JSON) + private function getCachedArray() { - return new StudipCachedArray( - md5(uniqid(__CLASS__, true)), - $partition_by, - $encoding, - $this->cache - ); + return new StudipCachedArray(__CLASS__); } /** - * @after + * @dataProvider StorageProvider */ - public function clearCache() - { - $this->cache->flush(); - } - - /** - * @dataProvider JSONStorageProvider - */ - public function testJSONStorage($key, $value) + public function testStorage($key, $value) { $cache = $this->getCachedArray(); @@ -68,202 +48,22 @@ class StudipCachedArrayTest extends \Codeception\Test\Unit $this->assertFalse(isset($cache[$key])); } - /** - * @depends testJSONStorage - */ - public function testJSONCountable() - { - $cache = $this->getCachedArray(); - - $this->assertEquals(0, count($cache)); - - $cache['foo'] = 'bar'; - $this->assertEquals(1, count($cache)); - - unset($cache['foo']); - $this->assertEquals(0, count($cache)); - } - - /** - * @depends testJSONCountable - */ - public function testJSONClear() - { - $cache = $this->getCachedArray(); - - $count = 100; - - for ($i = 0; $i < $count; $i += 1) { - $cache[$i] = $i; - } - - $this->assertEquals($count, count($cache)); - - $cache->clear(); - - $this->assertEquals(0, count($cache)); - } - - /** - * @dataProvider SerializedStorageProvider - */ - public function testSerializedStorage($key, $value) - { - $cache = $this->getCachedArray(1, StudipCachedArray::ENCODE_SERIALIZE); - - // Cache should be empty - $this->assertFalse(isset($cache[$key])); - - // Set value - $cache[$key] = $value; - - // Immediate response - $this->assertTrue(isset($cache[$key])); - $this->assertEquals($value, $cache[$key]); - - // When reading back from cache - $cache->reset(); - - $this->assertTrue(isset($cache[$key])); - $this->assertEquals($value, $cache[$key]); - - // Remove value - unset($cache[$key]); - $this->assertFalse(isset($cache[$key])); - - $cache->reset(); - - $this->assertFalse(isset($cache[$key])); - } - - /** - * @depends testSerializedStorage - */ - public function testSerializedCountable() - { - $cache = $this->getCachedArray(1, StudipCachedArray::ENCODE_SERIALIZE); - - $this->assertEquals(0, count($cache)); - - $cache['foo'] = 'bar'; - $this->assertEquals(1, count($cache)); - - unset($cache['foo']); - $this->assertEquals(0, count($cache)); - } - - /** - * @depends testSerializedCountable - */ - public function testSerializedClear() - { - $cache = $this->getCachedArray(1, StudipCachedArray::ENCODE_SERIALIZE); - - $count = 100; - - for ($i = 0; $i < $count; $i += 1) { - $cache[$i] = $i; - } - - $this->assertEquals($count, count($cache)); - - $cache->clear(); - - $this->assertEquals(0, count($cache)); - } - - /** - * This will test the partitioning by a slice of the key with the length 2. - */ - public function testPartitioningByInt1() - { - $cache = $this->getCachedArray(); - - $cache['abc'] = 1; - $cache['acd'] = 1; - $cache['def'] = 1; - - $this->assertEquals(3, count($this->cache->getCachedData())); - } - - /** - * This will test the partitioning by a slice of the key with the length 2. - */ - public function testPartitioningByInt2() - { - $cache = $this->getCachedArray(2); - - $cache['abc'] = 1; - $cache['acd'] = 1; - $cache['def'] = 1; - - $this->assertEquals(4, count($this->cache->getCachedData())); - } - - /** - * This will test the partitioning by a user defined function that - * always returns the same string. - */ - public function testPartitioningByFunction() - { - $cache = $this->getCachedArray(function () { - return 'test'; - }); - - $cache['abc'] = 1; - $cache['acd'] = 1; - $cache['def'] = 1; - - $this->assertEquals(2, count($this->cache->getCachedData())); - } - - /** - * This will test the getArrayCopy() method - */ - public function testGetArrayCopy() - { - $data = [23 => 42, 42 => 23]; - - $cache = $this->getCachedArray(); - foreach ($data as $key => $value) { - $cache[$key] = $value; - } - - $this->assertEquals($data, $cache->getArrayCopy()); - } - - public function JSONStorageProvider(): array + public function StorageProvider(): array { return [ - 'null' => [1, null], + // 'null' => [1, null], // Null is not really testable 'true' => [2, true], 'false' => [3, false], 'int' => [4, 42], 'string' => ['string', 'bar'], 'array' => ['array', ['foo']], + 'object' => ['object', new StudipCachedArrayTestClass()], ]; } - - public function SerializedStorageProvider(): array - { - return array_merge( - $this->JSONStorageProvider(), - ['object' => ['object', new TestClass()]] - ); - } -} - -// Extend memory cache so we will gain access to the internal data -class TestCache extends StudipMemoryCache -{ - public function getCachedData() - { - return $this->memory_cache; - } } // Simple test class -class TestClass +class StudipCachedArrayTestClass { private $foo = 42; }