Simplify filefetcher, remove chunking and use guzzle (#26)

GetDKAN · Feb 17, 2023 · 3b92fd5 · 3b92fd5
1 parent 938b79e
commit 3b92fd5
Show file tree

Hide file tree

Showing 17 changed files with 153 additions and 457 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,7 @@
 /vendor/
 composer.lock
 .idea
+.vscode
+.phpunit*
+coverage
 .phpunit.result.cache
diff --git a/composer.json b/composer.json
@@ -5,7 +5,9 @@
     "minimum-stability": "dev",
     "require-dev": {
         "phpunit/phpunit": "^9.0",
-        "getdkan/mock-chain": "dev-master"
+        "getdkan/mock-chain": "dev-master",
+        "squizlabs/php_codesniffer": "@stable",
+        "mikey179/vfsstream": "^2.0@dev"
     },
     "authors": [
         {
@@ -21,6 +23,13 @@
     },
     "require": {
         "getdkan/procrastinator": "^4.0.0",
-        "ext-curl": "*"
+        "ext-curl": "*",
+        "guzzlehttp/guzzle": "^6.3"
+    },
+    "scripts": {
+        "phpunit": "./vendor/bin/phpunit",
+        "phpunit-coverage": "./vendor/bin/phpunit --coverage-html ./coverage",
+        "phpcs": "./vendor/bin/phpcs -ps",
+        "phpcbf": "./vendor/bin/phpcbf -p"
     }
 }
diff --git a/phpcs.xml b/phpcs.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="getdkan-file-fetcher">
+    <arg name="extensions" value="inc,install,module,php,profile,test,theme,yml"/>
+    <description>PHP CodeSniffer configuration for File Fetcher</description>
+
+    <exclude-pattern>.circleci/</exclude-pattern>
+    <exclude-pattern>.ddev/</exclude-pattern>
+    <exclude-pattern>vendor/</exclude-pattern>
+
+    <file>.</file>
+
+    <rule ref="PSR1"/>
+    <rule ref="PSR2"/>
+</ruleset>
diff --git a/src/FileFetcher.php b/src/FileFetcher.php
@@ -5,7 +5,6 @@
 use FileFetcher\Processor\Local;
 use FileFetcher\Processor\ProcessorInterface;
 use FileFetcher\Processor\Remote;
-use FileFetcher\Processor\LastResort;
 use Procrastinator\Job\AbstractPersistentJob;
 
 /**
@@ -88,7 +87,6 @@ private static function getDefaultProcessors()
         $processors = [];
         $processors[Local::class] = new Local();
         $processors[Remote::class] = new Remote();
-        $processors[LastResort::class] = new LastResort();
         return $processors;
     }
 

diff --git a/src/LastResortException.php b/src/LastResortException.php
diff --git a/src/PhpFunctionsBridge.php b/src/PhpFunctionsBridge.php
diff --git a/src/PhpFunctionsBridgeTrait.php b/src/PhpFunctionsBridgeTrait.php
diff --git a/src/Processor/AbstractChunkedProcessor.php b/src/Processor/AbstractChunkedProcessor.php
@@ -2,15 +2,18 @@
 
 namespace FileFetcher\Processor;
 
-use FileFetcher\PhpFunctionsBridgeTrait;
-use FileFetcher\TemporaryFilePathFromUrl;
 use Procrastinator\Result;
 
-abstract class AbstractChunkedProcessor implements ProcessorInterface
+/**
+ * Base class for chunk-by-chunk file fetchers.
+ *
+ * @deprecated This is no longer used for included processors, but is included
+ * to maintain backwards compatibiliy with custom processors.
+ *
+ * @codeCoverageIgnore
+ */
+abstract class AbstractChunkedProcessor extends ProcessorBase implements ProcessorInterface
 {
-    use PhpFunctionsBridgeTrait;
-    use TemporaryFilePathFromUrl;
-
     abstract public function isServerCompatible(array $state): bool;
     abstract protected function getFileSize(string $filePath): int;
     abstract protected function getChunk(string $filePath, int $start, int $end);
@@ -19,7 +22,6 @@ abstract protected function getChunk(string $filePath, int $start, int $end);
      */
     public function __construct()
     {
-        $this->initializePhpFunctionsBridge();
     }
 
 

diff --git a/src/Processor/LastResort.php b/src/Processor/LastResort.php
@@ -2,144 +2,13 @@
 
 namespace FileFetcher\Processor;
 
-use FileFetcher\LastResortException;
-use FileFetcher\PhpFunctionsBridgeTrait;
-use FileFetcher\TemporaryFilePathFromUrl;
-use Procrastinator\Result;
-
 /**
- * Class LastResort
- *
- * The "last resort" processor does a regular copy of a file if non of the safer options were possible. This
- * processor will attempt at getting all of the data in one shot and placing it in a file.
+ * @deprecated No longer needed, but class name maintained for backwards
+ * compatibility.
  *
- * @package FileFetcher\Processor
+ * @codeCoverageIgnore
  */
-class LastResort implements ProcessorInterface
+class LastResort extends Local
 {
-    use TemporaryFilePathFromUrl;
-    use PhpFunctionsBridgeTrait;
-
-    /**
-     * LastResort constructor.
-     */
-    public function __construct()
-    {
-        $this->initializePhpFunctionsBridge();
-    }
-
-    public function isServerCompatible(array $state): bool
-    {
-        return true;
-    }
-
-    public function setupState(array $state): array
-    {
-        $state['total_bytes'] = PHP_INT_MAX;
-        $state['temporary'] = true;
-        $state['destination'] = $this->getTemporaryFilePath($state);
-
-        return $state;
-    }
-
-    public function isTimeLimitIncompatible(): bool
-    {
-        return true;
-    }
-
-    public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX): array
-    {
-        list($from, $to) = $this->validateAndGetInfoFromState($state);
-
-        $bytesToRead = 10 * 1000 * 1000;
-        $bytesCopied = 0;
-
-        $fin = $this->ensureExistsForReading($from);
-        $fout = $this->ensureCreatingForWriting($to);
-
-        while (!feof($fin)) {
-            $bytesCopied += $this->readAndWrite(
-                $fin,
-                $fout,
-                $bytesToRead,
-                $state
-            );
-        }
-
-        $result->setStatus(Result::DONE);
-        fclose($fin);
-        fclose($fout);
-        $state['total_bytes_copied'] = $bytesCopied;
-        $state['total_bytes'] = $bytesCopied;
-
-        return ['state' => $state, 'result' => $result];
-    }
-
-    private function readAndWrite($fin, $fout, $bytesToRead, $state): int
-    {
-        list($from, $to) = $this->validateAndGetInfoFromState($state);
-
-        $bytesRead = $this->php->fread($fin, $bytesToRead);
-        if ($bytesRead === false) {
-            throw new LastResortException("reading from", $from);
-        }
-        $bytesWritten = fwrite($fout, $bytesRead);
-        if ($bytesWritten === false) {
-            throw new LastResortException("writing to", $to);
-        }
-        return $bytesWritten;
-    }
-
-    private function validateAndGetInfoFromState($state)
-    {
-        if (!isset($state['source']) && !isset($state['destination'])) {
-            throw new \Exception("Incorrect state missing source, destination, or both.");
-        }
-        return [$state['source'], $state['destination']];
-    }
-
-    /**
-     * Ensure the target file can be read from.
-     *
-     * @param string $from
-     *   The target filename.
-     *
-     * @return false|resource
-     * @throws \FileFetcher\LastResortException
-     */
-    private function ensureExistsForReading(string $from)
-    {
-        $fin = @$this->php->fopen($from, "rb");
-        if ($fin === false) {
-            throw new LastResortException("opening", $from);
-        }
-        return $fin;
-    }
-
-    /**
-     * Ensure the destination file can be created.
-     *
-     * @param string $to
-     *   The destination filename.
-     *
-     * @return false|resource
-     * @throws \FileFetcher\LastResortException
-     */
-    private function ensureCreatingForWriting(string $to)
-    {
-        // Delete destination first to avoid appending if existing.
-        $this->deleteFile($to);
-        $fout = $this->php->fopen($to, "w");
-        if ($fout === false) {
-            throw new LastResortException("creating", $to);
-        }
-        return $fout;
-    }
 
-    private function deleteFile($file)
-    {
-        if (file_exists($file)) {
-            unlink($file);
-        }
-    }
 }
diff --git a/src/Processor/Local.php b/src/Processor/Local.php
@@ -2,29 +2,15 @@
 
 namespace FileFetcher\Processor;
 
-use FileFetcher\PhpFunctionsBridgeTrait;
-use FileFetcher\TemporaryFilePathFromUrl;
 use Procrastinator\Result;
 
-class Local implements ProcessorInterface
+class Local extends ProcessorBase implements ProcessorInterface
 {
-
-    use PhpFunctionsBridgeTrait;
-    use TemporaryFilePathFromUrl;
-
-    /**
-     * Local constructor.
-     */
-    public function __construct()
-    {
-        $this->initializePhpFunctionsBridge();
-    }
-
     public function isServerCompatible(array $state): bool
     {
         $path = $state['source'];
 
-        if ($this->php->file_exists($path) && !$this->php->is_dir($path)) {
+        if (file_exists($path) && !is_dir($path)) {
             return true;
         }
 
@@ -34,7 +20,7 @@ public function isServerCompatible(array $state): bool
     public function setupState(array $state): array
     {
         $state['total_bytes'] = PHP_INT_MAX;
-        $state['total_bytes'] = $this->php->filesize($state['source']);
+        $state['total_bytes'] = filesize($state['source']);
         $state['temporary'] = true;
         $state['destination'] = $this->getTemporaryFilePath($state);
 
@@ -48,9 +34,9 @@ public function isTimeLimitIncompatible(): bool
 
     public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX): array
     {
-        $this->php->copy($state['source'], $state['destination']);
-        $state['total_bytes_copied'] = $this->php->filesize($state['destination']);
+        copy($state['source'], $state['destination']);
         $result->setStatus(Result::DONE);
+        $state['total_bytes_copied'] = $state['total_bytes'] = filesize($state['destination']);
 
         return ['state' => $state, 'result' => $result];
     }