Ensure processors are determined at runtime (#35)

phpunit.xml

@@ -1,13 +1,17 @@
 <?xml version="1.0"?>
-<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" verbose="false">
+<phpunit
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.6/phpunit.xsd"
+    colors="true"
+>
   <coverage processUncoveredFiles="true">
     <include>
       <directory suffix=".php">src</directory>
     </include>
   </coverage>
   <testsuites>
     <testsuite name="all">
-      <directory suffix="Test.php" phpVersion="7.2" phpVersionOperator="&gt;=">test</directory>
+      <directory phpVersion="7.4" phpVersionOperator=">=">test</directory>
     </testsuite>
   </testsuites>
 </phpunit>

src/FileFetcher.php

@@ -17,14 +17,59 @@
 class FileFetcher extends AbstractPersistentJob
 {
 
+    /**
+     * Array of processor class names provided by the configuration.
+     *
+     * @var string[]
+     */
     protected array $customProcessorClasses = [];
 
+    /**
+     * The processor this file fetcher will use.
+     *
+     * Stored here so that we don't have to recompute it.
+     *
+     * @var \FileFetcher\Processor\ProcessorInterface|null
+     */
+    private ?ProcessorInterface $processor = null;
+
+    /**
+     * {@inheritDoc}
+     *
+     * We override ::get() because it can set values for both newly constructed
+     * objects and re-hydrated ones.
+     */
+    public static function get(string $identifier, $storage, array $config = null)
+    {
+        $ff = parent::get($identifier, $storage, $config);
+        // If we see that a processor is configured, we need to handle some
+        // special cases. It might be that the hydrated values for
+        // $customProcessorClasses are the same, but the caller could also be
+        // telling us to use a different processor than any that were hydrated
+        // from storage. We keep the existing ones and prepend the new ones.
+        $ff->addProcessors($config);
+        $storage->store(json_encode($ff), $identifier);
+        return $ff;
+    }
+
     /**
      * Constructor.
+     *
+     * Constructor is protected. Use static::get() to instantiate a file
+     * fetcher.
+     *
+     * @param string $identifier
+     *   File fetcher job identifier.
+     * @param $storage
+     *   File fetcher job storage object.
+     * @param array|NULL $config
+     *   Configuration for the file fetcher.
+     *
+     * @see static::get()
      */
     protected function __construct(string $identifier, $storage, array $config = null)
     {
-        parent::__construct($identifier, $storage, $config);
+        parent::__construct($identifier, $storage);
 
         $this->setProcessors($config);
 
@@ -42,15 +87,6 @@ protected function __construct(string $identifier, $storage, array $config = nul
             'temporary_directory' => $config['temporaryDirectory'],
         ];
 
-        // [State]
-
-        foreach ($this->getProcessors() as $processor) {
-            if ($processor->isServerCompatible($state)) {
-                $state['processor'] = get_class($processor);
-                break;
-            }
-        }
-
         $this->getResult()->setData(json_encode($state));
     }
 
@@ -62,6 +98,9 @@ public function setTimeLimit(int $seconds): bool
         return parent::setTimeLimit($seconds);
     }
 
+    /**
+     * {@inheritDoc}
+     */
     protected function runIt()
     {
         $state = $this->getProcessor()->setupState($this->getState());
@@ -71,10 +110,19 @@ protected function runIt()
         return $info['result'];
     }
 
+    /**
+     * Gets the combined custom and default processors list.
+     *
+     * @return array
+     *   The combined custom and default processors list, prioritizing the
+     *   custom ones in the order they were defined.
+     */
     protected function getProcessors(): array
     {
         $processors = self::getDefaultProcessors();
-        foreach ($this->customProcessorClasses as $processorClass) {
+        // Reverse the array so when we merge it all back together it's in the
+        // correct order of precedent.
+        foreach (array_reverse($this->customProcessorClasses) as $processorClass) {
             if ($processor = $this->getCustomProcessorInstance($processorClass)) {
                 $processors = array_merge([get_class($processor) => $processor], $processors);
             }
@@ -84,15 +132,32 @@ protected function getProcessors(): array
 
     private static function getDefaultProcessors()
     {
-        $processors = [];
-        $processors[Local::class] = new Local();
-        $processors[Remote::class] = new Remote();
-        return $processors;
+        return [
+            Local::class => new Local(),
+            Remote::class => new Remote(),
+        ];
     }
 
-    protected function getProcessor(): ProcessorInterface
+    /**
+     * Get the processor used by this file fetcher object.
+     *
+     * @return \FileFetcher\Processor\ProcessorInterface|null
+     *   A processor object, determined by configuration, or NULL if none is
+     *   suitable.
+     */
+    protected function getProcessor(): ?ProcessorInterface
     {
-        return $this->getProcessors()[$this->getStateProperty('processor')];
+        if ($this->processor) {
+            return $this->processor;
+        }
+        $state = $this->getState();
+        foreach ($this->getProcessors() as $processor) {
+            if ($processor->isServerCompatible($state)) {
+                $this->processor = $processor;
+                break;
+            }
+        }
+        return $this->processor;
     }
 
     private function validateConfig($config): array
@@ -109,30 +174,108 @@ private function validateConfig($config): array
         return $config;
     }
 
+    /**
+     * Set custom processors for this file fetcher object.
+     *
+     * @param $config
+     *   Configuration array, as passed to __construct() or Job::get(). Should
+     *   contain an array of processor class names under the key 'processors'.
+     *
+     * @see self::addProcessors()
+     */
     protected function setProcessors($config)
     {
-        if (!isset($config['processors'])) {
-            return;
+        $this->processor = null;
+        $processors = $config['processors'] ?? [];
+        if (!is_array($processors)) {
+            $processors = [];
         }
+        $this->customProcessorClasses = $processors;
+    }
 
-        if (!is_array($config['processors'])) {
+    /**
+     * Add configured processors to the ones already set in the object.
+     *
+     * Existing custom processor classes will be preserved, but any present in
+     * the new config will be prioritized.
+     *
+     * @param array $config
+     *   Configuration array, as passed to __construct() or Job::get(). Should
+     *   contain an array of processor class names under the key 'processors'.
+     *
+     * @see self::setProcessors()
+     */
+    protected function addProcessors(array $config): void
+    {
+        // If we have config, and we don't have custom classes already, do the
+        // easy thing.
+        if (($config['processors'] ?? false) && empty($this->customProcessorClasses)) {
+            $this->setProcessors($config);
             return;
         }
+        if ($config_processors = $config['processors'] ?? false) {
+            $this->processor = null;
+            // Unset the configured processors from customProcessorClasses.
+            $this->unsetDuplicateCustomProcessorClasses($config_processors);
+            // Merge in the configuration.
+            $this->customProcessorClasses = array_merge(
+                $config_processors,
+                $this->customProcessorClasses
+            );
+        }
+    }
 
-        $this->customProcessorClasses = $config['processors'];
+    /**
+     * Unset items in $this->customProcessorClasses present in the given array.
+     *
+     * @param array $processors
+     *   Processor class names to be removed from the list of custom processor
+     *   classes.
+     */
+    private function unsetDuplicateCustomProcessorClasses(array $processors):void
+    {
+        foreach ($processors as $processor) {
+            // Use array_keys() with its search parameter.
+            foreach (array_keys($this->customProcessorClasses, $processor) as $existing) {
+                unset($this->customProcessorClasses[$existing]);
+            }
+        }
     }
 
-    private function getCustomProcessorInstance($processorClass)
+    /**
+     * Get an instance of the given custom processor class.
+     *
+     * @param $processorClass
+     *   Processor class name.
+     *
+     * @return \FileFetcher\Processor\ProcessorInterface|null
+     *   An instance of the processor class. If the given class name does not
+     *   exist, or does not implement ProcessorInterface, then null is
+     *   returned.
+     */
+    private function getCustomProcessorInstance($processorClass): ?ProcessorInterface
     {
         if (!class_exists($processorClass)) {
-            return;
+            return null;
         }
 
         $classes = class_implements($processorClass);
         if (!in_array(ProcessorInterface::class, $classes)) {
-            return;
+            return null;
         }
 
         return new $processorClass();
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function serializeIgnoreProperties(): array
+    {
+        // Tell our serializer to ignore processor information.
+        return array_merge(
+            parent::serializeIgnoreProperties(),
+            ['processor']
+        );
+    }
 }

src/Processor/Local.php

@@ -8,7 +8,7 @@ class Local extends ProcessorBase implements ProcessorInterface
 {
     public function isServerCompatible(array $state): bool
     {
-        $path = $state['source'];
+        $path = $state['source'] ?? '';
 
         try {
             if (file_exists($path) && !is_dir($path)) {

src/Processor/Remote.php

@@ -13,7 +13,8 @@ class Remote extends ProcessorBase implements ProcessorInterface
 
     public function isServerCompatible(array $state): bool
     {
-        return preg_match(self::HTTP_URL_REGEX, $state['source']) === 1;
+        $source = $state['source'] ?? '';
+        return preg_match(self::HTTP_URL_REGEX, $source) === 1;
     }
 
     public function setupState(array $state): array