refactor: ensure we can modify header loader, and mark all deprecations

- Adds `@deprecated` and `@todo` annotations to the `$pluginClassLoader` property and the `setPluginClassLoader()` and `getPluginClassLoader()` methods - Adds `getHeaderLoader()` and `setHeaderLoader()` methods to allow providing alternate implementations. - Adds tests to ensure that deprecation notices are emitted. Signed-off-by: Matthew Weier O'Phinney <[email protected]>
laminas · Jul 28, 2020 · ba55363 · glensc · Jul 29, 2020 · ba55363
1 parent b6fdc6e
commit ba55363
Show file tree

Hide file tree

Showing 5 changed files with 169 additions and 45 deletions.
diff --git a/composer.json b/composer.json
@@ -54,6 +54,9 @@
         }
     },
     "autoload-dev": {
+        "files": [
+            "test/TestAsset/PluginClassLocator.php"
+        ],
         "psr-4": {
             "LaminasTest\\Mail\\": "test/"
         }

diff --git a/src/Headers.php b/src/Headers.php
@@ -11,6 +11,8 @@
 use ArrayIterator;
 use Countable;
 use Iterator;
+use Laminas\Loader\PluginClassLoader;
+use Laminas\Loader\PluginClassLocator;
 use Laminas\Mail\Header\GenericHeader;
 use Laminas\Mail\Header\HeaderInterface;
 use Traversable;
@@ -29,7 +31,9 @@ class Headers implements Countable, Iterator
     const FOLDING = "\r\n ";
 
     /**
-     * @var Header\HeaderLoader
+     * @todo Rename to $headerLoader for 3.0.0
+     * @todo Remove ability to use PluginClassLoader for 3.0.0
+     * @var null|Header\HeaderLoader|PluginClassLoader
      */
     protected $pluginClassLoader = null;
 
@@ -118,46 +122,73 @@ public static function fromString($string, $EOL = self::EOL)
     }
 
     /**
-     * Set an alternate implementation for the PluginClassLoader
+     * Set an alternate implementation for loading header classes.
      *
+     * @deprecated since 2.12.0
+     * @todo Remove for version 3.0.0
      * @param  Header\HeaderLoader|\Laminas\Loader\PluginClassLocator $pluginClassLoader
      * @return Headers
      */
     public function setPluginClassLoader($pluginClassLoader)
     {
-        if ($pluginClassLoader instanceof \Laminas\Loader\PluginClassLocator) {
-            trigger_error(
-                sprintf(
-                    'The class %s has been deprecated; please use %s',
-                    __CLASS__,
-                    Header\HeaderLoader::class
-                ),
-                E_USER_DEPRECATED
-            );
+        if ($pluginClassLoader instanceof PluginClassLocator) {
+            trigger_error(sprintf(
+                'Usage of %s instances for loading headers has been deprecated; please use %s',
+                PluginClassLoader::class,
+                Header\HeaderLoader::class
+            ), E_USER_DEPRECATED);
         }
 
         $this->pluginClassLoader = $pluginClassLoader;
         return $this;
     }
 
     /**
-     * Return an instance of a Header\HeaderLoader or \Laminas\Loader\PluginClassLocator, lazyload and inject map if necessary
+     * Return an instance of a Header\HeaderLoader or PluginClassLocator
      *
-     * @return Header\HeaderLoader|\Laminas\Loader\PluginClassLocator
+     * Lazyloads a Header\HeaderLoader if necessary.
+     *
+     * @deprecated since 2.12.0
+     * @todo Remove for version 3.0.0
+     * @return Header\HeaderLoader|PluginClassLocator
      */
     public function getPluginClassLoader()
     {
-        trigger_error(
-            'The method has been deprecated; please use ::resolveHeaderClass',
-            E_USER_DEPRECATED
-        );
+        trigger_error(sprintf(
+            'The method %s has been deprecated; please use getHeaderLoader() or resolveHeaderClass() instead',
+            __METHOD__
+        ), E_USER_DEPRECATED);
 
         if ($this->pluginClassLoader === null) {
             $this->pluginClassLoader = new Header\HeaderLoader();
         }
+
+        return $this->pluginClassLoader;
+    }
+
+    /**
+     * Retrieve the header class loader.
+     *
+     * @todo remove ability to return PluginClassLoader for 3.0.0
+     * @return Header\HeaderLoader|PluginClassLoader
+     */
+    public function getHeaderLoader()
+    {
+        if (! $this->pluginClassLoader) {
+            $this->setHeaderLoader(new Header\HeaderLoader());
+        }
         return $this->pluginClassLoader;
     }
 
+    /**
+     * @return $this
+     */
+    public function setHeaderLoader(Header\HeaderLoader $headerLoader)
+    {
+        $this->pluginClassLoader = $headerLoader;
+        return $this;
+    }
+
     /**
      * Set the header encoding
      *
@@ -550,10 +581,6 @@ protected function normalizeFieldName($fieldName)
      */
     private function resolveHeaderClass($key)
     {
-        if ($this->pluginClassLoader === null) {
-            $this->pluginClassLoader = new Header\HeaderLoader();
-        }
-
-        return $this->pluginClassLoader->get($key, Header\GenericHeader::class);
+        return $this->getHeaderLoader()->get($key, Header\GenericHeader::class);
     }
 }
diff --git a/test/Header/HeaderLoaderTest.php b/test/Header/HeaderLoaderTest.php
@@ -26,9 +26,9 @@ public function setUp()
     public function provideHeaderNames()
     {
         return [
-            'with existing name' => ['to', Header\To::class],
+            'with existing name'     => ['to', Header\To::class],
             'with non-existent name' => ['foo', null],
-            'with default value' => ['foo', Header\GenericHeader::class, Header\GenericHeader::class],
+            'with default value'     => ['foo', Header\GenericHeader::class, Header\GenericHeader::class],
         ];
     }
 
@@ -70,30 +70,30 @@ public function testHeaderCanBeRemoved()
     public static function expectedHeaders()
     {
         return [
-            ['bcc', Header\Bcc::class],
-            ['cc', Header\Cc::class],
-            ['contenttype', Header\ContentType::class],
-            ['content_type', Header\ContentType::class],
-            ['content-type', Header\ContentType::class],
-            ['date', Header\Date::class],
-            ['from', Header\From::class],
-            ['mimeversion', Header\MimeVersion::class],
-            ['mime_version', Header\MimeVersion::class],
-            ['mime-version', Header\MimeVersion::class],
-            ['received', Header\Received::class],
-            ['replyto', Header\ReplyTo::class],
-            ['reply_to', Header\ReplyTo::class],
-            ['reply-to', Header\ReplyTo::class],
-            ['sender', Header\Sender::class],
-            ['subject', Header\Subject::class],
-            ['to', Header\To::class],
+            'bcc'          => ['bcc', Header\Bcc::class],
+            'cc'           => ['cc', Header\Cc::class],
+            'contenttype'  => ['contenttype', Header\ContentType::class],
+            'content_type' => ['content_type', Header\ContentType::class],
+            'content-type' => ['content-type', Header\ContentType::class],
+            'date'         => ['date', Header\Date::class],
+            'from'         => ['from', Header\From::class],
+            'mimeversion'  => ['mimeversion', Header\MimeVersion::class],
+            'mime_version' => ['mime_version', Header\MimeVersion::class],
+            'mime-version' => ['mime-version', Header\MimeVersion::class],
+            'received'     => ['received', Header\Received::class],
+            'replyto'      => ['replyto', Header\ReplyTo::class],
+            'reply_to'     => ['reply_to', Header\ReplyTo::class],
+            'reply-to'     => ['reply-to', Header\ReplyTo::class],
+            'sender'       => ['sender', Header\Sender::class],
+            'subject'      => ['subject', Header\Subject::class],
+            'to'           => ['to', Header\To::class],
         ];
     }
 
     /**
      * @dataProvider expectedHeaders
-     * @param $name
-     * @param $class
+     * @param string $name
+     * @param Header\HeaderInterface $class
      */
     public function testDefaultHeadersMapResolvesProperHeader($name, $class)
     {

diff --git a/test/HeadersTest.php b/test/HeadersTest.php
@@ -8,12 +8,13 @@
 
 namespace LaminasTest\Mail;
 
+use Laminas\Loader\PluginClassLocator;
 use Laminas\Mail;
 use Laminas\Mail\Header;
 use Laminas\Mail\Header\Exception;
 use Laminas\Mail\Header\GenericHeader;
 use Laminas\Mail\Header\GenericMultiHeader;
-use Laminas\Mail\Header\To;
+use PHPUnit\Framework\Error\Deprecated;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -109,7 +110,9 @@ public function testHeadersFromStringFactoryCreatesMultipleObjects()
     public function testHeadersFromStringMultiHeaderWillAggregateLazyLoadedHeaders()
     {
         $headers = new Mail\Headers();
-        $headers->addHeaderLine('foo', ['[email protected]', '[email protected]', '[email protected]']);
+        $loader  = $headers->getHeaderLoader();
+        $loader->add('foo', GenericMultiHeader::class);
+        $headers->addHeaderLine('foo: bar1,bar2,bar3');
         $headers->forceLoading();
         $this->assertEquals(3, $headers->count());
     }
@@ -558,4 +561,75 @@ public function testAddHeaderCallsSetEncoding()
         // now UTF-8 via addHeader() call
         $this->assertSame('UTF-8', $subject->getEncoding());
     }
+
+    /**
+     * @todo Remove for 3.0.0
+     */
+    public function testGetPluginClassLoaderEmitsDeprecationNotice()
+    {
+        if (! class_exists(Deprecated::class)) {
+            $this->markTestSkipped('Requires a PHPUnit version that contains Error exceptions');
+        }
+
+        $headers = new Mail\Headers();
+
+        $this->expectException(Deprecated::class);
+        $this->expectExceptionMessage('getPluginClassLoader has been deprecated');
+        $headers->getPluginClassLoader();
+    }
+
+    /**
+     * @todo Remove for 3.0.0
+     */
+    public function testSetPluginClassLoaderEmitsDeprecationNoticeWhenPluginClassLocatorUsed()
+    {
+        if (! class_exists(Deprecated::class)) {
+            $this->markTestSkipped('Requires a PHPUnit version that contains Error exceptions');
+        }
+
+        $headers = new Mail\Headers();
+        $loader  = $this->prophesize(PluginClassLocator::class)->reveal();
+
+        $this->expectException(Deprecated::class);
+        $this->expectExceptionMessage('deprecated');
+        $headers->setPluginClassLoader($loader);
+    }
+
+    /**
+     * @todo Remove for 3.0.0
+     */
+    public function testGetPluginClassLoaderReturnsHeaderLoaderInstanceByDefault()
+    {
+        $headers = new Mail\Headers();
+        $loader  = @$headers->getPluginClassLoader();
+        $this->assertInstanceOf(Mail\Header\HeaderLoader::class, $loader);
+    }
+
+    /**
+     * @todo Remove for 3.0.0
+     */
+    public function testSetPluginClassLoaderAcceptsHeaderLoaderInstance()
+    {
+        $headers = new Mail\Headers();
+        $loader  = new Mail\Header\HeaderLoader();
+
+        @$headers->setPluginClassLoader($loader);
+        $this->assertSame($loader, $headers->getHeaderLoader());
+    }
+
+    public function testGetHeaderLoaderReturnsHeaderLoaderInstanceByDefault()
+    {
+        $headers = new Mail\Headers();
+        $loader  = $headers->getHeaderLoader();
+        $this->assertInstanceOf(Mail\Header\HeaderLoader::class, $loader);
+    }
+
+    public function testCanInjectAlternateHeaderLoaderInstance()
+    {
+        $headers = new Mail\Headers();
+        $loader  = new Mail\Header\HeaderLoader();
+
+        $headers->setHeaderLoader($loader);
+        $this->assertSame($loader, $headers->getHeaderLoader());
+    }
 }
diff --git a/test/TestAsset/PluginClassLocator.php b/test/TestAsset/PluginClassLocator.php
@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * @see       https://github.com/laminas/laminas-mail for the canonical source repository
+ * @copyright https://github.com/laminas/laminas-mail/blob/master/COPYRIGHT.md
+ * @license   https://github.com/laminas/laminas-mail/blob/master/LICENSE.md New BSD License
+ */
+
+namespace Laminas\Loader;
+
+/**
+ * @todo Remove this file for 3.0.0
+ */
+if (! class_exists(PluginClassLocator::class)
+    && ! interface_exists(PluginClassLocator::class)
+) {
+    class PluginClassLocator
+    {
+    }
+}