feat(media-library): add option to return original media file

[Keimeno]

Description

Currently, when using Glide as Media Library Image Service, I am unable to display uploaded SVGs, since Glide does not support SVGs.

There is a workaround to this problem, you can set the environment variable GLIDE_DRIVER to imagick. This “solves” the problem, meaning the SVG will be converted into a JPEG and thus losing image quality and transparency.

The workaround, however, is not perfect, and I'd prefer getting my unmodified SVG back instead. This is what this PR solves.

---

This may result in a breaking change, and it might make more sense to add an option in the config that allows the original SVG to be returned, such as twill.glide.use_original_svg with its default value being set to false.

Open to ideas and feedback

[pboivin]

Hi @Keimeno,

That's cool! I've also used a (hacky) homegrown helper to get the actual raw image when working with GIFs :

function safe_image_array($object, $role, $crop, $params=[], $media=null)
{
    if (! $media) {
        $media = $object->imageObject($role, $crop);
    }

    if (! $media) {
       return [];
    }

    $data = $object->imageAsArray($role, $crop, $params, $media);

    if (preg_match('/.gif$/', $media->filename)) {
        $data['src'] = '/storage/uploads/' . $media->uuid;
    }

    return $data;
}

I see the value of having a more robust solution in Twill core. Also very curious to know what others have been doing for this :)

[Keimeno]

To make things more robust, I've added support for Azure & S3 as well.
There are still some open questions, though.

1. Should the helper function getMediaUrl be made available for all Media Libraries? This would mean, that we make the function protected and move it into the ImageServiceDefaults.php file.
2. Is it enough if we offer this functionality for getRawUrl, or should this be added to other functions as well (getUrl, etc.)?
3. And since in your use case @pboi20 you need to return the original .gif file, it probably makes most sense if we add a configuration option, that allows us to specify an array of file endings. If a file ending matches, then we would return the original file.

[pboivin]

@Keimeno Thanks for taking the time to refine this!

1. As far as I can tell, the fix is not needed for the Local service. I haven't used Imgix, so I'm not sure about the behavior of getRawUrl in this context.

2. I don't think this should extend to getUrl.

3. A config option to toggle this behavior by file extension makes sense. Defaulting to an empty list would also avoid a breaking change, as you mentionned in your PR description.

Do you think that this feature could live in its own helper method? Something in the spirit of getRealRawUrl. Perhaps getOriginalFileUrl?

[Keimeno]

I'm unsure if getOriginalFileUrl should be the way to go. Wouldn't this mean that the method always returns the original URL, so the user has to check for the file extension himself? Also, we would have some inconsistencies in the public API between each media library service, so changing media drivers could become problematic, as long as we don't add this method to the ImageServiceDefaults trait.

Besides, after further consideration regarding getRealUrl, I think we should extend it to getUrl, since currently, when trying to access an SVG with the API described above we'd have to write something like the following (inside a blade file):

\A17\Twill\Services\MediaLibrary\ImageService::getRawUrl($block->imageObject('role')->uuid)

And since the HasMedias trait uses methods, which internally call the getUrl method, the following syntax would work as well, if we were to extend the getUrl method.

$block->image('role')

[pboivin]

Hey @Keimeno, thanks for the feedback! I see that getOriginalFileUrl may not be as desirable as I initially thought. Let's scratch that one :)

From my perspective (ie. my use case), I think the main issue remains that getRawUrl could behave better for certain file extensions. Your solution adresses this very cleanly.

I'm still unsure about changing the behavior of getUrl. Would there be any side effects in the CMS (Media Library or medias field)?

[Keimeno]

Thanks a million for the input @pboi20!
I don't think that it would cause any side effects for getUrl, since it only adds parameters to the URL, and these parameters would be ignored if specifying the original media URL.
Also, I kept the name getOriginalMediaUrl, think its the better naming choice than getMediaUrl

[Keimeno]

Thanks, @ifox, for the feedback on the PR, the requested changes have been made. Also found a small bug in my code, should have returned null instead of false in the getOriginalMediaUrl method.