Custom Routing is broken in scout 9.1.0+

[spiritinlife]

This pr https://github.com/laravel/scout/pull/471 breaks custom routing since the deserialized model does not have the needed properties to define the routing property

[babenkoivan]

Hey @spiritinlife, I need more details here. What elastic-scout-driver-plus version do you think is affected?

In the current version, you only need to define shardRouting method:

public function shardRouting()
{
        return $this->user->id;
}

I don't see how it should break the routing. Even if you use a relation like in the example above, then it should be loaded if missing. Do you have a specific error message?

[spiritinlife]

Hey, sorry for not giving more details, will provide more soon. Deletes are broken with this scout pr and queues enabled.

[spiritinlife]

@babenkoivan i hope this gives more info.

"babenkoivan/elastic-scout-driver": "^2.0.0",
"babenkoivan/elastic-scout-driver-plus": "^3.1.0",
"laravel/scout": "^9.3.4",

Scout version is the issue here, anything over 9.1.0 should fail due to this pr https://github.com/laravel/scout/pull/471.

Maybe we can override the RemoveFromSearch job to deserialise the shardRouting value along with the id.

The error stacktrace when RemoveFromSearch job fails.

[2022-01-06 11:11:03] local.ERROR: Return value of App\Models\Ad::shardRouting() must be of the type string, null returned {"exception":"[object] (TypeError(code: 0): Return value of App\\Models\\Ad::shardRouting() must be of the type string, null returned at /var/www/classifieds/app/Models/Ad.php:292)
[stacktrace]
#0 /var/www/classifieds/vendor/babenkoivan/elastic-scout-driver-plus/src/Factories/RoutingFactory.php(15): App\\Models\\Ad->shardRouting()
#1 /var/www/classifieds/vendor/babenkoivan/elastic-scout-driver-plus/src/Engine.php(63): ElasticScoutDriverPlus\\Factories\\RoutingFactory->makeFromModels(Object(Illuminate\\Database\\Eloquent\\Collection))
#2 /var/www/classifieds/vendor/laravel/scout/src/Jobs/RemoveFromSearch.php(41): ElasticScoutDriverPlus\\Engine->delete(Object(Illuminate\\Database\\Eloquent\\Collection))
#3 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36): Laravel\\Scout\\Jobs\\RemoveFromSearch->handle()
#4 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/Util.php(40): Illuminate\\Container\\BoundMethod::Illuminate\\Container\\{closure}()
#5 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(93): Illuminate\\Container\\Util::unwrapIfClosure(Object(Closure))
#6 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(37): Illuminate\\Container\\BoundMethod::callBoundMethod(Object(Illuminate\\Foundation\\Application), Array, Object(Closure))
#7 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/Container.php(653): Illuminate\\Container\\BoundMethod::call(Object(Illuminate\\Foundation\\Application), Array, Array, NULL)
#8 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Bus/Dispatcher.php(128): Illuminate\\Container\\Container->call(Array)
#9 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Bus\\Dispatcher->Illuminate\\Bus\\{closure}(Object(Laravel\\Scout\\Jobs\\RemoveFromSearch))
#10 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Laravel\\Scout\\Jobs\\RemoveFromSearch))
#11 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Bus/Dispatcher.php(132): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#12 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(120): Illuminate\\Bus\\Dispatcher->dispatchNow(Object(Laravel\\Scout\\Jobs\\RemoveFromSearch), false)
#13 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Queue\\CallQueuedHandler->Illuminate\\Queue\\{closure}(Object(Laravel\\Scout\\Jobs\\RemoveFromSearch))
#14 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Laravel\\Scout\\Jobs\\RemoveFromSearch))
#15 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(122): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#16 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(70): Illuminate\\Queue\\CallQueuedHandler->dispatchThroughMiddleware(Object(Illuminate\\Queue\\Jobs\\BeanstalkdJob), Object(Laravel\\Scout\\Jobs\\RemoveFromSearch))
#17 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Jobs/Job.php(98): Illuminate\\Queue\\CallQueuedHandler->call(Object(Illuminate\\Queue\\Jobs\\BeanstalkdJob), Array)
#18 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(428): Illuminate\\Queue\\Jobs\\Job->fire()
#19 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(378): Illuminate\\Queue\\Worker->process('beanstalkd', Object(Illuminate\\Queue\\Jobs\\BeanstalkdJob), Object(Illuminate\\Queue\\WorkerOptions))
#20 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(172): Illuminate\\Queue\\Worker->runJob(Object(Illuminate\\Queue\\Jobs\\BeanstalkdJob), 'beanstalkd', Object(Illuminate\\Queue\\WorkerOptions))
#21 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Console/WorkCommand.php(117): Illuminate\\Queue\\Worker->daemon('beanstalkd', 'high', Object(Illuminate\\Queue\\WorkerOptions))
#22 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Queue/Console/WorkCommand.php(101): Illuminate\\Queue\\Console\\WorkCommand->runWorker('beanstalkd', 'high')
#23 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36): Illuminate\\Queue\\Console\\WorkCommand->handle()
#24 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/Util.php(40): Illuminate\\Container\\BoundMethod::Illuminate\\Container\\{closure}()
#25 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(93): Illuminate\\Container\\Util::unwrapIfClosure(Object(Closure))
#26 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(37): Illuminate\\Container\\BoundMethod::callBoundMethod(Object(Illuminate\\Foundation\\Application), Array, Object(Closure))
#27 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Container/Container.php(653): Illuminate\\Container\\BoundMethod::call(Object(Illuminate\\Foundation\\Application), Array, Array, NULL)
#28 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Console/Command.php(136): Illuminate\\Container\\Container->call(Array)
#29 /var/www/classifieds/vendor/symfony/console/Command/Command.php(298): Illuminate\\Console\\Command->execute(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Illuminate\\Console\\OutputStyle))
#30 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Console/Command.php(121): Symfony\\Component\\Console\\Command\\Command->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Illuminate\\Console\\OutputStyle))
#31 /var/www/classifieds/vendor/symfony/console/Application.php(1005): Illuminate\\Console\\Command->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#32 /var/www/classifieds/vendor/symfony/console/Application.php(299): Symfony\\Component\\Console\\Application->doRunCommand(Object(Illuminate\\Queue\\Console\\WorkCommand), Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#33 /var/www/classifieds/vendor/symfony/console/Application.php(171): Symfony\\Component\\Console\\Application->doRun(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#34 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Console/Application.php(94): Symfony\\Component\\Console\\Application->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#35 /var/www/classifieds/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(129): Illuminate\\Console\\Application->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#36 /var/www/classifieds/artisan(35): Illuminate\\Foundation\\Console\\Kernel->handle(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#37 {main}
"}

[babenkoivan]

Thanks for the details, @spiritinlife! The problem is that restoreCollection only fills models with ids and the other fields are missing. The solution would be to override RemoveFromSearch:: restoreCollection() (to make a query and fetch all the fields) and Searchable::queueRemoveFromSearch() (to invoke custom RemoveFromSearch), but before we do anything crazy, let's summon @stevebauman here and ask his opinion.

I'm not even sure if restoreCollection in https://github.com/laravel/scout/pull/471 is valid. From my point of view it doesn't cover scenarios when scout key is not model's identifier (i.e. when Searchable::getScoutKey() and Searchable::getScoutKeyName() are overwritten). Maybe this problem can be solved in the Scout repository and not in the driver?

[stevebauman]

Hey guys! I'll do my best to help out here 👍

Thanks for the details, @spiritinlife! The problem is that restoreCollection only fills models with ids and the other fields are missing. The solution would be to override RemoveFromSearch:: restoreCollection() (to make a query and fetch all the fields) and Searchable::queueRemoveFromSearch() (to invoke custom RemoveFromSearch), but before we do anything crazy, let's summon @stevebauman here and ask his opinion.

This won't be possible without querying the database unfortunately. When a queued job is serialized, Laravel will process each property on the job class (which may be the model or collection of models), and overwrite them with a ModelIdentifier instance containing only the ID's of the models (which would be their scout key or primary key), discarding all other data from the models:

https://github.com/laravel/framework/blob/8.x/src/Illuminate/Queue/SerializesModels.php

https://github.com/laravel/framework/blob/b0a2d855cdbec8638e9eb7fa9d433ceb84137a84/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php#L20-L41

Unless you're referring to querying the ElasticSearch instance with the scout keys and hydrating the model(s) with its data instead? Could be tricky, but do-able.

Update: Actually this may be very tricky, especially in the OP's scenario above due to the routing key being dependent on a relationship. Maybe we could override the getSeralizedPropertyValue() method on the job itself and return an extended ModeIdentifier instance that could include the shardRouting() value?

I'm not even sure if restoreCollection in laravel/scout#471 is valid. From my point of view it doesn't cover scenarios when scout key is not model's identifier (i.e. when Searchable::getScoutKey() and Searchable::getScoutKeyName() are overwritten). Maybe this problem can be solved in the Scout repository and not in the driver?

This was actually resolved here: https://github.com/laravel/scout/pull/480 shortly after https://github.com/laravel/scout/pull/471.

[spiritinlife]

Wouldn't that problem be fixed if we didn't use SerializeModels on the RemoveFromSearch job ?

Also another thing that we could do is separate the option of queueing deletes from queuing indexing. So add another config option SCOUT_QUEUE_DELETES.

[stevebauman]

Wouldn't that problem be fixed if we didn't use SerializeModels on the RemoveFromSearch job ?

To my knowledge, Laravel does this to ensure all models can be serialized, as they may contain closures inside of their property values which cannot be serialized without an external package (PHP cannot serialize closures by default). When Laravel unserializes the job, it uses the serialized queuable ID's to query for all the models that were in the job, and re-hydrates the jobs properties with the resulting models, bypassing this issue of unserializable values.

Overriding this job and removing this trait will break environments if the above case is true.

Also another thing that we could do is separate the option of queueing deletes from queuing indexing. So add another config option SCOUT_QUEUE_DELETES.

Yea we could do that, though we'd have to PR Scout itself and see if that's something they'd like to be included.

With Laravel's current implementation of serialization, we'd have to slap a heavy layer of overrides to get this working smoothly.

[babenkoivan]

This won't be possible without querying the database unfortunately.

Doesn't Laravel query DB by default?

protected function restoreCollection($value)
{
    if (! $value->class || count($value->id) === 0) {
        return new EloquentCollection;
    }

    $collection = $this->getQueryForModelRestoration(
        (new $value->class)->setConnection($value->connection), $value->id
    )->useWritePdo()->get();

    if (is_a($value->class, Pivot::class, true) ||
        in_array(AsPivot::class, class_uses($value->class))) {
        return $collection;
    }

    $collection = $collection->keyBy->getKey();

    $collectionClass = get_class($collection);

    return new $collectionClass(
        collect($value->id)->map(function ($id) use ($collection) {
            return $collection[$id] ?? null;
        })->filter()
    );
}

It is just overwritten here. We can restore this code in Elastic Driver Plus, I think this would solve the issue or am I missing something?

[spiritinlife]

The problem with querying the database is that the model will most likely don't exist. This job runs in response to a model deletion.

To me the only viable solution is to either remove SerializesModels so that the whole model get's serialized and deserialized or better yet create/override SerializesModels in a way that serializes and deserializes only what is necessary to execute the document delete on the search engine.

[babenkoivan]

@spiritinlife indeed! I didn't think it through 🙃

Yeah, we probably need a custom serializer then. I will try to work on it next week.

[babenkoivan]

@spiritinlife I've just released v3.2.2 with a fix that should eliminate the problem. Unfortunately, I was only able to test it with the sync queue driver, perhaps @spiritinlife you can test it in your env with the real data?

[spiritinlife]

@babenkoivan Thank you, will do a test and inform you.

[spiritinlife]

@babenkoivan I can confirm it works. Thank you 🙏