From 39493a8c7db008626148971d6b4936d4dcd64a57 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:26:14 +0200 Subject: [PATCH 01/25] Search coverage cases --- src/Commands/stubs/repository.stub | 29 ---- src/Repositories/Repository.php | 8 - src/Services/Search/SearchService.php | 69 +++++---- src/Traits/InteractWithSearch.php | 2 +- .../RepositoryIndexControllerTest.php | 138 +++++++++++++++++- tests/Factories/PostFactory.php | 22 +++ tests/Fixtures/Post.php | 19 +++ tests/Fixtures/User.php | 14 +- tests/InteractWithModels.php | 35 ++++- .../2019_12_22_000005_create_posts_table.php | 34 +++++ 10 files changed, 289 insertions(+), 81 deletions(-) create mode 100644 tests/Factories/PostFactory.php create mode 100644 tests/Fixtures/Post.php create mode 100644 tests/Migrations/2019_12_22_000005_create_posts_table.php diff --git a/src/Commands/stubs/repository.stub b/src/Commands/stubs/repository.stub index c823db513..d61bda223 100644 --- a/src/Commands/stubs/repository.stub +++ b/src/Commands/stubs/repository.stub @@ -13,33 +13,4 @@ class DummyClass extends Repository * @var string */ public static $model = 'DummyFullModel'; - - /** - * The single value that should be used to represent the repository when being displayed. - * - * @var string - */ - public static $title = 'id'; - - /** - * The columns that should be searched. - * - * @var array - */ - public static $search = [ - 'id', - ]; - - /** - * Get the fields displayed by the repository. - * - * @param \Illuminate\Http\Request $request - * @return array - */ - public function fields(Request $request) - { - return [ - ID::make()->sortable(), - ]; - } } diff --git a/src/Repositories/Repository.php b/src/Repositories/Repository.php index 492b51a96..29ff1b3ba 100644 --- a/src/Repositories/Repository.php +++ b/src/Repositories/Repository.php @@ -30,14 +30,6 @@ public function __construct($resource) $this->modelInstance = $resource; } - /** - * Get the fields displayed by the resource. - * - * @param \Illuminate\Http\Request $request - * @return array - */ - abstract public function fields(Request $request); - /** * Get the underlying model instance for the resource. * diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index d810641f0..ef9d2e981 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -39,7 +39,7 @@ public function search(RestifyRequest $request, Model $model) protected function prepare() { $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) - ->prepareMatchFields($this->request->get('match', [])) + ->prepareMatchFields() ->prepareIn($this->request->get('in', [])) ->prepareOperator($this->request->get('operator', [])) ->prepareOrders($this->request->get('sort', '')) @@ -78,6 +78,7 @@ protected function prepareIn($fields) foreach ($value as $val) { switch ($this->model::getInFields()[$key]) { case 'integer': + case 'int': default: $this->builder->whereIn($this->model->qualifyColumn($field), explode(',', $val)); break; @@ -86,6 +87,7 @@ protected function prepareIn($fields) } elseif (is_array($value) === false && isset($this->model::getInFields()[$key]) === true) { switch ($this->model::getInFields()[$key]) { case 'integer': + case 'int': default: $this->builder->whereIn($this->model->qualifyColumn($field), explode(',', $value)); break; @@ -142,44 +144,40 @@ protected function prepareOperator($fields) * * @return $this */ - protected function prepareMatchFields($fields) + protected function prepareMatchFields() { - if (isset($this->fixedInput['match']) === true) { - if (is_array($fields) === false) { - $fields = $this->fixedInput['match']; - } else { - $fields = array_merge($this->fixedInput['match'], $fields); + foreach($this->model::getMatchByFields() as $key => $type) { + if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + continue; } - } - if (is_array($fields) === true) { - foreach ($fields as $key => $value) { - if (isset($this->model::getMatchByFields()[$key]) === true) { - $field = $this->model->qualifyColumn($key); - - $values = explode(',', $value); - foreach ($values as $match) { - switch ($this->model::getMatchByFields()[$key]) { - case RestifySearchable::MATCH_TEXT: - $this->builder->where($field, '=', $match); - break; - case RestifySearchable::MATCH_BOOL: - case 'boolean': - if ($match === 'false') { - $this->builder->where(function ($query) use ($field) { - return $query->where($field, '=', false)->orWhereNull($field); - }); - break; - } - $this->builder->where($field, '=', true); - break; - case RestifySearchable::MATCH_INTEGER: - case 'number': - case 'int': - $this->builder->where($field, '=', (int) $match); - break; + $value = $this->request->get($key) ?: data_get($this->fixedInput, "match.$key"); + + $field = $this->model->qualifyColumn($key); + + $values = explode(',', $value); + + foreach ($values as $match) { + switch ($this->model::getMatchByFields()[$key]) { + case RestifySearchable::MATCH_TEXT: + case 'string': + $this->builder->where($field, '=', $match); + break; + case RestifySearchable::MATCH_BOOL: + case 'boolean': + if ($match === 'false') { + $this->builder->where(function ($query) use ($field) { + return $query->where($field, '=', false)->orWhereNull($field); + }); + break; } - } + $this->builder->where($field, '=', true); + break; + case RestifySearchable::MATCH_INTEGER: + case 'number': + case 'int': + $this->builder->where($field, '=', (int) $match); + break; } } } @@ -313,6 +311,7 @@ public function setOrder($param) if ($order === '-') { $this->builder->orderBy($field, 'desc'); } + if ($order === '+') { $this->builder->orderBy($field, 'asc'); } diff --git a/src/Traits/InteractWithSearch.php b/src/Traits/InteractWithSearch.php index 59002a6cc..f6133b5fd 100644 --- a/src/Traits/InteractWithSearch.php +++ b/src/Traits/InteractWithSearch.php @@ -50,7 +50,7 @@ public static function getMatchByFields() */ public static function getOrderByFields() { - return static::$order ?? []; + return static::$sort ?? []; } /** diff --git a/tests/Controllers/RepositoryIndexControllerTest.php b/tests/Controllers/RepositoryIndexControllerTest.php index 36d97c1bf..68ac6b001 100644 --- a/tests/Controllers/RepositoryIndexControllerTest.php +++ b/tests/Controllers/RepositoryIndexControllerTest.php @@ -13,7 +13,7 @@ class RepositoryIndexControllerTest extends IntegrationTest { public function test_list_resource() - { + { factory(User::class)->create(); factory(User::class)->create(); factory(User::class)->create(); @@ -46,7 +46,7 @@ public function users() $this->assertEquals(count($class->users()->getData()->data->data), User::$defaultPerPage); } - public function test_per_page() + public function test_that_default_per_page_works() { User::$defaultPerPage = 40; $this->mockUsers(50); @@ -68,4 +68,138 @@ public function users() $this->assertEquals(count($class->users()->getData()->data->data), 40); User::$defaultPerPage = RestifySearchable::DEFAULT_PER_PAGE; } + + public function test_search_query_works() + { + $users = $this->mockUsers(10, ['eduard.lupacescu@binarcode.com']); + $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(request()); + $this->withExceptionHandling() + ->getJson('/restify-api/users?search=eduard.lupacescu@binarcode.com') + ->assertStatus(200) + ->assertJson([ + 'data' => [ + 'data' => [$expected], + 'current_page' => 1, + 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'from' => 1, + 'last_page' => 1, + 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'next_page_url' => null, + 'path' => "http://localhost/restify-api/users", + 'per_page' => 15, + 'prev_page_url' => null, + 'to' => 1, + 'total' => 1, + ], + 'errors' => [], + ]); + + $this->withExceptionHandling() + ->getJson('/restify-api/users?search=some_unexpected_string_here') + ->assertStatus(200) + ->assertJson([ + 'data' => [ + 'data' => [], + 'current_page' => 1, + 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'from' => 1, + 'last_page' => 1, + 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'next_page_url' => null, + 'path' => "http://localhost/restify-api/users", + 'per_page' => 15, + 'prev_page_url' => null, + 'to' => 1, + 'total' => 1, + ], + 'errors' => [], + ]); + } + + public function test_that_desc_sort_query_param_works() + { + $this->mockUsers(10); + $response = $this->withExceptionHandling()->get('/restify-api/users?sort=-id') + ->assertStatus(200) + ->getOriginalContent(); + + $this->assertSame($response->data['data'][0]['id'], 10); + $this->assertSame($response->data['data'][9]['id'], 1); + + + } + + public function test_that_asc_sort_query_param_works() + { + $this->mockUsers(10); + + $response = $this->withExceptionHandling()->get('/restify-api/users?sort=+id') + ->assertStatus(200) + ->getOriginalContent(); + + $this->assertSame($response->data['data'][0]['id'], 1); + $this->assertSame($response->data['data'][9]['id'], 10); + + $response = $this->withExceptionHandling()->get('/restify-api/users?sort=id')//assert default ASC sort + ->assertStatus(200) + ->getOriginalContent(); + + $this->assertSame($response->data['data'][0]['id'], 1); + $this->assertSame($response->data['data'][9]['id'], 10); + } + + public function test_that_default_asc_sort_query_param_works() + { + $this->mockUsers(10); + + $response = $this->withExceptionHandling()->get('/restify-api/users?sort=id') + ->assertStatus(200) + ->getOriginalContent(); + + $this->assertSame($response->data['data'][0]['id'], 1); + $this->assertSame($response->data['data'][9]['id'], 10); + } + + public function test_that_match_param_works() + { + User::$match = ['email' => RestifySearchable::MATCH_TEXT]; // it will automatically filter over these queries (email='test@email.com') + $users = $this->mockUsers(10, ['eduard.lupacescu@binarcode.com']); + $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(request()); + + $this->withExceptionHandling() + ->get('/restify-api/users?email=eduard.lupacescu@binarcode.com') + ->assertStatus(200) + ->assertJson([ + 'data' => [ + 'data' => [$expected], + 'current_page' => 1, + 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'from' => 1, + 'last_page' => 1, + 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'next_page_url' => null, + 'path' => "http://localhost/restify-api/users", + 'per_page' => 15, + 'prev_page_url' => null, + 'to' => 1, + 'total' => 1, + ], + 'errors' => [], + ]); + } + + public function test_that_with_param_works() + { + User::$match = ['email' => RestifySearchable::MATCH_TEXT]; // it will automatically filter over these queries (email='test@email.com') + $users = $this->mockUsers(1); + $posts = $this->mockPosts(1, 2); + $expected = $users->first()->serializeForIndex(request()); + $expected['posts'] = $posts->toArray(); + $r = $this->withExceptionHandling() + ->get('/restify-api/users?with=posts') + ->assertStatus(200) + ->getOriginalContent(); + + $this->assertSameSize($r->data['data']->first()['posts'], $expected['posts']); + } } diff --git a/tests/Factories/PostFactory.php b/tests/Factories/PostFactory.php new file mode 100644 index 000000000..6894c5c32 --- /dev/null +++ b/tests/Factories/PostFactory.php @@ -0,0 +1,22 @@ +define(Binaryk\LaravelRestify\Tests\Fixtures\Post::class, function (Faker $faker) { + return [ + 'title' => $faker->title, + 'description' => $faker->text, + ]; +}); diff --git a/tests/Fixtures/Post.php b/tests/Fixtures/Post.php new file mode 100644 index 000000000..58710f20f --- /dev/null +++ b/tests/Fixtures/Post.php @@ -0,0 +1,19 @@ + + */ +class Post extends Model +{ + protected $fillable = [ + 'id', + 'user_id', + 'title', + 'description' + ]; +} diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php index f747abfac..fe2ee0139 100644 --- a/tests/Fixtures/User.php +++ b/tests/Fixtures/User.php @@ -20,10 +20,11 @@ class User extends Authenticatable implements Passportable, MustVerifyEmail, Res use Notifiable, InteractWithSearch; - public static $search = ['id']; - public static $match = [ - 'id' => 'int', - ]; + public static $search = ['id', 'email']; + public static $sort = ['id',]; + public static $match = ['id' => 'int', 'email' => 'string',]; + public static $in = ['id' => 'int',]; + public static $withs = ['posts']; /** * The attributes that are mass assignable. @@ -71,4 +72,9 @@ public function tokens() { return Mockery::mock(Builder::class); } + + public function posts() + { + return $this->hasMany(Post::class); + } } diff --git a/tests/InteractWithModels.php b/tests/InteractWithModels.php index 970625aac..80292d118 100644 --- a/tests/InteractWithModels.php +++ b/tests/InteractWithModels.php @@ -2,6 +2,7 @@ namespace Binaryk\LaravelRestify\Tests; +use Binaryk\LaravelRestify\Tests\Fixtures\Post; use Binaryk\LaravelRestify\Tests\Fixtures\User; /** @@ -9,7 +10,12 @@ */ trait InteractWithModels { - public function mockUsers($count = 1) + /** + * @param int $count + * @param array $predefinedEmails + * @return \Illuminate\Support\Collection + */ + public function mockUsers($count = 1, $predefinedEmails = []) { $users = collect([]); $i = 0; @@ -18,6 +24,31 @@ public function mockUsers($count = 1) $i++; } - return $users; + foreach($predefinedEmails as $email) { + $users->push(factory(User::class)->create([ + 'email' => $email, + ])); + } + + return $users->shuffle(); // randomly shuffles the items in the collection + } + + /** + * @param $userId + * @param int $count + * @return \Illuminate\Support\Collection + */ + public function mockPosts($userId, $count = 1) + { + $users = collect([]); + $i = 0; + while ($i < $count) { + $users->push(factory(Post::class)->create( + ['user_id' => $userId,] + )); + $i++; + } + + return $users->shuffle(); // randomly shuffles the items in the collection } } diff --git a/tests/Migrations/2019_12_22_000005_create_posts_table.php b/tests/Migrations/2019_12_22_000005_create_posts_table.php new file mode 100644 index 000000000..b6a7b6a94 --- /dev/null +++ b/tests/Migrations/2019_12_22_000005_create_posts_table.php @@ -0,0 +1,34 @@ +increments('id'); + $table->unsignedInteger('user_id')->index(); + $table->string('title'); + $table->longText('description'); + $table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('password_resets'); + } +} From b28ba0020ba44a307952b83fbc787f5ca08c2800 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:26:33 +0200 Subject: [PATCH 02/25] Apply fixes from StyleCI (#49) --- src/Controllers/RestController.php | 2 -- .../Requests/InteractWithRepositories.php | 3 +- src/Http/Requests/RestifyRequest.php | 1 - src/Repositories/Repository.php | 1 - src/Services/Search/SearchService.php | 32 +++++++++---------- src/Services/Search/Searchable.php | 6 +--- src/Traits/AuthorizableModels.php | 4 +-- .../RepositoryIndexControllerTest.php | 23 ++++++------- tests/Factories/PostFactory.php | 1 - tests/Fixtures/Post.php | 3 +- tests/Fixtures/User.php | 6 ++-- tests/InteractWithModels.php | 4 +-- 12 files changed, 36 insertions(+), 50 deletions(-) diff --git a/src/Controllers/RestController.php b/src/Controllers/RestController.php index 39f2b530d..eae58dfc2 100644 --- a/src/Controllers/RestController.php +++ b/src/Controllers/RestController.php @@ -125,7 +125,6 @@ protected function response($data = null, $status = 200, array $headers = []) return $this->response; } - /** * @param $modelClass * @param array $filters @@ -144,7 +143,6 @@ public function search($modelClass, $filters = []) $paginator = $results->paginate($this->request()->get('perPage') ?? ($modelClass::$defaultPerPage ?? RestifySearchable::DEFAULT_PER_PAGE)); $items = $paginator->getCollection()->map->serializeForIndex($this->request()); - return array_merge($paginator->toArray(), [ 'data' => $items, ]); diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index d86a2fb10..164827970 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -9,7 +9,6 @@ use Illuminate\Database\Eloquent\Model; /** - * @package Binaryk\LaravelRestify\Http\Requests; * @author Eduard Lupacescu */ trait InteractWithRepositories @@ -45,7 +44,7 @@ public function repository() ]), 404); } - if ( ! $repository::authorizedToViewAny($this)) { + if (! $repository::authorizedToViewAny($this)) { throw new UnauthorizedException(__('Unauthorized to view repository :name.', [ 'name' => $repository, ]), 403); diff --git a/src/Http/Requests/RestifyRequest.php b/src/Http/Requests/RestifyRequest.php index d57f75a34..711031f43 100644 --- a/src/Http/Requests/RestifyRequest.php +++ b/src/Http/Requests/RestifyRequest.php @@ -10,5 +10,4 @@ class RestifyRequest extends FormRequest { use InteractWithRepositories; - } diff --git a/src/Repositories/Repository.php b/src/Repositories/Repository.php index 29ff1b3ba..9e16c2bc8 100644 --- a/src/Repositories/Repository.php +++ b/src/Repositories/Repository.php @@ -5,7 +5,6 @@ use Binaryk\LaravelRestify\Traits\AuthorizableModels; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; -use Illuminate\Http\Request; use Illuminate\Support\Str; /** diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index ef9d2e981..f8d670484 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -32,7 +32,7 @@ public function search(RestifyRequest $request, Model $model) } /** - * Will prepare the eloquent array to return + * Will prepare the eloquent array to return. * * @return array */ @@ -54,7 +54,7 @@ protected function prepare() } /** - * Prepare eloquent exact fields + * Prepare eloquent exact fields. * * @param $fields * @@ -100,7 +100,7 @@ protected function prepareIn($fields) } /** - * Prepare eloquent exact fields + * Prepare eloquent exact fields. * * @param $fields * @@ -117,16 +117,16 @@ protected function prepareOperator($fields) foreach ($values as $field => $value) { $qualifiedField = $this->model->qualifyColumn($field); switch ($key) { - case "gte": + case 'gte': $this->builder->where($qualifiedField, '>=', $value); break; - case "gt": + case 'gt': $this->builder->where($qualifiedField, '>', $value); break; - case "lte": + case 'lte': $this->builder->where($qualifiedField, '<=', $value); break; - case "lt": + case 'lt': $this->builder->where($qualifiedField, '<', $value); break; } @@ -138,7 +138,7 @@ protected function prepareOperator($fields) } /** - * Prepare eloquent exact fields + * Prepare eloquent exact fields. * * @param $fields * @@ -146,7 +146,7 @@ protected function prepareOperator($fields) */ protected function prepareMatchFields() { - foreach($this->model::getMatchByFields() as $key => $type) { + foreach ($this->model::getMatchByFields() as $key => $type) { if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -186,7 +186,7 @@ protected function prepareMatchFields() } /** - * Prepare eloquent order by + * Prepare eloquent order by. * * @param $sort * @@ -214,7 +214,7 @@ protected function prepareOrders($sort) } /** - * Prepare relations + * Prepare relations. * * @return $this */ @@ -243,7 +243,7 @@ protected function prepareRelations() } /** - * Prepare search + * Prepare search. * * @param $search * @return $this @@ -252,7 +252,7 @@ protected function prepareSearchFields($search) { $this->builder->where(function (Builder $query) use ($search) { /** - * @var RestifySearchable|Model $model + * @var RestifySearchable|Model */ $model = $query->getModel(); @@ -263,7 +263,6 @@ protected function prepareSearchFields($search) ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && in_array($query->getModel()->getKeyName(), $model::getSearchableFields()); - if ($canSearchPrimaryKey) { $query->orWhere($query->getModel()->getQualifiedKeyName(), $search); } @@ -271,7 +270,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); @@ -279,7 +278,7 @@ protected function prepareSearchFields($search) } /** - * Set order + * Set order. * * @param $param * @@ -289,6 +288,7 @@ public function setOrder($param) { if ($param === 'random') { $this->builder->inRandomOrder(); + return $this; } diff --git a/src/Services/Search/Searchable.php b/src/Services/Search/Searchable.php index 87e880b62..35ca074c1 100644 --- a/src/Services/Search/Searchable.php +++ b/src/Services/Search/Searchable.php @@ -6,9 +6,6 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Http\Request; -/** - * @package Binaryk\LaravelRestify\Services\Search; - */ abstract class Searchable { /** @@ -17,7 +14,7 @@ abstract class Searchable protected $request; /** - * @var RestifySearchable|Model $model + * @var RestifySearchable|Model */ protected $model; @@ -26,7 +23,6 @@ abstract class Searchable */ protected $fixedInput; - /** * @param $input * diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 319ee36b9..9580f2381 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -9,8 +9,8 @@ use Illuminate\Support\Str; /** - * Could be used as a trait in a model class and in a repository class - * + * Could be used as a trait in a model class and in a repository class. + * * @author Eduard Lupacescu */ trait AuthorizableModels diff --git a/tests/Controllers/RepositoryIndexControllerTest.php b/tests/Controllers/RepositoryIndexControllerTest.php index 68ac6b001..c60496309 100644 --- a/tests/Controllers/RepositoryIndexControllerTest.php +++ b/tests/Controllers/RepositoryIndexControllerTest.php @@ -13,7 +13,7 @@ class RepositoryIndexControllerTest extends IntegrationTest { public function test_list_resource() - { + { factory(User::class)->create(); factory(User::class)->create(); factory(User::class)->create(); @@ -24,7 +24,6 @@ public function test_list_resource() $response->assertJsonCount(3, 'data.data'); } - public function test_the_rest_controller_can_paginate() { $this->mockUsers(50); @@ -80,12 +79,12 @@ public function test_search_query_works() 'data' => [ 'data' => [$expected], 'current_page' => 1, - 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'first_page_url' => 'http://localhost/restify-api/users?page=1', 'from' => 1, 'last_page' => 1, - 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'last_page_url' => 'http://localhost/restify-api/users?page=1', 'next_page_url' => null, - 'path' => "http://localhost/restify-api/users", + 'path' => 'http://localhost/restify-api/users', 'per_page' => 15, 'prev_page_url' => null, 'to' => 1, @@ -101,12 +100,12 @@ public function test_search_query_works() 'data' => [ 'data' => [], 'current_page' => 1, - 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'first_page_url' => 'http://localhost/restify-api/users?page=1', 'from' => 1, 'last_page' => 1, - 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'last_page_url' => 'http://localhost/restify-api/users?page=1', 'next_page_url' => null, - 'path' => "http://localhost/restify-api/users", + 'path' => 'http://localhost/restify-api/users', 'per_page' => 15, 'prev_page_url' => null, 'to' => 1, @@ -125,8 +124,6 @@ public function test_that_desc_sort_query_param_works() $this->assertSame($response->data['data'][0]['id'], 10); $this->assertSame($response->data['data'][9]['id'], 1); - - } public function test_that_asc_sort_query_param_works() @@ -173,12 +170,12 @@ public function test_that_match_param_works() 'data' => [ 'data' => [$expected], 'current_page' => 1, - 'first_page_url' => "http://localhost/restify-api/users?page=1", + 'first_page_url' => 'http://localhost/restify-api/users?page=1', 'from' => 1, 'last_page' => 1, - 'last_page_url' => "http://localhost/restify-api/users?page=1", + 'last_page_url' => 'http://localhost/restify-api/users?page=1', 'next_page_url' => null, - 'path' => "http://localhost/restify-api/users", + 'path' => 'http://localhost/restify-api/users', 'per_page' => 15, 'prev_page_url' => null, 'to' => 1, diff --git a/tests/Factories/PostFactory.php b/tests/Factories/PostFactory.php index 6894c5c32..95ceef360 100644 --- a/tests/Factories/PostFactory.php +++ b/tests/Factories/PostFactory.php @@ -1,7 +1,6 @@ */ class Post extends Model @@ -14,6 +13,6 @@ class Post extends Model 'id', 'user_id', 'title', - 'description' + 'description', ]; } diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php index fe2ee0139..b71e68a8a 100644 --- a/tests/Fixtures/User.php +++ b/tests/Fixtures/User.php @@ -21,9 +21,9 @@ class User extends Authenticatable implements Passportable, MustVerifyEmail, Res InteractWithSearch; public static $search = ['id', 'email']; - public static $sort = ['id',]; - public static $match = ['id' => 'int', 'email' => 'string',]; - public static $in = ['id' => 'int',]; + public static $sort = ['id']; + public static $match = ['id' => 'int', 'email' => 'string']; + public static $in = ['id' => 'int']; public static $withs = ['posts']; /** diff --git a/tests/InteractWithModels.php b/tests/InteractWithModels.php index 80292d118..7aa7e5be0 100644 --- a/tests/InteractWithModels.php +++ b/tests/InteractWithModels.php @@ -24,7 +24,7 @@ public function mockUsers($count = 1, $predefinedEmails = []) $i++; } - foreach($predefinedEmails as $email) { + foreach ($predefinedEmails as $email) { $users->push(factory(User::class)->create([ 'email' => $email, ])); @@ -44,7 +44,7 @@ public function mockPosts($userId, $count = 1) $i = 0; while ($i < $count) { $users->push(factory(Post::class)->create( - ['user_id' => $userId,] + ['user_id' => $userId] )); $i++; } From ea9e8e5ac8957481e80fce7359e617eca7b5bb85 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:45:05 +0200 Subject: [PATCH 03/25] Clean up --- src/Exceptions/InstanceOfException.php | 14 ++++ .../Requests/InteractWithRepositories.php | 9 +++ src/Services/Search/SearchService.php | 80 +++++-------------- 3 files changed, 42 insertions(+), 61 deletions(-) create mode 100644 src/Exceptions/InstanceOfException.php diff --git a/src/Exceptions/InstanceOfException.php b/src/Exceptions/InstanceOfException.php new file mode 100644 index 000000000..50190b985 --- /dev/null +++ b/src/Exceptions/InstanceOfException.php @@ -0,0 +1,14 @@ + + */ +class InstanceOfException extends Exception +{ + +} diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index d86a2fb10..ff920b79e 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -64,4 +64,13 @@ public function rules() // ]; } + + /** + * Get the route handling the request. + * + * @param string|null $param + * @param mixed $default + * @return \Illuminate\Routing\Route|object|string + */ + abstract public function route($param = null, $default = null); } diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index ef9d2e981..f1349f3fc 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -3,14 +3,16 @@ namespace Binaryk\LaravelRestify\Services\Search; use Binaryk\LaravelRestify\Contracts\RestifySearchable; +use Binaryk\LaravelRestify\Exceptions\InstanceOfException; use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Query\Builder as QueryBuilder; class SearchService extends Searchable { /** - * @var Builder + * @var Builder|QueryBuilder */ protected $builder; @@ -18,6 +20,7 @@ class SearchService extends Searchable * @param RestifyRequest $request * @param Model $model * @return Builder + * @throws InstanceOfException */ public function search(RestifyRequest $request, Model $model) { @@ -35,15 +38,21 @@ public function search(RestifyRequest $request, Model $model) * Will prepare the eloquent array to return * * @return array + * @throws InstanceOfException */ protected function prepare() { - $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) - ->prepareMatchFields() - ->prepareIn($this->request->get('in', [])) - ->prepareOperator($this->request->get('operator', [])) - ->prepareOrders($this->request->get('sort', '')) - ->prepareRelations(); + if ($this->model instanceof RestifySearchable) { + $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) + ->prepareMatchFields() + ->prepareOperator($this->request->get('operator', [])) + ->prepareOrders($this->request->get('sort', '')) + ->prepareRelations(); + } else { + throw new InstanceOfException(__("Model is not an instance of :parent class", [ + 'parent' => RestifySearchable::class, + ])); + } $results = $this->builder->get(); @@ -53,52 +62,6 @@ protected function prepare() ]; } - /** - * Prepare eloquent exact fields - * - * @param $fields - * - * @return $this - */ - protected function prepareIn($fields) - { - if (isset($this->fixedInput['in']) === true) { - $fields = $this->fixedInput['in']; - } - - if (is_array($fields) === true) { - foreach ($fields as $key => $value) { - $field = $key; - - if ($field === null) { - continue; - } - - if (is_array($value) === true && isset($this->model::getInFields()[$key]) === true) { - foreach ($value as $val) { - switch ($this->model::getInFields()[$key]) { - case 'integer': - case 'int': - default: - $this->builder->whereIn($this->model->qualifyColumn($field), explode(',', $val)); - break; - } - } - } elseif (is_array($value) === false && isset($this->model::getInFields()[$key]) === true) { - switch ($this->model::getInFields()[$key]) { - case 'integer': - case 'int': - default: - $this->builder->whereIn($this->model->qualifyColumn($field), explode(',', $value)); - break; - } - } - } - } - - return $this; - } - /** * Prepare eloquent exact fields * @@ -251,17 +214,12 @@ protected function prepareRelations() protected function prepareSearchFields($search) { $this->builder->where(function (Builder $query) use ($search) { - /** - * @var RestifySearchable|Model $model - */ - $model = $query->getModel(); - - $connectionType = $query->getModel()->getConnection()->getDriverName(); + $connectionType = $this->model->getConnection()->getDriverName(); $canSearchPrimaryKey = is_numeric($search) && in_array($query->getModel()->getKeyType(), ['int', 'integer']) && ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && - in_array($query->getModel()->getKeyName(), $model::getSearchableFields()); + in_array($query->getModel()->getKeyName(), $this->model::getSearchableFields()); if ($canSearchPrimaryKey) { @@ -271,7 +229,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); } }); From 086f464fa7d0a2a499886ff3c6c440ed92cd9de2 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:45:42 +0200 Subject: [PATCH 04/25] Apply fixes from StyleCI (#50) --- src/Exceptions/InstanceOfException.php | 2 -- src/Services/Search/SearchService.php | 30 +++++++++++++------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/Exceptions/InstanceOfException.php b/src/Exceptions/InstanceOfException.php index 50190b985..a26d0f1c0 100644 --- a/src/Exceptions/InstanceOfException.php +++ b/src/Exceptions/InstanceOfException.php @@ -5,10 +5,8 @@ use Exception; /** - * @package Binaryk\LaravelRestify\Exceptions; * @author Eduard Lupacescu */ class InstanceOfException extends Exception { - } diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index f1349f3fc..fc60b5ade 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -35,7 +35,7 @@ public function search(RestifyRequest $request, Model $model) } /** - * Will prepare the eloquent array to return + * Will prepare the eloquent array to return. * * @return array * @throws InstanceOfException @@ -49,7 +49,7 @@ protected function prepare() ->prepareOrders($this->request->get('sort', '')) ->prepareRelations(); } else { - throw new InstanceOfException(__("Model is not an instance of :parent class", [ + throw new InstanceOfException(__('Model is not an instance of :parent class', [ 'parent' => RestifySearchable::class, ])); } @@ -63,7 +63,7 @@ protected function prepare() } /** - * Prepare eloquent exact fields + * Prepare eloquent exact fields. * * @param $fields * @@ -80,16 +80,16 @@ protected function prepareOperator($fields) foreach ($values as $field => $value) { $qualifiedField = $this->model->qualifyColumn($field); switch ($key) { - case "gte": + case 'gte': $this->builder->where($qualifiedField, '>=', $value); break; - case "gt": + case 'gt': $this->builder->where($qualifiedField, '>', $value); break; - case "lte": + case 'lte': $this->builder->where($qualifiedField, '<=', $value); break; - case "lt": + case 'lt': $this->builder->where($qualifiedField, '<', $value); break; } @@ -101,7 +101,7 @@ protected function prepareOperator($fields) } /** - * Prepare eloquent exact fields + * Prepare eloquent exact fields. * * @param $fields * @@ -109,7 +109,7 @@ protected function prepareOperator($fields) */ protected function prepareMatchFields() { - foreach($this->model::getMatchByFields() as $key => $type) { + foreach ($this->model::getMatchByFields() as $key => $type) { if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -149,7 +149,7 @@ protected function prepareMatchFields() } /** - * Prepare eloquent order by + * Prepare eloquent order by. * * @param $sort * @@ -177,7 +177,7 @@ protected function prepareOrders($sort) } /** - * Prepare relations + * Prepare relations. * * @return $this */ @@ -206,7 +206,7 @@ protected function prepareRelations() } /** - * Prepare search + * Prepare search. * * @param $search * @return $this @@ -221,7 +221,6 @@ protected function prepareSearchFields($search) ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && in_array($query->getModel()->getKeyName(), $this->model::getSearchableFields()); - if ($canSearchPrimaryKey) { $query->orWhere($query->getModel()->getQualifiedKeyName(), $search); } @@ -229,7 +228,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); @@ -237,7 +236,7 @@ protected function prepareSearchFields($search) } /** - * Set order + * Set order. * * @param $param * @@ -247,6 +246,7 @@ public function setOrder($param) { if ($param === 'random') { $this->builder->inRandomOrder(); + return $this; } From 104c7fe7cc8a51707a9d16efec684696e9a097da Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:46:24 +0200 Subject: [PATCH 05/25] With instead of relation --- src/Services/Search/SearchService.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index f1349f3fc..b25af4961 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -185,11 +185,11 @@ protected function prepareRelations() { $relations = null; - if (isset($this->fixedInput['relations']) === true) { - $relations = $this->fixedInput['relations']; + if (isset($this->fixedInput['with']) === true) { + $relations = $this->fixedInput['with']; } - if (isset($this->fixedInput['relations']) === false) { + if (isset($this->fixedInput['with']) === false) { $relations = $this->request->get('with', null); } From 799b9df00c6554e9c560445e8294ad648ab7ce2d Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:54:56 +0200 Subject: [PATCH 06/25] Throw if no model --- src/Traits/AuthorizableModels.php | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 9580f2381..05d77e8ca 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -4,6 +4,7 @@ use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Http\Request; use Illuminate\Support\Facades\Gate; use Illuminate\Support\Str; @@ -41,7 +42,7 @@ public static function authorizable() */ public function authorizeToViewAny(Request $request) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return; } @@ -58,7 +59,7 @@ public function authorizeToViewAny(Request $request) */ public static function authorizedToViewAny(Request $request) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return true; } @@ -198,11 +199,11 @@ public function authorizedToForceDelete(Request $request) */ public function authorizedToAdd(Request $request, $model) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return true; } - $method = 'add'.class_basename($model); + $method = 'add' . class_basename($model); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, $this->model()) @@ -218,11 +219,11 @@ public function authorizedToAdd(Request $request, $model) */ public function authorizedToAttachAny(Request $request, $model) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return true; } - $method = 'attachAny'.Str::singular(class_basename($model)); + $method = 'attachAny' . Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model()]) @@ -238,11 +239,11 @@ public function authorizedToAttachAny(Request $request, $model) */ public function authorizedToAttach(Request $request, $model) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return true; } - $method = 'attach'.Str::singular(class_basename($model)); + $method = 'attach' . Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model(), $model]) @@ -259,11 +260,11 @@ public function authorizedToAttach(Request $request, $model) */ public function authorizedToDetach(Request $request, $model, $relationship) { - if (! static::authorizable()) { + if ( ! static::authorizable()) { return true; } - $method = 'detach'.Str::singular(class_basename($model)); + $method = 'detach' . Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model(), $model]) @@ -299,9 +300,14 @@ public function authorizedTo(Request $request, $ability) /** * @return AuthorizableModels|Model|mixed + * @throws \Throwable */ public function determineModel() { - return $this instanceof Model ? $this : $this->modelInstance; + $model = $this instanceof Model ? $this : ($this->modelInstance ?? null); + + throw_if(is_null($model), new ModelNotFoundException(__('Model does not declared in :class', ['class' => self::class,]))); + + return $model; } } From f2ac5ad08a2e257642526b32ac1ccaf8d9069112 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:55:11 +0200 Subject: [PATCH 07/25] Apply fixes from StyleCI (#51) --- src/Traits/AuthorizableModels.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 05d77e8ca..8c3270d74 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -42,7 +42,7 @@ public static function authorizable() */ public function authorizeToViewAny(Request $request) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return; } @@ -59,7 +59,7 @@ public function authorizeToViewAny(Request $request) */ public static function authorizedToViewAny(Request $request) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return true; } @@ -199,11 +199,11 @@ public function authorizedToForceDelete(Request $request) */ public function authorizedToAdd(Request $request, $model) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return true; } - $method = 'add' . class_basename($model); + $method = 'add'.class_basename($model); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, $this->model()) @@ -219,11 +219,11 @@ public function authorizedToAdd(Request $request, $model) */ public function authorizedToAttachAny(Request $request, $model) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return true; } - $method = 'attachAny' . Str::singular(class_basename($model)); + $method = 'attachAny'.Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model()]) @@ -239,11 +239,11 @@ public function authorizedToAttachAny(Request $request, $model) */ public function authorizedToAttach(Request $request, $model) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return true; } - $method = 'attach' . Str::singular(class_basename($model)); + $method = 'attach'.Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model(), $model]) @@ -260,11 +260,11 @@ public function authorizedToAttach(Request $request, $model) */ public function authorizedToDetach(Request $request, $model, $relationship) { - if ( ! static::authorizable()) { + if (! static::authorizable()) { return true; } - $method = 'detach' . Str::singular(class_basename($model)); + $method = 'detach'.Str::singular(class_basename($model)); return method_exists(Gate::getPolicyFor($this->model()), $method) ? Gate::check($method, [$this->model(), $model]) @@ -306,7 +306,7 @@ public function determineModel() { $model = $this instanceof Model ? $this : ($this->modelInstance ?? null); - throw_if(is_null($model), new ModelNotFoundException(__('Model does not declared in :class', ['class' => self::class,]))); + throw_if(is_null($model), new ModelNotFoundException(__('Model does not declared in :class', ['class' => self::class]))); return $model; } From 3e48dc846d95741c4e1bfae20ea4aa460b7bdb4b Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:59:04 +0200 Subject: [PATCH 08/25] wip --- src/Services/Search/SearchService.php | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 12d957301..cde32867e 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -42,17 +42,15 @@ public function search(RestifyRequest $request, Model $model) */ protected function prepare() { - if ($this->model instanceof RestifySearchable) { - $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) - ->prepareMatchFields() - ->prepareOperator($this->request->get('operator', [])) - ->prepareOrders($this->request->get('sort', '')) - ->prepareRelations(); - } else { - throw new InstanceOfException(__('Model is not an instance of :parent class', [ - 'parent' => RestifySearchable::class, - ])); - } + throw_unless($this->model instanceof RestifySearchable, new InstanceOfException(__('Model is not an instance of :parent class', [ + 'parent' => RestifySearchable::class, + ]))); + + $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) + ->prepareMatchFields() + ->prepareOperator($this->request->get('operator', [])) + ->prepareOrders($this->request->get('sort', '')) + ->prepareRelations(); $results = $this->builder->get(); @@ -110,7 +108,7 @@ protected function prepareOperator($fields) protected function prepareMatchFields() { foreach ($this->model::getMatchByFields() as $key => $type) { - if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -228,7 +226,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); } }); From 87fa12bb52067cc79fe39d27bf969c8053610fad Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:59:19 +0200 Subject: [PATCH 09/25] Apply fixes from StyleCI (#53) --- src/Services/Search/SearchService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index cde32867e..a225167a6 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -108,7 +108,7 @@ protected function prepareOperator($fields) protected function prepareMatchFields() { foreach ($this->model::getMatchByFields() as $key => $type) { - if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -226,7 +226,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); From 78656f0be60c49f9f90deaf4964623bd1d4c5bbe Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 20:59:21 +0200 Subject: [PATCH 10/25] wip --- src/Services/Search/SearchService.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index cde32867e..35c1f6ecf 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -39,6 +39,7 @@ public function search(RestifyRequest $request, Model $model) * * @return array * @throws InstanceOfException + * @throws \Throwable */ protected function prepare() { From 09da9f567df87e072a8a0903e212349f902fba02 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 21:06:49 +0200 Subject: [PATCH 11/25] Add test when post no instance of searchable --- .../RepositoryIndexControllerTest.php | 8 +++++++ tests/Fixtures/PostRepository.php | 24 +++++++++++++++++++ tests/Fixtures/UserRepository.php | 8 ------- tests/IntegrationTest.php | 2 ++ 4 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 tests/Fixtures/PostRepository.php diff --git a/tests/Controllers/RepositoryIndexControllerTest.php b/tests/Controllers/RepositoryIndexControllerTest.php index c60496309..0e939353e 100644 --- a/tests/Controllers/RepositoryIndexControllerTest.php +++ b/tests/Controllers/RepositoryIndexControllerTest.php @@ -199,4 +199,12 @@ public function test_that_with_param_works() $this->assertSameSize($r->data['data']->first()['posts'], $expected['posts']); } + + public function test_should_throw_if_user_not_instance_of_searchable() + { + $this->withExceptionHandling() + ->get('/restify-api/posts') + ->assertStatus(500) + ->getOriginalContent(); + } } diff --git a/tests/Fixtures/PostRepository.php b/tests/Fixtures/PostRepository.php new file mode 100644 index 000000000..746c011b8 --- /dev/null +++ b/tests/Fixtures/PostRepository.php @@ -0,0 +1,24 @@ + + */ +class PostRepository extends Repository +{ + public static $model = Post::class; + + /** + * Get the URI key for the resource. + * + * @return string + */ + public static function uriKey() + { + return 'posts'; + } +} diff --git a/tests/Fixtures/UserRepository.php b/tests/Fixtures/UserRepository.php index 02e198b13..1a61aa016 100644 --- a/tests/Fixtures/UserRepository.php +++ b/tests/Fixtures/UserRepository.php @@ -12,14 +12,6 @@ class UserRepository extends Repository { public static $model = User::class; - /** - * {@inheritdoc} - */ - public function fields(Request $request) - { - return []; - } - /** * Get the URI key for the resource. * diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 98f86b272..e5fac821e 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -4,6 +4,7 @@ use Binaryk\LaravelRestify\LaravelRestifyServiceProvider; use Binaryk\LaravelRestify\Restify; +use Binaryk\LaravelRestify\Tests\Fixtures\PostRepository; use Binaryk\LaravelRestify\Tests\Fixtures\User; use Binaryk\LaravelRestify\Tests\Fixtures\UserRepository; use Illuminate\Contracts\Translation\Translator; @@ -34,6 +35,7 @@ protected function setUp(): void Restify::repositories([ UserRepository::class, + PostRepository::class, ]); } From 360339ce195a74553392be878a453860b33a2ba3 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Sun, 22 Dec 2019 21:07:04 +0200 Subject: [PATCH 12/25] Apply fixes from StyleCI (#54) --- tests/Fixtures/PostRepository.php | 1 - tests/Fixtures/UserRepository.php | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/Fixtures/PostRepository.php b/tests/Fixtures/PostRepository.php index 746c011b8..c551686ab 100644 --- a/tests/Fixtures/PostRepository.php +++ b/tests/Fixtures/PostRepository.php @@ -3,7 +3,6 @@ namespace Binaryk\LaravelRestify\Tests\Fixtures; use Binaryk\LaravelRestify\Repositories\Repository; -use Illuminate\Http\Request; /** * @author Eduard Lupacescu diff --git a/tests/Fixtures/UserRepository.php b/tests/Fixtures/UserRepository.php index 1a61aa016..57292911d 100644 --- a/tests/Fixtures/UserRepository.php +++ b/tests/Fixtures/UserRepository.php @@ -3,7 +3,6 @@ namespace Binaryk\LaravelRestify\Tests\Fixtures; use Binaryk\LaravelRestify\Repositories\Repository; -use Illuminate\Http\Request; /** * @author Eduard Lupacescu From 1c68a356bad75513306fc65eef9ca109acbab28d Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 12:08:23 +0200 Subject: [PATCH 13/25] Add extra repository --- src/Controllers/RestController.php | 2 ++ src/Http/Requests/InteractWithRepositories.php | 15 +++++++++++++++ src/Repositories/Repository.php | 13 +++++++------ src/Services/Search/SearchService.php | 4 ++-- src/Traits/AuthorizableModels.php | 12 ++++++++---- tests/Factories/PostFactory.php | 1 + tests/Fixtures/Post.php | 6 +++++- 7 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/Controllers/RestController.php b/src/Controllers/RestController.php index eae58dfc2..a6a2c3465 100644 --- a/src/Controllers/RestController.php +++ b/src/Controllers/RestController.php @@ -5,6 +5,7 @@ use Binaryk\LaravelRestify\Contracts\RestifySearchable; use Binaryk\LaravelRestify\Exceptions\Guard\EntityNotFoundException; use Binaryk\LaravelRestify\Exceptions\Guard\GatePolicy; +use Binaryk\LaravelRestify\Exceptions\InstanceOfException; use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Binaryk\LaravelRestify\Services\Search\SearchService; use Binaryk\LaravelRestify\Traits\PerformsQueries; @@ -130,6 +131,7 @@ protected function response($data = null, $status = 200, array $headers = []) * @param array $filters * @return array * @throws BindingResolutionException + * @throws InstanceOfException */ public function search($modelClass, $filters = []) { diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index b27f31e23..a5ab05d03 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -72,4 +72,19 @@ public function rules() * @return \Illuminate\Routing\Route|object|string */ abstract public function route($param = null, $default = null); + + /** + * Get a new instance of the repository being requested. + * + * @return Repository + * @throws EntityNotFoundException + * @throws UnauthorizedException + */ + public function newRepository() + { + $repository = $this->repository(); + + return new $repository($repository::newModel()); + } + } diff --git a/src/Repositories/Repository.php b/src/Repositories/Repository.php index 9e16c2bc8..273222b7b 100644 --- a/src/Repositories/Repository.php +++ b/src/Repositories/Repository.php @@ -2,7 +2,8 @@ namespace Binaryk\LaravelRestify\Repositories; -use Binaryk\LaravelRestify\Traits\AuthorizableModels; +use Binaryk\LaravelRestify\Contracts\RestifySearchable; +use Binaryk\LaravelRestify\Traits\InteractWithSearch; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Str; @@ -10,9 +11,9 @@ /** * @author Eduard Lupacescu */ -abstract class Repository +abstract class Repository implements RestifySearchable { - use AuthorizableModels; + use InteractWithSearch; /** * @var Model @@ -22,11 +23,11 @@ abstract class Repository /** * Create a new resource instance. * - * @param \Illuminate\Database\Eloquent\Model $resource + * @param \Illuminate\Database\Eloquent\Model $model */ - public function __construct($resource) + public function __construct($model) { - $this->modelInstance = $resource; + $this->modelInstance = $model; } /** diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index d088a76a8..35c1f6ecf 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -109,7 +109,7 @@ protected function prepareOperator($fields) protected function prepareMatchFields() { foreach ($this->model::getMatchByFields() as $key => $type) { - if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -227,7 +227,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); } }); diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 8c3270d74..77c3e66fb 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -38,7 +38,9 @@ public static function authorizable() * Determine if the resource should be available for the given request. * * @param \Illuminate\Http\Request $request - * @return bool + * @return void + * @throws AuthorizationException + * @throws \Throwable */ public function authorizeToViewAny(Request $request) { @@ -72,9 +74,10 @@ public static function authorizedToViewAny(Request $request) * Determine if the current user can view the given resource or throw an exception. * * @param \Illuminate\Http\Request $request - * @return void + * @return bool * * @throws \Illuminate\Auth\Access\AuthorizationException + * @throws \Throwable */ public function authorizeToView(Request $request) { @@ -93,12 +96,13 @@ public function authorizedToView(Request $request) } /** - * Determine if the current user can create new resources or throw an exception. + * Determine if the current user can create new repositories or throw an exception. * * @param \Illuminate\Http\Request $request * @return void * * @throws \Illuminate\Auth\Access\AuthorizationException + * @throws \Throwable */ public static function authorizeToCreate(Request $request) { @@ -106,7 +110,7 @@ public static function authorizeToCreate(Request $request) } /** - * Determine if the current user can create new resources. + * Determine if the current user can create new repositories. * * @param \Illuminate\Http\Request $request * @return bool diff --git a/tests/Factories/PostFactory.php b/tests/Factories/PostFactory.php index 95ceef360..8247515ee 100644 --- a/tests/Factories/PostFactory.php +++ b/tests/Factories/PostFactory.php @@ -15,6 +15,7 @@ $factory->define(Binaryk\LaravelRestify\Tests\Fixtures\Post::class, function (Faker $faker) { return [ + 'user_id' => 1, 'title' => $faker->title, 'description' => $faker->text, ]; diff --git a/tests/Fixtures/Post.php b/tests/Fixtures/Post.php index 8e558ba4d..1ba5be0af 100644 --- a/tests/Fixtures/Post.php +++ b/tests/Fixtures/Post.php @@ -2,13 +2,17 @@ namespace Binaryk\LaravelRestify\Tests\Fixtures; +use Binaryk\LaravelRestify\Contracts\RestifySearchable; +use Binaryk\LaravelRestify\Traits\InteractWithSearch; use Illuminate\Database\Eloquent\Model; /** * @author Eduard Lupacescu */ -class Post extends Model +class Post extends Model implements RestifySearchable { + use InteractWithSearch; + protected $fillable = [ 'id', 'user_id', From 437567751b5300aa81df9c0aaf28a2209447997a Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 12:08:54 +0200 Subject: [PATCH 14/25] Apply fixes from StyleCI (#55) --- src/Http/Requests/InteractWithRepositories.php | 1 - src/Services/Search/SearchService.php | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index a5ab05d03..0dfbf2a65 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -86,5 +86,4 @@ public function newRepository() return new $repository($repository::newModel()); } - } diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 35c1f6ecf..d088a76a8 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -109,7 +109,7 @@ protected function prepareOperator($fields) protected function prepareMatchFields() { foreach ($this->model::getMatchByFields() as $key => $type) { - if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -227,7 +227,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); From aaac309a1418a12520aa176f45e8372a4ee3cad7 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 12:15:09 +0200 Subject: [PATCH 15/25] wip --- src/Services/Search/SearchService.php | 195 +++++++----------- .../RepositoryIndexControllerTest.php | 8 - 2 files changed, 73 insertions(+), 130 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 35c1f6ecf..ab1a8fb9d 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -21,82 +21,25 @@ class SearchService extends Searchable * @param Model $model * @return Builder * @throws InstanceOfException + * @throws \Throwable */ public function search(RestifyRequest $request, Model $model) { - $this->request = $request; $this->model = $model; + $this->request = $request; $this->builder = $model->newQuery(); - $this->prepare(); - - return $this->builder; - } - - /** - * Will prepare the eloquent array to return. - * - * @return array - * @throws InstanceOfException - * @throws \Throwable - */ - protected function prepare() - { throw_unless($this->model instanceof RestifySearchable, new InstanceOfException(__('Model is not an instance of :parent class', [ 'parent' => RestifySearchable::class, ]))); $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) ->prepareMatchFields() - ->prepareOperator($this->request->get('operator', [])) ->prepareOrders($this->request->get('sort', '')) ->prepareRelations(); - $results = $this->builder->get(); - - return [ - 'data' => $results, - 'aggregations' => null, - ]; - } - - /** - * Prepare eloquent exact fields. - * - * @param $fields - * - * @return $this - */ - protected function prepareOperator($fields) - { - if (isset($this->fixedInput['operator']) === true) { - $fields = $this->fixedInput['operator']; - } - - if (is_array($fields) === true) { - foreach ($fields as $key => $values) { - foreach ($values as $field => $value) { - $qualifiedField = $this->model->qualifyColumn($field); - switch ($key) { - case 'gte': - $this->builder->where($qualifiedField, '>=', $value); - break; - case 'gt': - $this->builder->where($qualifiedField, '>', $value); - break; - case 'lte': - $this->builder->where($qualifiedField, '<=', $value); - break; - case 'lt': - $this->builder->where($qualifiedField, '<', $value); - break; - } - } - } - } - - return $this; + return $this->builder; } /** @@ -108,38 +51,40 @@ protected function prepareOperator($fields) */ protected function prepareMatchFields() { - foreach ($this->model::getMatchByFields() as $key => $type) { - if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { - continue; - } + if ($this->model instanceof RestifySearchable) { + foreach ($this->model::getMatchByFields() as $key => $type) { + if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + continue; + } - $value = $this->request->get($key) ?: data_get($this->fixedInput, "match.$key"); + $value = $this->request->get($key) ?: data_get($this->fixedInput, "match.$key"); - $field = $this->model->qualifyColumn($key); + $field = $this->model->qualifyColumn($key); - $values = explode(',', $value); + $values = explode(',', $value); - foreach ($values as $match) { - switch ($this->model::getMatchByFields()[$key]) { - case RestifySearchable::MATCH_TEXT: - case 'string': - $this->builder->where($field, '=', $match); - break; - case RestifySearchable::MATCH_BOOL: - case 'boolean': - if ($match === 'false') { - $this->builder->where(function ($query) use ($field) { - return $query->where($field, '=', false)->orWhereNull($field); - }); + foreach ($values as $match) { + switch ($this->model::getMatchByFields()[$key]) { + case RestifySearchable::MATCH_TEXT: + case 'string': + $this->builder->where($field, '=', $match); + break; + case RestifySearchable::MATCH_BOOL: + case 'boolean': + if ($match === 'false') { + $this->builder->where(function ($query) use ($field) { + return $query->where($field, '=', false)->orWhereNull($field); + }); + break; + } + $this->builder->where($field, '=', true); break; - } - $this->builder->where($field, '=', true); - break; - case RestifySearchable::MATCH_INTEGER: - case 'number': - case 'int': - $this->builder->where($field, '=', (int) $match); - break; + case RestifySearchable::MATCH_INTEGER: + case 'number': + case 'int': + $this->builder->where($field, '=', (int) $match); + break; + } } } } @@ -182,21 +127,23 @@ protected function prepareOrders($sort) */ protected function prepareRelations() { - $relations = null; + if ($this->model instanceof RestifySearchable) { + $relations = null; - if (isset($this->fixedInput['with']) === true) { - $relations = $this->fixedInput['with']; - } + if (isset($this->fixedInput['with']) === true) { + $relations = $this->fixedInput['with']; + } - if (isset($this->fixedInput['with']) === false) { - $relations = $this->request->get('with', null); - } + if (isset($this->fixedInput['with']) === false) { + $relations = $this->request->get('with', null); + } - if (empty($relations) === false) { - $foundRelations = explode(',', $relations); - foreach ($foundRelations as $relation) { - if (in_array($relation, $this->model->getWiths())) { - $this->builder->with($relation); + if (empty($relations) === false) { + $foundRelations = explode(',', $relations); + foreach ($foundRelations as $relation) { + if (in_array($relation, $this->model::getWiths())) { + $this->builder->with($relation); + } } } } @@ -212,24 +159,26 @@ protected function prepareRelations() */ protected function prepareSearchFields($search) { - $this->builder->where(function (Builder $query) use ($search) { - $connectionType = $this->model->getConnection()->getDriverName(); + if ($this->model instanceof RestifySearchable) { + $this->builder->where(function (Builder $query) use ($search) { + $connectionType = $this->model->getConnection()->getDriverName(); - $canSearchPrimaryKey = is_numeric($search) && - in_array($query->getModel()->getKeyType(), ['int', 'integer']) && - ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && - in_array($query->getModel()->getKeyName(), $this->model::getSearchableFields()); + $canSearchPrimaryKey = is_numeric($search) && + in_array($query->getModel()->getKeyType(), ['int', 'integer']) && + ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && + in_array($query->getModel()->getKeyName(), $this->model::getSearchableFields()); - if ($canSearchPrimaryKey) { - $query->orWhere($query->getModel()->getQualifiedKeyName(), $search); - } + if ($canSearchPrimaryKey) { + $query->orWhere($query->getModel()->getQualifiedKeyName(), $search); + } - $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; + $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; - foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); - } - }); + foreach ($this->model::getSearchableFields() as $column) { + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + } + }); + } return $this; } @@ -264,18 +213,20 @@ public function setOrder($param) $field = $param; } - if (in_array($field, $this->model::getOrderByFields()) === true) { - if ($order === '-') { - $this->builder->orderBy($field, 'desc'); - } + if (isset($field) && $this->model instanceof RestifySearchable) { + if (in_array($field, $this->model::getOrderByFields()) === true) { + if ($order === '-') { + $this->builder->orderBy($field, 'desc'); + } - if ($order === '+') { - $this->builder->orderBy($field, 'asc'); + if ($order === '+') { + $this->builder->orderBy($field, 'asc'); + } } - } - if ($field === 'random') { - $this->builder->orderByRaw('RAND()'); + if ($field === 'random') { + $this->builder->orderByRaw('RAND()'); + } } return $this; diff --git a/tests/Controllers/RepositoryIndexControllerTest.php b/tests/Controllers/RepositoryIndexControllerTest.php index 0e939353e..c60496309 100644 --- a/tests/Controllers/RepositoryIndexControllerTest.php +++ b/tests/Controllers/RepositoryIndexControllerTest.php @@ -199,12 +199,4 @@ public function test_that_with_param_works() $this->assertSameSize($r->data['data']->first()['posts'], $expected['posts']); } - - public function test_should_throw_if_user_not_instance_of_searchable() - { - $this->withExceptionHandling() - ->get('/restify-api/posts') - ->assertStatus(500) - ->getOriginalContent(); - } } From 1c7d6f825d8cdc58a92d0bf8fc555be07390e170 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 12:16:11 +0200 Subject: [PATCH 16/25] Apply fixes from StyleCI (#56) --- src/Services/Search/SearchService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index ab1a8fb9d..2674f9caa 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -53,7 +53,7 @@ protected function prepareMatchFields() { if ($this->model instanceof RestifySearchable) { foreach ($this->model::getMatchByFields() as $key => $type) { - if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + if (! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { continue; } @@ -175,7 +175,7 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); } From 3e820348130a08a62d182f1954eb02a4674a1cde Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 14:43:04 +0200 Subject: [PATCH 17/25] Add coverage for SearchService unit tests --- src/Services/Search/SearchService.php | 160 +++++++++---------- src/Services/Search/Searchable.php | 10 -- tests/Fixtures/User.php | 11 ++ tests/IntegrationTest.php | 15 +- tests/SearchServiceTest.php | 212 ++++++++++++++++++++++++++ 5 files changed, 312 insertions(+), 96 deletions(-) create mode 100644 tests/SearchServiceTest.php diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index ab1a8fb9d..11b8e766b 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -7,14 +7,9 @@ use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Query\Builder as QueryBuilder; class SearchService extends Searchable { - /** - * @var Builder|QueryBuilder - */ - protected $builder; /** * @param RestifyRequest $request @@ -25,148 +20,144 @@ class SearchService extends Searchable */ public function search(RestifyRequest $request, Model $model) { - $this->model = $model; - $this->request = $request; - - $this->builder = $model->newQuery(); - - throw_unless($this->model instanceof RestifySearchable, new InstanceOfException(__('Model is not an instance of :parent class', [ + throw_unless($model instanceof RestifySearchable, new InstanceOfException(__('Model is not an instance of :parent class', [ 'parent' => RestifySearchable::class, ]))); - $this->prepareSearchFields($this->request->get('search', data_get($this->fixedInput, 'search', ''))) - ->prepareMatchFields() - ->prepareOrders($this->request->get('sort', '')) - ->prepareRelations(); - - return $this->builder; + $query = $this->prepareMatchFields($request, $this->prepareSearchFields($request, $model->newQuery(), $this->fixedInput), $this->fixedInput); + return $this->prepareRelations($request, $this->prepareOrders($request, $query), $this->fixedInput); } /** * Prepare eloquent exact fields. * - * @param $fields - * - * @return $this + * @param RestifyRequest $request + * @param Builder $query + * @param array $extra + * @return Builder */ - protected function prepareMatchFields() + public function prepareMatchFields(RestifyRequest $request, $query, $extra = []) { - if ($this->model instanceof RestifySearchable) { - foreach ($this->model::getMatchByFields() as $key => $type) { - if ( ! $this->request->has($key) && ! data_get($this->fixedInput, "match.$key")) { + $model = $query->getModel(); + if ($model instanceof RestifySearchable) { + foreach ($model::getMatchByFields() as $key => $type) { + if ( ! $request->has($key) && ! data_get($extra, "match.$key")) { continue; } - $value = $this->request->get($key) ?: data_get($this->fixedInput, "match.$key"); + $value = $request->get($key, data_get($extra, "match.$key")); - $field = $this->model->qualifyColumn($key); + if (empty($value)) { + continue; + } + + $field = $model->qualifyColumn($key); $values = explode(',', $value); foreach ($values as $match) { - switch ($this->model::getMatchByFields()[$key]) { + switch ($model::getMatchByFields()[$key]) { case RestifySearchable::MATCH_TEXT: case 'string': - $this->builder->where($field, '=', $match); + $query->where($field, '=', $match); break; case RestifySearchable::MATCH_BOOL: case 'boolean': if ($match === 'false') { - $this->builder->where(function ($query) use ($field) { + $query->where(function ($query) use ($field) { return $query->where($field, '=', false)->orWhereNull($field); }); break; } - $this->builder->where($field, '=', true); + $query->where($field, '=', true); break; case RestifySearchable::MATCH_INTEGER: case 'number': case 'int': - $this->builder->where($field, '=', (int) $match); + $query->where($field, '=', (int) $match); break; } } } } - return $this; + return $query; } /** * Prepare eloquent order by. * - * @param $sort - * - * @return $this + * @param RestifyRequest $request + * @param $query + * @param array $extra + * @return Builder */ - protected function prepareOrders($sort) + public function prepareOrders(RestifyRequest $request, $query, $extra = []) { - if (isset($this->fixedInput['sort'])) { - $sort = $this->fixedInput['sort']; + $sort = $request->get('sort', ''); + + if (isset($extra['sort'])) { + $sort = $extra['sort']; } $params = explode(',', $sort); if (is_array($params) === true && empty($params) === false) { foreach ($params as $param) { - $this->setOrder($param); + $this->setOrder($query, $param); } } if (empty($params) === true) { - $this->setOrder('+id'); + $this->setOrder($query, '+id'); } - return $this; + return $query; } /** * Prepare relations. * - * @return $this + * @param RestifyRequest $request + * @param Builder $query + * @param array $extra + * @return Builder */ - protected function prepareRelations() + public function prepareRelations(RestifyRequest $request, $query, $extra = []) { - if ($this->model instanceof RestifySearchable) { - $relations = null; - - if (isset($this->fixedInput['with']) === true) { - $relations = $this->fixedInput['with']; - } - - if (isset($this->fixedInput['with']) === false) { - $relations = $this->request->get('with', null); - } - - if (empty($relations) === false) { - $foundRelations = explode(',', $relations); - foreach ($foundRelations as $relation) { - if (in_array($relation, $this->model::getWiths())) { - $this->builder->with($relation); - } + $model = $query->getModel(); + if ($model instanceof RestifySearchable) { + $relations = array_merge($extra, explode(',', $request->get('with'))); + foreach ($relations as $relation) { + if (in_array($relation, $model::getWiths())) { + $query->with($relation); } } } - return $this; + return $query; } /** * Prepare search. * - * @param $search - * @return $this + * @param RestifyRequest $request + * @param Builder $query + * @param array $extra + * @return Builder */ - protected function prepareSearchFields($search) + public function prepareSearchFields(RestifyRequest $request, $query, $extra = []) { - if ($this->model instanceof RestifySearchable) { - $this->builder->where(function (Builder $query) use ($search) { - $connectionType = $this->model->getConnection()->getDriverName(); + $search = $request->get('search', data_get($extra, 'search', '')); + $model = $query->getModel(); + if ($model instanceof RestifySearchable) { + $query->where(function (Builder $query) use ($search, $model) { + $connectionType = $model->getConnection()->getDriverName(); $canSearchPrimaryKey = is_numeric($search) && in_array($query->getModel()->getKeyType(), ['int', 'integer']) && ($connectionType != 'pgsql' || $search <= PHP_INT_MAX) && - in_array($query->getModel()->getKeyName(), $this->model::getSearchableFields()); + in_array($query->getModel()->getKeyName(), $model::getSearchableFields()); if ($canSearchPrimaryKey) { $query->orWhere($query->getModel()->getQualifiedKeyName(), $search); @@ -174,28 +165,27 @@ protected function prepareSearchFields($search) $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; - foreach ($this->model::getSearchableFields() as $column) { - $query->orWhere($this->model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + foreach ($model::getSearchableFields() as $column) { + $query->orWhere($model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); } }); } - return $this; + return $query; } + /** - * Set order. - * + * @param Builder $query * @param $param - * - * @return $this + * @return Builder */ - public function setOrder($param) + public function setOrder($query, $param) { if ($param === 'random') { - $this->builder->inRandomOrder(); + $query->inRandomOrder(); - return $this; + return $query; } $order = substr($param, 0, 1); @@ -213,22 +203,24 @@ public function setOrder($param) $field = $param; } - if (isset($field) && $this->model instanceof RestifySearchable) { - if (in_array($field, $this->model::getOrderByFields()) === true) { + $model = $query->getModel(); + + if (isset($field) && $model instanceof RestifySearchable) { + if (in_array($field, $model::getOrderByFields()) === true) { if ($order === '-') { - $this->builder->orderBy($field, 'desc'); + $query->orderBy($field, 'desc'); } if ($order === '+') { - $this->builder->orderBy($field, 'asc'); + $query->orderBy($field, 'asc'); } } if ($field === 'random') { - $this->builder->orderByRaw('RAND()'); + $query->orderByRaw('RAND()'); } } - return $this; + return $query; } } diff --git a/src/Services/Search/Searchable.php b/src/Services/Search/Searchable.php index 35ca074c1..a3cc32e43 100644 --- a/src/Services/Search/Searchable.php +++ b/src/Services/Search/Searchable.php @@ -8,16 +8,6 @@ abstract class Searchable { - /** - * @var Request - */ - protected $request; - - /** - * @var RestifySearchable|Model - */ - protected $model; - /** * @var array|null */ diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php index b71e68a8a..6e0bf8dbf 100644 --- a/tests/Fixtures/User.php +++ b/tests/Fixtures/User.php @@ -77,4 +77,15 @@ public function posts() { return $this->hasMany(Post::class); } + + /** + * Set default test values + */ + public static function reset() { + static::$search = ['id', 'email']; + static::$sort = ['id']; + static::$match = ['id' => 'int', 'email' => 'string']; + static::$in = ['id' => 'int']; + static::$withs = ['posts']; + } } diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index e5fac821e..ff0467f6e 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -8,6 +8,7 @@ use Binaryk\LaravelRestify\Tests\Fixtures\User; use Binaryk\LaravelRestify\Tests\Fixtures\UserRepository; use Illuminate\Contracts\Translation\Translator; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Route; use Orchestra\Testbench\TestCase; @@ -26,11 +27,12 @@ abstract class IntegrationTest extends TestCase protected function setUp(): void { parent::setUp(); + DB::enableQueryLog(); Hash::driver('bcrypt')->setRounds(4); $this->repositoryMock(); $this->loadMigrations(); $this->loadRoutes(); - $this->withFactories(__DIR__.'/Factories'); + $this->withFactories(__DIR__ . '/Factories'); $this->injectTranslator(); Restify::repositories([ @@ -67,7 +69,7 @@ protected function loadMigrations() { $this->loadMigrationsFrom([ '--database' => 'sqlite', - '--realpath' => realpath(__DIR__.'/Migrations'), + '--realpath' => realpath(__DIR__ . '/Migrations'), ]); } @@ -140,4 +142,13 @@ public function loadRoutes() // AuthPassport -> resetPassword })->name('password.reset'); } + + /** + * @return array + */ + public function lastQuery() + { + $queries = DB::getQueryLog(); + return end($queries); + } } diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php new file mode 100644 index 000000000..650132ea9 --- /dev/null +++ b/tests/SearchServiceTest.php @@ -0,0 +1,212 @@ + + */ +class SearchServiceTest extends IntegrationTest +{ + /** + * @var SearchService $service + */ + private $service; + + protected function setUp(): void + { + parent::setUp(); + + $this->service = resolve(SearchService::class); + } + + public function test_should_attach_with_from_extra_in_eager_load() + { + $this->mockUsers(); + $this->mockPosts(1); + User::$withs = ['posts']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn(); + /** + * @var Builder $query + */ + $query = $this->service->prepareRelations($request, $builder, ['posts']); + $this->assertInstanceOf(Closure::class, $query->getEagerLoads()['posts']); + } + + public function test_should_attach_with_in_eager_load() + { + $this->mockUsers(); + $this->mockPosts(1); + User::$withs = ['posts']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn('posts'); + /** + * @var Builder $query + */ + $query = $this->service->prepareRelations($request, $builder); + $this->assertInstanceOf(Closure::class, $query->getEagerLoads()['posts']); + } + + public function test_should_order_desc_by_field() + { + $this->mockUsers(5); + User::$sort = ['id']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn('-id'); + /** + * @var Builder $query + */ + $query = $this->service->prepareOrders($request, $builder); + $this->assertEquals('id', $query->getQuery()->orders[0]['column']); + $this->assertEquals('desc', $query->getQuery()->orders[0]['direction']); + } + + public function test_should_order_asc_by_field() + { + $this->mockUsers(5); + User::$sort = ['id']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn('id'); + /** + * @var Builder $query + */ + $query = $this->service->prepareOrders($request, $builder); + $this->assertEquals('id', $query->getQuery()->orders[0]['column']); + $this->assertEquals('asc', $query->getQuery()->orders[0]['direction']); + } + + public function test_should_order_asc_by_extra_passed_field() + { + $this->mockUsers(5); + User::$sort = ['id']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn(); + /** + * @var Builder $query + */ + $query = $this->service->prepareOrders($request, $builder, [ + 'sort' => 'id', + ]); + $this->assertEquals('id', $query->getQuery()->orders[0]['column']); + $this->assertEquals('asc', $query->getQuery()->orders[0]['direction']); + } + + public function test_match_fields_should_add_equal_where_clause() + { + $this->mockUsers(); + User::$match = ['email' => 'string']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn('eduard.lupacescu@binarcode.com'); + + $request->shouldReceive('has') + ->andReturnTrue(); + /** + * @var Builder $query + */ + $query = $this->service->prepareMatchFields($request, $builder); + $this->assertCount(count(User::$match), $query->getQuery()->getRawBindings()['where']); + $this->assertEquals('eduard.lupacescu@binarcode.com', $query->getQuery()->getRawBindings()['where'][0]); + $query->get(); + $raw = $this->lastQuery(); + $this->assertEquals($raw['query'], 'select * from "users" where "users"."email" = ?'); + $this->assertEquals($raw['bindings'], ['eduard.lupacescu@binarcode.com']); + User::reset(); + } + + public function test_match_fields_from_extra_should_add_equal_where_clause() + { + $this->mockUsers(); + User::$match = ['email' => 'string']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturnArg(1); //returns the default value + + $request->shouldReceive('has') + ->andReturnFalse(); + /** + * @var Builder $query + */ + $query = $this->service->prepareMatchFields($request, $builder, [ + 'match' => [ + 'email' => 'eduard.lupacescu@binarcode.com', + ], + ]); + $this->assertCount(count(User::$match), $query->getQuery()->getRawBindings()['where']); + $this->assertEquals('eduard.lupacescu@binarcode.com', $query->getQuery()->getRawBindings()['where'][0]); + $query->get(); + $raw = $this->lastQuery(); + $this->assertEquals($raw['query'], 'select * from "users" where "users"."email" = ?'); + $this->assertEquals($raw['bindings'], ['eduard.lupacescu@binarcode.com']); + User::reset(); + } + + public function test_match_fields_should_not_add_equal_where_if_value_passed_in_query_is_empty() + { + $this->mockUsers(); + User::$match = ['email' => 'string']; + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn(''); + + $request->shouldReceive('has') + ->andReturnTrue(); + /** + * @var Builder $query + */ + $query = $this->service->prepareMatchFields($request, $builder); + $this->assertCount(0, $query->getQuery()->getRawBindings()['where']); + User::reset(); + } + + public function test_search_throw_exception_if_not_searchable_instance() + { + $this->expectException(InstanceOfException::class); + $request = MockeryAlias::mock(RestifyRequest::class); + $class = (new class extends Model { + }); + + $this->service->search($request, $class); + } + + public function test_prepare_search_should_add_where_clause() + { + $this->mockUsers(1); + $request = MockeryAlias::mock(RestifyRequest::class); + $builder = User::query(); + $request->shouldReceive('get') + ->andReturn('some search'); + /** + * @var Builder $query + */ + $query = $this->service->prepareSearchFields($request, $builder); + $this->assertArrayHasKey('where', $query->getQuery()->getRawBindings()); + $this->assertCount(count(User::$search), $query->getQuery()->getRawBindings()['where']); + foreach ($query->getQuery()->getRawBindings()['where'] as $k => $queryString) { + $this->assertEquals('%some search%', $queryString); + } + } +} From b3cc1e222ba49b49e8feeb282d1d8b8a96a5c09f Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 14:45:54 +0200 Subject: [PATCH 18/25] wip --- src/Services/Search/SearchService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 11b8e766b..3eb66c3a7 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -176,7 +176,7 @@ public function prepareSearchFields(RestifyRequest $request, $query, $extra = [] /** - * @param Builder $query + * @param $query * @param $param * @return Builder */ From 03b182f299d638417e32abd983389b43b7b8f0d3 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 14:46:11 +0200 Subject: [PATCH 19/25] Apply fixes from StyleCI (#57) --- src/Services/Search/SearchService.php | 7 +++---- src/Services/Search/Searchable.php | 4 ---- tests/Fixtures/User.php | 5 +++-- tests/IntegrationTest.php | 5 +++-- tests/SearchServiceTest.php | 21 ++++++++++----------- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 3eb66c3a7..bd96aa950 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -10,7 +10,6 @@ class SearchService extends Searchable { - /** * @param RestifyRequest $request * @param Model $model @@ -25,6 +24,7 @@ public function search(RestifyRequest $request, Model $model) ]))); $query = $this->prepareMatchFields($request, $this->prepareSearchFields($request, $model->newQuery(), $this->fixedInput), $this->fixedInput); + return $this->prepareRelations($request, $this->prepareOrders($request, $query), $this->fixedInput); } @@ -41,7 +41,7 @@ public function prepareMatchFields(RestifyRequest $request, $query, $extra = []) $model = $query->getModel(); if ($model instanceof RestifySearchable) { foreach ($model::getMatchByFields() as $key => $type) { - if ( ! $request->has($key) && ! data_get($extra, "match.$key")) { + if (! $request->has($key) && ! data_get($extra, "match.$key")) { continue; } @@ -166,7 +166,7 @@ public function prepareSearchFields(RestifyRequest $request, $query, $extra = [] $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like'; foreach ($model::getSearchableFields() as $column) { - $query->orWhere($model->qualifyColumn($column), $likeOperator, '%' . $search . '%'); + $query->orWhere($model->qualifyColumn($column), $likeOperator, '%'.$search.'%'); } }); } @@ -174,7 +174,6 @@ public function prepareSearchFields(RestifyRequest $request, $query, $extra = [] return $query; } - /** * @param $query * @param $param diff --git a/src/Services/Search/Searchable.php b/src/Services/Search/Searchable.php index a3cc32e43..d0ea79fc6 100644 --- a/src/Services/Search/Searchable.php +++ b/src/Services/Search/Searchable.php @@ -2,10 +2,6 @@ namespace Binaryk\LaravelRestify\Services\Search; -use Binaryk\LaravelRestify\Contracts\RestifySearchable; -use Illuminate\Database\Eloquent\Model; -use Illuminate\Http\Request; - abstract class Searchable { /** diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php index 6e0bf8dbf..09e94a2b6 100644 --- a/tests/Fixtures/User.php +++ b/tests/Fixtures/User.php @@ -79,9 +79,10 @@ public function posts() } /** - * Set default test values + * Set default test values. */ - public static function reset() { + public static function reset() + { static::$search = ['id', 'email']; static::$sort = ['id']; static::$match = ['id' => 'int', 'email' => 'string']; diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index ff0467f6e..910fc60fa 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -32,7 +32,7 @@ protected function setUp(): void $this->repositoryMock(); $this->loadMigrations(); $this->loadRoutes(); - $this->withFactories(__DIR__ . '/Factories'); + $this->withFactories(__DIR__.'/Factories'); $this->injectTranslator(); Restify::repositories([ @@ -69,7 +69,7 @@ protected function loadMigrations() { $this->loadMigrationsFrom([ '--database' => 'sqlite', - '--realpath' => realpath(__DIR__ . '/Migrations'), + '--realpath' => realpath(__DIR__.'/Migrations'), ]); } @@ -149,6 +149,7 @@ public function loadRoutes() public function lastQuery() { $queries = DB::getQueryLog(); + return end($queries); } } diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php index 650132ea9..85ed32982 100644 --- a/tests/SearchServiceTest.php +++ b/tests/SearchServiceTest.php @@ -12,13 +12,12 @@ use Mockery as MockeryAlias; /** - * @package Binaryk\LaravelRestify\Tests; * @author Eduard Lupacescu */ class SearchServiceTest extends IntegrationTest { /** - * @var SearchService $service + * @var SearchService */ private $service; @@ -39,7 +38,7 @@ public function test_should_attach_with_from_extra_in_eager_load() $request->shouldReceive('get') ->andReturn(); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareRelations($request, $builder, ['posts']); $this->assertInstanceOf(Closure::class, $query->getEagerLoads()['posts']); @@ -55,7 +54,7 @@ public function test_should_attach_with_in_eager_load() $request->shouldReceive('get') ->andReturn('posts'); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareRelations($request, $builder); $this->assertInstanceOf(Closure::class, $query->getEagerLoads()['posts']); @@ -70,7 +69,7 @@ public function test_should_order_desc_by_field() $request->shouldReceive('get') ->andReturn('-id'); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareOrders($request, $builder); $this->assertEquals('id', $query->getQuery()->orders[0]['column']); @@ -86,7 +85,7 @@ public function test_should_order_asc_by_field() $request->shouldReceive('get') ->andReturn('id'); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareOrders($request, $builder); $this->assertEquals('id', $query->getQuery()->orders[0]['column']); @@ -102,7 +101,7 @@ public function test_should_order_asc_by_extra_passed_field() $request->shouldReceive('get') ->andReturn(); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareOrders($request, $builder, [ 'sort' => 'id', @@ -123,7 +122,7 @@ public function test_match_fields_should_add_equal_where_clause() $request->shouldReceive('has') ->andReturnTrue(); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareMatchFields($request, $builder); $this->assertCount(count(User::$match), $query->getQuery()->getRawBindings()['where']); @@ -147,7 +146,7 @@ public function test_match_fields_from_extra_should_add_equal_where_clause() $request->shouldReceive('has') ->andReturnFalse(); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareMatchFields($request, $builder, [ 'match' => [ @@ -175,7 +174,7 @@ public function test_match_fields_should_not_add_equal_where_if_value_passed_in_ $request->shouldReceive('has') ->andReturnTrue(); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareMatchFields($request, $builder); $this->assertCount(0, $query->getQuery()->getRawBindings()['where']); @@ -200,7 +199,7 @@ public function test_prepare_search_should_add_where_clause() $request->shouldReceive('get') ->andReturn('some search'); /** - * @var Builder $query + * @var Builder */ $query = $this->service->prepareSearchFields($request, $builder); $this->assertArrayHasKey('where', $query->getQuery()->getRawBindings()); From 7ed86d3b044f1ac1ec032a3e66a05d691d362cf0 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 16:33:42 +0200 Subject: [PATCH 20/25] Use RestifyRequest instead of Request --- src/Contracts/RestifySearchable.php | 6 +++--- src/Controllers/RestController.php | 18 +++++++++++----- .../Controllers/RepositoryIndexController.php | 4 +--- .../Requests/InteractWithRepositories.php | 16 ++++++++++++++ src/Repositories/Repository.php | 21 +++++++++++++++---- src/Services/Search/SearchService.php | 6 +++--- src/Traits/AuthorizableModels.php | 2 +- src/Traits/InteractWithSearch.php | 6 +++--- .../RepositoryIndexControllerTest.php | 8 ++++--- 9 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/Contracts/RestifySearchable.php b/src/Contracts/RestifySearchable.php index e761cdd75..237127b60 100644 --- a/src/Contracts/RestifySearchable.php +++ b/src/Contracts/RestifySearchable.php @@ -2,7 +2,7 @@ namespace Binaryk\LaravelRestify\Contracts; -use Illuminate\Http\Request; +use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; /** * @author Eduard Lupacescu @@ -16,11 +16,11 @@ interface RestifySearchable const MATCH_INTEGER = 'integer'; /** - * @param Request $request + * @param RestifyRequest $request * @param array $fields * @return array */ - public function serializeForIndex(Request $request, array $fields = []); + public function serializeForIndex(RestifyRequest $request, array $fields = []); /** * @return array diff --git a/src/Controllers/RestController.php b/src/Controllers/RestController.php index a6a2c3465..acfa9ccb8 100644 --- a/src/Controllers/RestController.php +++ b/src/Controllers/RestController.php @@ -9,8 +9,8 @@ use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Binaryk\LaravelRestify\Services\Search\SearchService; use Binaryk\LaravelRestify\Traits\PerformsQueries; -use Illuminate\Config\Repository; use Illuminate\Config\Repository as Config; +use Binaryk\LaravelRestify\Repositories\Repository; use Illuminate\Container\Container; use Illuminate\Contracts\Auth\Access\Gate; use Illuminate\Contracts\Auth\Authenticatable; @@ -85,8 +85,8 @@ public function config() { $container = Container::getInstance(); - if (($this->config instanceof Repository) === false) { - $this->config = $container->make(Repository::class); + if (($this->config instanceof Config) === false) { + $this->config = $container->make(Config::class); } return $this->config; @@ -137,13 +137,21 @@ public function search($modelClass, $filters = []) { $results = SearchService::instance() ->setPredefinedFilters($filters) - ->search($this->request(), new $modelClass); + ->search($this->request(), $modelClass instanceof Repository ? $modelClass->model() : new $modelClass); + $results->tap(function ($query) { static::indexQuery($this->request(), $query); }); + /** + * @var \Illuminate\Pagination\Paginator $paginator + */ $paginator = $results->paginate($this->request()->get('perPage') ?? ($modelClass::$defaultPerPage ?? RestifySearchable::DEFAULT_PER_PAGE)); - $items = $paginator->getCollection()->map->serializeForIndex($this->request()); + if ($modelClass instanceof Repository) { + $items = $paginator->getCollection()->mapInto(get_class($modelClass))->map->serializeForIndex($this->request()); + } else { + $items = $paginator->getCollection()->map->serializeForIndex($this->request()); + } return array_merge($paginator->toArray(), [ 'data' => $items, diff --git a/src/Http/Controllers/RepositoryIndexController.php b/src/Http/Controllers/RepositoryIndexController.php index 894733d1e..9303cf7ea 100644 --- a/src/Http/Controllers/RepositoryIndexController.php +++ b/src/Http/Controllers/RepositoryIndexController.php @@ -18,9 +18,7 @@ class RepositoryIndexController extends RepositoryController */ public function handle(RestifyRequest $request) { - $resource = $request->repository(); - - $data = $this->search($resource::newModel()); + $data = $this->search($request->newRepository()); return $this->respond($data); } diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index 0dfbf2a65..a53bd704d 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -86,4 +86,20 @@ public function newRepository() return new $repository($repository::newModel()); } + + /** + * Check if the route is resolved by the Repository class, or it uses the classical Models + * @return bool + */ + public function isResolvedByRestify() + { + try { + $this->repository(); + return true; + } catch (EntityNotFoundException $e) { + return false; + } catch (UnauthorizedException $e) { + return true; + } + } } diff --git a/src/Repositories/Repository.php b/src/Repositories/Repository.php index 273222b7b..caa2b0481 100644 --- a/src/Repositories/Repository.php +++ b/src/Repositories/Repository.php @@ -6,6 +6,7 @@ use Binaryk\LaravelRestify\Traits\InteractWithSearch; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; +use Illuminate\Http\Resources\DelegatesToResource; use Illuminate\Support\Str; /** @@ -13,12 +14,14 @@ */ abstract class Repository implements RestifySearchable { - use InteractWithSearch; + use InteractWithSearch, + DelegatesToResource; /** + * This is named `resource` because of the forwarding properties from DelegatesToResource trait * @var Model */ - public $modelInstance; + public $resource; /** * Create a new resource instance. @@ -27,7 +30,7 @@ abstract class Repository implements RestifySearchable */ public function __construct($model) { - $this->modelInstance = $model; + $this->resource = $model; } /** @@ -37,7 +40,7 @@ public function __construct($model) */ public function model() { - return $this->modelInstance; + return $this->resource; } /** @@ -69,4 +72,14 @@ public static function query() { return static::newModel()->query(); } + + /** + * @return array + */ + public function toArray() + { + $model = $this->model(); + + return $model->toArray(); + } } diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 3eb66c3a7..f9b962e81 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -20,9 +20,9 @@ class SearchService extends Searchable */ public function search(RestifyRequest $request, Model $model) { - throw_unless($model instanceof RestifySearchable, new InstanceOfException(__('Model is not an instance of :parent class', [ - 'parent' => RestifySearchable::class, - ]))); + if ( ! $model instanceof RestifySearchable) { + return $model->newQuery(); + } $query = $this->prepareMatchFields($request, $this->prepareSearchFields($request, $model->newQuery(), $this->fixedInput), $this->fixedInput); return $this->prepareRelations($request, $this->prepareOrders($request, $query), $this->fixedInput); diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 77c3e66fb..8e56c4904 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -308,7 +308,7 @@ public function authorizedTo(Request $request, $ability) */ public function determineModel() { - $model = $this instanceof Model ? $this : ($this->modelInstance ?? null); + $model = $this instanceof Model ? $this : ($this->resource ?? null); throw_if(is_null($model), new ModelNotFoundException(__('Model does not declared in :class', ['class' => self::class]))); diff --git a/src/Traits/InteractWithSearch.php b/src/Traits/InteractWithSearch.php index f6133b5fd..e6d04333a 100644 --- a/src/Traits/InteractWithSearch.php +++ b/src/Traits/InteractWithSearch.php @@ -2,7 +2,7 @@ namespace Binaryk\LaravelRestify\Traits; -use Illuminate\Http\Request; +use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; /** * @author Eduard Lupacescu @@ -56,11 +56,11 @@ public static function getOrderByFields() /** * Prepare the resource for JSON serialization. * - * @param Request $request + * @param RestifyRequest $request * @param array $fields * @return array */ - public function serializeForIndex(Request $request, array $fields = null) + public function serializeForIndex(RestifyRequest $request, array $fields = null) { return array_merge($fields ?: $this->toArray(), [ 'authorizedToView' => $this->authorizedToView($request), diff --git a/tests/Controllers/RepositoryIndexControllerTest.php b/tests/Controllers/RepositoryIndexControllerTest.php index c60496309..9872a5f92 100644 --- a/tests/Controllers/RepositoryIndexControllerTest.php +++ b/tests/Controllers/RepositoryIndexControllerTest.php @@ -4,8 +4,10 @@ use Binaryk\LaravelRestify\Contracts\RestifySearchable; use Binaryk\LaravelRestify\Controllers\RestController; +use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Binaryk\LaravelRestify\Tests\Fixtures\User; use Binaryk\LaravelRestify\Tests\IntegrationTest; +use Mockery; /** * @author Eduard Lupacescu @@ -71,7 +73,7 @@ public function users() public function test_search_query_works() { $users = $this->mockUsers(10, ['eduard.lupacescu@binarcode.com']); - $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(request()); + $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(Mockery::mock(RestifyRequest::class)); $this->withExceptionHandling() ->getJson('/restify-api/users?search=eduard.lupacescu@binarcode.com') ->assertStatus(200) @@ -161,7 +163,7 @@ public function test_that_match_param_works() { User::$match = ['email' => RestifySearchable::MATCH_TEXT]; // it will automatically filter over these queries (email='test@email.com') $users = $this->mockUsers(10, ['eduard.lupacescu@binarcode.com']); - $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(request()); + $expected = $users->where('email', 'eduard.lupacescu@binarcode.com')->first()->serializeForIndex(Mockery::mock(RestifyRequest::class)); $this->withExceptionHandling() ->get('/restify-api/users?email=eduard.lupacescu@binarcode.com') @@ -190,7 +192,7 @@ public function test_that_with_param_works() User::$match = ['email' => RestifySearchable::MATCH_TEXT]; // it will automatically filter over these queries (email='test@email.com') $users = $this->mockUsers(1); $posts = $this->mockPosts(1, 2); - $expected = $users->first()->serializeForIndex(request()); + $expected = $users->first()->serializeForIndex(Mockery::mock(RestifyRequest::class)); $expected['posts'] = $posts->toArray(); $r = $this->withExceptionHandling() ->get('/restify-api/users?with=posts') From fc37221a3e772550da01ae23cd8e9a31a65ce03d Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 16:34:08 +0200 Subject: [PATCH 21/25] Apply fixes from StyleCI (#58) --- src/Controllers/RestController.php | 4 ++-- src/Http/Requests/InteractWithRepositories.php | 3 ++- src/Repositories/Repository.php | 2 +- src/Services/Search/SearchService.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Controllers/RestController.php b/src/Controllers/RestController.php index acfa9ccb8..6f00bd83b 100644 --- a/src/Controllers/RestController.php +++ b/src/Controllers/RestController.php @@ -7,10 +7,10 @@ use Binaryk\LaravelRestify\Exceptions\Guard\GatePolicy; use Binaryk\LaravelRestify\Exceptions\InstanceOfException; use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; +use Binaryk\LaravelRestify\Repositories\Repository; use Binaryk\LaravelRestify\Services\Search\SearchService; use Binaryk\LaravelRestify\Traits\PerformsQueries; use Illuminate\Config\Repository as Config; -use Binaryk\LaravelRestify\Repositories\Repository; use Illuminate\Container\Container; use Illuminate\Contracts\Auth\Access\Gate; use Illuminate\Contracts\Auth\Authenticatable; @@ -144,7 +144,7 @@ public function search($modelClass, $filters = []) }); /** - * @var \Illuminate\Pagination\Paginator $paginator + * @var \Illuminate\Pagination\Paginator */ $paginator = $results->paginate($this->request()->get('perPage') ?? ($modelClass::$defaultPerPage ?? RestifySearchable::DEFAULT_PER_PAGE)); if ($modelClass instanceof Repository) { diff --git a/src/Http/Requests/InteractWithRepositories.php b/src/Http/Requests/InteractWithRepositories.php index a53bd704d..ccc374529 100644 --- a/src/Http/Requests/InteractWithRepositories.php +++ b/src/Http/Requests/InteractWithRepositories.php @@ -88,13 +88,14 @@ public function newRepository() } /** - * Check if the route is resolved by the Repository class, or it uses the classical Models + * Check if the route is resolved by the Repository class, or it uses the classical Models. * @return bool */ public function isResolvedByRestify() { try { $this->repository(); + return true; } catch (EntityNotFoundException $e) { return false; diff --git a/src/Repositories/Repository.php b/src/Repositories/Repository.php index caa2b0481..9e59ac952 100644 --- a/src/Repositories/Repository.php +++ b/src/Repositories/Repository.php @@ -18,7 +18,7 @@ abstract class Repository implements RestifySearchable DelegatesToResource; /** - * This is named `resource` because of the forwarding properties from DelegatesToResource trait + * This is named `resource` because of the forwarding properties from DelegatesToResource trait. * @var Model */ public $resource; diff --git a/src/Services/Search/SearchService.php b/src/Services/Search/SearchService.php index 2f50f1183..4bb048518 100644 --- a/src/Services/Search/SearchService.php +++ b/src/Services/Search/SearchService.php @@ -19,7 +19,7 @@ class SearchService extends Searchable */ public function search(RestifyRequest $request, Model $model) { - if ( ! $model instanceof RestifySearchable) { + if (! $model instanceof RestifySearchable) { return $model->newQuery(); } From b273b8689f0d67c89ee115b2b7f7ae2f8f708438 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 16:35:06 +0200 Subject: [PATCH 22/25] Remove unused test --- tests/SearchServiceTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php index 85ed32982..2d101f05e 100644 --- a/tests/SearchServiceTest.php +++ b/tests/SearchServiceTest.php @@ -181,16 +181,6 @@ public function test_match_fields_should_not_add_equal_where_if_value_passed_in_ User::reset(); } - public function test_search_throw_exception_if_not_searchable_instance() - { - $this->expectException(InstanceOfException::class); - $request = MockeryAlias::mock(RestifyRequest::class); - $class = (new class extends Model { - }); - - $this->service->search($request, $class); - } - public function test_prepare_search_should_add_where_clause() { $this->mockUsers(1); From 4bc3afc71efcd9bb1081959cf5bff869aee21a90 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 16:35:30 +0200 Subject: [PATCH 23/25] Apply fixes from StyleCI (#59) --- tests/SearchServiceTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php index 2d101f05e..80b71246b 100644 --- a/tests/SearchServiceTest.php +++ b/tests/SearchServiceTest.php @@ -2,13 +2,11 @@ namespace Binaryk\LaravelRestify\Tests; -use Binaryk\LaravelRestify\Exceptions\InstanceOfException; use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Binaryk\LaravelRestify\Services\Search\SearchService; use Binaryk\LaravelRestify\Tests\Fixtures\User; use Closure; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Database\Eloquent\Model; use Mockery as MockeryAlias; /** From dbc27cc537e4e9a5a4c0f62c2365f1823fd6842e Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 17:00:47 +0200 Subject: [PATCH 24/25] Add test for not searchable instance --- tests/SearchServiceTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php index 2d101f05e..286330ef4 100644 --- a/tests/SearchServiceTest.php +++ b/tests/SearchServiceTest.php @@ -181,6 +181,22 @@ public function test_match_fields_should_not_add_equal_where_if_value_passed_in_ User::reset(); } + public function test_should_not_call_anything_from_search_service_if_not_searchable_instance() + { + $service = MockeryAlias::spy(SearchService::class); + $this->instance(SearchService::class, $service); + $request = MockeryAlias::mock(RestifyRequest::class); + $class = (new class extends Model { + }); + $resolvedService = resolve(SearchService::class); + $resolvedService->search($request, $class); + $service->shouldHaveReceived('search'); + $service->shouldNotReceive('prepareSearchFields'); + $service->shouldNotReceive('prepareMatchFields'); + $service->shouldNotReceive('prepareRelations'); + $service->shouldNotReceive('prepareOrders'); + } + public function test_prepare_search_should_add_where_clause() { $this->mockUsers(1); From be8748d1cae8445afab4cadcf4ecdfb8c86c9c05 Mon Sep 17 00:00:00 2001 From: Lupacescu Eduard Date: Mon, 23 Dec 2019 17:05:20 +0200 Subject: [PATCH 25/25] Scrutinizer fix --- src/Services/Search/Searchable.php | 4 ++-- src/Traits/AuthorizableModels.php | 1 + tests/SearchServiceTest.php | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Services/Search/Searchable.php b/src/Services/Search/Searchable.php index d0ea79fc6..42d235dc3 100644 --- a/src/Services/Search/Searchable.php +++ b/src/Services/Search/Searchable.php @@ -5,9 +5,9 @@ abstract class Searchable { /** - * @var array|null + * @var array */ - protected $fixedInput; + protected $fixedInput = []; /** * @param $input diff --git a/src/Traits/AuthorizableModels.php b/src/Traits/AuthorizableModels.php index 8e56c4904..553837db8 100644 --- a/src/Traits/AuthorizableModels.php +++ b/src/Traits/AuthorizableModels.php @@ -12,6 +12,7 @@ /** * Could be used as a trait in a model class and in a repository class. * + * @property Model resource * @author Eduard Lupacescu */ trait AuthorizableModels diff --git a/tests/SearchServiceTest.php b/tests/SearchServiceTest.php index 2b93dec4a..7042d7769 100644 --- a/tests/SearchServiceTest.php +++ b/tests/SearchServiceTest.php @@ -7,6 +7,7 @@ use Binaryk\LaravelRestify\Tests\Fixtures\User; use Closure; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Model; use Mockery as MockeryAlias; /**