Finalized FetchExceptionHandler interface and added accompanying test.

Changed "provider" FEH name to "resource" fetch exception handler.
Changed resource FEH type from callable to FetchExceptionHandler.

Author: Bilge

src/Connector/ConnectionContext.php

@@ -13,16 +13,16 @@ final class ConnectionContext
     /**
      * User-defined exception handler called when a recoverable exception is thrown by Connector::fetch().
      *
-     * @var callable
+     * @var FetchExceptionHandler
      */
     private $fetchExceptionHandler;
 
     /**
-     * Provider-defined exception handler called when a recoverable exception is thrown by Connector::fetch().
+     * Resource-defined exception handler called when a recoverable exception is thrown by Connector::fetch().
      *
-     * @var callable
+     * @var FetchExceptionHandler
      */
-    private $providerFetchExceptionHandler;
+    private $resourceFetchExceptionHandler;
 
     private $maxFetchAttempts;
 
@@ -52,46 +52,59 @@ public function mustCache()
      */
     public function retry(callable $callback)
     {
-        $userHandlerReset = false;
+        $userHandlerCloned = $providerHandlerCloned = false;
 
         return \ScriptFUSION\Retry\retry(
             $this->maxFetchAttempts,
             $callback,
-            function (\Exception $exception) use (&$userHandlerReset) {
+            function (\Exception $exception) use (&$userHandlerCloned, &$providerHandlerCloned) {
                 // Throw exception instead of retrying, if unrecoverable.
                 if (!$exception instanceof RecoverableConnectorException) {
                     throw $exception;
                 }
 
                 // Call provider's exception handler, if defined.
-                if ($this->providerFetchExceptionHandler) {
-                    call_user_func($this->providerFetchExceptionHandler, $exception);
+                if ($this->resourceFetchExceptionHandler) {
+                    self::invokeHandler($this->resourceFetchExceptionHandler, $exception, $providerHandlerCloned);
                 }
 
-                if (!$userHandlerReset) {
-                    $this->fetchExceptionHandler->reset();
-                    $userHandlerReset = true;
-                }
-
-                // TODO: Remove call_user_func calls when PHP 5 support dropped.
-                call_user_func($this->fetchExceptionHandler, $exception);
+                // Call user's exception handler.
+                self::invokeHandler($this->fetchExceptionHandler, $exception, $userHandlerCloned);
             }
         );
     }
 
+    /**
+     * Invokes the specified fetch exception handler, cloning it if required.
+     *
+     * @param FetchExceptionHandler $handler Fetch exception handler.
+     * @param \Exception $exception Exception to pass to the handler.
+     * @param bool $cloned False if handler requires cloning, true if handler has already been cloned.
+     */
+    private static function invokeHandler(FetchExceptionHandler &$handler, \Exception $exception, &$cloned)
+    {
+        if (!$cloned) {
+            $handler = clone $handler;
+            $handler->initialize();
+            $cloned = true;
+        }
+
+        $handler($exception);
+    }
+
     /**
      * Sets an exception handler to be called when a recoverable exception is thrown by Connector::fetch().
      *
-     * This handler is intended to be set by provider resources only and is called before the user-defined handler.
+     * The handler is intended to be set by provider resources only once and is called before the user-defined handler.
      *
-     * @param callable $providerFetchExceptionHandler Exception handler.
+     * @param FetchExceptionHandler $resourceFetchExceptionHandler Exception handler.
      */
-    public function setProviderFetchExceptionHandler(callable $providerFetchExceptionHandler)
+    public function setResourceFetchExceptionHandler(FetchExceptionHandler $resourceFetchExceptionHandler)
     {
-        if ($this->providerFetchExceptionHandler !== null) {
-            throw new \LogicException('Cannot set provider fetch exception handler: already set!');
+        if ($this->resourceFetchExceptionHandler !== null) {
+            throw new \LogicException('Cannot set resource fetch exception handler: already set!');
         }
 
-        $this->providerFetchExceptionHandler = $providerFetchExceptionHandler;
+        $this->resourceFetchExceptionHandler = $resourceFetchExceptionHandler;
     }
 }

src/Connector/FetchExceptionHandler/ExponentialSleepFetchExceptionHandler.php

@@ -10,7 +10,7 @@ class ExponentialSleepFetchExceptionHandler implements FetchExceptionHandler
 {
     private $handler;
 
-    public function reset()
+    public function initialize()
     {
         $this->handler = new ExponentialBackoffExceptionHandler;
     }

src/Connector/FetchExceptionHandler/FetchExceptionHandler.php

@@ -1,9 +1,34 @@
 <?php
 namespace ScriptFUSION\Porter\Connector\FetchExceptionHandler;
 
+/**
+ * Provides methods for handling exceptions thrown by Connector::fetch().
+ *
+ * This interface supports a prototype cloning model that guarantees the object can be cloned and reset to its
+ * initial state at any time, any number of times. This is needed because a given import can spawn any number of
+ * subsequent fetches, some of which may execute concurrently, and all of which share the same exception handler
+ * prototype.
+ *
+ * This approach is better than relying on __clone because handlers may employ generators which cannot be cloned.
+ * If generators are part of the object's state they must be recreated during initialize().
+ */
 interface FetchExceptionHandler
 {
-    public function reset();
+    /**
+     * Initializes this handler to its starting state. Should be idempotent because it may be called multiple times.
+     *
+     * This method must always be called before the first call to __invoke.
+     *
+     * @return void
+     */
+    public function initialize();
 
+    /**
+     * Handles a fetch() exception.
+     *
+     * @param \Exception $exception Exception thrown by Connector::fetch().
+     *
+     * @return void
+     */
     public function __invoke(\Exception $exception);
 }

src/Connector/FetchExceptionHandler/StatelessFetchExceptionHandler.php

@@ -13,7 +13,7 @@ public function __construct(callable $handler)
         $this->handler = $handler;
     }
 
-    public function reset()
+    public function initialize()
     {
         // Intentionally empty.
     }

src/Connector/ImportConnector.php

@@ -47,6 +47,6 @@ public function getWrappedConnector()
      */
     public function setExceptionHandler(callable $exceptionHandler)
     {
-        $this->context->setProviderFetchExceptionHandler($exceptionHandler);
+        $this->context->setResourceFetchExceptionHandler($exceptionHandler);
     }
 }

src/Specification/ImportSpecification.php

@@ -242,8 +242,7 @@ final public function setMaxFetchAttempts($attempts)
      */
     final public function getFetchExceptionHandler()
     {
-        return $this->fetchExceptionHandler ?: $this->fetchExceptionHandler
-            = new ExponentialSleepFetchExceptionHandler;
+        return $this->fetchExceptionHandler ?: $this->fetchExceptionHandler = new ExponentialSleepFetchExceptionHandler;
     }
 
     /**

test/Integration/Porter/Connector/ConnectionContextTest.php

@@ -0,0 +1,46 @@
+<?php
+namespace ScriptFUSIONTest\Integration\Porter\Connector;
+
+use ScriptFUSION\Porter\Connector\ConnectionContext;
+use ScriptFUSION\Porter\Connector\RecoverableConnectorException;
+use ScriptFUSIONTest\FixtureFactory;
+use ScriptFUSIONTest\Stubs\TestFetchExceptionHandler;
+
+final class ConnectionContextTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * Tests that when retry() is called multiple times, the original fetch exception handler is unmodified.
+     * This is expected because the handler must be cloned using the prototype pattern to ensure multiple concurrent
+     * fetches do not conflict.
+     *
+     * @dataProvider provideHandlerAndContext
+     */
+    public function testFetchExceptionHandlerCloned(TestFetchExceptionHandler $handler, ConnectionContext $context)
+    {
+        $handler->initialize();
+        $initial = $handler->getCurrent();
+
+        $context->retry(static function () {
+            static $invocationCount;
+
+            if (!$invocationCount++) {
+                throw new RecoverableConnectorException;
+            }
+        });
+
+        self::assertSame($initial, $handler->getCurrent());
+    }
+
+    public function provideHandlerAndContext()
+    {
+        yield 'User exception handler' => [
+            $handler = new TestFetchExceptionHandler,
+            FixtureFactory::buildConnectionContext(false, $handler),
+        ];
+
+        $context = FixtureFactory::buildConnectionContext();
+        // It should be OK to reuse the handler here because the whole point of the test is that it's not modified.
+        $context->setResourceFetchExceptionHandler($handler);
+        yield 'Resource exception handler' => [$handler, $context];
+    }
+}

test/Integration/Porter/PorterTest.php

@@ -294,7 +294,7 @@ public function testCustomFetchExceptionHandler()
     {
         $this->specification->setFetchExceptionHandler(
             \Mockery::mock(FetchExceptionHandler::class)
-                ->shouldReceive('reset')
+                ->shouldReceive('initialize')
                     ->once()
                 ->shouldReceive('__invoke')
                     ->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS - 1)
@@ -323,11 +323,13 @@ public function testCustomProviderFetchExceptionHandler()
         $this->resource
             ->shouldReceive('fetch')
             ->andReturnUsing(function (ImportConnector $connector) use ($connectorException) {
-                $connector->setExceptionHandler(function (\Exception $exception) use ($connectorException) {
-                    self::assertSame($connectorException, $exception);
+                $connector->setExceptionHandler(new StatelessFetchExceptionHandler(
+                    function (\Exception $exception) use ($connectorException) {
+                        self::assertSame($connectorException, $exception);
 
-                    throw new \RuntimeException('This exception is thrown by the provider handler.');
-                });
+                        throw new \RuntimeException('This exception is thrown by the provider handler.');
+                    }
+                ));
 
                 yield $connector->fetch('foo');
             })

test/Stubs/Invokable.php

@@ -0,0 +1,35 @@
+<?php
+namespace ScriptFUSIONTest\Stubs;
+
+use ScriptFUSION\Porter\Connector\FetchExceptionHandler\FetchExceptionHandler;
+
+final class TestFetchExceptionHandler implements FetchExceptionHandler
+{
+    /**
+     * @var \Generator
+     */
+    private $series;
+
+    public function initialize()
+    {
+        $this->series = call_user_func(function () {
+            foreach (range(1, 10) as $value) {
+                yield $value;
+            }
+        });
+    }
+
+    public function __invoke(\Exception $exception)
+    {
+        $current = $this->getCurrent();
+
+        $this->series->next();
+
+        return $current;
+    }
+
+    public function getCurrent()
+    {
+        return $this->series->current();
+    }
+}