outl1ne / nova-settings

A Laravel Nova tool for editing custom settings using native Nova fields.
MIT License
278 stars 97 forks source link

nova_get_setting() returns cached results when using queued jobs #182

Open nathan-io opened 8 months ago

nathan-io commented 8 months ago

We have jobs which call nova_get_setting(). We're using Laravel Horizon to manage the job queue.

I've found that when you use Nova to change some nova-settings value, nova_get_setting() continues to return the old value until you restart the Horizon process.

Our jobs also run actions which themselves call nova_get_setting(). It returns the old value there as well.

I believe this behavior has the same cause as #158, where I reported that old values persist across a Tinker session, even if you are using the Nova UI to change some setting (and thus conceivably flushing the cache).

Using clearCache() does fix this:

NovaSettings::getStore()->clearCache();
nova_get_setting('some_key'); // now returns current result without needing to restart Horizon

However, I'm hoping to avoid calling this before every call to nova_get_setting(), which seems really messy.

I suppose we could create some helper which first clears the cache and then calls nova_get_setting(), but that doesn't really feel like a great solution either.

Thanks for any insight or assistance you can provide.

KasparRosin commented 8 months ago

Hey, @nathan-io

This is because we are using a singleton class that holds the "cache." Since Tinker and Horizon spawn new instances, they also create new singletons. The singletons, between instances, don't communicate with each other. Thus, when a browser instance clears its singleton's cache, other instances keep theirs until the instance is killed (or clearCache is called). This solution is fine as long as we have short-lived instances, but for long instances (Horizon, Tinker, etc.), it won't indeed work.

Here is how the essential setup is, you can see the $cache variable that holds the values. https://github.com/outl1ne/nova-settings/blob/main/src/NovaSettingsStore.php#L9 https://github.com/outl1ne/nova-settings/blob/main/src/NovaSettingsServiceProvider.php#L50

Essentially there are two solutions, either to drop the singleton based cache and request the values from database each time or implement the user defined cache (Redis, Filesystem etc).

nathan-io commented 8 months ago

Thanks @KasparRosin!

Would you consider supporting a config key to disable the cache, for those who don't want this behavior?

If not, any guidance on how to best implement either solution?

nathan-io commented 7 months ago

Hi, just wanted to circle back to this. I forgot to mention before, it seems this would likely also affect apps being served via Laravel Octane.