ENH Refactor FileIDHelper classes to reduce code duplication (#587)

silverstripe · Jan 25, 2024 · 3326749 · 3326749
1 parent 2ed92f8
commit 3326749
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 121 deletions.
diff --git a/src/FilenameParsing/AbstractFileIDHelper.php b/src/FilenameParsing/AbstractFileIDHelper.php
@@ -0,0 +1,73 @@
+<?php
+
+namespace SilverStripe\Assets\FilenameParsing;
+
+use SilverStripe\Core\Injector\Injectable;
+
+abstract class AbstractFileIDHelper implements FileIDHelper
+{
+    use Injectable;
+
+    public function buildFileID($filename, $hash = null, $variant = null, $cleanfilename = true)
+    {
+        if ($filename instanceof ParsedFileID) {
+            $hash =  $filename->getHash();
+            $variant =  $filename->getVariant();
+            $filename =  $filename->getFilename();
+        }
+
+        $this->validateFileParts($filename, $hash, $variant);
+
+        // Since we use double underscore to delimit variants, eradicate them from filename
+        if ($cleanfilename) {
+            $filename = $this->cleanFilename($filename);
+        }
+
+        $name = basename($filename ?? '');
+
+        // Split extension
+        $extension = null;
+        if (($pos = strpos($name ?? '', '.')) !== false) {
+            $extension = substr($name ?? '', $pos ?? 0);
+            $name = substr($name ?? '', 0, $pos);
+        }
+
+        $fileID = $this->getFileIDBase($name, $filename, $hash, $variant);
+
+        // Add directory
+        $dirname = ltrim(dirname($filename ?? ''), '.');
+        if ($dirname) {
+            $fileID = $dirname . '/' . $fileID;
+        }
+
+        // Add variant
+        if ($variant) {
+            $fileID .= '__' . $variant;
+        }
+
+        // Add extension
+        if ($extension) {
+            $fileID .= $extension;
+        }
+
+        return $fileID;
+    }
+
+    public function cleanFilename($filename)
+    {
+        // Swap backslash for forward slash
+        $filename = str_replace('\\', '/', $filename ?? '');
+
+        // Since we use double underscore to delimit variants, eradicate them from filename
+        return preg_replace('/_{2,}/', '_', $filename ?? '');
+    }
+
+    public function lookForVariantRecursive(): bool
+    {
+        return false;
+    }
+
+    abstract protected function getFileIDBase($shortFilename, $fullFilename, $hash, $variant): string;
+
+    abstract protected function validateFileParts($filename, $hash, $variant): void;
+}
diff --git a/src/FilenameParsing/HashFileIDHelper.php b/src/FilenameParsing/HashFileIDHelper.php
@@ -3,7 +3,6 @@
 namespace SilverStripe\Assets\FilenameParsing;
 
 use InvalidArgumentException;
-use SilverStripe\Core\Injector\Injectable;
 
 /**
  * Parsed Hash path URLs. Hash paths group a file and its variant under a directory based on a hash generated from the
@@ -14,70 +13,13 @@
  *
  * e.g.: `Uploads/a1312bc34d/sam__ResizedImageWzYwLDgwXQ.jpg`
  */
-class HashFileIDHelper implements FileIDHelper
+class HashFileIDHelper extends AbstractFileIDHelper
 {
-    use Injectable;
-
     /**
      * Default length at which hashes are truncated.
      */
     const HASH_TRUNCATE_LENGTH = 10;
 
-    public function buildFileID($filename, $hash = null, $variant = null, $cleanfilename = true)
-    {
-        if ($filename instanceof ParsedFileID) {
-            $hash =  $filename->getHash();
-            $variant =  $filename->getVariant();
-            $filename =  $filename->getFilename();
-        }
-
-        if (empty($hash)) {
-            throw new InvalidArgumentException('HashFileIDHelper::buildFileID requires an $hash value.');
-        }
-
-        // Since we use double underscore to delimit variants, eradicate them from filename
-        if ($cleanfilename) {
-            $filename = $this->cleanFilename($filename);
-        }
-        $name = basename($filename ?? '');
-
-        // Split extension
-        $extension = null;
-        if (($pos = strpos($name ?? '', '.')) !== false) {
-            $extension = substr($name ?? '', $pos ?? 0);
-            $name = substr($name ?? '', 0, $pos);
-        }
-
-        $fileID = $this->truncate($hash) . '/' . $name;
-
-        // Add directory
-        $dirname = ltrim(dirname($filename ?? ''), '.');
-        if ($dirname) {
-            $fileID = $dirname . '/' . $fileID;
-        }
-
-        // Add variant
-        if ($variant) {
-            $fileID .= '__' . $variant;
-        }
-
-        // Add extension
-        if ($extension) {
-            $fileID .= $extension;
-        }
-
-        return $fileID;
-    }
-
-    public function cleanFilename($filename)
-    {
-        // Swap backslash for forward slash
-        $filename = str_replace('\\', '/', $filename ?? '');
-
-        // Since we use double underscore to delimit variants, eradicate them from filename
-        return preg_replace('/_{2,}/', '_', $filename ?? '');
-    }
-
     public function parseFileID($fileID)
     {
         $pattern = '#^(?<folder>([^/]+/)*)(?<hash>[a-f0-9]{10})/(?<basename>((?<!__)[^/.])+)(__(?<variant>[^.]+))?(?<extension>(\..+)*)$#';
@@ -115,6 +57,18 @@ public function lookForVariantIn(ParsedFileID $parsedFileID)
         return  $folder . $this->truncate($parsedFileID->getHash());
     }
 
+    protected function validateFileParts($filename, $hash, $variant): void
+    {
+        if (empty($hash)) {
+            throw new InvalidArgumentException('HashFileIDHelper::buildFileID requires an $hash value.');
+        }
+    }
+
+    protected function getFileIDBase($shortFilename, $fullFilename, $hash, $variant): string
+    {
+        return $this->truncate($hash) . '/' . $shortFilename;
+    }
+
     /**
      * Truncate a hash to a predefined length
      * @param $hash
@@ -124,9 +78,4 @@ private function truncate($hash)
     {
         return substr($hash ?? '', 0, self::HASH_TRUNCATE_LENGTH);
     }
-
-    public function lookForVariantRecursive(): bool
-    {
-        return false;
-    }
 }
diff --git a/src/FilenameParsing/NaturalFileIDHelper.php b/src/FilenameParsing/NaturalFileIDHelper.php
@@ -11,62 +11,8 @@
  *
  * e.g.: `Uploads/sam__ResizedImageWzYwLDgwXQ.jpg`
  */
-class NaturalFileIDHelper implements FileIDHelper
+class NaturalFileIDHelper extends AbstractFileIDHelper
 {
-    use Injectable;
-
-    public function buildFileID($filename, $hash = null, $variant = null, $cleanfilename = true)
-    {
-        if ($filename instanceof ParsedFileID) {
-            $hash =  $filename->getHash();
-            $variant =  $filename->getVariant();
-            $filename =  $filename->getFilename();
-        }
-
-        // Since we use double underscore to delimit variants, eradicate them from filename
-        if ($cleanfilename) {
-            $filename = $this->cleanFilename($filename);
-        }
-        $name = basename($filename ?? '');
-
-        // Split extension
-        $extension = null;
-        if (($pos = strpos($name ?? '', '.')) !== false) {
-            $extension = substr($name ?? '', $pos ?? 0);
-            $name = substr($name ?? '', 0, $pos);
-        }
-
-        $fileID = $name;
-
-        // Add directory
-        $dirname = ltrim(dirname($filename ?? ''), '.');
-        if ($dirname) {
-            $fileID = $dirname . '/' . $fileID;
-        }
-
-        // Add variant
-        if ($variant) {
-            $fileID .= '__' . $variant;
-        }
-
-        // Add extension
-        if ($extension) {
-            $fileID .= $extension;
-        }
-
-        return $fileID;
-    }
-
-
-    public function cleanFilename($filename)
-    {
-        // Swap backslash for forward slash
-        $filename = str_replace('\\', '/', $filename ?? '');
-
-        // Since we use double underscore to delimit variants, eradicate them from filename
-        return preg_replace('/_{2,}/', '_', $filename ?? '');
-    }
-
     public function parseFileID($fileID)
     {
         $pattern = '#^(?<folder>([^/]+/)*)(?<basename>((?<!__)[^/.])+)(__(?<variant>[^.]+))?(?<extension>(\..+)*)$#';
@@ -97,8 +43,13 @@ public function lookForVariantIn(ParsedFileID $parsedFileID)
         return $folder == '.' ? '' : $folder;
     }
 
-    public function lookForVariantRecursive(): bool
+    protected function getFileIDBase($shortFilename, $fullFilename, $hash, $variant): string
+    {
+        return $shortFilename;
+    }
+
+    protected function validateFileParts($filename, $hash, $variant): void
     {
-        return false;
+        // no-op
     }
 }