Fix model serialization issue in RemoveFromSearch job when using custom Scout key values

[DarronEngelbrechtEdge]

This PR attempts to fix a bug with model serialization in the RemoveFromSearch job that appears when using custom Scout key values that that modify a column by concatenation or hashing etc.

More information may be found in this issue: #653

This PR removes the old "test_models_are_deserialized_without_the_database_using_custom_scout_key" test as it is my understanding that the SerializesModels trait in the RemoveFromSearch job forces a database lookup for custom attributes as it only stores the primary key in the queue? This test is "replaced" by one that tests for custom key values.

[DarronEngelbrechtEdge]

@mmachatschek @driesvints - I'm wondering if the test_models_are_deserialized_without_the_database_using_custom_scout_key test should be present. If my understanding is correct the SerializesModels trait in the RemoveFromSearch job will ensure that only the model ID is stored in the job and the model must then be retrieved from the database to fetch the custom attribute?

Removing the SerializesModels trait from the RemoveFromSearch job gets the tests to pass but is likely a terrible solution.

[driesvints]

These changes were made here: #480 to solve #479. @DarronEngelbrechtEdge did you also set the getScoutKeyName in your model as detailed in PR #480? (before the changes in this PR).

Ping @stevebauman can you maybe have a look here?

[DarronEngelbrechtEdge]

@driesvints - No I do not modify getScoutKeyName other than to specify it now as the string of id which is the primary key for the model.

Had a look at those PRs and #479 explains the problem I'm having but #480 fixes the use case where both getScoutKeyName and getScoutKey have been modified not the more simple case where only getScoutKey is modified?

I believe this is not caught in testing because SCOUT_QUEUE is false and the models are not serialized and everything works correctly (setting SCOUT_QUEUE=false) in the project also "fixes" it for me.

Also suspect that for this issue to appear the custom key must be modified by concatenation or hashing etc and can't just do something like this as running it multiple times won't change anything...

getScoutKey() {
     return $this->other_id
 }

Perhaps smarter people can figure out how if it's possible that "models_are_deserialized_without_the_database_using_custom_scout_key" so we can restore that test?

[stevebauman]

Hey guys -- going to take a look into this, thanks for the ping @driesvints 👍

Give me til end of day -- have a bit of a full plate but I'll be able to look into this worst case later tonight.

[stevebauman]

This test will pass because the MeiliSearchCustomKeySearchableModel fixture is being instantiated with primary key that is being used in the getScoutKey() method:

public function getScoutKey()
{
    return 'my-meilisearch-key.'.$this->getKey();
}

https://github.com/DarronEngelbrechtEdge/scout/blob/246bf96e1042f1b1adc586f69f6260e389a1dccb/tests/Unit/MeiliSearchEngineTest.php#L475-L478

If another model attribute is used to construct the scout key, then this will fail, as it cannot be retrieved from the faux model that is created after the job is unserialized. All other model properties will not exist.

[stevebauman]

We can't make this change. The RemoveableScoutCollection instance must be present in the constructor for the getQueueableIds() method to return the proper scout keys. This collection method will be called when the job is serialized and placed onto the queue:

scout/src/Jobs/RemoveableScoutCollection.php

Line 15 in 3352fbc

public function getQueueableIds()

The test passes because the Eloquent\Collection instance will (by default) utilize the model's primary key (which is being populated in the test model) in it's getQueuableIds() method, so it will be available inside of the faux model, and thus the getScoutKey() will return the correct key:

https://github.com/laravel/framework/blob/b89f0d95275baf8a2b5adcc265203d7d365950be/src/Illuminate/Database/Eloquent/Collection.php#L697-L706