[9.x] Fix custom scout keys not being utilized when deleting from queue (#656)

stevebauman · taylorotwell · web-flow · commit 19d77a49fa32 · 2022-10-04T09:10:50.000-05:00
* Add getUnqualifiedScoutKeyName method

* Add RemoveableScoutCollection tests

* Change order of update indexing so a custom key cannot be overridden and resolve deleting with custom key

* Return a RemoveableScoutCollection instance and fix key type when scout key is a string

* Add `RemoveFromSearch` meilisearch tests

* Flush the container on tear down

* Remove unneeded tearDown

* Update algolia engine to delete using custom keys

* Add Algolia deletion tests with custom keys

* Rename "$values" to "$keys" for clarity

* Spacing

* Remove unused imports

* CS fixes

* Comment clarification

* Move scout key array merge priority

* Update RemoveFromSearch.php

Co-authored-by: Taylor Otwell &lt;taylor@laravel.com&gt;
diff --git a/src/Builder.php b/src/Builder.php
@@ -5,7 +5,6 @@
 use Illuminate\Container\Container;
 use Illuminate\Pagination\LengthAwarePaginator;
 use Illuminate\Pagination\Paginator;
-use Illuminate\Support\Str;
 use Illuminate\Support\Traits\Macroable;
 use Laravel\Scout\Contracts\PaginatesEloquentModels;
 
@@ -444,7 +443,7 @@ protected function getTotalCount($results)
 
         $ids = $engine->mapIdsFrom(
             $results,
-            Str::afterLast($this->model->getScoutKeyName(), '.')
+            $this->model->getUnqualifiedScoutKeyName()
         )->all();
 
         if (count($ids) < $totalCount) {
diff --git a/src/Engines/AlgoliaEngine.php b/src/Engines/AlgoliaEngine.php
@@ -7,6 +7,7 @@
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Support\LazyCollection;
 use Laravel\Scout\Builder;
+use Laravel\Scout\Jobs\RemoveableScoutCollection;
 
 class AlgoliaEngine extends Engine
 {
@@ -63,9 +64,9 @@ public function update($models)
             }
 
             return array_merge(
-                ['objectID' => $model->getScoutKey()],
                 $searchableData,
-                $model->scoutMetadata()
+                $model->scoutMetadata(),
+                ['objectID' => $model->getScoutKey()],
             );
         })->filter()->values()->all();
 
@@ -82,13 +83,17 @@ public function update($models)
      */
     public function delete($models)
     {
+        if ($models->isEmpty()) {
+            return;
+        }
+
         $index = $this->algolia->initIndex($models->first()->searchableAs());
 
-        $index->deleteObjects(
-            $models->map(function ($model) {
-                return $model->getScoutKey();
-            })->values()->all()
-        );
+        $keys = $models instanceof RemoveableScoutCollection
+            ? $models->pluck($models->first()->getUnqualifiedScoutKeyName())
+            : $models->map->getScoutKey();
+
+        $index->deleteObjects($keys->all());
     }
 
     /**
diff --git a/src/Engines/MeiliSearchEngine.php b/src/Engines/MeiliSearchEngine.php
@@ -3,8 +3,8 @@
 namespace Laravel\Scout\Engines;
 
 use Illuminate\Support\LazyCollection;
-use Illuminate\Support\Str;
 use Laravel\Scout\Builder;
+use Laravel\Scout\Jobs\RemoveableScoutCollection;
 use MeiliSearch\Client as MeiliSearchClient;
 use MeiliSearch\MeiliSearch;
 use MeiliSearch\Search\SearchResult;
@@ -64,9 +64,9 @@ public function update($models)
             }
 
             return array_merge(
-                [$model->getKeyName() => $model->getScoutKey()],
                 $searchableData,
-                $model->scoutMetadata()
+                $model->scoutMetadata(),
+                [$model->getKeyName() => $model->getScoutKey()],
             );
         })->filter()->values()->all();
 
@@ -83,13 +83,17 @@ public function update($models)
      */
     public function delete($models)
     {
+        if ($models->isEmpty()) {
+            return;
+        }
+
         $index = $this->meilisearch->index($models->first()->searchableAs());
 
-        $index->deleteDocuments(
-            $models->map->getScoutKey()
-                ->values()
-                ->all()
-        );
+        $keys = $models instanceof RemoveableScoutCollection
+            ? $models->pluck($models->first()->getUnqualifiedScoutKeyName())
+            : $models->map->getScoutKey();
+
+        $index->deleteDocuments($keys->all());
     }
 
     /**
@@ -244,7 +248,7 @@ public function mapIdsFrom($results, $key)
      */
     public function keys(Builder $builder)
     {
-        $scoutKey = Str::afterLast($builder->model->getScoutKeyName(), '.');
+        $scoutKey = $builder->model->getUnqualifiedScoutKeyName();
 
         return $this->mapIdsFrom($this->search($builder), $scoutKey);
     }
diff --git a/src/Jobs/RemoveFromSearch.php b/src/Jobs/RemoveFromSearch.php
@@ -4,9 +4,7 @@
 
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
-use Illuminate\Database\Eloquent\Collection as EloquentCollection;
 use Illuminate\Queue\SerializesModels;
-use Illuminate\Support\Str;
 
 class RemoveFromSearch implements ShouldQueue
 {
@@ -15,7 +13,7 @@ class RemoveFromSearch implements ShouldQueue
     /**
      * The models to be removed from the search index.
      *
-     * @var \Illuminate\Database\Eloquent\Collection
+     * @var \Laravel\Scout\Jobs\RemoveableScoutCollection
      */
     public $models;
 
@@ -46,35 +44,24 @@ public function handle()
      * Restore a queueable collection instance.
      *
      * @param  \Illuminate\Contracts\Database\ModelIdentifier  $value
-     * @return \Illuminate\Database\Eloquent\Collection
+     * @return \Laravel\Scout\Jobs\RemoveableScoutCollection
      */
     protected function restoreCollection($value)
     {
         if (! $value->class || count($value->id) === 0) {
-            return new EloquentCollection;
+            return new RemoveableScoutCollection;
         }
 
-        return new EloquentCollection(
+        return new RemoveableScoutCollection(
             collect($value->id)->map(function ($id) use ($value) {
                 return tap(new $value->class, function ($model) use ($id) {
-                    $keyName = $this->getUnqualifiedScoutKeyName(
-                        $model->getScoutKeyName()
-                    );
-
-                    $model->forceFill([$keyName => $id]);
+                    $model->setKeyType(
+                        is_string($id) ? 'string' : 'int'
+                    )->forceFill([
+                        $model->getUnqualifiedScoutKeyName() => $id,
+                    ]);
                 });
             })
         );
     }
-
-    /**
-     * Get the unqualified Scout key name.
-     *
-     * @param  string  $keyName
-     * @return string
-     */
-    protected function getUnqualifiedScoutKeyName($keyName)
-    {
-        return Str::afterLast($keyName, '.');
-    }
 }
diff --git a/src/Searchable.php b/src/Searchable.php
@@ -4,6 +4,7 @@
 
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Support\Collection as BaseCollection;
+use Illuminate\Support\Str;
 
 trait Searchable
 {
@@ -389,6 +390,16 @@ public function getScoutKeyName()
         return $this->getQualifiedKeyName();
     }
 
+    /**
+     * Get the unqualified Scout key name.
+     *
+     * @return string
+     */
+    public function getUnqualifiedScoutKeyName()
+    {
+        return Str::afterLast($this->getScoutKeyName(), '.');
+    }
+
     /**
      * Determine if the current class should use soft deletes with searching.
      *
diff --git a/tests/Unit/AlgoliaEngineTest.php b/tests/Unit/AlgoliaEngineTest.php
@@ -3,11 +3,14 @@
 namespace Laravel\Scout\Tests\Unit;
 
 use Algolia\AlgoliaSearch\SearchClient;
+use Illuminate\Container\Container;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\LazyCollection;
 use Laravel\Scout\Builder;
+use Laravel\Scout\EngineManager;
 use Laravel\Scout\Engines\AlgoliaEngine;
+use Laravel\Scout\Jobs\RemoveFromSearch;
 use Laravel\Scout\Tests\Fixtures\EmptySearchableModel;
 use Laravel\Scout\Tests\Fixtures\SearchableModel;
 use Laravel\Scout\Tests\Fixtures\SoftDeletedEmptySearchableModel;
@@ -25,6 +28,7 @@ protected function setUp(): void
 
     protected function tearDown(): void
     {
+        Container::getInstance()->flush();
         m::close();
     }
 
@@ -51,6 +55,59 @@ public function test_delete_removes_objects_to_index()
         $engine->delete(Collection::make([new SearchableModel(['id' => 1])]));
     }
 
+    public function test_delete_removes_objects_to_index_with_a_custom_search_key()
+    {
+        $client = m::mock(SearchClient::class);
+        $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(Indexes::class));
+        $index->shouldReceive('deleteObjects')->once()->with(['my-algolia-key.5']);
+
+        $engine = new AlgoliaEngine($client);
+        $engine->delete(Collection::make([new AlgoliaCustomKeySearchableModel(['id' => 5])]));
+    }
+
+    public function test_delete_with_removeable_scout_collection_using_custom_search_key()
+    {
+        $job = new RemoveFromSearch(Collection::make([
+            new AlgoliaCustomKeySearchableModel(['id' => 5]),
+        ]));
+
+        $job = unserialize(serialize($job));
+
+        $client = m::mock(SearchClient::class);
+        $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class));
+        $index->shouldReceive('deleteObjects')->once()->with(['my-algolia-key.5']);
+
+        $engine = new AlgoliaEngine($client);
+        $engine->delete($job->models);
+    }
+
+    public function test_remove_from_search_job_uses_custom_search_key()
+    {
+        $job = new RemoveFromSearch(Collection::make([
+            new AlgoliaCustomKeySearchableModel(['id' => 5]),
+        ]));
+
+        $job = unserialize(serialize($job));
+
+        Container::getInstance()->bind(EngineManager::class, function () {
+            $engine = m::mock(AlgoliaEngine::class);
+
+            $engine->shouldReceive('delete')->once()->with(m::on(function ($collection) {
+                $keyName = ($model = $collection->first())->getUnqualifiedScoutKeyName();
+
+                return $model->getAttributes()[$keyName] === 'my-algolia-key.5';
+            }));
+
+            $manager = m::mock(EngineManager::class);
+
+            $manager->shouldReceive('engine')->andReturn($engine);
+
+            return $manager;
+        });
+
+        $job->handle();
+    }
+
     public function test_search_sends_correct_parameters_to_algolia()
     {
         $client = m::mock(SearchClient::class);
diff --git a/tests/Unit/MeiliSearchEngineTest.php b/tests/Unit/MeiliSearchEngineTest.php
@@ -2,10 +2,14 @@
 
 namespace Laravel\Scout\Tests\Unit;
 
+use Illuminate\Container\Container;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Support\Facades\Config;
 use Illuminate\Support\LazyCollection;
 use Laravel\Scout\Builder;
+use Laravel\Scout\EngineManager;
 use Laravel\Scout\Engines\MeiliSearchEngine;
+use Laravel\Scout\Jobs\RemoveFromSearch;
 use Laravel\Scout\Tests\Fixtures\EmptySearchableModel;
 use Laravel\Scout\Tests\Fixtures\SearchableModel;
 use Laravel\Scout\Tests\Fixtures\SoftDeletedEmptySearchableModel;
@@ -18,6 +22,18 @@
 
 class MeiliSearchEngineTest extends TestCase
 {
+    protected function setUp(): void
+    {
+        Config::shouldReceive('get')->with('scout.after_commit', m::any())->andReturn(false);
+        Config::shouldReceive('get')->with('scout.soft_delete', m::any())->andReturn(false);
+    }
+
+    protected function tearDown(): void
+    {
+        Container::getInstance()->flush();
+        m::close();
+    }
+
     public function test_update_adds_objects_to_index()
     {
         $client = m::mock(Client::class);
@@ -43,6 +59,59 @@ public function test_delete_removes_objects_to_index()
         $engine->delete(Collection::make([new SearchableModel(['id' => 1])]));
     }
 
+    public function test_delete_removes_objects_to_index_with_a_custom_search_key()
+    {
+        $client = m::mock(Client::class);
+        $client->shouldReceive('index')->with('table')->andReturn($index = m::mock(Indexes::class));
+        $index->shouldReceive('deleteDocuments')->once()->with(['my-meilisearch-key.5']);
+
+        $engine = new MeiliSearchEngine($client);
+        $engine->delete(Collection::make([new MeiliSearchCustomKeySearchableModel(['id' => 5])]));
+    }
+
+    public function test_delete_with_removeable_scout_collection_using_custom_search_key()
+    {
+        $job = new RemoveFromSearch(Collection::make([
+            new MeiliSearchCustomKeySearchableModel(['id' => 5]),
+        ]));
+
+        $job = unserialize(serialize($job));
+
+        $client = m::mock(Client::class);
+        $client->shouldReceive('index')->with('table')->andReturn($index = m::mock(Indexes::class));
+        $index->shouldReceive('deleteDocuments')->once()->with(['my-meilisearch-key.5']);
+
+        $engine = new MeiliSearchEngine($client);
+        $engine->delete($job->models);
+    }
+
+    public function test_remove_from_search_job_uses_custom_search_key()
+    {
+        $job = new RemoveFromSearch(Collection::make([
+            new MeiliSearchCustomKeySearchableModel(['id' => 5]),
+        ]));
+
+        $job = unserialize(serialize($job));
+
+        Container::getInstance()->bind(EngineManager::class, function () {
+            $engine = m::mock(MeiliSearchEngine::class);
+
+            $engine->shouldReceive('delete')->once()->with(m::on(function ($collection) {
+                $keyName = ($model = $collection->first())->getUnqualifiedScoutKeyName();
+
+                return $model->getAttributes()[$keyName] === 'my-meilisearch-key.5';
+            }));
+
+            $manager = m::mock(EngineManager::class);
+
+            $manager->shouldReceive('engine')->andReturn($engine);
+
+            return $manager;
+        });
+
+        $job->handle();
+    }
+
     public function test_search_sends_correct_parameters_to_meilisearch()
     {
         $client = m::mock(Client::class);
@@ -171,7 +240,7 @@ public function test_returns_primary_keys_when_custom_array_order_present()
         $builder = m::mock(Builder::class);
 
         $model = m::mock(stdClass::class);
-        $model->shouldReceive(['getScoutKeyName' => 'table.custom_key']);
+        $model->shouldReceive(['getUnqualifiedScoutKeyName' => 'custom_key']);
         $builder->model = $model;
 
         $engine->shouldReceive('keys')->passthru();
@@ -300,7 +369,7 @@ public function test_a_model_is_indexed_with_a_custom_meilisearch_key()
         $client = m::mock(Client::class);
         $client->shouldReceive('index')->with('table')->andReturn($index = m::mock(Indexes::class));
         $index->shouldReceive('addDocuments')->once()->with([[
-            'id' => 5,
+            'id' => 'my-meilisearch-key.5',
         ]], 'id');
 
         $engine = new MeiliSearchEngine($client);
diff --git a/tests/Unit/RemoveableScoutCollectionTest.php b/tests/Unit/RemoveableScoutCollectionTest.php
