From 232173753ac56bee752d3c4144056812c4ebc883 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 4 Jul 2024 14:15:28 +1200 Subject: [PATCH 1/2] FIX Ensure cache is shared between CLI and webserver --- _config/cache.yml | 1 - src/Control/Email/TransportFactory.php | 2 +- src/Core/Cache/AbstractCacheFactory.php | 100 ++++++++++ src/Core/Cache/ApcuCacheFactory.php | 65 ++++--- src/Core/Cache/CacheFactory.php | 7 +- src/Core/Cache/DefaultCacheFactory.php | 173 ++++++------------ src/Core/Cache/FilesystemCacheFactory.php | 3 +- src/Core/Cache/InMemoryCacheFactory.php | 17 ++ src/Core/Cache/ManifestCacheFactory.php | 9 +- src/Core/Cache/MemcachedCacheFactory.php | 57 +++--- src/Core/Cache/RedisCacheFactory.php | 64 +++++++ src/Core/Injector/Factory.php | 2 +- src/Core/Injector/InjectionCreator.php | 11 +- tests/php/Core/Cache/CacheTest.php | 84 --------- .../Core/Cache/DefaultCacheFactoryTest.php | 156 ++++++++++++++++ tests/php/Core/Injector/InjectorTest.php | 29 +-- .../Injector/InjectorTest/EmptyFactory.php | 2 +- .../Injector/InjectorTest/SSObjectCreator.php | 2 +- 18 files changed, 494 insertions(+), 290 deletions(-) create mode 100644 src/Core/Cache/AbstractCacheFactory.php create mode 100644 src/Core/Cache/InMemoryCacheFactory.php create mode 100644 src/Core/Cache/RedisCacheFactory.php delete mode 100644 tests/php/Core/Cache/CacheTest.php create mode 100644 tests/php/Core/Cache/DefaultCacheFactoryTest.php diff --git a/_config/cache.yml b/_config/cache.yml index 9400a527038..5f69d6e4f74 100644 --- a/_config/cache.yml +++ b/_config/cache.yml @@ -7,7 +7,6 @@ SilverStripe\Core\Injector\Injector: constructor: args: directory: '`TEMP_PATH`' - version: null logger: '%$Psr\Log\LoggerInterface' Psr\SimpleCache\CacheInterface.cacheblock: factory: SilverStripe\Core\Cache\CacheFactory diff --git a/src/Control/Email/TransportFactory.php b/src/Control/Email/TransportFactory.php index c8c656c80bc..99ff56919c6 100644 --- a/src/Control/Email/TransportFactory.php +++ b/src/Control/Email/TransportFactory.php @@ -12,7 +12,7 @@ */ class TransportFactory implements Factory { - public function create($service, array $params = []) + public function create(string $service, array $params = []): object { $dsn = Environment::getEnv('MAILER_DSN') ?: $params['dsn']; $dispatcher = $params['dispatcher']; diff --git a/src/Core/Cache/AbstractCacheFactory.php b/src/Core/Cache/AbstractCacheFactory.php new file mode 100644 index 00000000000..3a9251c7348 --- /dev/null +++ b/src/Core/Cache/AbstractCacheFactory.php @@ -0,0 +1,100 @@ +logger = $logger; + } + + /** + * Creates an object with a PSR-16 interface, usually from a PSR-6 class name. + * + * Quick explanation of caching standards: + * - Symfony cache implements the PSR-6 standard + * - Symfony provides adapters which wrap a PSR-6 backend with a PSR-16 interface + * - Silverstripe uses the PSR-16 interface to interact with caches. It does not directly interact with the PSR-6 classes + * - Psr\SimpleCache\CacheInterface is the php interface of the PSR-16 standard. All concrete cache classes Silverstripe code interacts with should implement this interface + * + * Further reading: + * - https://symfony.com/doc/current/components/cache/psr6_psr16_adapters.html#using-a-psr-6-cache-object-as-a-psr-16-cache + * - https://github.com/php-fig/simple-cache + */ + protected function createCache( + string $class, + array $args, + bool $useInjector = true + ): CacheInterface { + $classIsPsr6 = is_a($class, CacheItemPoolInterface::class, true); + $classIsPsr16 = is_a($class, CacheInterface::class, true); + if (!$classIsPsr6 && !$classIsPsr16) { + throw new InvalidArgumentException("class $class must implement one of " . CacheItemPoolInterface::class . ' or ' . CacheInterface::class); + } + $cacheAdapter = $this->instantiateCache($class, $args, $useInjector); + $psr16Cache = $this->prepareCacheForUse($cacheAdapter, $useInjector); + return $psr16Cache; + } + + /** + * Prepare a cache adapter for use. + * This wraps a PSR6 adapter inside a PSR16 one. It also adds the loggers. + */ + protected function prepareCacheForUse( + CacheItemPoolInterface|CacheInterface $cacheAdapter, + bool $useInjector + ): CacheInterface { + $loggerAdded = false; + if ($cacheAdapter instanceof CacheItemPoolInterface) { + $loggerAdded = $this->addLogger($cacheAdapter); + // Wrap the PSR-6 class inside a class with a PSR-16 interface + $cacheAdapter = $this->instantiateCache(Psr16Cache::class, [$cacheAdapter], $useInjector); + } + if (!$loggerAdded) { + $this->addLogger($cacheAdapter); + } + return $cacheAdapter; + } + + /** + * Instantiates a cache adapter, either via the dependency injector or using the new keyword. + */ + protected function instantiateCache( + string $class, + array $args, + bool $useInjector + ): CacheItemPoolInterface|CacheInterface { + if ($useInjector) { + // Injector is used for in most instances to allow modification of the cache implementations + return Injector::inst()->createWithArgs($class, $args); + } + // ManifestCacheFactory cannot use Injector because config is not available at that point + return new $class(...$args); + } + + protected function addLogger(CacheItemPoolInterface|CacheInterface $cache): bool + { + if ($this->logger && ($cache instanceof LoggerAwareInterface)) { + $cache->setLogger($this->logger); + return true; + } + return false; + } +} diff --git a/src/Core/Cache/ApcuCacheFactory.php b/src/Core/Cache/ApcuCacheFactory.php index 188224063b2..4c5a0803701 100644 --- a/src/Core/Cache/ApcuCacheFactory.php +++ b/src/Core/Cache/ApcuCacheFactory.php @@ -2,39 +2,62 @@ namespace SilverStripe\Core\Cache; -use SilverStripe\Core\Injector\Injector; +use Psr\Cache\CacheItemPoolInterface; +use Psr\SimpleCache\CacheInterface; +use RuntimeException; +use SilverStripe\Control\Director; +use SilverStripe\Core\Environment; use Symfony\Component\Cache\Adapter\ApcuAdapter; -use Symfony\Component\Cache\Psr16Cache; -class ApcuCacheFactory implements CacheFactory +/** + * Factory to instantiate an ApcuAdapter for use in caching. + * + * Note that APCu cache may not be shared between your webserver and the CLI. + * Flushing the cache from your terminal may not flush the cache used by the webserver. + * See https://github.com/symfony/symfony/discussions/54066 + */ +class ApcuCacheFactory extends AbstractCacheFactory implements InMemoryCacheFactory { /** - * @var string - */ - protected $version; - - /** - * @param string $version + * @inheritdoc */ - public function __construct($version = null) + public function create(string $service, array $params = []): CacheInterface { - $this->version = $version; + $psr6Cache = $this->createPsr6($service, $params); + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + return $this->prepareCacheForUse($psr6Cache, $useInjector); } - /** - * @inheritdoc - */ - public function create($service, array $params = []) + public function createPsr6(string $service, array $params = []): CacheItemPoolInterface { + if (!$this->isSupported()) { + throw new RuntimeException('APCu is not supported in the current environment. Cannot use APCu cache.'); + } + $namespace = isset($params['namespace']) ? $params['namespace'] . '_' . md5(BASE_PATH) : md5(BASE_PATH); $defaultLifetime = isset($params['defaultLifetime']) ? $params['defaultLifetime'] : 0; - $psr6Cache = Injector::inst()->createWithArgs(ApcuAdapter::class, [ - $namespace, - $defaultLifetime, - $this->version - ]); - return Injector::inst()->createWithArgs(Psr16Cache::class, [$psr6Cache]); + // $version is optional - defaults to null. + $version = isset($params['version']) ? $params['version'] : Environment::getEnv('SS_APCU_VERSION'); + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + + return $this->instantiateCache( + ApcuAdapter::class, + [$namespace, $defaultLifetime, $version], + $useInjector + ); + } + + private function isSupported(): bool + { + static $isSupported = null; + if (null === $isSupported) { + // Need to check for CLI because Symfony won't: https://github.com/symfony/symfony/pull/25080 + $isSupported = Director::is_cli() + ? filter_var(ini_get('apc.enable_cli'), FILTER_VALIDATE_BOOL) && ApcuAdapter::isSupported() + : ApcuAdapter::isSupported(); + } + return $isSupported; } } diff --git a/src/Core/Cache/CacheFactory.php b/src/Core/Cache/CacheFactory.php index 994ac948226..83efbb68766 100644 --- a/src/Core/Cache/CacheFactory.php +++ b/src/Core/Cache/CacheFactory.php @@ -7,14 +7,9 @@ interface CacheFactory extends InjectorFactory { - /** * Note: While the returned object is used as a singleton (by the originating Injector->get() call), * this cache object shouldn't be a singleton itself - it has varying constructor args for the same service name. - * - * @param string $service - * @param array $params - * @return CacheInterface */ - public function create($service, array $params = []); + public function create(string $service, array $params = []): CacheInterface; } diff --git a/src/Core/Cache/DefaultCacheFactory.php b/src/Core/Cache/DefaultCacheFactory.php index b82d6f08a2b..7d8d0a75f6d 100644 --- a/src/Core/Cache/DefaultCacheFactory.php +++ b/src/Core/Cache/DefaultCacheFactory.php @@ -2,38 +2,24 @@ namespace SilverStripe\Core\Cache; -use InvalidArgumentException; -use Psr\Log\LoggerAwareInterface; +use LogicException; use Psr\Log\LoggerInterface; use Psr\Cache\CacheItemPoolInterface; use Psr\SimpleCache\CacheInterface; -use SilverStripe\Control\Director; +use SilverStripe\Core\Environment; use SilverStripe\Core\Injector\Injector; -use Symfony\Component\Cache\Adapter\ApcuAdapter; use Symfony\Component\Cache\Adapter\ChainAdapter; use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Cache\Adapter\PhpFilesAdapter; -use Symfony\Component\Cache\Psr16Cache; /** - * Returns the most performant combination of caches available on the system: - * - `PhpFilesCache` (PHP 7 with opcache enabled) - * - `ApcuCache` (requires APC) with a `FilesystemCache` fallback (for larger cache volumes) - * - `FilesystemCache` if none of the above is available - * - * Modelled after `Symfony\Component\Cache\Adapter\AbstractAdapter::createSystemCache()` + * Creates the following cache adapters: + * - `PhpFilesAdapter` (falls back to `FilesystemAdapter` if `PhpFilesAdapter` isn't supported) + * - An optional in-memory cache such as Redis, Memcached, or APCu. */ -class DefaultCacheFactory implements CacheFactory +class DefaultCacheFactory extends AbstractCacheFactory { - /** - * @var string Absolute directory path - */ - protected $args = []; - - /** - * @var LoggerInterface - */ - protected $logger; + protected array $args = []; /** * @param array $args List of global options to merge with args during create() @@ -42,148 +28,97 @@ class DefaultCacheFactory implements CacheFactory public function __construct($args = [], LoggerInterface $logger = null) { $this->args = $args; - $this->logger = $logger; + parent::__construct($logger); } /** * @inheritdoc */ - public function create($service, array $args = []) + public function create(string $service, array $args = []): CacheInterface { // merge args with default $args = array_merge($this->args, $args); $namespace = isset($args['namespace']) ? $args['namespace'] : ''; - $defaultLifetime = isset($args['defaultLifetime']) ? $args['defaultLifetime'] : 0; + $defaultLifetime = (int) (isset($args['defaultLifetime']) ? $args['defaultLifetime'] : 0); $directory = isset($args['directory']) ? $args['directory'] : null; - $version = isset($args['version']) ? $args['version'] : null; $useInjector = isset($args['useInjector']) ? $args['useInjector'] : true; // In-memory caches are typically more resource constrained (number of items and storage space). // Give cache consumers an opt-out if they are expecting to create large caches with long lifetimes. $useInMemoryCache = isset($args['useInMemoryCache']) ? $args['useInMemoryCache'] : true; + $inMemoryCacheFactory = Environment::getEnv('SS_MEMORY_CACHEFACTORY'); - // Check support - $apcuSupported = ($this->isAPCUSupported() && $useInMemoryCache); - $phpFilesSupported = $this->isPHPFilesSupported(); - - // If apcu isn't supported, phpfiles is the next best preference - if (!$apcuSupported && $phpFilesSupported) { - return $this->createCache(PhpFilesAdapter::class, [$namespace, $defaultLifetime, $directory], $useInjector); + $filesystemCache = $this->instantiateFilesystemCache($namespace, $defaultLifetime, $directory, $useInjector); + if (!$useInMemoryCache || !$inMemoryCacheFactory) { + return $this->prepareCacheForUse($filesystemCache, $useInjector); } - // Create filesystem cache - if (!$apcuSupported) { - return $this->createCache( - FilesystemAdapter::class, - [$namespace, $defaultLifetime, $directory], - $useInjector + // Check if SS_MEMORY_CACHEFACTORY is a factory + if (!is_a($inMemoryCacheFactory, InMemoryCacheFactory::class, true)) { + throw new LogicException( + 'SS_MEMORY_CACHEFACTORY is not a valid InMemoryCacheFactory class name' ); } - // Create PSR6 filesystem + apcu cache's wrapped in a PSR6 chain adapter, then wrap in a PSR16 class - $fs = $this->instantiateCache( - FilesystemAdapter::class, - [$namespace, $defaultLifetime, $directory], + // Note that the cache lifetime will be shorter there by default, to ensure there's enough + // resources for "hot cache" items as a resource constrained in memory cache. + $inMemoryLifetime = (int) ($defaultLifetime / 5); + $inMemoryCache = $this->instantiateInMemoryCache( + $service, + $inMemoryCacheFactory, + ['namespace' => $namespace, 'defaultLifetime' => $inMemoryLifetime, 'useInjector' => $useInjector], $useInjector ); - // Note that the cache lifetime will be shorter there by default, to ensure there's enough - // resources for "hot cache" items in APCu as a resource constrained in memory cache. - $apcuNamespace = $namespace . ($namespace ? '_' : '') . md5(BASE_PATH); - $lifetime = (int) $defaultLifetime / 5; - $apcu = $this->instantiateCache(ApcuAdapter::class, [$apcuNamespace, $lifetime, $version], $useInjector); + // The ChainAdapter doesn't take a logger, so we need to make sure to add it to the child cache adapters. + $this->addLogger($filesystemCache); - return $this->createCache(ChainAdapter::class, [[$apcu, $fs]], $useInjector); - } - - /** - * Determine if apcu is supported - * - * @return bool - */ - protected function isAPCUSupported() - { - static $apcuSupported = null; - if (null === $apcuSupported) { - // Need to check for CLI because Symfony won't: https://github.com/symfony/symfony/pull/25080 - $apcuSupported = Director::is_cli() - ? filter_var(ini_get('apc.enable_cli'), FILTER_VALIDATE_BOOL) && ApcuAdapter::isSupported() - : ApcuAdapter::isSupported(); - } - return $apcuSupported; + return $this->createCache(ChainAdapter::class, [[$inMemoryCache, $filesystemCache]], $useInjector); } /** * Determine if PHP files is supported - * - * @return bool */ - protected function isPHPFilesSupported() + protected function isPHPFilesSupported(): bool { static $phpFilesSupported = null; if (null === $phpFilesSupported) { - $phpFilesSupported = PhpFilesAdapter::isSupported(); + // Only consider to be enabled if opcache is enabled in CLI, or else + // filesystem cache won't be shared between webserver and CLI. + $phpFilesSupported = PhpFilesAdapter::isSupported() && + filter_var(ini_get('opcache.enable_cli'), FILTER_VALIDATE_BOOL); } return $phpFilesSupported; } /** - * Creates an object with a PSR-16 interface, usually from a PSR-6 class name - * - * Quick explanation of caching standards: - * - Symfony cache implements the PSR-6 standard - * - Symfony provides adapters which wrap a PSR-6 backend with a PSR-16 interface - * - Silverstripe uses the PSR-16 interface to interact with caches. It does not directly interact with the PSR-6 classes - * - Psr\SimpleCache\CacheInterface is the php interface of the PSR-16 standard. All concrete cache classes Silverstripe code interacts with should implement this interface - * - * Further reading: - * - https://symfony.com/doc/current/components/cache/psr6_psr16_adapters.html#using-a-psr-6-cache-object-as-a-psr-16-cache - * - https://github.com/php-fig/simple-cache + * Instantiate the cache adapter for the filesystem cache. */ - protected function createCache( - string $class, - array $args, - bool $useInjector = true - ): CacheInterface { - $loggerAdded = false; - $classIsPsr6 = is_a($class, CacheItemPoolInterface::class, true); - $classIsPsr16 = is_a($class, CacheInterface::class, true); - if (!$classIsPsr6 && !$classIsPsr16) { - throw new InvalidArgumentException("class $class must implement one of " . CacheItemPoolInterface::class . ' or ' . CacheInterface::class); - } - if ($classIsPsr6) { - $psr6Cache = $this->instantiateCache($class, $args, $useInjector); - $loggerAdded = $this->addLogger($psr6Cache, $loggerAdded); - // Wrap the PSR-6 class inside a class with a PSR-16 interface - $psr16Cache = $this->instantiateCache(Psr16Cache::class, [$psr6Cache], $useInjector); - } else { - $psr16Cache = $this->instantiateCache($class, $args, $useInjector); - } - if (!$loggerAdded) { - $this->addLogger($psr16Cache, $loggerAdded); - } - return $psr16Cache; - } - - private function instantiateCache( - string $class, - array $args, + private function instantiateFilesystemCache( + string $namespace, + int $defaultLifetime, + string $directory, bool $useInjector - ): CacheItemPoolInterface|CacheInterface { - if ($useInjector) { - // Injector is used for in most instances to allow modification of the cache implementations - return Injector::inst()->createWithArgs($class, $args); + ): CacheInterface|CacheItemPoolInterface { + if ($this->isPHPFilesSupported()) { + return $this->instantiateCache(PhpFilesAdapter::class, [$namespace, $defaultLifetime, $directory], $useInjector); } - // ManifestCacheFactory cannot use Injector because config is not available at that point - return new $class(...$args); + return $this->instantiateCache(FilesystemAdapter::class, [$namespace, $defaultLifetime, $directory], $useInjector); } - private function addLogger(CacheItemPoolInterface|CacheInterface $cache): bool + /** + * Instantiate the cache adapter for the in-memory cache. + */ + private function instantiateInMemoryCache(string $service, string $inMemoryCacheFactory, array $args): CacheItemPoolInterface { - if ($this->logger && $cache instanceof LoggerAwareInterface) { - $cache->setLogger($this->logger); - return true; + if ($args['useInjector']) { + $factory = Injector::inst()->create($inMemoryCacheFactory); + } else { + $factory = new $inMemoryCacheFactory(); } - return false; + /** @var InMemoryCacheFactory $factory */ + $adapter = $factory->createPsr6($service, $args); + $this->addLogger($adapter); + return $adapter; } } diff --git a/src/Core/Cache/FilesystemCacheFactory.php b/src/Core/Cache/FilesystemCacheFactory.php index 822f582f54d..4de5c7d3f36 100644 --- a/src/Core/Cache/FilesystemCacheFactory.php +++ b/src/Core/Cache/FilesystemCacheFactory.php @@ -2,6 +2,7 @@ namespace SilverStripe\Core\Cache; +use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Injector\Injector; use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\Cache\Psr16Cache; @@ -24,7 +25,7 @@ public function __construct($directory) /** * @inheritdoc */ - public function create($service, array $params = []) + public function create(string $service, array $params = []): CacheInterface { $psr6Cache = Injector::inst()->createWithArgs(FilesystemAdapter::class, [ (isset($params['namespace'])) ? $params['namespace'] : '', diff --git a/src/Core/Cache/InMemoryCacheFactory.php b/src/Core/Cache/InMemoryCacheFactory.php new file mode 100644 index 00000000000..39bfdade13c --- /dev/null +++ b/src/Core/Cache/InMemoryCacheFactory.php @@ -0,0 +1,17 @@ +get() call), - * this cache object shouldn't be a singleton itself - it has varying constructor args for the same service name. - * - * @param string $service The class name of the service. - * @param array $params The constructor parameters. - * @return CacheInterface + * @inheritDoc */ - public function create($service, array $params = []) + public function create(string $service, array $params = []): CacheInterface { // Override default cache generation with SS_MANIFESTCACHE $cacheClass = Environment::getEnv('SS_MANIFESTCACHE'); diff --git a/src/Core/Cache/MemcachedCacheFactory.php b/src/Core/Cache/MemcachedCacheFactory.php index 1752baf5ba9..9dfe1655fc5 100644 --- a/src/Core/Cache/MemcachedCacheFactory.php +++ b/src/Core/Cache/MemcachedCacheFactory.php @@ -2,41 +2,52 @@ namespace SilverStripe\Core\Cache; -use SilverStripe\Core\Injector\Injector; +use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\MemcachedAdapter; -use Symfony\Component\Cache\Psr16Cache; -use Memcached; +use Psr\SimpleCache\CacheInterface; +use RuntimeException; +use SilverStripe\Core\Environment; -class MemcachedCacheFactory implements CacheFactory +/** + * Factory to instantiate a MemcachedAdapter for use in caching. + * + * SS_MEMCACHED_DSN must be set in environment variables. + * See https://symfony.com/doc/current/components/cache/adapters/memcached_adapter.html#configure-the-connection + */ +class MemcachedCacheFactory extends AbstractCacheFactory implements InMemoryCacheFactory { - - /** - * @var Memcached - */ - protected $memcachedClient; - /** - * @param Memcached $memcachedClient + * @inheritdoc */ - public function __construct(Memcached $memcachedClient = null) + public function create(string $service, array $params = []): CacheInterface { - $this->memcachedClient = $memcachedClient; + $psr6Cache = $this->createPsr6($service, $params); + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + return $this->prepareCacheForUse($psr6Cache, $useInjector); } - /** - * @inheritdoc - */ - public function create($service, array $params = []) + public function createPsr6(string $service, array $params = []): CacheItemPoolInterface { + if (!MemcachedAdapter::isSupported()) { + throw new RuntimeException('Memcached is not supported in the current environment. Cannot use Memcached cache.'); + } + + $dsn = Environment::getEnv('SS_MEMCACHED_DSN'); + if (!$dsn) { + throw new RuntimeException('The SS_MEMCACHED_DSN environment variable must be set to use Memcached cache.'); + } + $namespace = isset($params['namespace']) ? $params['namespace'] . '_' . md5(BASE_PATH) : md5(BASE_PATH); $defaultLifetime = isset($params['defaultLifetime']) ? $params['defaultLifetime'] : 0; - $psr6Cache = Injector::inst()->createWithArgs(MemcachedAdapter::class, [ - $this->memcachedClient, - $namespace, - $defaultLifetime - ]); - return Injector::inst()->createWithArgs(Psr16Cache::class, [$psr6Cache]); + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + $client = MemcachedAdapter::createConnection($dsn); + + return $this->instantiateCache( + MemcachedAdapter::class, + [$client, $namespace, $defaultLifetime], + $useInjector + ); } } diff --git a/src/Core/Cache/RedisCacheFactory.php b/src/Core/Cache/RedisCacheFactory.php new file mode 100644 index 00000000000..514b47fc874 --- /dev/null +++ b/src/Core/Cache/RedisCacheFactory.php @@ -0,0 +1,64 @@ +createPsr6($service, $params); + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + return $this->prepareCacheForUse($psr6Cache, $useInjector); + } + + public function createPsr6(string $service, array $params = []): CacheItemPoolInterface + { + if (!$this->isSupported()) { + throw new RuntimeException('Redis is not supported in the current environment. Cannot use Redis cache.'); + } + + $dsn = Environment::getEnv('SS_REDIS_DSN'); + if (!$dsn) { + throw new RuntimeException('The SS_REDIS_DSN environment variable must be set to use Redis cache.'); + } + + $namespace = isset($params['namespace']) + ? $params['namespace'] . '_' . md5(BASE_PATH) + : md5(BASE_PATH); + $defaultLifetime = isset($params['defaultLifetime']) ? $params['defaultLifetime'] : 0; + $useInjector = isset($params['useInjector']) ? $params['useInjector'] : true; + $client = RedisAdapter::createConnection($dsn, ['lazy' => true]); + + return $this->instantiateCache( + RedisAdapter::class, + [$client, $namespace, $defaultLifetime], + $useInjector + ); + } + + public function isSupported() + { + return class_exists(PredisClient::class) || + class_exists(Relay::class) || + extension_loaded('redis'); + } +} diff --git a/src/Core/Injector/Factory.php b/src/Core/Injector/Factory.php index 36aaa00e199..c027c40d353 100644 --- a/src/Core/Injector/Factory.php +++ b/src/Core/Injector/Factory.php @@ -15,5 +15,5 @@ interface Factory * @param array $params The constructor parameters. * @return object The created service instances. */ - public function create($service, array $params = []); + public function create(string $service, array $params = []): ?object; } diff --git a/src/Core/Injector/InjectionCreator.php b/src/Core/Injector/InjectionCreator.php index ad91f11d281..6b1ae121a2f 100644 --- a/src/Core/Injector/InjectionCreator.php +++ b/src/Core/Injector/InjectionCreator.php @@ -12,19 +12,10 @@ class InjectionCreator implements Factory /** * Create a new instance of a class * - * Passing an object for $class will result from using an anonymous class in unit testing, e.g. - * Injector::inst()->load([SomeClass::class => ['class' => new class { ... }]]); - * * @param string|object $class - string: The FQCN of the class, object: A class instance */ - public function create($class, array $params = []) + public function create(string $class, array $params = []): object { - if (is_object($class ?? '')) { - $class = get_class($class); - } - if (!is_string($class ?? '')) { - throw new InvalidArgumentException('$class parameter must be a string or an object'); - } if (!class_exists($class)) { throw new InjectorNotFoundException("Class {$class} does not exist"); } diff --git a/tests/php/Core/Cache/CacheTest.php b/tests/php/Core/Cache/CacheTest.php deleted file mode 100644 index 228b1121a4f..00000000000 --- a/tests/php/Core/Cache/CacheTest.php +++ /dev/null @@ -1,84 +0,0 @@ -load([ - ApcuCacheFactory::class => [ - 'constructor' => [ 'version' => 'ss40test' ] - ], - 'MemcachedClient' => Memcached::class, - MemcachedCacheFactory::class => [ - 'constructor' => [ 'memcachedClient' => '%$MemcachedClient' ] - ], - CacheInterface::class . '.TestApcuCache' => [ - 'factory' => ApcuCacheFactory::class, - 'constructor' => [ - 'namespace' => 'TestApcuCache', - 'defaultLifetime' => 2600, - ], - ], - CacheInterface::class . '.TestMemcache' => [ - 'factory' => MemcachedCacheFactory::class, - 'constructor' => [ - 'namespace' => 'TestMemCache', - 'defaultLifetime' => 5600, - ], - ], - Psr16Cache::class => MockCache::class, - ApcuAdapter::class => MockCache::class, - MemcachedAdapter::class => MockCache::class, - ]); - } - - public function testApcuCacheFactory() - { - $psr16Cache = Injector::inst()->get(CacheInterface::class . '.TestApcuCache'); - $this->assertInstanceOf(MockCache::class, $psr16Cache); - $this->assertEquals(MockCache::class, get_class($psr16Cache->getArgs()[0])); - $this->assertEquals( - [ - 'TestApcuCache_' . md5(BASE_PATH), - 2600, - 'ss40test' - ], - $psr16Cache->getArgs()[0]->getArgs() - ); - } - - public function testMemCacheFactory() - { - if (!class_exists(Memcached::class)) { - $this->markTestSkipped('Memcached is not installed'); - } - $psr16Cache = Injector::inst()->get(CacheInterface::class . '.TestMemcache'); - $this->assertInstanceOf(MockCache::class, $psr16Cache); - $this->assertEquals(MockCache::class, get_class($psr16Cache->getArgs()[0])); - $this->assertEquals( - [ - new MemCached(), - 'TestMemCache_' . md5(BASE_PATH), - 5600 - ], - $psr16Cache->getArgs()[0]->getArgs() - ); - } -} diff --git a/tests/php/Core/Cache/DefaultCacheFactoryTest.php b/tests/php/Core/Cache/DefaultCacheFactoryTest.php new file mode 100644 index 00000000000..50548e0486f --- /dev/null +++ b/tests/php/Core/Cache/DefaultCacheFactoryTest.php @@ -0,0 +1,156 @@ + [ + 'useInMemoryCache' => true, + 'useInjector' => true, + ], + 'inMemoryCacheFactory' => null, + ], + [ + 'args' => [ + 'useInMemoryCache' => false, + ], + 'inMemoryCacheFactory' => null, + ], + [ + 'args' => [ + 'useInjector' => false, + ], + 'inMemoryCacheFactory' => null, + ], + ]; + $cacheFactories = [ + null, + ]; + // Add the in-memory cache factories the current test environment supports + // If a factory isn't supported and we add it anyway it'll throw an exception. + if (filter_var(ini_get('apc.enable_cli'), FILTER_VALIDATE_BOOL)) { + $cacheFactories[] = ApcuCacheFactory::class; + } + if (MemcachedAdapter::isSupported()) { + $cacheFactories[] = MemcachedCacheFactory::class; + } + if (class_exists(PredisClient::class) || + class_exists(Relay::class) || + extension_loaded('redis') + ) { + $cacheFactories[] = RedisCacheFactory::class; + } + // Use all of the above test scenarios with each supported cache factory + $allScenarios = []; + foreach ($cacheFactories as $cacheFactory) { + foreach ($scenarios as $scenario) { + $scenario['inMemoryCacheFactory'] = $cacheFactory; + $allScenarios[] = $scenario; + } + } + return $allScenarios; + } + + /** + * @dataProvider provideCreate + */ + public function testCreate(array $args, ?string $inMemoryCacheFactory): void + { + $oldFactoryValue = Environment::getEnv('SS_MEMORY_CACHEFACTORY'); + $oldMemcachedDSNValue = Environment::getEnv('SS_MEMCACHED_DSN'); + $oldRedisDSNValue = Environment::getEnv('SS_REDIS_DSN'); + Environment::setEnv('SS_MEMORY_CACHEFACTORY', $inMemoryCacheFactory); + // These are obviously not real connections, but it seems a real connection is not required + // to just instantiate the cache adapter, which allows us to validate the correct adapter + // is instantiated. + Environment::setEnv('SS_MEMCACHED_DSN', "memcached://example.com:1234"); + Environment::setEnv('SS_REDIS_DSN', "redis://password@example.com:1234"); + + try { + $logger = new Logger('test-cache'); + $defaultArgs = [ + 'namespace' => __FUNCTION__, + 'directory' => TEMP_PATH, + ]; + $factory = new DefaultCacheFactory($defaultArgs, $logger); + $psr16Wrapper = $factory->create('test-cache', $args); + + $reflectionPoolProperty = new ReflectionProperty($psr16Wrapper, 'pool'); + $reflectionPoolProperty->setAccessible(true); + $cacheBucket = $reflectionPoolProperty->getValue($psr16Wrapper); + + if (!$inMemoryCacheFactory || (isset($args['useInMemoryCache']) && !$args['useInMemoryCache'])) { + $filesystemCache = $cacheBucket; + } else { + $this->assertInstanceOf(ChainAdapter::class, $cacheBucket); + + $reflectionAdaptersProperty = new ReflectionProperty($cacheBucket, 'adapters'); + $reflectionAdaptersProperty->setAccessible(true); + $adapters = $reflectionAdaptersProperty->getValue($cacheBucket); + + $this->assertCount(2, $adapters); + + // in-memory cache always comes first + $inMemoryCache = array_shift($adapters); + $filesystemCache = array_shift($adapters); + + // Check we have the right adapter for the given factory + switch ($inMemoryCacheFactory) { + case RedisCacheFactory::class: + $this->assertInstanceOf(RedisAdapter::class, $inMemoryCache); + break; + case MemcachedCacheFactory::class: + $this->assertInstanceOf(MemcachedAdapter::class, $inMemoryCache); + break; + case ApcuCacheFactory::class: + $this->assertInstanceOf(ApcuAdapter::class, $inMemoryCache); + break; + default: + throw new RuntimeException("Unexpected factory while running test: $inMemoryCacheFactory"); + } + + // Check the adapter got the right logger + $reflectionLoggerProperty = new ReflectionProperty($inMemoryCache, 'logger'); + $reflectionLoggerProperty->setAccessible(true); + $this->assertTrue($logger === $reflectionLoggerProperty->getValue($inMemoryCache)); + } + + // Check filesystem cache is correct + if (filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOL) && filter_var(ini_get('opcache.enable_cli'), FILTER_VALIDATE_BOOL)) { + $this->assertInstanceOf(PhpFilesAdapter::class, $filesystemCache); + } else { + $this->assertInstanceOf(FilesystemAdapter::class, $filesystemCache); + } + // Check the adapter got the right logger + $reflectionLoggerProperty = new ReflectionProperty($filesystemCache, 'logger'); + $reflectionLoggerProperty->setAccessible(true); + $this->assertTrue($logger === $reflectionLoggerProperty->getValue($filesystemCache)); + } finally { + Environment::setEnv('SS_MEMORY_CACHEFACTORY', $oldFactoryValue); + Environment::setEnv('SS_MEMCACHED_DSN', $oldMemcachedDSNValue); + Environment::setEnv('SS_REDIS_DSN', $oldRedisDSNValue); + } + } +} diff --git a/tests/php/Core/Injector/InjectorTest.php b/tests/php/Core/Injector/InjectorTest.php index 9d914feeb93..cb7f7b24173 100644 --- a/tests/php/Core/Injector/InjectorTest.php +++ b/tests/php/Core/Injector/InjectorTest.php @@ -1155,24 +1155,25 @@ public function testNest() public function testAnonymousClass() { + $class = get_class(new class ('abc') { + private string $property; + public function __construct(string $value) + { + $this->property = $value; + } + public function foo(): string + { + return $this->property; + } + }); Injector::inst()->load([ 'Some\\Project\\Class' => [ - // the php anonymous class syntax will instantiate a new anonymous class object, with ('abc') - // passed to the constructor - 'class' => new class ('abc') { - private string $property; - public function __construct(string $value) - { - $this->property = $value; - } - public function foo(): string - { - return $this->property; - } - } + 'class' => $class, ], ]); // assert that Injector creates a new instance of the anonymous class, with ('def') passed to the constructor - $this->assertSame('def', Injector::inst()->create('Some\\Project\\Class', 'def')->foo()); + $injectedObject = Injector::inst()->create('Some\\Project\\Class', 'def'); + $this->assertInstanceOf($class, $injectedObject); + $this->assertSame('def', $injectedObject->foo()); } } diff --git a/tests/php/Core/Injector/InjectorTest/EmptyFactory.php b/tests/php/Core/Injector/InjectorTest/EmptyFactory.php index e713f4689ac..62fb930a4b3 100644 --- a/tests/php/Core/Injector/InjectorTest/EmptyFactory.php +++ b/tests/php/Core/Injector/InjectorTest/EmptyFactory.php @@ -6,7 +6,7 @@ class EmptyFactory implements Factory { - public function create($service, array $params = []) + public function create(string $service, array $params = []): ?object { return null; } diff --git a/tests/php/Core/Injector/InjectorTest/SSObjectCreator.php b/tests/php/Core/Injector/InjectorTest/SSObjectCreator.php index 7fff12b3659..868175d2053 100644 --- a/tests/php/Core/Injector/InjectorTest/SSObjectCreator.php +++ b/tests/php/Core/Injector/InjectorTest/SSObjectCreator.php @@ -25,7 +25,7 @@ public function __construct($injector) $this->injector = $injector; } - public function create($class, array $params = []) + public function create(string $class, array $params = []): object { if (strpos($class ?? '', '(') === false) { return parent::create($class, $params); From ed321f8ce7ce136ce1c844cb26df9e348b794ffc Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 9 Jul 2024 13:57:33 +1200 Subject: [PATCH 2/2] MNT Test with all cache factories in CI --- .github/workflows/ci.yml | 6 ++++++ phpunit.xml.dist | 4 ++++ src/Core/Cache/DefaultCacheFactory.php | 7 ++++--- tests/php/Core/Cache/DefaultCacheFactoryTest.php | 6 +++--- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2f9a0ba413..035b6ef0e93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,3 +12,9 @@ jobs: with: # Turn phpcoverage off because it causes a segfault phpcoverage_force_off: true + phpunit_skip_suites: framework-cache-only + # Ensure we test all in-memory cache factories provided in core + extra_jobs: | + - phpunit: true + phpunit_suite: framework-cache-only + install_in_memory_cache_exts: true diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f435f9a4541..78fdb6bd38a 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -21,6 +21,10 @@ Requires PHPUnit ^9 vendor/silverstripe/cms/tests + + + tests/php/Core/Cache + diff --git a/src/Core/Cache/DefaultCacheFactory.php b/src/Core/Cache/DefaultCacheFactory.php index 7d8d0a75f6d..5dc712ef18c 100644 --- a/src/Core/Cache/DefaultCacheFactory.php +++ b/src/Core/Cache/DefaultCacheFactory.php @@ -46,17 +46,18 @@ public function create(string $service, array $args = []): CacheInterface // In-memory caches are typically more resource constrained (number of items and storage space). // Give cache consumers an opt-out if they are expecting to create large caches with long lifetimes. $useInMemoryCache = isset($args['useInMemoryCache']) ? $args['useInMemoryCache'] : true; - $inMemoryCacheFactory = Environment::getEnv('SS_MEMORY_CACHEFACTORY'); + $inMemoryCacheFactory = Environment::getEnv('SS_IN_MEMORY_CACHE_FACTORY'); $filesystemCache = $this->instantiateFilesystemCache($namespace, $defaultLifetime, $directory, $useInjector); if (!$useInMemoryCache || !$inMemoryCacheFactory) { return $this->prepareCacheForUse($filesystemCache, $useInjector); } - // Check if SS_MEMORY_CACHEFACTORY is a factory + // Check if SS_IN_MEMORY_CACHE_FACTORY is a factory if (!is_a($inMemoryCacheFactory, InMemoryCacheFactory::class, true)) { throw new LogicException( - 'SS_MEMORY_CACHEFACTORY is not a valid InMemoryCacheFactory class name' + 'The value in your SS_IN_MEMORY_CACHE_FACTORY environment variable' + . ' is not a valid InMemoryCacheFactory class name' ); } diff --git a/tests/php/Core/Cache/DefaultCacheFactoryTest.php b/tests/php/Core/Cache/DefaultCacheFactoryTest.php index 50548e0486f..9041f4a3ed0 100644 --- a/tests/php/Core/Cache/DefaultCacheFactoryTest.php +++ b/tests/php/Core/Cache/DefaultCacheFactoryTest.php @@ -78,10 +78,10 @@ class_exists(Relay::class) || */ public function testCreate(array $args, ?string $inMemoryCacheFactory): void { - $oldFactoryValue = Environment::getEnv('SS_MEMORY_CACHEFACTORY'); + $oldFactoryValue = Environment::getEnv('SS_IN_MEMORY_CACHE_FACTORY'); $oldMemcachedDSNValue = Environment::getEnv('SS_MEMCACHED_DSN'); $oldRedisDSNValue = Environment::getEnv('SS_REDIS_DSN'); - Environment::setEnv('SS_MEMORY_CACHEFACTORY', $inMemoryCacheFactory); + Environment::setEnv('SS_IN_MEMORY_CACHE_FACTORY', $inMemoryCacheFactory); // These are obviously not real connections, but it seems a real connection is not required // to just instantiate the cache adapter, which allows us to validate the correct adapter // is instantiated. @@ -148,7 +148,7 @@ public function testCreate(array $args, ?string $inMemoryCacheFactory): void $reflectionLoggerProperty->setAccessible(true); $this->assertTrue($logger === $reflectionLoggerProperty->getValue($filesystemCache)); } finally { - Environment::setEnv('SS_MEMORY_CACHEFACTORY', $oldFactoryValue); + Environment::setEnv('SS_IN_MEMORY_CACHE_FACTORY', $oldFactoryValue); Environment::setEnv('SS_MEMCACHED_DSN', $oldMemcachedDSNValue); Environment::setEnv('SS_REDIS_DSN', $oldRedisDSNValue); }