matchish / laravel-scout-elasticsearch

Search among multiple models with ElasticSearch and Laravel Scout
MIT License
721 stars 115 forks source link

[Feature] Add scout builder variable on creating HitsIteratorAggregate #234

Open krio-rogue opened 1 year ago

krio-rogue commented 1 year ago

Thank you very much for your job. Your package is very interesting and functional. The ability to store data on different models similar in structure in one index due to the "__class_name" attribute is very useful. At the same time, this function changes the standard functionality of the scout package and will not be able to work with an already created database.

Is your feature request related to a problem? Please describe. When testing different packages for scout + elastic without any callback and additional functions of the package, I expected to get the same search results

Describe the solution you'd like Search result will contain models instance of target search model (not imported model).

Describe alternatives you've considered In first i try use custom binding for HitsIteratorAggregate

class CustomIteratorAggregate implements IteratorAggregate
{
    protected static $builder;

    public static function setBuilder(Builder $builder)
    {
        static::$builder = $builder;

        return $builder;
    }

    public static function getBuilder()
    {
        return static::$builder;
    }

    public function getIterator(): Traversable
    {
        $builder = static::getBuilder();
        //....
        $hits = $builder->model->getScoutModelsByIds($builder, $objectIds);
        //....
    }
}
//using
$query = CustomIteratorAggregate::setBuilder(Products::search('car'));

But this is not a very good solution, because it is necessary to specify the model class each time and you cannot make requests to the search engine until the previous request is completed.

Perhaps this method will be more useful and will not break compatibility.

final class ElasticSearchEngine extends Engine
{
    //.....
    public function map(BaseBuilder $builder, $results, $model)
    {
        $hits = app()->makeWith(
            HitsIteratorAggregate::class,
            [
                'results'  => $results,
                'callback' => $builder->queryCallback,
                'builder' => $builder,
            ]
        );

        return new Collection($hits);
    }
//.....
class CustomIteratorAggregate implements IteratorAggregate
{
    //.....
    /**
     * @var Builder|null
     */
    protected $builder;

    public function __construct(array $results, callable $callback = null, Builder $builder = null)
    {
        $this->results = $results;
        $this->callback = $callback;
        $this->builder = $builder;
    }

    public function getIterator(): Traversable
    {
        //....
        $hits = $this->builder->model->getScoutModelsByIds($this->builder, $objectIds);
        //....
    }
matchish commented 1 year ago

Thank you) I'm not sure I got what issue you're trying to solve. Could you provide some example or even better to write a test that not pass?

krio-rogue commented 1 year ago

On importing model to storage, __class_name has class name of imported model. On searching we retrieve models by same class stored in __class_name. Its nice for a project without any additional services (other language, other model name). Search result will be losted when we rename model class, move model to other namespace, use several models for frontend and backend, fill elastic database with other service or try to use already existed data.

App\Models\Product::makeAllSearchable(); // __class_name is 'App\Models\Product'

class MyProduct extends Product {}
App\AnyModule\MyProduct::search()->get(); // return 'App\Models\Product' instead 'App\AnyModule\MyProduct'

Using a custom HitsIteratorAggregate with scout $builder is a quick way for retrieve in search result models with initial model class for searching.

Long way

Laravel eloquent has polymorphic with custom types https://laravel.com/docs/9.x/eloquent-relationships#custom-polymorphic-types By default on database will saved App\Models\Product. For using custom type we update model and setup morph map.

use Illuminate\Database\Eloquent\Model;

class Product extends Model {
   protected $morphClass = 'ProductAlias'; 
use Illuminate\Database\Eloquent\Relations\Relation;
Relation::morphMap([
    'ProductAlias' => App\Models\Product::class,
]);

Eloquent will save to database 'ProductAlias' instead App\Models\Product. For retrieve assigned class for alias used Model::getActualClassNameForMorph('ProductAlias') => 'App\Models\Product' This approach is more flexible to the organization of the code and allows you to reassign the class at any time. This solution is not quick.

matchish commented 1 year ago

Thanks for the explanation. Now I think I got the problem. Let me think