Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Ensure cache is shared between CLI and webserver #11300

Merged
merged 2 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion _config/cache.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ SilverStripe\Core\Injector\Injector:
constructor:
args:
directory: '`TEMP_PATH`'
version: null
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version is effectively a flush mechanism - if the version changes, all cache with the previous version is invalidated.

Given this config isn't available when instantiating manifest cache, I've moved this to being an environment variable. In practice I don't think anyone would ever bother setting it since Silverstripe CMS has so many ways to flush cache.

logger: '%$Psr\Log\LoggerInterface'
Psr\SimpleCache\CacheInterface.cacheblock:
factory: SilverStripe\Core\Cache\CacheFactory
Expand Down
4 changes: 4 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ Requires PHPUnit ^9
<testsuite name="cms">
<directory>vendor/silverstripe/cms/tests</directory>
</testsuite>
<!-- Cache suite is separate so it only runs what it needs to run. Intentionally also included in core above. -->
<testsuite name="framework-cache-only">
<directory>tests/php/Core/Cache</directory>
</testsuite>
</testsuites>
<filter>
<whitelist addUncoveredFilesFromWhitelist="true">
Expand Down
2 changes: 1 addition & 1 deletion src/Control/Email/TransportFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down
100 changes: 100 additions & 0 deletions src/Core/Cache/AbstractCacheFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
<?php

namespace SilverStripe\Core\Cache;

use InvalidArgumentException;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerInterface;
use Psr\Cache\CacheItemPoolInterface;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Injector\Injector;
use Symfony\Component\Cache\Psr16Cache;

/**
* Abstract implementation of CacheFactory which provides methods to easily instantiate PSR6 and PSR16 cache adapters.
*/
abstract class AbstractCacheFactory implements CacheFactory
{
protected ?LoggerInterface $logger;

/**
* @param LoggerInterface $logger Logger instance to assign
*/
public function __construct(LoggerInterface $logger = null)
{
$this->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;
}
}
65 changes: 44 additions & 21 deletions src/Core/Cache/ApcuCacheFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +52 to 62
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is moved here from DefaultCacheFactory

}
7 changes: 1 addition & 6 deletions src/Core/Cache/CacheFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this is PSR16 - this is intended for direct use.

}
Loading