Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
krio-rogue opened this issue Feb 9, 2023 · 3 comments
Open
Labels
enhancement New feature or request

Comments

@krio-rogue
Copy link

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);
        //....
    }
@krio-rogue krio-rogue added the enhancement New feature or request label Feb 9, 2023
@matchish
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants