From 47ba00850488481694d6e2e31218cc059b48c17a Mon Sep 17 00:00:00 2001 From: Dale Davies Date: Mon, 4 Sep 2023 14:18:04 +0100 Subject: [PATCH] MDL-79165 cachestore_file: Add igbinary serializer option AMOS BEGIN CPY [serializer_igbinary,cachestore_redis],[serializer_igbinary,cachestore_file] CPY [serializer_php,cachestore_redis],[serializer_php,cachestore_file] AMOS END --- cache/stores/file/addinstanceform.php | 6 ++ cache/stores/file/lang/en/cachestore_file.php | 5 + cache/stores/file/lib.php | 96 ++++++++++++++++++- cache/stores/file/tests/store_test.php | 39 ++++++++ 4 files changed, 143 insertions(+), 3 deletions(-) diff --git a/cache/stores/file/addinstanceform.php b/cache/stores/file/addinstanceform.php index 5a2ac837f77..e3704e33e85 100644 --- a/cache/stores/file/addinstanceform.php +++ b/cache/stores/file/addinstanceform.php @@ -53,5 +53,11 @@ class cachestore_file_addinstance_form extends cachestore_addinstance_form { $form->setDefault('lockwait', 60); $form->setType('lockwait', PARAM_INT); $form->addHelpButton('lockwait', 'lockwait', 'cachestore_file'); + + $serializeroptions = cachestore_file::config_get_serializer_options(); + $form->addElement('select', 'serializer', get_string('useserializer', 'cachestore_file'), $serializeroptions); + $form->setDefault('serializer', cachestore_file::SERIALIZER_PHP); + $form->setType('serializer', PARAM_ALPHA); + $form->addHelpButton('serializer', 'useserializer', 'cachestore_file'); } } diff --git a/cache/stores/file/lang/en/cachestore_file.php b/cache/stores/file/lang/en/cachestore_file.php index 1562d752698..49b641ff22f 100644 --- a/cache/stores/file/lang/en/cachestore_file.php +++ b/cache/stores/file/lang/en/cachestore_file.php @@ -40,6 +40,8 @@ $string['pluginname'] = 'File cache'; $string['privacy:metadata'] = 'The File cache cachestore plugin stores data briefly as part of its caching functionality but this data is regularly cleared.'; $string['prescan'] = 'Prescan directory'; $string['prescan_help'] = 'If enabled the directory is scanned when the cache is first used and requests for files are first checked against the scan data. This can help if you have a slow file system and are finding that file operations are causing you a bottle neck.'; +$string['serializer_igbinary'] = 'The igbinary serializer.'; +$string['serializer_php'] = 'The default PHP serializer.'; $string['singledirectory'] = 'Single directory store'; $string['singledirectory_help'] = 'If enabled files (cached items) will be stored in a single directory rather than being broken up into multiple directories. @@ -50,6 +52,9 @@ It is advisable to only turn this on if the following is true: * If you know the number of items in the cache is going to be small enough that it won\'t cause issues on the file system you are running with. * The data being cached is not expensive to generate. If it is then sticking with the default may still be the better option as it reduces the chance of issues.'; $string['task_asyncpurge'] = 'Asynchronously purge file store old cache revision directories'; +$string['useserializer'] = 'Use serializer'; +$string['useserializer_help'] = 'Specifies the serializer to use for serializing. +If available the igbinary serializer can reduce the storage requirements for large caches, this is supported only when the igbinary extension is loaded.'; /** * This is is like the file store, but designed for siutations where: diff --git a/cache/stores/file/lib.php b/cache/stores/file/lib.php index cf02e0ce2c2..b4559c334ed 100644 --- a/cache/stores/file/lib.php +++ b/cache/stores/file/lib.php @@ -39,6 +39,16 @@ class cachestore_file extends store implements searchable_cache_interface, lockable_cache_interface { + /** + * Value to represent use of the PHP serializer. + */ + public const SERIALIZER_PHP = 'php'; + + /** + * Value to represent use of the Igbinary serializer. + */ + public const SERIALIZER_IGBINARY = 'igbinary'; + /** * The name of the store. * @var string @@ -145,6 +155,37 @@ class cachestore_file extends store implements */ protected $locks = []; + /** + * Serializer for this store. + * + * @var string + */ + protected $serializer = self::SERIALIZER_PHP; + + /** + * Determine if igbinary functions are available for use. + * + * @return boolean + */ + public static function igbinary_available(): bool { + return function_exists('igbinary_serialize'); + } + + /** + * Gets an array of options to use as the serialiser. + * + * @return array + */ + public static function config_get_serializer_options(): array { + $options = [ + self::SERIALIZER_PHP => get_string('serializer_php', 'cachestore_file'), + ]; + if (self::igbinary_available()) { + $options[self::SERIALIZER_IGBINARY] = get_string('serializer_igbinary', 'cachestore_file'); + } + return $options; + } + /** * Constructs the store instance. * @@ -225,6 +266,11 @@ class cachestore_file extends store implements // File locking is disabled in config, fall back to default lock factory. $this->lockfactory = \core\lock\lock_config::get_lock_factory('cachestore_file'); } + + // Set the serializer to use based on configuration. + if (array_key_exists('serializer', $configuration)) { + $this->serializer = (string)$configuration['serializer']; + } } /** @@ -549,7 +595,7 @@ class cachestore_file extends store implements * @return string */ protected function prep_data_before_save($data) { - return serialize($data); + return $this->serialize($data); } /** @@ -560,8 +606,8 @@ class cachestore_file extends store implements * @return mixed */ protected function prep_data_after_read($data, $path) { - $result = @unserialize($data); - if ($result === false && $data != serialize(false)) { + $result = @$this->unserialize($data); + if ($result === false && $data != @$this->serialize(false)) { debugging('Failed to unserialise data from cache file: ' . $path . '. Data: ' . $data, DEBUG_DEVELOPER); return false; } @@ -720,6 +766,9 @@ class cachestore_file extends store implements if (isset($data->lockwait)) { $config['lockwait'] = $data->lockwait; } + if (isset($data->serializer)) { + $config['serializer'] = $data->serializer; + } return $config; } @@ -750,6 +799,9 @@ class cachestore_file extends store implements if (isset($config['lockwait'])) { $data['lockwait'] = (int)$config['lockwait']; } + if (isset($config['serializer'])) { + $data['serializer'] = (string)$config['serializer']; + } $editform->set_data($data); } @@ -1037,4 +1089,42 @@ class cachestore_file extends store implements } return $unlocked; } + + /** + * Serializes the data according to the configured serializer. + * + * @param mixed $value + * @return string + */ + protected function serialize($value): string { + switch ($this->serializer) { + case self::SERIALIZER_PHP: + return serialize($value); + case self::SERIALIZER_IGBINARY: + if (self::igbinary_available()) { + return igbinary_serialize($value); + } + } + debugging("Unknown or unavailable serializer {$this->serializer}"); + return serialize($value); + } + + /** + * Unserializes the data according to the configured serializer. + * + * @param mixed $value + * @return mixed + */ + protected function unserialize($value) { + switch ($this->serializer) { + case self::SERIALIZER_PHP: + return unserialize($value); + case self::SERIALIZER_IGBINARY: + if (self::igbinary_available()) { + return igbinary_unserialize($value); + } + } + debugging("Unknown or unavailable serializer: {$this->serializer}"); + return unserialize($value); + } } diff --git a/cache/stores/file/tests/store_test.php b/cache/stores/file/tests/store_test.php index 9e3d1d47490..f4a6cf8be4e 100644 --- a/cache/stores/file/tests/store_test.php +++ b/cache/stores/file/tests/store_test.php @@ -44,6 +44,45 @@ class store_test extends \cachestore_tests { return 'cachestore_file'; } + /** + * Provider for set/get tests with all combinations of serializer. + * + * @return array + */ + public static function getset_serialization_test_provider(): array { + $data = [ + [ + 'PHP serializer', + \cachestore_file::SERIALIZER_PHP, + ], + ]; + if (function_exists('igbinary_serialize')) { + $data[] = [ + 'Igbinary serializer', + \cachestore_file::SERIALIZER_IGBINARY, + ]; + } + + return $data; + } + + /** + * Test get and set function correctly with all combinations of serializer. + * + * @dataProvider getset_serialization_test_provider + * @param string $name + * @param string $serializer + */ + public function test_getset_serialization(string $name, string $serializer): void { + $definition = definition::load_adhoc(store::MODE_APPLICATION, 'cachestore_file', 'phpunit_test'); + $store = new \cachestore_file('Test', ['serializer' => $serializer]); + $store->initialise($definition); + $originalvalue = 'value12345'; + $store->set('key', $originalvalue); + $unserializedvalue = $store->get('key'); + self::assertSame($originalvalue, $unserializedvalue, "Invalid serialisation/unserialisation for: {$name}"); + } + /** * Testing cachestore_file::get with prescan enabled and with * deleting the cache between the prescan and the call to get.