babenkoivan / elastic-scout-driver-plus

Extension for Elastic Scout Driver
MIT License
267 stars 52 forks source link

Using `null` driver is broken #156

Closed oprypkhantc closed 1 year ago

oprypkhantc commented 1 year ago
Software Version
PHP 8.1.9
Elasticsearch 8.x
Laravel 9.11.0
Laravel Scout 9.4.9
Elastic Scout Driver 3.1.0

Describe the bug

Laravel Scout supports null driver out of the box, which serves testing purposes and literally does nothing. We use that by default for all of our tests to avoid hitting Elastic and slowing down our tests. Only in tests where we actually test anything Elastic-related do we enable the "real" driver. Before 3.x of elastic-scout-driver and before 4.x of elastic-scout-driver-plus, it used to work fine.

However, now thatSearchParametersBuilder::$engine is typed against an Engine class from elastic-scout-driver-plus, which the Scout's NullEngine doesn't extend, it causes this error when trying to use a "raw" elastic search:

User::searchQuery();

TypeError : Cannot assign Laravel\Scout\Engines\NullEngine to property Elastic\ScoutDriverPlus\Builders\SearchParametersBuilder::$engine of type Elastic\ScoutDriverPlus\Engine

To Reproduce

  1. Set SCOUT_DRIVER to null string
  2. Call searchQuery() method on any Searchable model

Current behavior A type error is thrown.

Expected behavior It works as expected. To achieve this, it'd probably be best to have custom methods that the Elastic\ScoutDriverPlus\Engine has be a part of a new interface, and use that interface instead of directly typing against Elastic\ScoutDriverPlus\Engine. Then we'd need an implementation of a new driver, ElasticNullDriver, with something along the lines of:

class ElasticNullEngine extends NullEngine implements ElasticEngine
{
    public function searchWithParameters(SearchParameters $searchParameters): SearchResult
    {
        return new SearchResult([
            'hits' => [
                'total' => ['value' => 0],
                'hits'  => [],
            ],
        ]);
    }

        // more functions mocked down here
}

If this is something that this packages wants to support - let me know and I'll make a pull request :) Thanks.

babenkoivan commented 1 year ago

Hey @oprypkhantc, thank you for raising this issue! I've just released v4.6.0 with proper null engine support 🎉

Please feel free to reopen the issue if you face any troubles.

oprypkhantc commented 1 year ago

Awesome, thank you :)