Caching the icon content?

[kohenkatz]

First, thank you for this great package. I was planning to implement something similar to this myself, and then I found out about this package through Laravel-News.com.

I want to be able to cache not only the URL and data about the favicon, but also the favicon itself, effectively combining the cache() and storeAs() functions. The purpose would be to avoid retrieving the icon over and over again every time it is used.

I assume that many projects have good reasons NOT to do this, so I'm interested in your thoughts about adding an argument $withContent to the cache() method.

I can think of several ways to handle this, including:

1. Include the output of content() in the toCache() function
2. Store the file (using storeAs) and include the path to that file in toCache()

There are, of course, pros and cons or both methods, and there could be other options as well.

Any thoughts?

[ash-jc-allen]

Hey! Sorry for such a slow reply. I've fallen behind with a few things, so I'm just trying to catch up now haha.

I'm definitely open to the idea of this. But I can't quite decide whether it's something that should be baked into the package or added in app code.

The original intention of the package was that it'd fetch favicon URLs that would probably be passed to a web browser for displaying in a webpage. So I was using the caching to prevent the calls from our app server to an external site. Then I was relying on the web browser for caching the favicons for us.

But of course, this was just my original intention for my own use case. So it probably doesn't fit everyone's use case. I reckon if you can give me a bit more context on where you'd use this type of functionality, we can probably come up with something really nice and get it added to the package 😄

[kohenkatz]

Thanks for the reply! No need to be sorry, I'm behind on several projects myself.

I reckon if you can give me a bit more context on where you'd use this type of functionality, ...

My use-case is pulling customer icons based on their provided domain names, but I don't want to retrieve the icons from the customer's website every time. My application has a concept of "dealers" who manage the product for multiple customer organizations, so I don't want every customer's website to be hit every time the dealer views the customer list. I also don't trust that the customers' websites are set up with proper caching settings, just based on experience dealing with some of them. Some of the customers' websites are also very slow, and I don't want my application to appear to be slow because it is waiting for content from someone else's server.

I can't quite decide whether it's something that should be baked into the package or added in app code.

I was originally planning to do it in the app code, but the structure of this package makes that difficult.

Here is the controller method I wrote so far:

    public function showIcon(Organization $organization, ShowRequest $request)
    {
        $cacheKey = "organization-main-domain-{$organization->id}";
        // We can assume primary domains don't change much
        $cacheTtl = Date::now()->addDays(30);
        $domain = Cache::remember($cacheKey, $cacheTtl, function() use ($organization) {
            return $organization->domains()->where('is_default', true)->firstOr(function () use ($organization) {
                return $organization->domains()->first();
            })->domain_name;
        });

        $iconPath = Favicon::fetch("https://{$domain}/")->cache($cacheTtl)->storeAs('customer-icons', "org-{$domain}");
        if ($iconPath) {
            return Storage::response($iconPath);
        }

        // TODO: fallback generic icon
        abort(404);
    }

My original plan was to wrap the entire thing in a check for the existence of the icon file. The problem is that I don't know the filename because storeAs() uses the protected method buildStoragePath() - first, I don't want to re-implement this in my own code in case this method changes later in the package and no longer matches; second, this method relies on guessFileExtension() which requires knowing the URL extension and/or mime type.

So in order to accomplish what I want, something in the package needs to change. The simplest thing that can change is changing buildStoragePath() to public - then I could probably do something like this:

// This will load metadata from cache if it exists there
$icon = Favicon::fetch("https://{$domain}/")->cache($cacheTtl);
// This will get the local filesystem path
$iconPath = $icon->buildStoragePath('customer-icons', "org-{$domain}");
// If the file does not exist, get and store it
if (Storage::fileExists($iconPath)) {
    $iconPath = $icon->storeAs('customer-icons', "org-{$domain}");
}
return Storage::response($iconPath);

This option feels messy to me: first, because it relies on internal data from the Favicon within the controller, and second, because it ends up caching the icon split across two separate locations: the metadata in the cache and the icon data on disk.

That makes me think that having the option to include the icon content itself in the toCache() method (and therefore also in the cache() method) makes a lot of sense. Along with those, I would add an asResponse() method that returns a response with the correct headers (mime type, size, etc.) set. Then I could do something like this:

$icon = Favicon::fetch("https://{$domain}/")->cache($cacheTtl, withContent: true);
return $icon->asResponse();

At the same time, doing it that way would not be useful if someone wants to store the icons using a different storage driver, such as S3.

At this point, I have a lot of ideas, but I'm not sure what is the best way forward.

I'm happy to do a Pull Request for this - I'm always happy to contribute - but I wanted to get some discussion going about the right way to do it before I build something that might not be useful.