Allow file variant with different extensions POC #411

maxime-rainville · 2020-07-19T10:20:18Z

This is just a proof-of-concept for now. Matching RFC #441

Background

Our asset system allows you to define "variants" of a file. Variants are are variation of the parent file. e.g.: A thumbnail of an image, or a cropped version of the original image.

This is quite a useful feature and allows any operation on the main file to be reflected on its variants. e.g: Deleting, publishing, unpublishing a file will do the same thing to all its variant. If you are granted access to a file, you automatically get access to its variants.

Currently variants are tracked by creating file with a variation on the name of the original file. e.g.: If you have an image called "Upload/logo.png", all it's variant will be called something like "Upload/logo__someVariantstring.png"

Problem

Currently, variants must have the same extensions as the parent file. This foreclose a lot of interesting scenarios and in-effect means that variants are only useful for resizing/cropping images.

If we could have variants with extension that differ from their parents we could open this kind of possibilities:

provide thumbnails for more file types (e.g.: We could take thumbnails for videos or documents)
convert files to alternative formats (e.g.: .docx document could be converted to PDF on the fly or we could convert a PNG to JPG)

We probably wouldn't want to provide all these features out of the box in core, but we could provide a simple API that third parties could hook into to provide conversion features.

Approach

This PR builds on some of our previous work with FileIDHelper concept. FileIDHelpers are responsible for:

converting file tuple (an array with a file name, file hash, variant) to their equivalent File ID (path as a string),
parsing File IDs back to their original file tuple,
matching a file variant back to its original file.

This PR updates FileIDHelper classes so they can understand a special extRewrite variant. This variant as two parameters: the extension of the parent file and the extension that the new variant should have.

When building the fileID from a tuple, the extension will swap to the new extension. When parsing a file ID back to its original tuple, the extension in the filename key is restored to the extension of the parent file.

For example, this tuple would be converted to this file ID:
['Filename' => 'video.mp4', 'Hash' => 'b67fcb469a', 'variant' => 'extRewriteWyJtcDQiLCJqcGciXQ'] becomes b67fcb469a/video__extRewriteWyJtcDQiLCJqcGciXQ.jpg. WyJtcDQiLCJqcGciXQ is a base64 encoded representation of ['mp4','jpg'].

What would the API for this look like

This is based on the existing logic for creating thumbnails and the manipulateImage method. We might be able to provide a nicer API to do this.

public function ToImage()
{
    return $file->manipulateExtension(
        'jpg',
        function (AssetStore $store, string $filename, string $hash, string $variant) use ($file) {
            $results = $file->getImageBackend();
            $results->setImageResource(VideoConverter::magicalMethod($file->getStream()));
            $tuple = $results->writeToStore($store, $filename, $hash, $variant, ['conflict' =>     AssetStore::CONFLICT_USE_EXISTING]);
            return [$tuple, $results];
        }
    );
}

Extra complications

Most of the logic around generating thumbnails in assets and asset-admin has some safe guard to make sure you don't try generating thumbnails for regular files. Generating images for regular files is kind of the entire point of this, so we'll need to rewrite some of those safe guards or remove them altogether.

We might want to have dedicated API for this purpose as well. This will require some more thoughts.

On a UX consideration, the asset-admin GalleryItem currently only has thumbnails for images. If we move into a direction where PDF, Videos and other types of files can have thumbnails as well, we'll need to have a way to differentiate those files from regular images

Related PR

WIP Allow generation of thumbnail for non-image file silverstripe-asset-admin#1133

maxime-rainville · 2020-08-21T12:18:36Z

This video showcases what it can look like https://youtu.be/org5FDYAbL4

clarkepaul · 2020-08-23T21:55:22Z

@maxime-rainville looking great! are there other variations of icons we would require like doc, pdf etc. ? would be good to have a list so we could ensure iconography would still make sense for both thumb and list view.

maxime-rainville · 2020-08-25T22:55:58Z

As far as I know, these are all the file categories for which we have icons/styles:

Video
Audio
Document (e.g.: DOCX, DOC, PDF, XSLX, XSL, CSV)
Any other file
Image
Archives (e.g.: ZIP, TAR.GZ, 7ZIP)

So we don't currently differentiate between a DOC and a PDF file ... or between a PNG vs a JPG.

So if we want to allow any file type to have a thumbnail, then maybe we need have some generic logic that overlays some file type icon over the thumbnail ... if the thumbnail is provided.

sachajudd · 2020-09-25T04:06:25Z

I've created a PDF thumbnail icon to differentiate between a DOC extension icon. This will hopefully help with more clarity about extensions. Note this will need to be exported by myself to a PNG for use in admin as I'm pretty sure this is how they are implemented currently. See File icons (PDF) for design sizes in 32 and 92 (Note: Still working on the 32). @maxime-rainville could you set up a demo where I could take a look at these implemented? Otherwise let me know if you'd like separate issues raised.

I've also had a little think about where we could add an extension type more clearly in the CMS. I was thinking potentially a readonly field with the type but this feels like adding clutter. The original "Filename" does usually include the extension but this can be removed and easy to loose this information. As a quick win I think we could add the extension to the end of the editor specs (Size of image, size of file, extension type) e.g 10x10px, 2MB, JPG. Let me know your thoughts, see Edit file / Details tab for design.

tractorcow · 2021-04-08T00:52:35Z

I wrote a module that lets you transcode images in templates.

https://github.com/tractorcow/silverstripe-image-formatter

It works but it breaks a lot of core filesystem in order to do so;

images are forced to be public
hack to disable hash-comparison for file->exists()

What you propose here @maxime-rainville would be much nicer :)

maxime-rainville · 2021-04-08T01:20:19Z

I've got 4 people who have expressed that this feature would be helpful in the real world:

@DorsetDigital on the forum
@tractorcow on this PR
@scott-nz and @blueo on the office slack (Silverstripe LTD only)

I'll spend some time cleaning up the PR and adding a bit of doc. Then see if we can get a bit more more momentum to get this over the line.

I'm thinking we can probably just get the assets bit merged initially. If this works well and we don't find any major bugs, we can have a look at enabling the asset-admin part.

maxime-rainville · 2021-04-08T08:59:55Z

src/File.php

@@ -1503,4 +1503,10 @@ protected function getFilter()
    {
        return FileNameFilter::create();
    }
+
+
+    public function ToImage(): ?AssetContainer


Strictly speaking, this method is not needed to introduce the ability to create variants with other extension. If we want to minimise the risk, we could ship the PR without it and still ship something useful.

So you could still do something like $file->ToJPG(). However, you couldn't perform image manipulation on the result.

maxime-rainville · 2021-04-08T09:00:19Z

src/Image.php

@@ -60,4 +63,15 @@ public function PreviewLink($action = null)
        $this->extend('updatePreviewLink', $link, $action);
        return $link;
    }
+
+    public function ToImage(): ?AssetContainer


See #411 (comment)

maxime-rainville · 2021-04-08T09:00:35Z

src/ImageBackendFactory.php

@@ -51,7 +51,7 @@ public function create($service, array $params = [])

        // Verify file exists before creating backend
        $backend = null;
-        if ($store->exists() && $store->getIsImage()) {
+        if ($store->exists()) {


See #411 (comment)

maxime-rainville · 2021-04-08T09:00:51Z

src/ImageManipulation.php

@@ -731,7 +731,7 @@ public function getImageBackend()
        }

        // Skip files we know won't be an image
-        if (!$this->getIsImage() || !$this->getHash()) {
+        if (!$this->getHash()) {


See #411 (comment)

maxime-rainville · 2021-04-08T09:01:13Z

src/ImageManipulation.php

                    return null;
                }

+                // If we don't have an image resource


See #411 (comment)

maxime-rainville · 2021-04-08T09:01:54Z

src/Storage/DBFile.php

@@ -628,4 +628,12 @@ public function copyFile($newName)
            ->getStore()
            ->copy($this->Filename, $this->Hash, $newName);
    }
+
+    public function ToImage(): ?AssetContainer


See #411 (comment)

maxime-rainville · 2021-04-08T09:50:09Z

src/FilenameParsing/AlternativeFileExtensionTrait.php

+ *
+ * This allows the creation of file variant with different extension to the original file.
+ */
+trait AlternativeFileExtensionTrait


I'm thinking maybe this should be some sort of service instead. There's not really any value in making that logic snipet part of the FIleIDHelper.

Also, there's bit of logic in ImageManipulate::manipulateExtension() and InterventionBackend that might make more sense in a centralised service.

maxime-rainville · 2021-04-08T09:52:40Z

src/ImageManipulation.php

+        $pathParts = pathinfo($this->getFilename());
+        $variant = $this->variantName(FileIDHelper::EXTENSION_REWRITE_VARIANT, $pathParts['extension'], $newExtension);


Maybe this bit should be in a centralised service

maxime-rainville · 2021-04-08T09:53:28Z

src/InterventionBackend.php

+            $url = $assetStore->getAsURL($filename, $hash, $variant, false);
+            $extension = pathinfo($url, PATHINFO_EXTENSION);


Maybe this bit should be in a centralised service.

maxime-rainville · 2021-04-08T10:14:13Z

Just looking at this again after a while, a couple question pop up:

Maybe AlternativeFileExtensionTrait should be some sort of service instead
Do we want to ship the tweaks to allow Image Manipulation of variants right away? This enable things like convert a PDF to an image and then resize that image.
There's some edge case where you chain up file conversion which I'm not sure how to handle e.g.:
- Maybe you start with a XLS file, then get it converted to a CSV, then get it converted to a TXT file
- Or maybe you have a JPG that you convert to a PNG that you then convert back to a JPG
Right now the way you create a new converters is you write an a new DataExtension for Image/File. That extension has a method that calls manipulateExtension with a callback. Maybe we could be more prescriptive than that: define a FileConverter interface. Then you register your new converter somewhere. Then, Image/File could have a convert method that looks for a suitable converter for your use case.

tractorcow · 2021-04-10T02:29:23Z

question, wouldn't the extRewrite variant only need to include the "from" variant? (since the "to" variant is the actual current extension). This would be simpler in terms of url length :)

The only thing that you need is the ability to generate the "original" file ID from any current tuple.

maxime-rainville · 2021-04-12T05:04:19Z

@tractorcow Possibly ... I remember pondering this question when I started working on this. From memory, it made the logic more difficult to follow because you had to decode the extRewrite variant to know if you were working with the original file name or the the one with the rewritten extension.

I don't recall spending hours trying to get it working, so it's probably just a shortcut I took to try to ship something in one hackday. Probably worth having a second though about it.

kinglozzer · 2022-04-13T09:32:37Z

#486 just reminded me of this PR. This API would definitely be valuable in the context of webp images, because most content authors we’ve mentioned webp to have absolutely no idea the format exists, or how to create images for it. The ability to upload JPEGs (and other formats, but our content authors typically deal with JPEGs) and have the CMS handle conversion for you would be awesome.

nathanbrauer · 2023-09-16T00:02:33Z

I've got 4 people who have expressed that this feature would be helpful in the real world:

@DorsetDigital on the forum

@tractorcow on this PR

@scott-nz and @blueo on the office slack (Silverstripe LTD only)

You can add me and @Marketera to the list. We have several projects/clients that need similar functionality. Especially generating thumbnails for video assets.

Attn: @muskie9

Reference: https://forum.silverstripe.org/t/thumbnails-for-every-file-types-and-convert-files-between-different-formats/3564

GuySartorelli · 2024-01-16T22:01:07Z

Closed in favour of #585

maxime-rainville mentioned this pull request Aug 14, 2020

WIP Allow generation of thumbnail for non-image file silverstripe/silverstripe-asset-admin#1133

Closed

emteknetnz added affects/v4 change/minor complexity/medium impact/medium type/enhancement labels Sep 8, 2020

kinglozzer mentioned this pull request Mar 18, 2021

RFC: Option to transcode images (similar to image manipulations). #441

Closed

NEW Allow file variant with different extensions

a435f86

maxime-rainville force-pushed the pulls/1/allow-variant-with-different-extension branch from 5d9ce7a to a435f86 Compare April 8, 2021 08:49

maxime-rainville commented Apr 8, 2021

View reviewed changes

DOC AlternativeFileExtensionTrait

e8941ba

maxime-rainville commented Apr 8, 2021

View reviewed changes

maxime-rainville mentioned this pull request May 3, 2022

Allow file variant with different extension #488

Closed

7 tasks

emteknetnz removed the change/minor label Jun 20, 2022

GuySartorelli changed the base branch from 1 to 2 January 16, 2024 20:50

GuySartorelli changed the base branch from 2 to 1 January 16, 2024 20:51

GuySartorelli closed this Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow file variant with different extensions POC #411

Allow file variant with different extensions POC #411

maxime-rainville commented Jul 19, 2020 •

edited

Loading

maxime-rainville commented Aug 21, 2020

clarkepaul commented Aug 23, 2020

maxime-rainville commented Aug 25, 2020

sachajudd commented Sep 25, 2020 •

edited

Loading

tractorcow commented Apr 8, 2021

maxime-rainville commented Apr 8, 2021

maxime-rainville Apr 8, 2021

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021

maxime-rainville Apr 8, 2021

maxime-rainville Apr 8, 2021

maxime-rainville commented Apr 8, 2021

tractorcow commented Apr 10, 2021

maxime-rainville commented Apr 12, 2021

kinglozzer commented Apr 13, 2022

nathanbrauer commented Sep 16, 2023 •

edited

Loading

GuySartorelli commented Jan 16, 2024

		$pathParts = pathinfo($this->getFilename());
		$variant = $this->variantName(FileIDHelper::EXTENSION_REWRITE_VARIANT, $pathParts['extension'], $newExtension);

		$url = $assetStore->getAsURL($filename, $hash, $variant, false);
		$extension = pathinfo($url, PATHINFO_EXTENSION);

Allow file variant with different extensions POC #411

Allow file variant with different extensions POC #411

Conversation

maxime-rainville commented Jul 19, 2020 • edited Loading

Background

Problem

Approach

What would the API for this look like

Extra complications

Related PR

maxime-rainville commented Aug 21, 2020

clarkepaul commented Aug 23, 2020

maxime-rainville commented Aug 25, 2020

sachajudd commented Sep 25, 2020 • edited Loading

tractorcow commented Apr 8, 2021

maxime-rainville commented Apr 8, 2021

maxime-rainville Apr 8, 2021

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021 • edited by GuySartorelli Loading

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021 • edited by GuySartorelli Loading

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021 • edited by GuySartorelli Loading

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021 • edited by GuySartorelli Loading

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021 • edited by GuySartorelli Loading

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021

Choose a reason for hiding this comment

maxime-rainville Apr 8, 2021

Choose a reason for hiding this comment

maxime-rainville commented Apr 8, 2021

tractorcow commented Apr 10, 2021

maxime-rainville commented Apr 12, 2021

kinglozzer commented Apr 13, 2022

nathanbrauer commented Sep 16, 2023 • edited Loading

GuySartorelli commented Jan 16, 2024

maxime-rainville commented Jul 19, 2020 •

edited

Loading

sachajudd commented Sep 25, 2020 •

edited

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

maxime-rainville Apr 8, 2021 •

edited by GuySartorelli

Loading

nathanbrauer commented Sep 16, 2023 •

edited

Loading