Merge pull request #6693 from open-sausages/pulls/4.0/cache-logging

API Apply logging to cache system

_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:

_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:

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

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());

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;
     }
 }

src/Core/Config/CoreConfigFactory.php

@@ -2,6 +2,10 @@
 
 namespace SilverStripe\Core\Config;
 
+use Monolog\Handler\ErrorLogHandler;
+use Monolog\Handler\StreamHandler;
+use Monolog\Logger;
+use Psr\Log\LoggerInterface;
 use SilverStripe\Config\Collections\CachedConfigCollection;
 use SilverStripe\Config\Collections\MemoryConfigCollection;
 use SilverStripe\Config\Transformer\PrivateStaticTransformer;
@@ -48,7 +52,7 @@ class CoreConfigFactory
         $instance = new CachedConfigCollection();
 
         // Set root cache
-        $instance->setPool(new FilesystemAdapter('configcache', 0, getTempFolder()));
+        $instance->setPool($this->createPool());
         $instance->setFlush($flush);
 
         // Set collection creator
@@ -168,4 +172,32 @@ class CoreConfigFactory
                 return ModuleLoader::instance()->getManifest()->moduleExists($module);
             });
     }
+
+    /**
+     * @todo Refactor bootstrapping of manifest caching into app object
+     * @return FilesystemAdapter
+     */
+    protected function createPool()
+    {
+        $cache = new FilesystemAdapter('configcache', 0, getTempFolder());
+        $cache->setLogger($this->createLogger());
+        return $cache;
+    }
+
+    /**
+     * Create default error logger
+     *
+     * @todo Refactor bootstrapping of manifest logging into app object
+     * @return LoggerInterface
+     */
+    protected function createLogger()
+    {
+        $logger = new Logger("configcache-log");
+        if (Director::isDev()) {
+            $logger->pushHandler(new StreamHandler('php://output'));
+        } else {
+            $logger->pushHandler(new ErrorLogHandler());
+        }
+        return $logger;
+    }
 }

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);
     }
 }

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."

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 {