From 61c295ce46e6918c405fbe38dc0548d4635c10cd Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Mon, 1 Apr 2019 16:01:47 +0200 Subject: [PATCH 1/3] Import classes --- tests/AlgoliaEngineTest.php | 34 ++++++++++++++++++---------------- tests/SearchableTest.php | 3 ++- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/AlgoliaEngineTest.php b/tests/AlgoliaEngineTest.php index 930f034d..6a55009e 100644 --- a/tests/AlgoliaEngineTest.php +++ b/tests/AlgoliaEngineTest.php @@ -2,9 +2,11 @@ namespace Laravel\Scout\Tests; +use stdClass; use Mockery as m; use Laravel\Scout\Builder; use PHPUnit\Framework\TestCase; +use Algolia\AlgoliaSearch\SearchClient; use Laravel\Scout\Engines\AlgoliaEngine; use Illuminate\Database\Eloquent\Collection; use Laravel\Scout\Tests\Fixtures\EmptyTestModel; @@ -20,8 +22,8 @@ public function tearDown() public function test_update_adds_objects_to_index() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('saveObjects')->with([[ 'id' => 1, 'objectID' => 1, @@ -33,8 +35,8 @@ public function test_update_adds_objects_to_index() public function test_delete_removes_objects_to_index() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('deleteObjects')->with([1]); $engine = new AlgoliaEngine($client); @@ -43,8 +45,8 @@ public function test_delete_removes_objects_to_index() public function test_search_sends_correct_parameters_to_algolia() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('search')->with('zonda', [ 'numericFilters' => ['foo=1'], ]); @@ -57,10 +59,10 @@ public function test_search_sends_correct_parameters_to_algolia() public function test_map_correctly_maps_results_to_models() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); + $client = m::mock(SearchClient::class); $engine = new AlgoliaEngine($client); - $model = m::mock('StdClass'); + $model = m::mock(stdClass::class); $model->shouldReceive('newQuery')->andReturn($model); $model->shouldReceive('getKeyName')->andReturn('id'); $model->shouldReceive('getScoutModelsByIds')->andReturn($models = Collection::make([new AlgoliaEngineTestModel])); @@ -77,8 +79,8 @@ public function test_map_correctly_maps_results_to_models() public function test_a_model_is_indexed_with_a_custom_algolia_key() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('saveObjects')->with([[ 'id' => 1, 'objectID' => 'my-algolia-key.1', @@ -90,8 +92,8 @@ public function test_a_model_is_indexed_with_a_custom_algolia_key() public function test_a_model_is_removed_with_a_custom_algolia_key() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('deleteObjects')->with(['my-algolia-key.1']); $engine = new AlgoliaEngine($client); @@ -100,8 +102,8 @@ public function test_a_model_is_removed_with_a_custom_algolia_key() public function test_flush_a_model() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('clearObjects'); $engine = new AlgoliaEngine($client); @@ -110,8 +112,8 @@ public function test_flush_a_model() public function test_update_empty_searchable_array_does_not_add_objects_to_index() { - $client = m::mock('Algolia\AlgoliaSearch\SearchClient'); - $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock('StdClass')); + $client = m::mock(SearchClient::class); + $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldNotReceive('saveObjects'); $engine = new AlgoliaEngine($client); diff --git a/tests/SearchableTest.php b/tests/SearchableTest.php index 05678df4..51544658 100644 --- a/tests/SearchableTest.php +++ b/tests/SearchableTest.php @@ -4,6 +4,7 @@ use Mockery as m; use PHPUnit\Framework\TestCase; +use Illuminate\Database\Eloquent\Builder; use Laravel\Scout\Tests\Fixtures\SearchableTestModel; class SearchableTest extends TestCase @@ -63,7 +64,7 @@ class ModelStubForMakeAllSearchable extends SearchableTestModel { public function newQuery() { - $mock = \Mockery::mock('Illuminate\Database\Eloquent\Builder'); + $mock = m::mock(Builder::class); $mock->shouldReceive('orderBy') ->with('id') From 39d31b12f1d2a077baa9b86fd8bc43cac28a0a5f Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Mon, 1 Apr 2019 20:06:07 +0200 Subject: [PATCH 2/3] Refactor test suite For some reason there were way more fixtures then there were suppose to be. This commit refactors the fixtures so they're included in the only classes they're used and reduces them to only the minimum amount needed. --- tests/AlgoliaEngineTest.php | 43 ++++++++++++------- .../AlgoliaEngineTestCustomKeyModel.php | 11 ----- tests/Fixtures/AlgoliaEngineTestModel.php | 8 ---- tests/Fixtures/EmptyTestModel.php | 11 ----- tests/Fixtures/SearchableModel.php | 28 ++++++++++++ tests/Fixtures/SearchableTestModel.php | 10 ----- tests/Fixtures/TestModel.php | 39 ----------------- tests/SearchableTest.php | 12 +++--- 8 files changed, 62 insertions(+), 100 deletions(-) delete mode 100644 tests/Fixtures/AlgoliaEngineTestCustomKeyModel.php delete mode 100644 tests/Fixtures/AlgoliaEngineTestModel.php delete mode 100644 tests/Fixtures/EmptyTestModel.php create mode 100644 tests/Fixtures/SearchableModel.php delete mode 100644 tests/Fixtures/SearchableTestModel.php delete mode 100644 tests/Fixtures/TestModel.php diff --git a/tests/AlgoliaEngineTest.php b/tests/AlgoliaEngineTest.php index 6a55009e..13ae61c1 100644 --- a/tests/AlgoliaEngineTest.php +++ b/tests/AlgoliaEngineTest.php @@ -8,10 +8,8 @@ use PHPUnit\Framework\TestCase; use Algolia\AlgoliaSearch\SearchClient; use Laravel\Scout\Engines\AlgoliaEngine; +use Laravel\Scout\Tests\Fixtures\SearchableModel; use Illuminate\Database\Eloquent\Collection; -use Laravel\Scout\Tests\Fixtures\EmptyTestModel; -use Laravel\Scout\Tests\Fixtures\AlgoliaEngineTestModel; -use Laravel\Scout\Tests\Fixtures\AlgoliaEngineTestCustomKeyModel; class AlgoliaEngineTest extends TestCase { @@ -30,7 +28,7 @@ public function test_update_adds_objects_to_index() ]]); $engine = new AlgoliaEngine($client); - $engine->update(Collection::make([new AlgoliaEngineTestModel])); + $engine->update(Collection::make([new SearchableModel])); } public function test_delete_removes_objects_to_index() @@ -40,7 +38,7 @@ public function test_delete_removes_objects_to_index() $index->shouldReceive('deleteObjects')->with([1]); $engine = new AlgoliaEngine($client); - $engine->delete(Collection::make([new AlgoliaEngineTestModel])); + $engine->delete(Collection::make([new SearchableModel(['id' => 1])])); } public function test_search_sends_correct_parameters_to_algolia() @@ -52,7 +50,7 @@ public function test_search_sends_correct_parameters_to_algolia() ]); $engine = new AlgoliaEngine($client); - $builder = new Builder(new AlgoliaEngineTestModel, 'zonda'); + $builder = new Builder(new SearchableModel, 'zonda'); $builder->where('foo', 1); $engine->search($builder); } @@ -63,10 +61,9 @@ public function test_map_correctly_maps_results_to_models() $engine = new AlgoliaEngine($client); $model = m::mock(stdClass::class); - $model->shouldReceive('newQuery')->andReturn($model); - $model->shouldReceive('getKeyName')->andReturn('id'); - $model->shouldReceive('getScoutModelsByIds')->andReturn($models = Collection::make([new AlgoliaEngineTestModel])); - $model->shouldReceive('newCollection')->andReturn($models); + $model->shouldReceive('getScoutModelsByIds')->andReturn($models = Collection::make([ + new SearchableModel(['id' => 1]), + ])); $builder = m::mock(Builder::class); @@ -87,7 +84,7 @@ public function test_a_model_is_indexed_with_a_custom_algolia_key() ]]); $engine = new AlgoliaEngine($client); - $engine->update(Collection::make([new AlgoliaEngineTestCustomKeyModel])); + $engine->update(Collection::make([new CustomKeySearchableModel])); } public function test_a_model_is_removed_with_a_custom_algolia_key() @@ -97,17 +94,17 @@ public function test_a_model_is_removed_with_a_custom_algolia_key() $index->shouldReceive('deleteObjects')->with(['my-algolia-key.1']); $engine = new AlgoliaEngine($client); - $engine->delete(Collection::make([new AlgoliaEngineTestCustomKeyModel])); + $engine->delete(Collection::make([new CustomKeySearchableModel(['id' => 1])])); } - public function test_flush_a_model() + public function test_flush_a_model_with_a_custom_algolia_key() { $client = m::mock(SearchClient::class); $client->shouldReceive('initIndex')->with('table')->andReturn($index = m::mock(stdClass::class)); $index->shouldReceive('clearObjects'); $engine = new AlgoliaEngine($client); - $engine->flush(new AlgoliaEngineTestCustomKeyModel); + $engine->flush(new CustomKeySearchableModel); } public function test_update_empty_searchable_array_does_not_add_objects_to_index() @@ -117,6 +114,22 @@ public function test_update_empty_searchable_array_does_not_add_objects_to_index $index->shouldNotReceive('saveObjects'); $engine = new AlgoliaEngine($client); - $engine->update(Collection::make([new EmptyTestModel])); + $engine->update(Collection::make([new EmptySearchableModel])); + } +} + +class CustomKeySearchableModel extends SearchableModel +{ + public function getScoutKey() + { + return 'my-algolia-key.'.$this->getKey(); + } +} + +class EmptySearchableModel extends SearchableModel +{ + public function toSearchableArray() + { + return []; } } diff --git a/tests/Fixtures/AlgoliaEngineTestCustomKeyModel.php b/tests/Fixtures/AlgoliaEngineTestCustomKeyModel.php deleted file mode 100644 index 8172a756..00000000 --- a/tests/Fixtures/AlgoliaEngineTestCustomKeyModel.php +++ /dev/null @@ -1,11 +0,0 @@ -getKey(); - } -} diff --git a/tests/Fixtures/AlgoliaEngineTestModel.php b/tests/Fixtures/AlgoliaEngineTestModel.php deleted file mode 100644 index 2aadf1c0..00000000 --- a/tests/Fixtures/AlgoliaEngineTestModel.php +++ /dev/null @@ -1,8 +0,0 @@ -id; - } - - /** - * The default key of Models with Searchable-Trait - * @return mixed - */ - public function getScoutKey() - { - return $this->getKey(); - } - - public function toSearchableArray() - { - return ['id' => 1]; - } - - public function scoutMetadata() - { - return []; - } -} diff --git a/tests/SearchableTest.php b/tests/SearchableTest.php index 51544658..bd6eab80 100644 --- a/tests/SearchableTest.php +++ b/tests/SearchableTest.php @@ -5,7 +5,7 @@ use Mockery as m; use PHPUnit\Framework\TestCase; use Illuminate\Database\Eloquent\Builder; -use Laravel\Scout\Tests\Fixtures\SearchableTestModel; +use Laravel\Scout\Tests\Fixtures\SearchableModel; class SearchableTest extends TestCase { @@ -20,7 +20,7 @@ public function test_searchable_using_update_is_called_on_collection() $collection->shouldReceive('isEmpty')->andReturn(false); $collection->shouldReceive('first->searchableUsing->update')->with($collection); - $model = new SearchableTestModel; + $model = new SearchableModel(); $model->queueMakeSearchable($collection); } @@ -30,7 +30,7 @@ public function test_searchable_using_update_is_not_called_on_empty_collection() $collection->shouldReceive('isEmpty')->andReturn(true); $collection->shouldNotReceive('first->searchableUsing->update'); - $model = new SearchableTestModel; + $model = new SearchableModel; $model->queueMakeSearchable($collection); } @@ -40,7 +40,7 @@ public function test_searchable_using_delete_is_called_on_collection() $collection->shouldReceive('isEmpty')->andReturn(false); $collection->shouldReceive('first->searchableUsing->delete')->with($collection); - $model = new SearchableTestModel; + $model = new SearchableModel; $model->queueRemoveFromSearch($collection); } @@ -50,7 +50,7 @@ public function test_searchable_using_delete_is_not_called_on_empty_collection() $collection->shouldReceive('isEmpty')->andReturn(true); $collection->shouldNotReceive('first->searchableUsing->delete'); - $model = new SearchableTestModel; + $model = new SearchableModel; $model->queueRemoveFromSearch($collection); } @@ -60,7 +60,7 @@ public function test_make_all_searchable_uses_order_by() } } -class ModelStubForMakeAllSearchable extends SearchableTestModel +class ModelStubForMakeAllSearchable extends SearchableModel { public function newQuery() { From dafbfa74031c660f34f93a64b566e6164f9e5baa Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Mon, 1 Apr 2019 20:11:50 +0200 Subject: [PATCH 3/3] Correctly sort Algolia results This commit fixes the sorting of Algolia results. Instead of letting this get reset by the collection we should use the sorting results from Algolia itself since it already correctly sorts the search results. --- src/Engines/AlgoliaEngine.php | 2 ++ tests/AlgoliaEngineTest.php | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/Engines/AlgoliaEngine.php b/src/Engines/AlgoliaEngine.php index b607c36c..add0a093 100644 --- a/src/Engines/AlgoliaEngine.php +++ b/src/Engines/AlgoliaEngine.php @@ -179,6 +179,8 @@ public function map(Builder $builder, $results, $model) $builder, $objectIds )->filter(function ($model) use ($objectIds) { return in_array($model->getScoutKey(), $objectIds); + })->sortBy(function($model) use ($objectIds) { + return array_search($model->getScoutKey(), $objectIds); }); } diff --git a/tests/AlgoliaEngineTest.php b/tests/AlgoliaEngineTest.php index 13ae61c1..518fbd10 100644 --- a/tests/AlgoliaEngineTest.php +++ b/tests/AlgoliaEngineTest.php @@ -74,6 +74,32 @@ public function test_map_correctly_maps_results_to_models() $this->assertEquals(1, count($results)); } + public function test_map_method_respects_order() + { + $client = m::mock(SearchClient::class); + $engine = new AlgoliaEngine($client); + + $model = m::mock(stdClass::class); + $model->shouldReceive('getScoutModelsByIds')->andReturn($models = Collection::make([ + new SearchableModel(['id' => 1]), + new SearchableModel(['id' => 2]), + new SearchableModel(['id' => 3]), + new SearchableModel(['id' => 4]), + ])); + + $builder = m::mock(Builder::class); + + $results = $engine->map($builder, ['nbHits' => 4, 'hits' => [ + ['objectID' => 1, 'id' => 1], + ['objectID' => 2, 'id' => 2], + ['objectID' => 4, 'id' => 4], + ['objectID' => 3, 'id' => 3], + ]], $model); + + $this->assertEquals(4, count($results)); + $this->assertEquals([1, 2, 4, 3], $results->pluck('id')->all()); + } + public function test_a_model_is_indexed_with_a_custom_algolia_key() { $client = m::mock(SearchClient::class);