Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows models to define their own Url Generator implementation #2809

Conversation

belzaaron
Copy link

@belzaaron belzaaron commented Feb 24, 2022

This should allow models to have more control over their url generator without impeding on the default functionality (using config media-library.url_generator). This is super useful when you'd like one specific model to have media secured with something like a signed url.

By allowing a model that HasMedia to define an optional non-static method: getUrlGeneratorClass which takes no parameters and returns a string, we can drastically modify how the package generates urls JUST to that model. I've written a simple example below.

Here's the example model:

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Spatie\MediaLibrary\HasMedia;
use Spatie\MediaLibrary\InteractsWithMedia;

class MedicalCard extends Model implements HasMedia
{
    use HasFactory, InteractsWithMedia;

    // Removed for simplicity

    /**
     * The user for the model.
     *
     * @return BelongsTo
     */
    public function user(): BelongsTo
    {
        return $this->belongsTo(User::class);
    }

    /**
     * Get the classpath of the url generator used by the model.
     *
     * @return string
     */
    public function getUrlGeneratorClass(): string
    {
        return \App\Services\SpatieMediaLibrary\SignedUrlGenerator::class;
    }
}

And the example Url Generator:

<?php

namespace App\Services\SpatieMediaLibrary;

use Illuminate\Support\Facades\URL;
use Spatie\MediaLibrary\Support\UrlGenerator\DefaultUrlGenerator;

class SignedUrlGenerator extends DefaultUrlGenerator
{
    /**
     * Get url for the media and conversion on the generator instance.
     *
     * @return string
     */
    public function getUrl(): string
    {
        return URL::signedRoute('signed-media', [
            'media' => $this->media->id,
            'conversion' => $this->conversion?->getName(),
        ]);
    }
}

And the example route:

Route::middleware(['signed', 'auth'])->get('/media/{media}/{conversion?}', SignedMediaController::class)->name('signed-media');

And the example controller:

<?php

namespace App\Http\Controllers;

use Spatie\MediaLibrary\MediaCollections\Models\Media;
use Symfony\Component\HttpFoundation\BinaryFileResponse;

class SignedMediaController extends Controller
{
    /**
     * Handle the incoming request.
     *
     * @param  Media   $media
     * @param  string  $conversion = ''
     * @return BinaryFileResponse
     */
    public function __invoke(Media $media, string $conversion = ''): BinaryFileResponse
    {
      // More of your own logic if needed.

      ob_get_clean();

      return response()->file($media->getPath($conversion), [
        'Content-Type' => $media->mime_type,
      ]);
    }
}

This would allow so many more custom implementations as well - and should be totally non-conflicting with existing behavior.

Credit where it is due, this blog post sparked this PR for me: https://felix-schmid.de/blog/2-securing-media-from-laravel-medialibrary. I was inspired to find a solution to keep my other models' functionality while also allowing specific models to "opt-out" of default behavior and "opt-into" custom logic.

As another note to this, the issue solved my primary issue - my Media Library Pro collection components needed to have private media still function without modifying any views or other components.

Also - I know that we could just create a Url generator then change the logic by using something like:

    /**
     * Get url for the media and conversion on the generator instance.
     *
     * @return string
     */
    public function getUrl(): string
    {
        if (! $this->media->model instanceof SomeModel) {
            return parent::getUrl();
        }

        return URL::signedRoute('signed-media', [
            'media' => $this->media->id,
            'conversion' => $this->conversion?->getName(),
        ]);
    }

But I cannot see how this would be easier than the optional method. Will keep things clean on the implementation side as well.

Just in case someone comes across this, and it was not merged - here's a Gist documenting a way this can be accomplished: https://gist.github.com/belzaaron/bff713e7a73a489784745b7efa2b085e without this pull request.

This should allow models to have more control over their url generator without impeding on the default functionality (using config media-library.url_generator). This is super useful when you'd like one specific model to have media secured with something like a signed url.
@freekmurze
Copy link
Member

Could you add tests to ensure that this works correctly?

@belzaaron
Copy link
Author

Could you add tests to ensure that this works correctly?

Yes, I will do this sometime very soon.

@freekmurze
Copy link
Member

Closing for now as this solution will break eager loading.

@freekmurze freekmurze closed this Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants