babenkoivan / elastic-scout-driver

Elasticsearch driver for Laravel Scout
MIT License
252 stars 33 forks source link

Specifying specific database connection #64

Closed spiritinlife closed 1 month ago

spiritinlife commented 2 months ago

Not sure if there is a way that i am missing but it would be beneficial to be able to specify a specific database connection so that we can utilise for example our readonly mysql servers for hydration of elasticsearch results.

babenkoivan commented 2 months ago

Hey @spiritinlife, there is refineModels method in the elastic-scout-driver-plus package that you can use to set different connections, e.g.:

$models = Book::searchQuery($query)
   ->refineModels(function (EloquentBuilder $query) {
       $query->setConnection('my_connection');
   })
   ->execute()
   ->models();

Would that resolve your issue?

spiritinlife commented 1 month ago

Hey @babenkoivan, That could work but laravel query builder doesn't allow to change the connection easily. I am digging into it to see if there are any viable workarounds and revert.

spiritinlife commented 1 month ago

I can't seem to find a solution to this unfortunately. I have not yet fully understood why that doesn't work though.

babenkoivan commented 1 month ago

Hey @spiritinlife, sorry for the late reply. I think my initial code example was misleading because setConnection belongs to the model and not to the query builder. I need to think of a better approach and implement it in the package.

Meanwhile, you can try to switch the connection as follows:

$models = Book::searchQuery($query)
   ->refineModels(function (EloquentBuilder $builder) {
       $conn = DB::connection('my_connection');
       $query = $builder->getQuery();
       $query->connection = $conn;
       $query->grammar = $conn->query()->getGrammar();
       $query->processor = $conn->query()->getProcessor();
   })
   ->execute()
   ->models();

I didn't try it, so not sure if this would work.

spiritinlife commented 1 month ago

Hey @babenkoivan ,

Thank you for providing the snipper. Indeed that works in my tests although it doesn't work for relationships when you do the eager loading through the search query. I am glad you reached the same conclusion that a bigger change is required as i was also trying to find the time to assist on this.

From the consumer of the package point of view imho we should provide an override on the model for the database connection related to hydrating the elastic results and if not given it should default to the default model's connection.

I was able to have a proof of concept by overriding the LazyModelFactory and setting the connection like so in mapModels. This solves the issue with the relationships in my tests.

Screenshot 2024-09-23 at 6 36 44 PM
babenkoivan commented 1 month ago

Hey @spiritinlife, this doesn't look like the latest version of the LazyModelFactory 😅 Would you be able to update to v5 if I do the changes?

spiritinlife commented 1 month ago

Indeed it's not the latest. Unfortunately I am not able to upgrade to latest because our elastic deployment is at 7 and cannot be easily upgraded to 8 due to plugin dependencies.

I wanted to ask you on that also if there is any reason that the latest versions of the package don't support elastic 7.

babenkoivan commented 1 month ago

Can you please tell me what versions of packages you have (driver and driver plus)?

If I remember correctly, there were some breaking changes in ES 8 and I always focus on maintaining the latest versions due to time/capacity limitations.

spiritinlife commented 1 month ago
  1. babenkoivan/elastic-adapter: v2.4.0
  2. babenkoivan/elastic-client: v1.2.0
  3. babenkoivan/elastic-scout-driver: v2.0.0
  4. babenkoivan/elastic-scout-driver-plus: v3.5.1
babenkoivan commented 1 month ago

Hey @spiritinlife, I spent some time drafting a solution but concluded that such a feature must be implemented on the Laravel Scout side. If I add something like this in the Searchable trait in the elastic-scout-driver-plus:

trait Searchable
{
    // you can override this method in your model to define a connection that will be used when mapping search results to models
    public function scoutConnection(): ?string
    {
         return $this-connection;
    }
}

then it works perfectly fine with Model::searchQuery() but not with Model::search(), because the latter uses the standard functionality of the Scout package. I always try to keep elastic-scout-driver as close as possible to the original Scout package and avoid changing its behavior.

If I add something like this to the query builder, which limits the usage to the elastic-scout-driver-plus package:

$result = Model::searchQuery()->scoutConnection('readonly');

then it solves the issue, but it's cumbersome cause normally, you would want to define it every time you do a search request.

This all feels a bit off because this feature should live in the Scout package and be supported by Laravel.


Now, let's talk about the options you currently have:

  1. Read-only models. Let's say you have a model User, now you want to use readonly connection when searching users. You can extend User as follows:
class ReadonlyUser extends User 
{
    protected $table = 'users';
    // alternatively, you can override getConnectionName() method if you need to define some logic
    protected $connection = 'readonly';
}

then when you want to make sure readonly connection is used for queries, you can search using the new model:

$result = ReadonlyUser::searchQuery();
  1. Use refineModels as suggested here https://github.com/babenkoivan/elastic-scout-driver/issues/64#issuecomment-2355581162. It is cumbersome, but if you extract it to a function it will be readable and reusable.

  2. Fork the repository and apply the changes you need. It would be really difficult to extend/override existing functionality without a fork because many methods are private.

  3. Change the default connection before/after making a search request:

$originalConnection = DB::getDefaultConnection();

DB::setDefaultConnection('readonly');
$result = Model::searchQuery();
DB::setDefaultConnection($originalConnection);
  1. Create a feature request in https://github.com/laravel/scout.

I hope this helps and at least one of the solutions above works for you 🙂

spiritinlife commented 1 month ago

Hey 👋

Thank you for taking time to look into this.

Indeed that sounds like something it should be done in scout first.