anlutro / laravel-settings

Persistent settings in Laravel
MIT License
940 stars 112 forks source link

Disabling the cache does not seem to disable the cache. #136

Closed tobz-nz closed 3 years ago

tobz-nz commented 4 years ago

I'm been debugging an issue in my app that seems to be coming from the fact that caching does not seem to be disabled in settings.

By this I mean fetching a value from the settings store twice in the save process does not refect changes that may have happened between those 2 calls.

In my case this is a scheduled task that refreshes an API access token as it expires - but any queued jobs (run with horizon) do not get the new value until the horizon process is restarted.

My quick fix for now is to manually fetch the required data in my queued jobs(in a container binding).

I'm using the JSON store.

My settings:

<?php

return [
    /*
    |--------------------------------------------------------------------------
    | Default Settings Store
    |--------------------------------------------------------------------------
    |
    | This option controls the default settings store that gets used while
    | using this settings library.
    |
    | Supported: "json", "database"
    |
    */
    'store' => 'json',

    /*
    |--------------------------------------------------------------------------
    | JSON Store
    |--------------------------------------------------------------------------
    |
    | If the store is set to "json", settings are stored in the defined
    | file path in JSON format. Use full path to file.
    |
    */
    'path' => storage_path('settings.json'),

    /*
    |--------------------------------------------------------------------------
    | Database Store
    |--------------------------------------------------------------------------
    |
    | The settings are stored in the defined file path in JSON format.
    | Use full path to JSON file.
    |
    */
    // If set to null, the default connection will be used.
    'connection' => null,

    // Name of the table used.
    'table' => 'settings',

    // Cache usage configurations.
    'enableCache' => false,
    'forgetCacheByWrite' => true,
    'cacheTtl' => 0,

    // If you want to use custom column names in database store you could
    // set them in this configuration
    'keyColumn' => 'key',
    'valueColumn' => 'value',

    /*
    |--------------------------------------------------------------------------
    | Default Settings
    |--------------------------------------------------------------------------
    |
    | Define all default settings that will be used before any settings are set,
    | this avoids all settings being set to false to begin with and avoids
    | hardcoding the same defaults in all 'Settings::get()' calls
    |
    */
    'defaults' => []
];

The cache is disabled and even tried setting the TTL to 0.

laravel v6.18.2 settings v0.11.0

anlutro commented 4 years ago

This is probably the JsonSettingStore keeping the settings in memory, not a caching issue. This library was very much written with HTTP handling in mind, so it kind of assumes that once settings are read into memory, that memory will be wiped in no more than a few seconds.

I don't know what the appopriate way to hook this into the Laravel task framework is, possibly $schedule->before(...), but what you'd want to do is find a way to call Settings::load(true) to force a reload at the beginning of any task run.

Ideally we should add an unload() function which just forces settings to be reloaded next time you try to get or set any settings, because forcing a load shouldn't really be necessary.

tobz-nz commented 4 years ago

Ah-ha, Settings::load(true) looks like it might solve the issue. Would be good to add this method to the documentation for cases such as mine.

Happy to PR that if you want.

anlutro commented 4 years ago

A PR would be really nice :)

bjhijmans commented 3 years ago

I had a similar issue and I looked at the Settings::load(true) solution, but that would require running it in everything that goes to the queue and uses settings, which is not just jobs, but also emails, notifications, some events and some other jobs added by vendor code that I don't want to change.

So instead I decided to create a custom setting store that restarts the queue after saving settings. Something similar would work for json as well.

Here's my code:

new class:

use anlutro\LaravelSettings\DatabaseSettingStore as Base;
use Artisan;
use Illuminate\Database\Connection;

class CustomDatabaseSettingStore extends Base
{
    public function __construct(Connection $connection)
    {
        parent::__construct(
            $connection,
            config('settings.table'),
            config('settings.keyColumn'),
            config('settings.valueColumn')
        );
    }

    protected function write(array $data)
    {
        parent::write($data);
        // restart queue workers to force them to read new settings.
        Artisan::call('queue:restart');
    }
}

in AppServiceProvider::boot()

Setting::extend('customDatabaseSettingStore', function($app) {
    return $app->make(CustomDatabaseSettingStore::class);
});

and set 'store' => 'customDatabaseSettingStore' in settings config.