From ce14060913fee01d86d73ca8513d3a68d87c627c Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 10 Mar 2017 11:20:58 +1300 Subject: [PATCH] =?UTF-8?q?API=20Apply=20default=20logger=20to=20all=20cac?= =?UTF-8?q?hes=20API=20Rename=20=E2=80=98Logger=E2=80=99=20service=20name?= =?UTF-8?q?=20to=20=E2=80=98Psr\Log\LoggerInterface=E2=80=99=20API=20Defau?= =?UTF-8?q?ltCacheFactory=20constructor=20now=20takes=20an=20array=20of=20?= =?UTF-8?q?default=20arguments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- _config/cache.yml | 5 +- _config/logging.yml | 4 +- .../07_Debugging/01_Error_Handling.md | 14 +-- src/Core/Cache/CacheFactory.php | 4 +- src/Core/Cache/DefaultCacheFactory.php | 103 +++++++++++++----- src/Logging/Log.php | 8 +- src/Logging/MonologErrorHandler.php | 9 +- src/conf/ConfigureFromEnv.php | 3 +- 8 files changed, 102 insertions(+), 48 deletions(-) diff --git a/_config/cache.yml b/_config/cache.yml index c5b0eca68..ac0e4247e 100644 --- a/_config/cache.yml +++ b/_config/cache.yml @@ -5,7 +5,10 @@ SilverStripe\Core\Injector\Injector: SilverStripe\Core\Cache\CacheFactory: class: 'SilverStripe\Core\Cache\DefaultCacheFactory' constructor: - directory: '`TEMP_FOLDER`' + args: + directory: '`TEMP_FOLDER`' + version: null + logger: %$Psr\Log\LoggerInterface Psr\SimpleCache\CacheInterface.GDBackend_Manipulations: factory: SilverStripe\Core\Cache\CacheFactory constructor: diff --git a/_config/logging.yml b/_config/logging.yml index 0d1c4f893..675a305b6 100644 --- a/_config/logging.yml +++ b/_config/logging.yml @@ -5,8 +5,8 @@ SilverStripe\Core\Injector\Injector: ErrorHandler: class: SilverStripe\Logging\MonologErrorHandler properties: - Logger: %$Logger - Logger: + Logger: %$Psr\Log\LoggerInterface + Psr\Log\LoggerInterface: type: singleton class: Monolog\Logger constructor: diff --git a/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md b/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md index 8c5b9cfc4..bd7618db5 100644 --- a/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md +++ b/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md @@ -14,8 +14,8 @@ For informational and debug logs, you can use the Logger directly. The Logger is can be accessed via the `Injector`: :::php - Injector::inst()->get('Logger')->info('User has logged in: ID #' . Member::currentUserID()); - Injector::inst()->get('Logger')->debug('Query executed: ' . $sql); + Injector::inst()->get(LoggerInterface::class)->info('User has logged in: ID #' . Member::currentUserID()); + Injector::inst()->get(LoggerInterface::class)->debug('Query executed: ' . $sql); Although you can raise more important levels of alerts in this way, we recommend using PHP's native error systems for these instead. @@ -48,16 +48,16 @@ but they can be caught with a try/catch clause. ### Accessing the logger via dependency injection. -It can quite verbose to call `Injector::inst()->get('Logger')` all the time. More importantly, it also means that you're -coupling your code to global state, which is a bad design practise. A better approach is to use depedency injection to -pass the logger in for you. The [Injector](../extending/Injector) can help with this. The most straightforward is to -specify a `dependencies` config setting, like this: +It can quite verbose to call `Injector::inst()->get(LoggerInterface::class)` all the time. More importantly, +it also means that you're coupling your code to global state, which is a bad design practise. A better +approach is to use depedency injection to pass the logger in for you. The [Injector](../extending/Injector) +can help with this. The most straightforward is to specify a `dependencies` config setting, like this: :::php class MyController { private static $dependencies = array( - 'logger' => '%$Logger', + 'logger' => '%$Psr\Log\LoggerInterface', ); // This will be set automatically, as long as MyController is instantiated via Injector diff --git a/src/Core/Cache/CacheFactory.php b/src/Core/Cache/CacheFactory.php index 1f216c1b0..0d1a4e500 100644 --- a/src/Core/Cache/CacheFactory.php +++ b/src/Core/Cache/CacheFactory.php @@ -12,8 +12,8 @@ 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 $class - * @param array $args + * @param string $service + * @param array $params * @return CacheInterface */ public function create($service, array $params = array()); diff --git a/src/Core/Cache/DefaultCacheFactory.php b/src/Core/Cache/DefaultCacheFactory.php index 818f0b8a4..963fba709 100644 --- a/src/Core/Cache/DefaultCacheFactory.php +++ b/src/Core/Cache/DefaultCacheFactory.php @@ -2,6 +2,9 @@ namespace SilverStripe\Core\Cache; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerInterface; +use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Injector\Injector; use Symfony\Component\Cache\Simple\FilesystemCache; use Symfony\Component\Cache\Simple\ApcuCache; @@ -20,25 +23,24 @@ use Symfony\Component\Cache\Adapter\PhpFilesAdapter; */ class DefaultCacheFactory implements CacheFactory { - /** * @var string Absolute directory path */ - protected $directory; + protected $args = []; /** - * @var string APC version for apcu_add() + * @var LoggerInterface */ - protected $version; + protected $logger; /** - * @param string $directory - * @param string $version + * @param array $args List of global options to merge with args during create() + * @param LoggerInterface $logger Logger instance to assign */ - public function __construct($directory, $version = null) + public function __construct($args = [], LoggerInterface $logger = null) { - $this->directory = $directory; - $this->version = $version; + $this->args = $args; + $this->logger = $logger; } /** @@ -46,33 +48,76 @@ class DefaultCacheFactory implements CacheFactory */ public function create($service, array $args = array()) { - $namespace = (isset($args['namespace'])) ? $args['namespace'] : ''; - $defaultLifetime = (isset($args['defaultLifetime'])) ? $args['defaultLifetime'] : 0; - $version = $this->version; - $directory = $this->directory; + // merge args with default + $args = array_merge($this->args, $args); + $namespace = isset($args['namespace']) ? $args['namespace'] : ''; + $defaultLifetime = isset($args['defaultLifetime']) ? $args['defaultLifetime'] : 0; + $directory = isset($args['directory']) ? $args['directory'] : null; + $version = isset($args['version']) ? $args['version'] : null; - $apcuSupported = null; - $phpFilesSupported = null; + // Check support + $apcuSupported = $this->isAPCUSupported(); + $phpFilesSupported = $this->isPHPFilesSupported(); - if (null === $apcuSupported) { - $apcuSupported = ApcuAdapter::isSupported(); + // If apcu isn't supported, phpfiles is the next best preference + if (!$apcuSupported && $phpFilesSupported) { + return $this->createCache(PhpFilesCache::class, [$namespace, $defaultLifetime, $directory]); } - if (!$apcuSupported && null === $phpFilesSupported) { - $phpFilesSupported = PhpFilesAdapter::isSupported(); - } - - if ($phpFilesSupported) { - $opcache = Injector::inst()->create(PhpFilesCache::class, $namespace, $defaultLifetime, $directory); - return $opcache; - } - - $fs = Injector::inst()->create(FilesystemCache::class, $namespace, $defaultLifetime, $directory); + // Create filessytem cache + $fs = $this->createCache(FilesystemCache::class, [$namespace, $defaultLifetime, $directory]); if (!$apcuSupported) { return $fs; } - $apcu = Injector::inst()->create(ApcuCache::class, $namespace, (int) $defaultLifetime / 5, $version); - return Injector::inst()->create(ChainCache::class, [$apcu, $fs]); + // Chain this cache with ApcuCache + $apcu = $this->createCache(ApcuCache::class, [$namespace, (int) $defaultLifetime / 5, $version]); + return $this->createCache(ChainCache::class, [[$apcu, $fs]]); + } + + /** + * Determine if apcu is supported + * + * @return bool + */ + protected function isAPCUSupported() + { + static $apcuSupported = null; + if (null === $apcuSupported) { + $apcuSupported = ApcuAdapter::isSupported(); + } + return $apcuSupported; + } + + /** + * Determine if PHP files is supported + * + * @return bool + */ + protected function isPHPFilesSupported() + { + static $phpFilesSupported = null; + if (null === $phpFilesSupported) { + $phpFilesSupported = PhpFilesAdapter::isSupported(); + } + return $phpFilesSupported; + } + + /** + * @param string $class + * @param array $args + * @return CacheInterface + */ + protected function createCache($class, $args) + { + /** @var CacheInterface $cache */ + $cache = Injector::inst()->createWithArgs($class, $args); + + // Assign cache logger + if ($this->logger && $cache instanceof LoggerAwareInterface) { + $cache->setLogger($this->logger); + } + + return $cache; } } diff --git a/src/Logging/Log.php b/src/Logging/Log.php index f25fb8e50..a1a399d64 100644 --- a/src/Logging/Log.php +++ b/src/Logging/Log.php @@ -36,8 +36,8 @@ class Log */ public static function get_logger() { - Deprecation::notice('5.0', 'Use Injector::inst()->get(\'Logger\') instead'); - return Injector::inst()->get('Logger'); + Deprecation::notice('5.0', 'Use Injector::inst()->get(LoggerInterface::class) instead'); + return Injector::inst()->get(LoggerInterface::class); } /** @@ -55,7 +55,7 @@ class Log */ public static function log($message, $priority) { - Deprecation::notice('5.0', 'Use Injector::inst()->get(\'Logger\')->log($priority, $message) instead'); - Injector::inst()->get('Logger')->log($priority, $message); + Deprecation::notice('5.0', 'Use Injector::inst()->get(LoggerInterface::class)->log($priority, $message) instead'); + Injector::inst()->get(LoggerInterface::class)->log($priority, $message); } } diff --git a/src/Logging/MonologErrorHandler.php b/src/Logging/MonologErrorHandler.php index e3f769e04..25ce3fe8f 100644 --- a/src/Logging/MonologErrorHandler.php +++ b/src/Logging/MonologErrorHandler.php @@ -10,17 +10,22 @@ use Monolog\ErrorHandler; */ class MonologErrorHandler { + /** + * @var LoggerInterface + */ private $logger; /** * Set the PSR-3 logger to send errors & exceptions to + * + * @param LoggerInterface $logger */ - function setLogger(LoggerInterface $logger) + public function setLogger(LoggerInterface $logger) { $this->logger = $logger; } - function start() + public function start() { if (!$this->logger) { throw new \InvalidArgumentException("No Logger property passed to MonologErrorHandler." diff --git a/src/conf/ConfigureFromEnv.php b/src/conf/ConfigureFromEnv.php index 3ffe44256..bf3738e3c 100644 --- a/src/conf/ConfigureFromEnv.php +++ b/src/conf/ConfigureFromEnv.php @@ -48,6 +48,7 @@ use Monolog\Logger; use Monolog\Handler\StreamHandler; +use Psr\Log\LoggerInterface; use SilverStripe\Control\Email\Email; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Install\DatabaseAdapterRegistry; @@ -138,7 +139,7 @@ if ($useBasicAuth = getenv('SS_USE_BASIC_AUTH')) { } if ($errorLog = getenv('SS_ERROR_LOG')) { - $logger = Injector::inst()->get('Logger'); + $logger = Injector::inst()->get(LoggerInterface::class); if ($logger instanceof Logger) { $logger->pushHandler(new StreamHandler(BASE_PATH . '/' . $errorLog, Logger::WARNING)); } else {