laravel / scout

Laravel Scout provides a driver based solution to searching your Eloquent models.
https://laravel.com/docs/scout
MIT License
1.57k stars 336 forks source link

Fix searchable queue configuration #840

Closed tklie closed 5 months ago

tklie commented 5 months ago

This PR fixes how the Searchable trait determines whether to queue jobs. Also, the published configuration file is updated to be compatible with the current version of the trait.


The Searchable trait currently determines the queue connection and queue using the following two methods:

    public function syncWithSearchUsing()
    {
        return config('scout.queue.connection') ?: config('queue.default');
    }

    public function syncWithSearchUsingQueue()
    {
        return config('scout.queue.queue');
    }

However, the config file that is published by this package only contained the following config option:

return [
    'queue' => env('SCOUT_QUEUE', false),
];

This has been updated to the keys actually used by the trait:

return [
    'queue' => [
        'connection' => env('SCOUT_CONNECTION'),
        'queue' => env('SCOUT_QUEUE', false),
    ],
]

Subsequently, another update to the Searchable trait itself was necessary, since this used the old scout.queue key to determine if jobs should be queued. It now correctly uses scout.queue.queue:

    public function queueMakeSearchable($models)
    {
        if ($models->isEmpty()) {
            return;
        }

        if (! config('scout.queue.queue')) {
            $models->first()->makeSearchableUsing($models)->first()->searchableUsing()->update($models);
            return;
        }

        dispatch((new Scout::$makeSearchableJob($models))
                ->onQueue($models->first()->syncWithSearchUsingQueue())
                ->onConnection($models->first()->syncWithSearchUsing()));
    }

Edit: The same was done for the queueRemoveFromSearch method.

taylorotwell commented 5 months ago

Would break all existing apps.

tklie commented 5 months ago

Will be fixed in future versions though? Because as it stands right now, the Searchable trait is inconsistent within itself, using two different config keys.

To clarify: You can use it as is right now by setting the entire scout.queue option in your config file to false. But having the config file like this

return [
    'queue' => [
        'connection' => env('SCOUT_CONNECTION'),
        'queue' => env('SCOUT_QUEUE', false),
    ],
]

and disabling queueing via an env variable is not possible right now, because scout.queue will always be truthy since it's an array.

bedus-creation commented 4 months ago

@taylorotwell it would be nice, if we could fix this by adding another options possibly like connection ?

'connection' => env('SCOUT_CONNECTION')

and

 public function syncWithSearchUsing()
 {
    return config('scout.connection') ?: config('queue.default');
}