NEW Allow file variants with different extensions

src/FilenameParsing/AbstractFileIDHelper.php

@@ -2,12 +2,28 @@
 
 namespace SilverStripe\Assets\FilenameParsing;
 
+use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injectable;
 
 abstract class AbstractFileIDHelper implements FileIDHelper
 {
     use Injectable;
 
+    /**
+     * A variant type for encoding a variant filename with a different extension than the original.
+     */
+    public const EXTENSION_REWRITE_VARIANT = 'extRewrite';
+
+    /**
+     * use the original file's extension
+     */
+    private const EXT_ORIGINAL = 0;
+
+    /**
+     * use the variant file's extension
+     */
+    private const EXT_VARIANT = 1;
+
     public function buildFileID($filename, $hash = null, $variant = null, $cleanfilename = true)
     {
         if ($filename instanceof ParsedFileID) {
@@ -23,6 +39,10 @@ public function buildFileID($filename, $hash = null, $variant = null, $cleanfile
             $filename = $this->cleanFilename($filename);
         }
 
+        if ($variant) {
+            $filename = $this->rewriteVariantExtension($filename, $variant);
+        }
+
         $name = basename($filename ?? '');
 
         // Split extension
@@ -53,6 +73,66 @@ public function buildFileID($filename, $hash = null, $variant = null, $cleanfile
         return $fileID;
     }
 
+    /**
+     * Get the original filename with the extension rewritten to be the same as the variant file extension.
+     *
+     * @param string $filename Original filename without variant
+     */
+    public function rewriteVariantExtension(string $filename, string $variant): string
+    {
+        return $this->swapExtension($filename, $variant, self::EXT_VARIANT);
+    }
+
+    /**
+     * Get the variant filename with the extension rewritten to be the same as the original file extension.
+     *
+     * @param string $filename Original filename without variant
+     */
+    public function restoreOriginalExtension(string $filename, string $variant): string
+    {
+        return $this->swapExtension($filename, $variant, self::EXT_ORIGINAL);
+    }
+
+    /**
+     * Get the original file's filename with the extension rewritten to be the same as either the original
+     * or the variant extension.
+     *
+     * @param string $filename Original filename without variant
+     * @param int $extIndex One of self::EXT_ORIGINAL or self::EXT_VARIANT
+     */
+    private function swapExtension(string $filename, string $variant, int $extIndex): string
+    {
+        // If there's no variant at all, we can rewrite the filenmane
+        if (empty($variant)) {
+            return $filename;
+        }
+
+        // Split variant string in variant list
+        $subVariants = explode('_', $variant);
+
+        // Split our filename into a filename and extension part
+        if (!preg_match('/(?<basename>.+)\.(?<ext>[a-z\d]+)$/i', $filename, $matches)) {
+            return $filename;
+        }
+        $filenameWithoutExtension = $matches['basename'];
+        $extension = $matches['ext'];
+
+        // Loop our variant list until we find our special file extension swap variant
+        // Reverse the list first so the variant extension we find is the last extension rewrite variant in a chain
+        $extSwapVariant = preg_quote(self::EXTENSION_REWRITE_VARIANT, '/');
+        foreach (array_reverse($subVariants) as $subVariant) {
+            if (preg_match("/^$extSwapVariant(?<base64>.+)$/", $subVariant, $matches)) {
+                // This array always contain 2 values: The original extension at index 0 and the variant extension at index 1
+                /** @var array $extensionData */
+                $extensionData = Convert::base64url_decode($matches['base64']);
+                $extension = $extensionData[$extIndex];
+                break;
+            }
+        }
+
+        return $filenameWithoutExtension . '.' . $extension;
+    }
+
     public function cleanFilename($filename)
     {
         // Swap backslash for forward slash

src/FilenameParsing/FileIDHelper.php

@@ -7,7 +7,6 @@
  */
 interface FileIDHelper
 {
-
     /**
      * Map file tuple (hash, name, variant) to a filename to be used by flysystem
      *

src/FilenameParsing/HashFileIDHelper.php

@@ -30,10 +30,16 @@ public function parseFileID($fileID)
         }
 
         $filename = $matches['folder'] . $matches['basename'] . $matches['extension'];
+        $variant = isset($matches['variant']) ? $matches['variant'] : '';
+
+        if (isset($variant)) {
+            $filename = $this->restoreOriginalExtension($filename, $variant);
+        }
+
         return new ParsedFileID(
             $filename,
             $matches['hash'],
-            isset($matches['variant']) ? $matches['variant'] : '',
+            $variant,
             $fileID
         );
     }

src/FilenameParsing/NaturalFileIDHelper.php

@@ -2,8 +2,6 @@
 
 namespace SilverStripe\Assets\FilenameParsing;
 
-use SilverStripe\Core\Injector\Injectable;
-
 /**
  * Parsed Natural path URLs. Natural path is the same hashless path that appears in the CMS.
  *
@@ -23,10 +21,16 @@ public function parseFileID($fileID)
         }
 
         $filename = $matches['folder'] . $matches['basename'] . $matches['extension'];
+        $variant = isset($matches['variant']) ? $matches['variant'] : '';
+
+        if (isset($variant)) {
+            $filename = $this->restoreOriginalExtension($filename, $variant);
+        }
+
         return new ParsedFileID(
             $filename,
             '',
-            isset($matches['variant']) ? $matches['variant'] : '',
+            $variant,
             $fileID
         );
     }

src/ImageBackendFactory.php

@@ -37,9 +37,15 @@ public function __construct(Factory $creator)
      */
     public function create($service, array $params = [])
     {
-        /** @var AssetContainer $assetContainer */
+        /** @var AssetContainer|null $assetContainer */
         $assetContainer = reset($params);
-        if (!$assetContainer instanceof AssetContainer) {
+
+        // If no asset container was passed in, create a new uncached image backend
+        if (!$assetContainer) {
+            return $this->creator->create($service, $params);
+        }
+
+        if (!($assetContainer instanceof AssetContainer)) {
             throw new BadMethodCallException("Can only create Image_Backend for " . AssetContainer::class);
         }
 

src/ImageManipulation.php

@@ -4,6 +4,7 @@
 
 use InvalidArgumentException;
 use LogicException;
+use SilverStripe\Assets\FilenameParsing\AbstractFileIDHelper;
 use SilverStripe\Assets\Storage\AssetContainer;
 use SilverStripe\Assets\Storage\AssetStore;
 use SilverStripe\Assets\Storage\DBFile;
@@ -840,6 +841,18 @@ public function isHeight($height)
         return $this->getHeight() === $height;
     }
 
+    /**
+     * Wrapper for manipulate() that creates a variant file with a different extension than the original file.
+     *
+     * @return DBFile The manipulated file
+     */
+    public function manipulateExtension(string $newExtension, callable $callback)
+    {
+        $pathParts = pathinfo($this->getFilename());
+        $variant = $this->variantName(AbstractFileIDHelper::EXTENSION_REWRITE_VARIANT, $pathParts['extension'], $newExtension);
+        return $this->manipulate($variant, $callback);
+    }
+
     /**
      * Wrapper for manipulate that passes in and stores Image_Backend objects instead of tuples
      *
@@ -999,6 +1012,7 @@ public function variantName($format, $arg = null)
      * For legacy reasons, there's no delimiter between this part, and the encoded arguments.
      * This means we have to use a whitelist of "known formats", based on methods
      * available on the {@link Image} class as the "main" user of this trait.
+     * The one exception to this is the variant for swapping file extensions, which is explicitly allowed.
      * This class is commonly decorated with additional manipulation methods through {@link DataExtension}.
      *
      * @param $variantName
@@ -1007,10 +1021,11 @@ public function variantName($format, $arg = null)
      */
     public function variantParts($variantName)
     {
-        $methods = array_map('preg_quote', singleton(Image::class)->allMethodNames() ?? []);
+        $allowedVariantTypes = array_map('preg_quote', singleton(Image::class)->allMethodNames() ?? []);
+        $allowedVariantTypes[] = preg_quote(AbstractFileIDHelper::EXTENSION_REWRITE_VARIANT);
 
         // Regex needs to be case insensitive since allMethodNames() is all lowercased
-        $regex = '#^(?<format>(' . implode('|', $methods) . '))(?<encodedargs>(.*))#i';
+        $regex = '#^(?<format>(' . implode('|', $allowedVariantTypes) . '))(?<encodedargs>(.*))#i';
         preg_match($regex ?? '', $variantName ?? '', $matches);
 
         if (!$matches) {

src/InterventionBackend.php

@@ -366,8 +366,10 @@ public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $v
                 throw new BadMethodCallException("Cannot write corrupt file to store");
             }
 
+            // Make sure we're using the extension of the variant file, which can differ from the original file
+            $url = $assetStore->getAsURL($filename, $hash, $variant, false);
+            $extension = pathinfo($url, PATHINFO_EXTENSION);
             // Save file
-            $extension = pathinfo($filename ?? '', PATHINFO_EXTENSION);
             $result = $assetStore->setFromString(
                 $resource->encode($extension, $this->getQuality())->getEncoded(),
                 $filename,

tests/php/FilenameParsing/HashFileIDHelperTest.php

@@ -4,6 +4,7 @@
 use InvalidArgumentException;
 use SilverStripe\Assets\FilenameParsing\HashFileIDHelper;
 use SilverStripe\Assets\FilenameParsing\ParsedFileID;
+use SilverStripe\Core\Convert;
 
 class HashFileIDHelperTest extends FileIDHelperTester
 {
@@ -173,4 +174,42 @@ public function testHashlessBuildFileID()
         $this->expectException(\InvalidArgumentException::class);
         $this->getHelper()->buildFileID('Filename.txt', '');
     }
+
+    public function provideRewriteExtension()
+    {
+        $jpgToPng = 'extRewrite' . Convert::base64url_encode(['jpg', 'png']);
+
+        return [
+            'no variant' => ['', 'hello.txt', 'hello.txt'],
+            'invalid extension' => ['xyz', 'hello.abc+', 'hello.abc+'],
+            'no extension' => ['', 'hello.', 'hello.'],
+            'no filename' => ['', '.htaccess', '.htaccess'],
+            'no rewrite' => ['xyz', 'hello.jpg', 'hello.jpg'],
+            'no rewrite multi variant' => ['xyz_abc', 'hello.jpg', 'hello.jpg'],
+            'rewitten extension' => [$jpgToPng, 'hello.jpg', 'hello.png'],
+            'rewitten extension with other variants' => ["{$jpgToPng}_xyz", 'hello.jpg', 'hello.png'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideRewriteExtension
+     */
+    public function testRewriteVariantExtension($variant, $inFilename, $outFilename)
+    {
+        $helper = new HashFileIDHelper();
+        $actualFilename = $helper->rewriteVariantExtension($inFilename, $variant);
+
+        $this->assertEquals($outFilename, $actualFilename);
+    }
+
+    /**
+     * @dataProvider provideRewriteExtension
+     */
+    public function testRestoreOriginalExtension($variant, $outFilename, $inFilename)
+    {
+        $helper = new HashFileIDHelper();
+        $actualFilename = $helper->restoreOriginalExtension($inFilename, $variant);
+
+        $this->assertEquals($outFilename, $actualFilename);
+    }
 }

tests/php/ImageManipulationTest.php

@@ -5,8 +5,10 @@
 use Prophecy\Prophecy\ObjectProphecy;
 use Silverstripe\Assets\Dev\TestAssetStore;
 use SilverStripe\Assets\File;
+use SilverStripe\Assets\FilenameParsing\AbstractFileIDHelper;
 use SilverStripe\Assets\Folder;
 use SilverStripe\Assets\Image;
+use SilverStripe\Assets\Image_Backend;
 use SilverStripe\Assets\InterventionBackend;
 use SilverStripe\Assets\Storage\AssetStore;
 use SilverStripe\Assets\Storage\DBFile;
@@ -470,4 +472,88 @@ public function testThumbnailURL()
             'Current user should not automatically be granted access to view thumbnail'
         );
     }
+
+    public function testManipulateExtension()
+    {
+        $image = $this->objFromFixture(Image::class, 'imageWithTitle');
+        $manipulated = $image->manipulateExtension(
+            'webp',
+            function (AssetStore $store, string $filename, string $hash, string $variant) use ($image) {
+                $backend = $image->getImageBackend();
+                $tuple = $backend->writeToStore(
+                    $store,
+                    $filename,
+                    $hash,
+                    $variant,
+                    ['conflict' => AssetStore::CONFLICT_USE_EXISTING]
+                );
+                return [$tuple, $backend];
+            }
+        );
+
+        $store = Injector::inst()->get(AssetStore::class);
+
+        // Having a valid image backend means all the image manipulation methods can be chained on top
+        $this->assertInstanceOf(Image_Backend::class, $manipulated->getImageBackend());
+        // Double check the variant was created and stored correctly
+        $this->assertSame([AbstractFileIDHelper::EXTENSION_REWRITE_VARIANT, 'png', 'webp'], $manipulated->variantParts($manipulated->getVariant()));
+        $this->assertTrue($store->exists($manipulated->getFilename(), $manipulated->getHash(), $manipulated->getVariant()));
+    }
+
+    public function testManipulateExtensionNonImageToImage()
+    {
+        $original = $this->objFromFixture(File::class, 'notImage');
+        $manipulated = $original->manipulateExtension(
+            'png',
+            function (AssetStore $store, string $filename, string $hash, string $variant) {
+                $backend = Injector::inst()->create(Image_Backend::class);
+                // In lieu of actually generating a screenshot of the txt file and making an image from it,
+                // we'll just load an image from the filesystem.
+                $backend->loadFrom(__DIR__ . '/ImageTest/test-image.png');
+                $tuple = $backend->writeToStore(
+                    $store,
+                    $filename,
+                    $hash,
+                    $variant,
+                    ['conflict' => AssetStore::CONFLICT_USE_EXISTING]
+                );
+                return [$tuple, $backend];
+            }
+        );
+
+        $store = Injector::inst()->get(AssetStore::class);
+
+        // Having a valid image backend means all the image manipulation methods can be chained on top
+        $this->assertInstanceOf(Image_Backend::class, $manipulated->getImageBackend());
+        // Double check the variant was created and stored correctly
+        $this->assertSame([AbstractFileIDHelper::EXTENSION_REWRITE_VARIANT, 'txt', 'png'], $manipulated->variantParts($manipulated->getVariant()));
+        $this->assertTrue($store->exists($manipulated->getFilename(), $manipulated->getHash(), $manipulated->getVariant()));
+    }
+
+    public function testManipulateExtensionNonImageToNonImage()
+    {
+        $original = $this->objFromFixture(File::class, 'notImage');
+        $manipulated = $original->manipulateExtension(
+            'csv',
+            function (AssetStore $store, string $filename, string $hash, string $variant) {
+                $tuple = $store->setFromString(
+                    'Any content will do - csv is just a text file afterall',
+                    $filename,
+                    $hash,
+                    $variant,
+                    ['conflict' => AssetStore::CONFLICT_USE_EXISTING]
+                );
+                return [$tuple, null];
+            }
+        );
+
+        $store = Injector::inst()->get(AssetStore::class);
+
+        // Backend should be null since the resulting variant isn't an image
+        $this->assertNull($manipulated->getImageBackend());
+        // Double check the variant was created and stored correctly
+        $this->assertSame([AbstractFileIDHelper::EXTENSION_REWRITE_VARIANT, 'txt', 'csv'], $manipulated->variantParts($manipulated->getVariant()));
+        $this->assertTrue($store->exists($manipulated->getFilename(), $manipulated->getHash(), $manipulated->getVariant()));
+        $this->assertSame('Any content will do - csv is just a text file afterall', $manipulated->getString());
+    }
 }