Merge branch 'master' into hotfix/refactor-search-all-entities

* master: Applying style patch. Tests for custom search conditions. Added searchPaginated method allowing only search not items listing StyleCI fixes Use method getMappingProperties() instead of mappingProperties field Refactored entity controller to build elasticsearch params for complexQuery and allow adding custom conditions in controllers. Applying style patch. Now checking if the result set for a nested model is empty before adding it to indexed data.
spira · Mar 22, 2016 · cc9401b · cc9401b
2 parents a481639 + 8430dc6
commit cc9401b
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 53 deletions.
diff --git a/Controllers/EntityController.php b/Controllers/EntityController.php
@@ -14,6 +14,7 @@
 use Elasticquent\ElasticquentTrait;
 use Illuminate\Http\Request;
 use Illuminate\Support\Str;
+use Spira\Core\Contract\Exception\BadRequestException;
 use Spira\Core\Model\Collection\Collection;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Spira\Core\Model\Model\BaseModel;
@@ -54,24 +55,14 @@ public function getAll(Request $request)
             ->collection($collection);
     }
 
-    public function getAllPaginated(Request $request, RangeRequest $rangeRequest)
+    public function searchPaginated(RangeRequest $rangeRequest)
     {
-        $totalCount = $this->countEntities();
-        $limit = $rangeRequest->getLimit($this->paginatorDefaultLimit, $this->paginatorMaxLimit);
-        $offset = $rangeRequest->isGetLast() ? $totalCount - $limit : $rangeRequest->getOffset();
-
-        if ($request->has('q')) {
-            $collection = $this->searchAllEntities($request->query('q'), $limit, $offset, $totalCount);
-        } else {
-            $collection = $this->getAllEntities($limit, $offset);
-        }
-
-        $collection = $this->getWithNested($collection, $request);
-        $this->checkPermission(static::class.'@getAllPaginated', ['model' => $collection]);
+        return $this->makePaginated($rangeRequest, 'searchPaginated', true, false);
+    }
 
-        return $this->getResponse()
-            ->transformer($this->getTransformer())
-            ->paginatedCollection($collection, $offset, $totalCount);
+    public function getAllPaginated(RangeRequest $rangeRequest)
+    {
+        return $this->makePaginated($rangeRequest, 'getAllPaginated');
     }
 
     /**
@@ -286,6 +277,41 @@ public function deleteMany(Request $request)
         return $this->getResponse()->noContent();
     }
 
+    /**
+     * Helper for paginated controller actions with restrictions of allowed methods for getting content: listing and\or search.
+     *
+     * @return ApiResponse
+     */
+    protected function makePaginated(RangeRequest $rangeRequest, $permission, $allow_search = true, $allow_listing = true)
+    {
+        $request = $rangeRequest->getRequest();
+
+        $totalCount = $this->countEntities();
+        $limit = $rangeRequest->getLimit($this->paginatorDefaultLimit, $this->paginatorMaxLimit);
+        $offset = $rangeRequest->isGetLast() ? $totalCount - $limit : $rangeRequest->getOffset();
+
+        if ($request->has('q')) {
+            if (! $allow_search) {
+                throw new BadRequestException('Search not allowed');
+            }
+
+            $collection = $this->searchAllEntities($request->query('q'), $limit, $offset, $totalCount);
+        } else {
+            if (! $allow_listing) {
+                throw new BadRequestException('Items listing not allowed');
+            }
+
+            $collection = $this->getAllEntities($limit, $offset);
+        }
+
+        $collection = $this->getWithNested($collection, $request);
+        $this->checkPermission(static::class.'@'.$permission, ['model' => $collection]);
+
+        return $this->getResponse()
+            ->transformer($this->getTransformer())
+            ->paginatedCollection($collection, $offset, $totalCount);
+    }
+
     /**
      * @param $id
      * @return BaseModel
@@ -399,24 +425,39 @@ protected function percolatedSearch($query)
         throw new BadRequestException('Percolated Search not available for this entity');
     }
 
-    /**
-     * @param array $queryArray
-     * @return mixed
-     * @internal param $model
-     */
-    protected function complexSearch(array $queryArray)
+    protected function convertQueryToElasticsearchRequest($query, $limit = null, $offset = null)
     {
         /* @var ElasticquentTrait $model */
         $model = $this->getModel();
-        $searchResults = $model->complexSearch(
-            [
+
+        // Complex query
+        if (is_array($query)) {
+            $params = [
                 'index' => $model->getIndexName(),
                 'type' => $model->getTypeName(),
-                'body' => $this->translateQuery($queryArray),
-            ]
-        );
+                'body' => $this->translateQuery($query),
+            ];
 
-        return $searchResults;
+        // Simple query
+        } else {
+            $params = $model->getBasicEsParams(true, true, true, $limit, $offset);
+            $params['body']['query'] = [
+                'match_phrase_prefix' => [
+                    '_all' => $query,
+                ],
+            ];
+        }
+
+        return $this->customSearchConditions($params);
+    }
+
+    /**
+     * Method for adding custom search conditions by overriding in controllers.
+     * Should return array for elasticsearch request for complexSearch method.
+     */
+    protected function customSearchConditions($params)
+    {
+        return $params;
     }
 
     /**

diff --git a/Model/Model/IndexedModel.php b/Model/Model/IndexedModel.php
@@ -123,20 +123,23 @@ public function getIndexDocumentData()
             foreach ($this->indexRelations as $nestedModelName) {
                 /** @var IndexedModel|IndexedCollection $results */
                 $results = $this->$nestedModelName()->getResults();
-                if ($results instanceof Collection) {
+
+                if (is_null($results)) {
+                    break;
+                } elseif ($results instanceof Collection) {
                     $nestedData = $results->map(function (IndexedModel $result) {
-                        return array_intersect_key($result->attributesToArray(), $result->mappingProperties);
+                        return array_intersect_key($result->attributesToArray(), $result->getMappingProperties());
                     });
 
                     $relations[snake_case($nestedModelName)] = $nestedData;
                 } else {
-                    $relations[snake_case($nestedModelName)] = array_intersect_key($results->attributesToArray(), $results->mappingProperties);
+                    $relations[snake_case($nestedModelName)] = array_intersect_key($results->attributesToArray(), $results->getMappingProperties());
                 }
             }
         }
 
         // Only include attributes present in mappingProperties
-        $attributes = array_intersect_key($this->attributesToArray(), $this->mappingProperties);
+        $attributes = array_intersect_key($this->attributesToArray(), $this->getMappingProperties());
 
         return array_merge($attributes, $relations);
     }

diff --git a/tests/integration/EntityTest.php b/tests/integration/EntityTest.php
@@ -57,6 +57,48 @@ protected function addRelatedEntities($model)
         });
     }
 
+    public function dataCustomSearchApplied()
+    {
+        return [
+            ['body.query.match_phrase_prefix._all', 'foobar'], // Simple search
+            ['body.query.bool.must.0.match_phrase_prefix.author_id', ['authorId' => ['foobar']]], // Complex search
+        ];
+    }
+
+    /**
+     * @dataProvider dataCustomSearchApplied
+     *
+     * @see \Spira\Core\tests\integration\TestController::customSearchConditions()
+     */
+    public function testCustomSearchApplied($arr_path, $query, $value = 'foobar')
+    {
+        // Force not found, we don't have to mock a success, just that 'searchByQuery' is called with the right params.
+        $resultsMock = Mockery::mock(ElasticquentResultCollection::class);
+        $resultsMock->shouldReceive('totalHits')->andReturn(0);
+
+        $mockModel = Mockery::mock(TestEntity::class)->makePartial();
+        $mockModel
+            ->shouldReceive('count')
+            ->andReturn(10)
+            ->shouldReceive('complexSearch')
+            ->with(Mockery::on(function ($arr) use ($arr_path, $value) {
+                return is_array($arr)
+                    && array_get($arr, $arr_path) == $value // Check does query processed correctly
+                    && $arr['custom_search'] == 'some_value'; // Check does custom rules are applied
+            }))
+            ->andReturn($resultsMock);
+
+        $this->app->instance(TestEntity::class, $mockModel);
+
+        $params = [
+            'q' => base64_encode(json_encode($query)),
+            'custom_search' => 1,
+        ];
+        $this->getJson('/test/entities/search?'.http_build_query($params), ['Range' => 'entities=0-']);
+
+        $this->assertResponseStatus(404);
+    }
+
     public function testSetTimeCarbon()
     {
         $entity = new TestEntity();
@@ -183,39 +225,48 @@ public function testGetAllPaginatedSimpleRange()
         $this->assertEquals($last, 19);
     }
 
-    public function testGetAllPaginatedSimpleSearch()
+    public function dataSearchPaths()
     {
-        // @todo wait for fix an issue with PHP 7.0.2 INF.0 https://github.com/padraic/mockery/issues/530
-        $this->markTestSkipped();
+        return [
+            ['/test/entities/pages'],
+            ['/test/entities/search'],
+        ];
+    }
 
+    /**
+     * @dataProvider dataSearchPaths
+     */
+    public function testGetAllPaginatedSimpleSearch($path)
+    {
         $resultsMock = Mockery::mock(ElasticquentResultCollection::class);
         $resultsMock->shouldReceive('totalHits')
             ->andReturn(0); // Force not found, we don't have to mock a success, just that 'searchByQuery' is called with the right params.
 
-        $mockModel = Mockery::mock(TestEntity::class);
+        $mockModel = Mockery::mock(TestEntity::class)->makePartial();
         $mockModel
             ->shouldReceive('count')
             ->andReturn(10)
-            ->shouldReceive('searchByQuery')
-            ->with([
-                'match_phrase_prefix' => [
-                    '_all' => 'foobar',
-                ],
-            ], null, null, 10, 0)
+            ->shouldReceive('complexSearch')
+            ->with(Mockery::on(function ($arr) {
+                    return is_array($arr)
+                        && $arr['size'] == 10
+                        && $arr['from'] == 0
+                        && $arr['body']['query']['match_phrase_prefix']['_all'] == 'foobar';
+                }))
             ->andReturn($resultsMock);
 
         $this->app->instance(TestEntity::class, $mockModel);
 
-        $this->getJson('/test/entities/pages?q='.base64_encode(json_encode('foobar')), ['Range' => 'entities=0-']);
+        $this->getJson($path.'?q='.base64_encode(json_encode('foobar')), ['Range' => 'entities=0-']);
 
         $this->assertResponseStatus(404);
     }
 
-    public function testGetAllPaginatedComplexSearch()
+    /**
+     * @dataProvider dataSearchPaths
+     */
+    public function testGetAllPaginatedComplexSearch($path)
     {
-        // @todo wait for fix an issue with PHP 7.0.2 INF.0 https://github.com/padraic/mockery/issues/530
-        $this->markTestSkipped();
-
         $resultsMock = Mockery::mock(ElasticquentResultCollection::class);
         $resultsMock->shouldReceive('totalHits')
             ->andReturn(0); // Force not found, we don't have to mock a success, just that 'searchByQuery' is called with the right params.
@@ -245,16 +296,16 @@ public function testGetAllPaginatedComplexSearch()
             ],
         ];
 
-        $this->getJson('/test/entities/pages?q='.base64_encode(json_encode($query)), ['Range' => 'entities=0-']);
+        $this->getJson($path.'?q='.base64_encode(json_encode($query)), ['Range' => 'entities=0-']);
 
         $this->assertResponseStatus(404);
     }
 
-    public function testGetAllPaginatedComplexSearchMatchAll()
+    /**
+     * @dataProvider dataSearchPaths
+     */
+    public function testGetAllPaginatedComplexSearchMatchAll($path)
     {
-        // @todo wait for fix an issue with PHP 7.0.2 INF.0 https://github.com/padraic/mockery/issues/530
-        $this->markTestSkipped();
-
         $results = $this->getFactory(TestEntity::class)->count(5)->make();
 
         $resultsMock = Mockery::mock(ElasticquentResultCollection::class);
@@ -291,11 +342,21 @@ public function testGetAllPaginatedComplexSearchMatchAll()
             'authorId' => [''],
         ];
 
-        $this->getJson('/test/entities/pages?q='.base64_encode(json_encode($query)), ['Range' => 'entities=0-']);
+        $this->getJson($path.'?q='.base64_encode(json_encode($query)), ['Range' => 'entities=0-']);
 
         $this->assertResponseStatus(206);
     }
 
+    public function testSearchPaginatedListingProhibitedError()
+    {
+        $this->getJson('/test/entities/search', ['Range' => 'entities=0-']);
+
+        $this->assertResponseStatus(Response::HTTP_BAD_REQUEST);
+        $result = $this->getJsonResponseAsArray();
+
+        $this->assertEquals('Items listing not allowed', $result['message']);
+    }
+
     public function testPaginationBadRanges()
     {
         $this->getFactory(TestEntity::class)->count(20)->create();

diff --git a/tests/integration/TestController.php b/tests/integration/TestController.php
@@ -10,6 +10,7 @@
 
 namespace Spira\Core\tests\integration;
 
+use Illuminate\Http\Request;
 use Spira\Core\Controllers\EntityController;
 use Spira\Core\Controllers\LocalizableTrait;
 use Spira\Core\Model\Test\TestEntity;
@@ -38,6 +39,18 @@ public function cors()
         return $this->getResponse()->item(['foo' => 'bar']);
     }
 
+    protected function customSearchConditions($params)
+    {
+        /** @var Request $request */
+        $request = app(Request::class);
+
+        if ($request->has('custom_search')) {
+            $params['custom_search'] = 'some_value'; // Simulation of applying custom rules for elasticsearch
+        }
+
+        return $params;
+    }
+
     /**
      * Test a standard internal exception.
      */

diff --git a/tests/integration/test_routes.php b/tests/integration/test_routes.php
@@ -47,6 +47,7 @@
     $app->get('test/fatal-error', 'TestController@fatalError');
     $app->get('test/entities', 'TestController@getAll');
     $app->get('test/entities/pages', 'TestController@getAllPaginated');
+    $app->get('test/entities/search', 'TestController@searchPaginated');
     $app->get('test/entities_encoded/{id}', 'TestController@urlEncode');
     $app->get('test/entities/{id}', ['as' => TestEntity::class, 'uses' => 'TestController@getOne']);
     $app->get('test/entities-second/{id}', ['as' => SecondTestEntity::class, 'uses' => 'TestController@getOne']);