silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

API/Service for registering "converters" that will create file variant under a different extensions #489

Closed maxime-rainville closed 3 months ago

maxime-rainville commented 2 years ago

As a CMS Developer, I want clear and simple direction when implementing logic that converts files between formats.

Acceptance criteria

Excludes

PRs

michalkleiner commented 2 years ago

Do we want to consider a file variant under the same file extension/same type of file? I guess the question is what is a file variant in this piece of work. Would e.g. adding a watermark and saving as the same file type constitute a variant? Or should it be a different file name and that will not be seen as a variant?

GuySartorelli commented 2 years ago

488 mentions that calling Jpeg() on a file that is already a .jpg or .jpeg file shouldn't create a duplicate, but @michalkleiner has a good point that there might be a use case for using the converter API to generate, instead, a different variant of the jpeg.... I think however that the API as defined in #488 would be too vague to cleanly support that.

I think the converter API should not explicitly say "this is already a jpeg so I'm not touching it" - i.e. developers could use the converter API to define their own converter that adds a watermark to a jpg and outputs a jpg - but they should define their own method in an extension on the Image class (e.g. Watermark()) which would then engage the converter to generate that image.

I think it would make sense for the result of that to be a variant, rather than its own file - but maybe there could be a way for developers to choose whether the output of conversion is a variant or a new File db entry?

maxime-rainville commented 2 years ago

There's currently nothing stopping you from creating your own variant methods to do something like Watermark() or BlackAndWhite() or ReverseColour() ... that's essentially what all our resize method do. It's just your watermark method needs to always output images in the same format as the original. e.g.: If you start with Maxime.png and call the Watermark() method on it, you would get a new image stored under Maxime__watermark.png. Our logic is smart enough to know that __watermark is a variant and that the original file was called Maxime.png.

It doesn't look like our image manipulation are smart enough right now to avoid de-duplicating themselves. e.g.: $Logo.CropHeight(100).CropHeight(100).CropHeight(100).CropHeight(100) creates a variant like /family_picnic_resized__CropHeightWyIxMDAiXQ_CropHeightWyIxMDAiXQ_CropHeightWyIxMDAiXQ_CropHeightWyIxMDAiXQ.jpg.

It could very well be that if you have a scenario like Logo.Jpeg().Png().Jpeg(), there's no practical way for that chain to realise that it's returning you to the same starting point. At the very least, I would hope that in a scenario where you do something like $MyJpegImage.Jpeg(), the Jpeg method can be smart enough to realise there's nothing to for it do and simply return you a reference to $MyJpegImage.

maxime-rainville commented 5 months ago

I'm thinking the interface for a converter can probably look something like this:

interface FileConverter
{
    public function canConvert(string|DBFile $from, $to): bool;

    public function convert(DBFile $file, $to): DBFile;
}

You would need some sort of conversion service to register all your converters. That service would have a convert method that just loops through all its converters until it finds ones that can do your desired conversion.

maxime-rainville commented 5 months ago

These notes were intially in the main issue, but they don't seem all that relevant right now.

Notes