MeiliSearchEngine does not work with custom Scout keys

[DarronEngelbrechtEdge]

• Scout Version: 9.4.10
• Scout Driver: MeiliSearch
• Laravel Version: 9.21.6
• PHP Version: 8.1.9
• Database Driver & Version: MySQL 8.0
• SDK Version (If using a third-party service): N/A
• Meilisearch CLI Version (If using Meilisearch): meilisearch-http 0.28.0

Description:

When using custom index keys with getScoutKey() in a Searchable model the MeiliSearchEngine does not correct add the custom key to index nor delete entries from the index.

1. The update() method in the MeiliSearchEngine class uses array_merge() to merge the Scout key index with the toSearchableArray() data but references the Scout key first so the custom key gets overwritten by $searchableData unless a custom key is used.

           return array_merge(
               // [$model->getKeyName() => $model->getScoutKey()],
                $searchableData,
                $model->scoutMetadata(),
                [$model->getKeyName() => $model->getScoutKey()]
            );

2. RemoveFromSearch job makes a RemoveableScoutCollection which runs the models through getScoutKey() and passes the result to the the delete() method in MeiliSearchEngine which then runs getScountKey() a second time.
   ie.

    /**
     * Remove the given model from the index.
     *
     * @param  \Illuminate\Database\Eloquent\Collection  $models
     * @return void
     */
    public function delete($models)
    {
        $index = $this->meilisearch->index($models->first()->searchableAs());

        $index->deleteDocuments(
            // $models->map->getScoutKey()
            //     ->values()
            //     ->all()
            $models->pluck($models->first()->getKeyName())->toArray()
        );
    }

PS. We may also want to look at replacing getKeyName() with getScoutKeyName() in this MeiliSearchEngine class?

Steps To Reproduce:

1. Create a searchable model which overrides the getScoutKey method to prepend a something...

    /**
     * Get the custom search index key
     *
     * @return void
     */
    public function getScoutKey()
    {
        return 1 . $this->getKey();
    }

3. Create and ->save() the model, notice in Meilisearch the ID is not prepended with 1.

4. Delete the model, notice the model is not removed from Meilisearch.

[driesvints]

Isn't this a duplicate of #651 and fixed by #652 which is pending review?

[mmachatschek]

@DarronEngelbrechtEdge if you want to override the value of the primary key in meilisearch you would need to override the ->toSearchableArray() method too.

Unfortunately the documentation is not clear enough in this case.

By default the toSearchableArray() method just creates an array form of the model and sends the payload to the underlying search engine.

@driesvints no duplicate of #651 as this is about the indexing. Though after fixing the searchable array on this instance @DarronEngelbrechtEdge will run into the issue if he tries to use the keys() method as described in #651

[DarronEngelbrechtEdge]

@mmachatschek and @driesvints - Thanks for the feedback,

@mmachatschek is 100% correct regarding point 1, a toSearchableArray() method as below will load the custom Scout key in the index and yes I believe after fixing this I may later run into that issue with keys(), but I had managed to get around this and first ran into the issue in Point 2 which I describe further below.

    public function toSearchableArray()
    {
        return [
            'id' => $this->getScoutKey()
        ];
    }

Regarding point 2,

Once you manage to get the custom scout key into the index the RemoveFromSearch job does not actually remove the model from the index due to sending in the incorrect ID, it sends in the incorrect ID because...

RemoveFromSearch uses RemoveableScoutCollection to build a collection using ->getScoutKeys() to return the correct IDs and then runs the delete() method on the configured engine passing in the RemoveableScoutCollection. All 100% up to here unless there are issues in other engines that I'm unaware of....

The delete() method in the MeiliSearchEngine however tries to map the collection through ->getScoutKeys() again which it does not need to do but could perhaps just pluck out the IDs from the collection?

PS. Here are some dumps I used to verify this...

     * Remove the given model from the index.
     *
     * @param  \Illuminate\Database\Eloquent\Collection  $models
     * @return void
     */
    public function delete($models)
    {
        $index = $this->meilisearch->index($models->first()->searchableAs());

        dump($models);
        dump($models->map->getScoutKey()->values()->all());
        dump($models->pluck($models->first()->getKeyName())->toArray());

        $index->deleteDocuments(
            $models->map->getScoutKey()
                ->values()
                ->all()
        );
    }

My output is as follows after, Blog::create() gave me an incrementing ID of 30 and deleting that model results in the following dumps. You'll notice the original collection and pluck as the correct IDs but mapping adds another 1 because it runs through getScountKeys() twice.

 //  dump($models); ( raw input )
Illuminate\Database\Eloquent\Collection {#2187 ▼
  #items: array:1 [▶
    0 => App\Models\Blog {#2186 ▶
      #connection: null
      #table: null
      #primaryKey: "id"
      #keyType: "int"
      +incrementing: true
      #with: []
      #withCount: []
      +preventsLazyLoading: false
      #perPage: 15
      +exists: false
      +wasRecentlyCreated: false
      #escapeWhenCastingToString: false
      #attributes: array:1 [▶
        "id" => "130"
      ]
      #original: []
      #changes: []
      #casts: []
      #classCastCache: []
      #attributeCastCache: []
      #dates: []
      #dateFormat: null
      #appends: []
      #dispatchesEvents: []
      #observables: []
      #relations: []
      #touches: []
      +timestamps: true
      #hidden: []
      #visible: []
      #fillable: []
      #guarded: array:1 [▶
        0 => "*"
      ]
      #scoutMetadata: []
    }
  ]
  #escapeWhenCastingToString: false
}

//  dump($models->map->getScoutKey()->values()->all()); ( current code)
array:1 [▼
  0 => "1130"
]

// dump($models->pluck($models->first()->getKeyName())->toArray()); ( fixes it one way )
array:1 [▼
  0 => 130
]

[francoism90]

@DarronEngelbrechtEdge
I'm having the issue, using your workaround indeed fixes the issue:

public function toSearchableArray(): array
    {
        return [
            'id' => $this->getScoutKey(), // this one
            'name' => $this->name,
            'email' => $this->email,
        ];
    }

[DarronEngelbrechtEdge]

@mmachatschek @driesvints - I have opened a draft PR #655 to demonstrate the issue with getScoutKey being calculated from a field by hashing the value in SearchableModelWithCustomKey. I'm not too sure how to link it to this issue. Hope this helps to understand the issue, will see if I can take a stab at fixing it a bit later today. Many thanks!

[DarronEngelbrechtEdge]

@mmachatschek @driesvints - I tried to replicate my issue directly using the unit test below but I was getting different results in the Scout package (SCOUT_QUEUE=false) as to what I get in my project with SCOUT_QUEUE=true. Switching SCOUT_QUEUE=false in the project gives me the same results as in the unit test (things would work 100%).

Thus, I believe my PR demonstrates the core issue as the unit test test_models_are_deserialized_without_the_database_using_custom_scout_key fails due to running through getScoutKey() twice?

    public function test_delete_removes_objects_to_index_using_custom_scout_key()
    {
        Config::shouldReceive('get')->with('scout.prefix')->andReturn('');

        $client = m::mock(Client::class);
        $client->shouldReceive('index')->with('table')->andReturn($index = m::mock(Indexes::class));
        $index->shouldReceive('deleteDocuments')->with(['my-meilisearch-key.1']);

        $engine = new MeiliSearchEngine($client);
        $engine->delete(RemoveableScoutCollection::make([new MeiliSearchCustomKeySearchableModel(['id' => 1])]));
    }

PS. Have verified the issue still exists in v9.4.11

[DarronEngelbrechtEdge]

For anybody waiting on the outcome here I have worked around this overriding the RemoveFromSearch job for the application using the changes in PR #655 - to override the job just add something like the following to your AppServiceProvider.php - thanks Laravel and Scout - that was easy! :-)

 Scout::removeFromSearchUsing(\App\Jobs\Scout\RemoveFromSearch::class);