ralphjsmit/laravel-seo

Huge delay with imageMeta when running tests

Opened this issue · 3 comments

HassanZahirnia commented

Hey 👋

I use spatie/laravel-medialibrary for managing files including images. And I noticed when I'm running tests there is a huge stall (around 5-6seconds) on the imageMeta creation. I pass the getFirstMediaUrl to the SEOData and it works fine in browser since it returns a full url to the image (like: http://www....)

view()->share('SEOData', new SEOData(
     image: $category->getFirstMediaUrl(Disk::CategoryPictures),
));

But during tests since the storage is faked, the getFirstMediaUrl returns a local path like /storage/1/.... and this seem to confuse this package's imageMeta() inside SEOData class and the test would get delayed for many seconds until it finally passes.

I wonder if this is something that can be fixed or taken into account for such scenario ?
Or is there a different approach I should take for passing the image path. I'm asking this since spatie/laravel-medialibrary is a popular package and people probably might end up with same pitfall.

Thanks in advance and thank you for your great package 😊🙌

ralphjsmit commented

Hey Hassan! Thanks a lot for your detailed and thoughtful issue. I totally agree testsuites should be fast and this should not take 5-6 seconds. 🙂

I think that it has to do with the function getimagesize() in the ImageMeta class. Could you try both the following:

  1. Update your code, so that it passes the path and not the url. Should be something like this:
$category->getFirstMedia(Disk::CategoryPictures)->getPath();
  1. Go to the ImageMeta class in your vendor files, add the following code and share with me the results:
<?php

namespace RalphJSmit\Laravel\SEO\Support;

use Exception;

class ImageMeta
{
    public ?int $width = null;

    public ?int $height = null;

    public function __construct(string $path)
    {
        $publicPath = public_path($path);

        if ( ! is_file($publicPath) ) {
            dump("Not a file");
            report(new Exception("Path {$publicPath} is not a file."));

            return;
        }

        [$width, $height] = getimagesize($publicPath);
        dump($width, $height);

        $this->width = $width;
        $this->height = $height;
    }
}

Thanks!

  • ❤️1
HassanZahirnia commented

Hey! Thanks for your quick reply. 😄
I applied your instructions and I got the "Not a file" message.

I want to point out that spatie/laravel-medialibrary returns a full disk path to it's media. It does not return or store path relative to the public directory, which I know is what your package is designed to use. So there is that. And I looked into laravel-medialibrary docs and I found no way to return a relative path.

As for now, I found a workaround for this problem. I did some investigations and found out laravel-medialibrary uses the disk url config to generate the media url. And as you know Laravel's Storage::fake() creates a dummy disk with only one config property and that is the root path so it's missing lots of properties that a normal disk in config/filesystems.php would do.

So I added a second parameter to the Storage::fake() and adjusted it like below:

Storage::fake(Disk::CategoryPictures, [
    'url' => config('app.url') . '/storage',
]);

Now the test will run fine with normal speeds ✅
Although I'm not sure if this is considered a "hack" or it's actually how a proper storage fake should look like.

  • ❤️1
ralphjsmit commented

Hey @HassanZahirnia! Hmm, I guess a good solution, but indeed slightly hacky. But it is a good fake. For now I guess that it would be nice, but I could add a method that would allow you to generate URLs based on paths (and vice versa). I don't have time to do that this year, so you might want to use the current solution for now. Thanks! 🙂

  • 👍1