Added StatelessFetchExceptionHandler cloning optimization in ConnectionContext.

Fixed type hints in ImportConnector from callable -> FetchExceptionHandler.

Author: Bilge

src/Connector/ConnectionContext.php

@@ -2,6 +2,7 @@
 namespace ScriptFUSION\Porter\Connector;
 
 use ScriptFUSION\Porter\Connector\FetchExceptionHandler\FetchExceptionHandler;
+use ScriptFUSION\Porter\Connector\FetchExceptionHandler\StatelessFetchExceptionHandler;
 
 /**
  * Specifies runtime connection settings and provides utility methods.
@@ -83,7 +84,7 @@ function (\Exception $exception) use (&$userHandlerCloned, &$providerHandlerClon
      */
     private static function invokeHandler(FetchExceptionHandler &$handler, \Exception $exception, &$cloned)
     {
-        if (!$cloned) {
+        if (!$cloned && !$handler instanceof StatelessFetchExceptionHandler) {
             $handler = clone $handler;
             $handler->initialize();
             $cloned = true;

src/Connector/FetchExceptionHandler/StatelessFetchExceptionHandler.php

@@ -2,7 +2,7 @@
 namespace ScriptFUSION\Porter\Connector\FetchExceptionHandler;
 
 /**
- * Contains a stateless fetch exception handler that does not respond to reset() calls.
+ * Contains a fetch exception handler that does not have private state and therefore does not require initialization.
  */
 final class StatelessFetchExceptionHandler implements FetchExceptionHandler
 {

src/Connector/ImportConnector.php

@@ -2,6 +2,7 @@
 namespace ScriptFUSION\Porter\Connector;
 
 use ScriptFUSION\Porter\Cache\CacheUnavailableException;
+use ScriptFUSION\Porter\Connector\FetchExceptionHandler\FetchExceptionHandler;
 
 /**
  * Connector whose lifecycle is synchronised with an import operation. Ensures correct ConnectionContext is delivered
@@ -43,9 +44,9 @@ public function getWrappedConnector()
     /**
      * Sets the exception handler to be called when a recoverable exception is thrown by Connector::fetch().
      *
-     * @param callable $exceptionHandler Exception handler.
+     * @param FetchExceptionHandler $exceptionHandler Fetch exception handler.
      */
-    public function setExceptionHandler(callable $exceptionHandler)
+    public function setExceptionHandler(FetchExceptionHandler $exceptionHandler)
     {
         $this->context->setResourceFetchExceptionHandler($exceptionHandler);
     }

src/Specification/ImportSpecification.php

@@ -44,7 +44,7 @@ class ImportSpecification
     private $maxFetchAttempts = self::DEFAULT_FETCH_ATTEMPTS;
 
     /**
-     * @var callable
+     * @var FetchExceptionHandler
      */
     private $fetchExceptionHandler;
 
@@ -68,7 +68,7 @@ function (Transformer $transformer) {
         ));
 
         is_object($this->context) && $this->context = clone $this->context;
-        is_object($this->fetchExceptionHandler) && $this->fetchExceptionHandler = clone $this->fetchExceptionHandler;
+        $this->fetchExceptionHandler && $this->fetchExceptionHandler = clone $this->fetchExceptionHandler;
     }
 
     /**

test/Integration/Porter/Connector/ConnectionContextTest.php

@@ -2,10 +2,14 @@
 namespace ScriptFUSIONTest\Integration\Porter\Connector;
 
 use ScriptFUSION\Porter\Connector\ConnectionContext;
+use ScriptFUSION\Porter\Connector\FetchExceptionHandler\StatelessFetchExceptionHandler;
 use ScriptFUSION\Porter\Connector\RecoverableConnectorException;
 use ScriptFUSIONTest\FixtureFactory;
 use ScriptFUSIONTest\Stubs\TestFetchExceptionHandler;
 
+/**
+ * @see ConnectionContext
+ */
 final class ConnectionContextTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -20,13 +24,7 @@ public function testFetchExceptionHandlerCloned(TestFetchExceptionHandler $handl
         $handler->initialize();
         $initial = $handler->getCurrent();
 
-        $context->retry(static function () {
-            static $invocationCount;
-
-            if (!$invocationCount++) {
-                throw new RecoverableConnectorException;
-            }
-        });
+        $context->retry(self::createExceptionThrowingClosure());
 
         self::assertSame($initial, $handler->getCurrent());
     }
@@ -43,4 +41,49 @@ public function provideHandlerAndContext()
         $context->setResourceFetchExceptionHandler($handler);
         yield 'Resource exception handler' => [$handler, $context];
     }
+
+    /**
+     * Tests that when retry() is called, a stateless fetch exception handler is neither cloned nor reinitialized.
+     * For stateless handlers, initialization is a NOOP, so avoiding cloning is a small optimization.
+     */
+    public function testStatelessExceptionHandlerNotCloned()
+    {
+        $context = FixtureFactory::buildConnectionContext(
+            false,
+            $handler = new StatelessFetchExceptionHandler(static function () {
+                // Intentionally empty.
+            })
+        );
+
+        $context->retry(self::createExceptionThrowingClosure());
+
+        self::assertSame(
+            $handler,
+            call_user_func(
+                \Closure::bind(
+                    function () {
+                        return $this->fetchExceptionHandler;
+                    },
+                    $context,
+                    $context
+                )
+            )
+        );
+    }
+
+    /**
+     * Creates a closure that only throws an exception on the first invocation.
+     *
+     * @return \Closure
+     */
+    private static function createExceptionThrowingClosure()
+    {
+        return static function () {
+            static $invocationCount;
+
+            if (!$invocationCount++) {
+                throw new RecoverableConnectorException;
+            }
+        };
+    }
 }