API Rename services to match FQN of interface / classes

.upgrade.yml

@@ -409,6 +409,10 @@ mappings:
   SilverStripe\Framework\Logging\DetailedErrorFormatter: SilverStripe\Logging\DetailedErrorFormatter
   SilverStripe\Framework\Logging\HTTPOutputHandler: SilverStripe\Logging\HTTPOutputHandler
   SilverStripe\Framework\Logging\MonologErrorHandler: SilverStripe\Logging\MonologErrorHandler
+  DebugViewFriendlyErrorFormatter: Monolog\Formatter\FormatterInterface.friendly
+  DetailedErrorFormatter: Monolog\Formatter\FormatterInterface.detailed
+  DisplayErrorHandler: Monolog\Handler\HandlerInterface
+  ErrorHandler: SilverStripe\Logging\ErrorHandler;
   SS_Log: SilverStripe\Logging\Log
   SilverStripe\Logging\SS_Log: SilverStripe\Logging\Log
   ComparisonFilter: SilverStripe\ORM\Filters\ComparisonFilter

_config/logging.yml

@@ -1,8 +1,13 @@
 ---
 Name: logging
 ---
+# Logging is built up of a chain containing:
+# - A top level \ErrorHandler which registers the error service
+# - A \Logger which acts as the error service
+# - A \HandlerInterface which handles errors for the logger
+# - One or more \FormatterInterface which format errors for the handler
 SilverStripe\Core\Injector\Injector:
-  ErrorHandler:
+  SilverStripe\Logging\ErrorHandler:
     class: SilverStripe\Logging\MonologErrorHandler
     properties:
       Logger: %$Psr\Log\LoggerInterface
@@ -12,35 +17,45 @@ SilverStripe\Core\Injector\Injector:
     constructor:
       - "error-log"
     calls:
-      DisplayErrorHandler: [ pushHandler, [ %$DisplayErrorHandler ] ]
+      DisplayErrorHandler: [ pushHandler, [ %$Monolog\Handler\HandlerInterface ] ]
-
+---
+Name: loggingformatters
+---
+SilverStripe\Core\Injector\Injector:
+  # Display detailed information on an error
+  Monolog\Formatter\FormatterInterface.detailed:
+    class: SilverStripe\Logging\DetailedErrorFormatter
+  # Display friendly error messages and suppresses possible disclosure of dev configuration
+  Monolog\Formatter\FormatterInterface.friendly:
+    class: SilverStripe\Logging\DebugViewFriendlyErrorFormatter
+    properties:
+      Title: "There has been an error"
+      Body: "The website server has not been able to respond to your request"
 ---
 Name: dev-logging
 Only:
   environment: dev
 ---
+# Dev handler outputs detailed information including notices
 SilverStripe\Core\Injector\Injector:
-  DisplayErrorHandler:
+  Monolog\Handler\HandlerInterface:
     class: SilverStripe\Logging\HTTPOutputHandler
     constructor:
       - "notice"
     properties:
-      Formatter: %$SilverStripe\Logging\DetailedErrorFormatter
+      DefaultFormatter: %$Monolog\Formatter\FormatterInterface.detailed
 ---
 Name: live-logging
 Except:
   environment: dev
 ---
+# Live handler outputs user-friendly error details, and ignores notices
+# CLI errors still show full details
 SilverStripe\Core\Injector\Injector:
-  DisplayErrorHandler:
+  Monolog\Handler\HandlerInterface:
     class: SilverStripe\Logging\HTTPOutputHandler
     constructor:
       - "error"
     properties:
-      Formatter: %$SilverStripe\Logging\DebugViewFriendlyErrorFormatter
+      DefaultFormatter: %$Monolog\Formatter\FormatterInterface.friendly
-      CLIFormatter: %$SilverStripe\Logging\DetailedErrorFormatter
+      CLIFormatter: %$Monolog\Formatter\FormatterInterface.detailed
-  SilverStripe\Logging\DebugViewFriendlyErrorFormatter:
-    class: SilverStripe\Logging\DebugViewFriendlyErrorFormatter
-    properties:
-      Title: "There has been an error"
-      Body: "The website server has not been able to respond to your request"

composer.json

@@ -66,6 +66,7 @@
       "SilverStripe\\i18n\\": "src/i18n/",
       "SilverStripe\\i18n\\Tests\\": "tests/php/i18n/",
       "SilverStripe\\Logging\\": "src/Logging/",
+      "SilverStripe\\Logging\\Tests\\": "tests/php/Logging/",
       "SilverStripe\\ORM\\": "src/ORM/",
       "SilverStripe\\ORM\\Tests\\": "tests/php/ORM/",
       "SilverStripe\\Security\\": "src/Security/",

docs/en/02_Developer_Guides/01_Templates/03_Requirements.md

@@ -176,25 +176,25 @@ replaced. For instance, the below will set a new set of dependencies to write to
       combine_hash_querystring: true
       default_combined_files_folder: 'combined'
     SilverStripe\Core\Injector\Injector:
-      MySiteAdapter:
+      # Create adapter that points to the custom directory root
-        class: 'SilverStripe\Filesystem\Flysystem\AssetAdapter'
+      SilverStripe\Assets\Flysystem\PublicAdapter.custom-adapter:
+        class: SilverStripe\Assets\Flysystem\PublicAssetAdapter
         constructor:
           Root: ./mysite/javascript
-      # Define the default filesystem
+      # Set flysystem filesystem that uses this adapter
-      MySiteBackend:
+      League\Flysystem\Filesystem.custom-filesystem:
         class: 'League\Flysystem\Filesystem'
         constructor:
-          Adapter: '%$MySiteAdapter'
+          Adapter: '%$SilverStripe\Assets\Flysystem\PublicAdapter.custom-adapter'
-        calls:
+      # Create handler to generate assets using this filesystem
-          PublicURLPlugin: [ addPlugin, [ %$FlysystemUrlPlugin ] ]
+      SilverStripe\Assets\Storage\GeneratedAssetHandler.custom-generated-assets:
-      # Requirements config
+        class: SilverStripe\Assets\Flysystem\GeneratedAssets
-      MySiteAssetHandler:
-        class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler
         properties:
-          Filesystem: '%$MySiteBackend'
+          Filesystem: %$League\Flysystem\Filesystem.custom-filesystem
+      # Assign this generator to the requirements builder
       SilverStripe\View\Requirements_Backend:
         properties:
-          AssetHandler: '%$MySiteAssetHandler'
+          AssetHandler: '%$SilverStripe\Assets\Storage\GeneratedAssetHandler.custom-generated-assets'
 
 In the above configuration, automatic expiry of generated files has been disabled, and it is necessary for
 the developer to maintain these files manually. This may be useful in environments where assets must

docs/en/02_Developer_Guides/14_Files/01_File_Management.md

@@ -35,12 +35,12 @@ Because the filesystem now uses the sha1 of file contents in order to version mu
 filename, the default storage paths in 4.0 will not be the same as in 3.
 
 In order to retain existing file paths in line with framework version 3 you should set the
-`\SilverStripe\Filesystem\Flysystem\FlysystemAssetStore.legacy_paths` config to true.
+`\SilverStripe\Assets\Flysystem\FlysystemAssetStore.legacy_filenames` config to true.
 Note that this will not allow you to utilise certain file versioning features in 4.0.
 
 	:::yaml
-	\SilverStripe\Filesystem\Flysystem\FlysystemAssetStore:
+	\SilverStripe\Assets\Flysystem\FlysystemAssetStore:
-		legacy_paths: true
+		legacy_filenames: true
 
 ## Loading content into `DBFile`
 

docs/en/02_Developer_Guides/14_Files/03_File_Security.md

@@ -19,7 +19,7 @@ config option:
 
 
     :::php
-    $store = singleton('AssetStore');
+    $store = singleton(AssetStore::class);
     $store->setFromString('My protected content', 'Documents/Mydocument.txt', null, null, array(
         'visibility' => AssetStore::VISIBILITY_PROTECTED
     ));
@@ -182,7 +182,7 @@ the web server, bypassing the need for additional PHP requests.
 
 
     :::php
-    $store = singleton('AssetStore');
+    $store = singleton(AssetStore::class);
     $store->publish('NewCompanyLogo.gif', 'a870de278b475cb75f5d9f451439b2d378e13af1');
 
 

src/Control/HTTPResponse.php

@@ -3,6 +3,8 @@
 namespace SilverStripe\Control;
 
 use InvalidArgumentException;
+use Monolog\Formatter\FormatterInterface;
+use Monolog\Handler\HandlerInterface;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
@@ -306,11 +308,12 @@ EOT
                 }
             }
 
-            // Only show error pages or generic "friendly" errors if the status code signifies
+            // If this is an error but no error body has yet been generated,
-            // an error, and the response doesn't have any body yet that might contain
+            // delegate formatting to current error handler.
-            // a more specific error description.
+            if ($this->isError() && !$this->body) {
-            if (Director::isLive() && $this->isError() && !$this->body) {
+                /** @var HandlerInterface $handler */
-                $formatter = Injector::inst()->get('FriendlyErrorFormatter');
+                $handler = Injector::inst()->get(HandlerInterface::class);
+                $formatter = $handler->getFormatter();
                 echo $formatter->format(array(
                     'code' => $this->statusCode
                 ));

src/Control/Session.php

@@ -183,7 +183,7 @@ class Session
      * Set a key/value pair in the session
      *
      * @param string $name Key
-     * @param string $val Value
+     * @param string|array $val Value
      */
     public static function set($name, $val)
     {

src/Core/Core.php

@@ -12,6 +12,7 @@ use SilverStripe\Control\Director;
 use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\Core\Manifest\ModuleManifest;
 use SilverStripe\i18n\i18n;
+use SilverStripe\Logging\ErrorHandler;
 
 /**
  * This file is the Framework bootstrap.  It will get your environment ready to call Director::direct().
@@ -116,7 +117,7 @@ if (Director::isLive()) {
 /**
  * Load error handlers
  */
-$errorHandler = Injector::inst()->get('ErrorHandler');
+$errorHandler = Injector::inst()->get(ErrorHandler::class);
 $errorHandler->start();
 
 ///////////////////////////////////////////////////////////////////////////////

src/Dev/CliDebugView.php

@@ -88,6 +88,11 @@ class CliDebugView extends DebugView
         return $output;
     }
 
+    public function renderParagraph($text)
+    {
+        return wordwrap($text, self::config()->columns) . "\n\n";
+    }
+
     /**
      * Render the information header for the view
      *

src/Dev/Debug.php

@@ -186,8 +186,8 @@ class Debug
     public static function create_debug_view()
     {
         $service = Director::is_cli() || Director::is_ajax()
-            ? 'SilverStripe\\Dev\\CliDebugView'
+            ? CliDebugView::class
-            : 'SilverStripe\\Dev\\DebugView';
+            : DebugView::class;
         return Injector::inst()->get($service);
     }
 

src/Logging/ErrorHandler.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\Logging;
+
+/**
+ * Core error handler for a SilverStripe application
+ */
+interface ErrorHandler
+{
+    /**
+     * Register and begin handling errors with this handler
+     */
+    public function start();
+}

src/Logging/HTTPOutputHandler.php

@@ -112,9 +112,31 @@ class HTTPOutputHandler extends AbstractProcessingHandler
             return $cliFormatter;
         }
 
+        return $this->getDefaultFormatter();
+    }
+
+    /**
+     * Check default formatter to use
+     *
+     * @return FormatterInterface
+     */
+    public function getDefaultFormatter()
+    {
         return parent::getFormatter();
     }
 
+    /**
+     * Set default formatter
+     *
+     * @param FormatterInterface $formatter
+     * @return $this
+     */
+    public function setDefaultFormatter(FormatterInterface $formatter)
+    {
+        parent::setFormatter($formatter);
+        return $this;
+    }
+
     /**
      * @param array $record
      * @return bool

src/Logging/MonologErrorHandler.php

@@ -3,12 +3,9 @@
 namespace SilverStripe\Logging;
 
 use Psr\Log\LoggerInterface;
-use Monolog\ErrorHandler;
+use Monolog\ErrorHandler as MonologHandler;
 
-/**
+class MonologErrorHandler implements ErrorHandler
- * Simple adaptor to start Monolog\ErrorHandler
- */
-class MonologErrorHandler
 {
     /**
      * @var LoggerInterface
@@ -32,6 +29,6 @@ class MonologErrorHandler
                 . "Is your Injector config correct?");
         }
 
-        ErrorHandler::register($this->logger);
+        MonologHandler::register($this->logger);
     }
 }

src/View/Requirements_Backend.php

@@ -1288,6 +1288,7 @@ class Requirements_Backend
      * @param array $fileList List of files to combine
      * @param string $type Either 'js' or 'css'
      * @return string|null URL to this resource, if there are files to combine
+     * @throws Exception
      */
     protected function getCombinedFileURL($combinedFile, $fileList, $type)
     {

tests/php/Core/Cache/CacheTest.php

@@ -5,7 +5,6 @@ namespace SilverStripe\Core\Tests\Cache;
 use Psr\SimpleCache\CacheInterface;
 use SilverStripe\Core\Cache\ApcuCacheFactory;
 use SilverStripe\Core\Cache\MemcachedCacheFactory;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Test\Cache\CacheTest\MockCache;
 use SilverStripe\Dev\SapphireTest;
@@ -18,38 +17,29 @@ class CacheTest extends SapphireTest
     {
         parent::setUp();
 
-        Config::modify()
+        Injector::inst()
-            ->set(
+            ->load([
-                Injector::class,
+                ApcuCacheFactory::class => [
-                ApcuCacheFactory::class,
+                    'constructor' => [ 'version' => 'ss40test' ]
-                [
+                ],
-                    'constructor' => [ 'version' => 4400 ]
+                MemcachedCacheFactory::class => MemcachedCacheFactory::class,
-                ]
+                CacheInterface::class . '.TestApcuCache' =>  [
-            )
-            ->set(
-                Injector::class,
-                CacheInterface::class . '.TestApcuCache',
-                [
                     'factory' => ApcuCacheFactory::class,
                     'constructor' => [
                         'namespace' => 'TestApcuCache',
                         'defaultLifetime' => 2600,
                     ],
-                ]
+                ],
-            )
+                CacheInterface::class . '.TestMemcache' => [
-            ->set(
-                Injector::class,
-                CacheInterface::class . '.TestMemcache',
-                [
                     'factory' => MemcachedCacheFactory::class,
                     'constructor' => [
                         'namespace' => 'TestMemCache',
                         'defaultLifetime' => 5600,
                     ],
-                ]
+                ],
-            )
+                ApcuCache::class => MockCache::class,
-            ->set(Injector::class, ApcuCache::class, MockCache::class)
+                MemcachedCache::class => MockCache::class,
-            ->set(Injector::class, MemcachedCache::class, MockCache::class);
+            ]);
     }
 
     public function testApcuCacheFactory()
@@ -63,7 +53,7 @@ class CacheTest extends SapphireTest
             [
                 'TestApcuCache_'.md5(BASE_PATH),
                 2600,
-                4400
+                'ss40test'
             ],
             $cache->getArgs()
         );

tests/php/Logging/DebugViewFriendlyErrorFormatterTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace SilverStripe\Logging\Tests;
+
+use SilverStripe\Control\Email\Email;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Logging\DebugViewFriendlyErrorFormatter;
+
+class DebugViewFriendlyErrorFormatterTest extends SapphireTest
+{
+    public function setUp()
+    {
+        parent::setUp();
+        Email::config()->set('admin_email', 'testy@mctest.face');
+    }
+
+    public function testOutput()
+    {
+        $formatter = new DebugViewFriendlyErrorFormatter();
+        $formatter->setTitle("There has been an error");
+        $formatter->setBody("The website server has not been able to respond to your request");
+
+        $expected = <<<TEXT
+WEBSITE ERROR
+There has been an error
+-----------------------
+The website server has not been able to respond to your request
+
+Contact an administrator: testy [at] mctest [dot] face
+
+
+TEXT
+        ;
+
+        $this->assertEquals($expected, $formatter->output(404));
+    }
+}

tests/php/Logging/DetailedErrorFormatterTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace SilverStripe\Logging\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Logging\DetailedErrorFormatter;
+use SilverStripe\Logging\Tests\DetailedErrorFormatterTest\ErrorGenerator;
+
+class DetailedErrorFormatterTest extends SapphireTest
+{
+    public function testFormat()
+    {
+        $generator = new ErrorGenerator();
+        $formatter = new DetailedErrorFormatter();
+        $exception = $generator->mockException();
+
+        $output = ''.$formatter->format(['context' => [
+            'exception' => $exception,
+        ]]);
+
+        $base = __DIR__;
+        $this->assertContains('ERROR [Emergency]: Uncaught Exception: Error', $output);
+        $this->assertContains("Line 32 in $base/DetailedErrorFormatterTest/ErrorGenerator.php", $output);
+        $this->assertContains('* 32:                  throw new Exception(\'Error\');', $output);
+        $this->assertContains(
+            'SilverStripe\\Logging\\Tests\\DetailedErrorFormatterTest\\ErrorGenerator->mockException(4)',
+            $output
+        );
+    }
+}

tests/php/Logging/DetailedErrorFormatterTest/ErrorGenerator.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace SilverStripe\Logging\Tests\DetailedErrorFormatterTest;
+
+use Exception;
+use SilverStripe\Dev\TestOnly;
+
+/**
+ * WARNING: This file is sensitive to whitespace changes
+ */
+class ErrorGenerator implements TestOnly
+{
+    /**
+     * Generate an exception with a trace depeth of at least 4
+     *
+     * @param int $depth
+     * @return Exception
+     * @throws Exception
+     */
+    public function mockException($depth = 0)
+    {
+        switch ($depth) {
+            case 0:
+                try {
+                    $this->mockException(1);
+                } catch (\Exception $ex) {
+                    return $ex;
+                }
+                return null;
+                break;
+            case 4:
+                throw new Exception('Error');
+            default:
+                return $this->mockException($depth + 1);
+        }
+    }
+}

tests/php/Logging/HTTPOutputHandlerTest.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace SilverStripe\Logging\Tests;
+
+use Monolog\Handler\HandlerInterface;
+use PhpParser\Node\Scalar\MagicConst\Dir;
+use SilverStripe\Control\Director;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Logging\DebugViewFriendlyErrorFormatter;
+use SilverStripe\Logging\DetailedErrorFormatter;
+use SilverStripe\Logging\HTTPOutputHandler;
+
+class HTTPOutputHandlerTest extends SapphireTest
+{
+    public function setUp()
+    {
+        parent::setUp();
+        if (!Director::is_cli()) {
+            $this->markTestSkipped("This test only runs in CLI mode");
+        }
+        if (!Director::isDev()) {
+            $this->markTestSkipped("This test only runs in dev mode");
+        }
+    }
+
+    public function testGetFormatter()
+    {
+        $handler = new HTTPOutputHandler();
+
+        $detailedFormatter = new DetailedErrorFormatter();
+        $friendlyFormatter = new DebugViewFriendlyErrorFormatter();
+
+        // Handler without CLIFormatter chooses correct formatter
+        $handler->setDefaultFormatter($detailedFormatter);
+        $this->assertInstanceOf(DetailedErrorFormatter::class, $handler->getFormatter());
+        $this->assertInstanceOf(DetailedErrorFormatter::class, $handler->getDefaultFormatter());
+
+        // Handler with CLIFormatter should return that, although default handler is still accessible
+        $handler->setCLIFormatter($friendlyFormatter);
+        $this->assertInstanceOf(DebugViewFriendlyErrorFormatter::class, $handler->getFormatter());
+        $this->assertInstanceOf(DetailedErrorFormatter::class, $handler->getDefaultFormatter());
+    }
+
+    /**
+     * Covers `#dev-logging` section in logging.yml
+     */
+    public function testDevConfig()
+    {
+        /** @var HTTPOutputHandler $handler */
+        $handler = Injector::inst()->get(HandlerInterface::class);
+        $this->assertInstanceOf(HTTPOutputHandler::class, $handler);
+
+        // Test only default formatter is set, but CLI specific formatter is left out
+        $this->assertNull($handler->getCLIFormatter());
+        $this->assertInstanceOf(DetailedErrorFormatter::class, $handler->getDefaultFormatter());
+        $this->assertInstanceOf(DetailedErrorFormatter::class, $handler->getFormatter());
+    }
+}