From 7e736074938f7bb691bb93ceaa5ae23bbc2ce6b9 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Mon, 12 Mar 2018 14:49:15 +0300 Subject: [PATCH 1/5] Support getting guard_name from extended model when using static methods --- src/Guard.php | 65 +++++++++++++++++++++++++++++++++++ src/Models/Permission.php | 14 +++++--- src/Models/Role.php | 14 +++++--- src/Traits/HasPermissions.php | 32 ++++++++--------- 4 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 src/Guard.php diff --git a/src/Guard.php b/src/Guard.php new file mode 100644 index 0000000..e14bf85 --- /dev/null +++ b/src/Guard.php @@ -0,0 +1,65 @@ +guard_name ?? null; + } + + if (! isset($guardName)) { + $class = \is_object($model) ? \get_class($model) : $model; + $guardName = (new \ReflectionClass($class))->getDefaultProperties()['guard_name'] ?? null; + } + + if ($guardName) { + return collect($guardName); + } + return collect(config('auth.guards')) + ->map(function ($guard) { + return config("auth.providers.{$guard['provider']}.model"); + }) + ->filter(function ($model) use ($class) { + return $class === $model; + }) + ->keys(); + } + + /** + * Return Default Guard name + * + * @param $class + * + * @return string + * @throws \ReflectionException + */ + public static function getDefaultName($class): string + { + $default = config('auth.defaults.guard'); + return static::getNames($class)->first() ?: $default; + } +} \ No newline at end of file diff --git a/src/Models/Permission.php b/src/Models/Permission.php index 061ecb8..2d1dfa2 100644 --- a/src/Models/Permission.php +++ b/src/Models/Permission.php @@ -9,6 +9,7 @@ use Maklad\Permission\Contracts\PermissionInterface; use Maklad\Permission\Exceptions\PermissionAlreadyExists; use Maklad\Permission\Exceptions\PermissionDoesNotExist; +use Maklad\Permission\Guard; use Maklad\Permission\Helpers; use Maklad\Permission\PermissionRegistrar; use Maklad\Permission\Traits\RefreshesPermissionCache; @@ -28,10 +29,12 @@ class Permission extends Model implements PermissionInterface * Permission constructor. * * @param array $attributes + * + * @throws \ReflectionException */ public function __construct(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? \config('auth.defaults.guard'); + $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); parent::__construct($attributes); @@ -47,11 +50,12 @@ public function __construct(array $attributes = []) * * @return $this|\Illuminate\Database\Eloquent\Model * @throws \Maklad\Permission\Exceptions\PermissionAlreadyExists + * @throws \ReflectionException */ public static function create(array $attributes = []) { $helpers = new Helpers(); - $attributes['guard_name'] = $attributes['guard_name'] ?? \config('auth.defaults.guard'); + $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); if (static::getPermissions()->where('name', $attributes['name'])->where( 'guard_name', @@ -77,10 +81,11 @@ public static function create(array $attributes = []) * * @return PermissionInterface * @throws \Maklad\Permission\Exceptions\PermissionAlreadyExists + * @throws \ReflectionException */ public static function findOrCreate(string $name, $guardName = null): PermissionInterface { - $guardName = $guardName ?? config('auth.defaults.guard'); + $guardName = $guardName ?? Guard::getDefaultName(static::class); $permission = static::getPermissions() ->where('name', $name) @@ -123,10 +128,11 @@ public function users(): BelongsToMany * * @return PermissionInterface * @throws PermissionDoesNotExist + * @throws \ReflectionException */ public static function findByName(string $name, $guardName = null): PermissionInterface { - $guardName = $guardName ?? \config('auth.defaults.guard'); + $guardName = $guardName ?? Guard::getDefaultName(static::class); $permission = static::getPermissions()->where('name', $name)->where('guard_name', $guardName)->first(); diff --git a/src/Models/Role.php b/src/Models/Role.php index d256c24..1724d7a 100644 --- a/src/Models/Role.php +++ b/src/Models/Role.php @@ -8,6 +8,7 @@ use Maklad\Permission\Exceptions\GuardDoesNotMatch; use Maklad\Permission\Exceptions\RoleAlreadyExists; use Maklad\Permission\Exceptions\RoleDoesNotExist; +use Maklad\Permission\Guard; use Maklad\Permission\Helpers; use Maklad\Permission\Traits\HasPermissions; use Maklad\Permission\Traits\RefreshesPermissionCache; @@ -28,10 +29,12 @@ class Role extends Model implements RoleInterface * Role constructor. * * @param array $attributes + * + * @throws \ReflectionException */ public function __construct(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? config('auth.defaults.guard'); + $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); parent::__construct($attributes); @@ -47,10 +50,11 @@ public function __construct(array $attributes = []) * @throws RoleAlreadyExists * @internal param array $attributes§ * + * @throws \ReflectionException */ public static function create(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? \config('auth.defaults.guard'); + $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); $helpers = new Helpers(); if (static::where('name', $attributes['name'])->where('guard_name', $attributes['guard_name'])->first()) { @@ -74,10 +78,11 @@ public static function create(array $attributes = []) * * @return RoleInterface * @throws \Maklad\Permission\Exceptions\RoleAlreadyExists + * @throws \ReflectionException */ public static function findOrCreate(string $name, $guardName = null): RoleInterface { - $guardName = $guardName ?? config('auth.defaults.guard'); + $guardName = $guardName ?? Guard::getDefaultName(static::class); $role = static::where('name', $name) ->where('guard_name', $guardName) @@ -98,10 +103,11 @@ public static function findOrCreate(string $name, $guardName = null): RoleInterf * * @return RoleInterface * @throws RoleDoesNotExist + * @throws \ReflectionException */ public static function findByName(string $name, $guardName = null): RoleInterface { - $guardName = $guardName ?? config('auth.defaults.guard'); + $guardName = $guardName ?? Guard::getDefaultName(static::class); $role = static::where('name', $name) ->where('guard_name', $guardName) diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 39a500d..db9c8e9 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -8,6 +8,7 @@ use Jenssegers\Mongodb\Relations\BelongsToMany; use Maklad\Permission\Contracts\PermissionInterface as Permission; use Maklad\Permission\Exceptions\GuardDoesNotMatch; +use Maklad\Permission\Guard; use Maklad\Permission\Helpers; use Maklad\Permission\PermissionRegistrar; @@ -107,6 +108,7 @@ public function revokePermissionTo(...$permissions) * @param string|Permission $permission * * @return Permission + * @throws \ReflectionException */ protected function getStoredPermission($permission): Permission { @@ -121,10 +123,11 @@ protected function getStoredPermission($permission): Permission * @param Model $roleOrPermission * * @throws GuardDoesNotMatch + * @throws \ReflectionException */ protected function ensureModelSharesGuard(Model $roleOrPermission) { - if (! $this->getGuardNames()->contains($roleOrPermission->guard_name)) { + if ( ! $this->getGuardNames()->contains($roleOrPermission->guard_name)) { $expected = $this->getGuardNames(); $given = $roleOrPermission->guard_name; $helpers = new Helpers(); @@ -133,27 +136,22 @@ protected function ensureModelSharesGuard(Model $roleOrPermission) } } + /** + * @return Collection + * @throws \ReflectionException + */ protected function getGuardNames(): Collection { - if ($this->guard_name) { - return collect($this->guard_name); - } - - return collect(config('auth.guards')) - ->map(function ($guard) { - return config("auth.providers.{$guard['provider']}.model"); - }) - ->filter(function ($model) { - return \get_class($this) === $model; - }) - ->keys(); + return Guard::getNames($this); } + /** + * @return string + * @throws \ReflectionException + */ protected function getDefaultGuardName(): string { - $default = config('auth.defaults.guard'); - - return $this->getGuardNames()->first() ?: $default; + return Guard::getDefaultName($this); } /** @@ -177,7 +175,7 @@ private function convertToPermissionModels($permissions): Collection $permissions = collect($permissions); } - if (! $permissions instanceof Collection) { + if ( ! $permissions instanceof Collection) { $permissions = collect([$permissions]); } From 67730a659fc1d21880ef21d9740bdb97368bbf2d Mon Sep 17 00:00:00 2001 From: mostafamaklad Date: Mon, 12 Mar 2018 15:35:17 +0000 Subject: [PATCH 2/5] Apply fixes from StyleCI --- src/Guard.php | 2 +- src/Traits/HasPermissions.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Guard.php b/src/Guard.php index e14bf85..c388e72 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -62,4 +62,4 @@ public static function getDefaultName($class): string $default = config('auth.defaults.guard'); return static::getNames($class)->first() ?: $default; } -} \ No newline at end of file +} diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index db9c8e9..8895e97 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -127,7 +127,7 @@ protected function getStoredPermission($permission): Permission */ protected function ensureModelSharesGuard(Model $roleOrPermission) { - if ( ! $this->getGuardNames()->contains($roleOrPermission->guard_name)) { + if (! $this->getGuardNames()->contains($roleOrPermission->guard_name)) { $expected = $this->getGuardNames(); $given = $roleOrPermission->guard_name; $helpers = new Helpers(); @@ -175,7 +175,7 @@ private function convertToPermissionModels($permissions): Collection $permissions = collect($permissions); } - if ( ! $permissions instanceof Collection) { + if (! $permissions instanceof Collection) { $permissions = collect([$permissions]); } From a0a643dc9e9308642baf5206891d95ef4b250861 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sat, 17 Mar 2018 23:44:15 +0300 Subject: [PATCH 3/5] fix code smells for codeclimate --- src/Guard.php | 12 +++------ src/Models/Permission.php | 8 +++--- src/Models/Role.php | 10 +++++--- src/Traits/HasPermissions.php | 4 +-- src/Traits/HasRoles.php | 46 +++++++++++++++++------------------ 5 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/Guard.php b/src/Guard.php index c388e72..6c15c89 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -1,10 +1,4 @@ guard_name ?? null; @@ -57,9 +51,9 @@ public static function getNames($model) : Collection * @return string * @throws \ReflectionException */ - public static function getDefaultName($class): string + public function getDefaultName($class): string { $default = config('auth.defaults.guard'); - return static::getNames($class)->first() ?: $default; + return $this->getNames($class)->first() ?: $default; } } diff --git a/src/Models/Permission.php b/src/Models/Permission.php index 2d1dfa2..b360710 100644 --- a/src/Models/Permission.php +++ b/src/Models/Permission.php @@ -34,7 +34,7 @@ class Permission extends Model implements PermissionInterface */ public function __construct(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); + $attributes['guard_name'] = $attributes['guard_name'] ?? (new Guard())->getDefaultName(static::class); parent::__construct($attributes); @@ -55,7 +55,7 @@ public function __construct(array $attributes = []) public static function create(array $attributes = []) { $helpers = new Helpers(); - $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); + $attributes['guard_name'] = $attributes['guard_name'] ?? (new Guard())->getDefaultName(static::class); if (static::getPermissions()->where('name', $attributes['name'])->where( 'guard_name', @@ -85,7 +85,7 @@ public static function create(array $attributes = []) */ public static function findOrCreate(string $name, $guardName = null): PermissionInterface { - $guardName = $guardName ?? Guard::getDefaultName(static::class); + $guardName = $guardName ?? (new Guard())->getDefaultName(static::class); $permission = static::getPermissions() ->where('name', $name) @@ -132,7 +132,7 @@ public function users(): BelongsToMany */ public static function findByName(string $name, $guardName = null): PermissionInterface { - $guardName = $guardName ?? Guard::getDefaultName(static::class); + $guardName = $guardName ?? (new Guard())->getDefaultName(static::class); $permission = static::getPermissions()->where('name', $name)->where('guard_name', $guardName)->first(); diff --git a/src/Models/Role.php b/src/Models/Role.php index 1724d7a..bcb86ef 100644 --- a/src/Models/Role.php +++ b/src/Models/Role.php @@ -12,6 +12,7 @@ use Maklad\Permission\Helpers; use Maklad\Permission\Traits\HasPermissions; use Maklad\Permission\Traits\RefreshesPermissionCache; +use ReflectionException; /** * Class Role @@ -34,7 +35,7 @@ class Role extends Model implements RoleInterface */ public function __construct(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); + $attributes['guard_name'] = $attributes['guard_name'] ?? (new Guard())->getDefaultName(static::class); parent::__construct($attributes); @@ -54,7 +55,7 @@ public function __construct(array $attributes = []) */ public static function create(array $attributes = []) { - $attributes['guard_name'] = $attributes['guard_name'] ?? Guard::getDefaultName(static::class); + $attributes['guard_name'] = $attributes['guard_name'] ?? (new Guard())->getDefaultName(static::class); $helpers = new Helpers(); if (static::where('name', $attributes['name'])->where('guard_name', $attributes['guard_name'])->first()) { @@ -82,7 +83,7 @@ public static function create(array $attributes = []) */ public static function findOrCreate(string $name, $guardName = null): RoleInterface { - $guardName = $guardName ?? Guard::getDefaultName(static::class); + $guardName = $guardName ?? (new Guard())->getDefaultName(static::class); $role = static::where('name', $name) ->where('guard_name', $guardName) @@ -107,7 +108,7 @@ public static function findOrCreate(string $name, $guardName = null): RoleInterf */ public static function findByName(string $name, $guardName = null): RoleInterface { - $guardName = $guardName ?? Guard::getDefaultName(static::class); + $guardName = $guardName ?? (new Guard())->getDefaultName(static::class); $role = static::where('name', $name) ->where('guard_name', $guardName) @@ -129,6 +130,7 @@ public static function findByName(string $name, $guardName = null): RoleInterfac * @return bool * * @throws GuardDoesNotMatch + * @throws ReflectionException */ public function hasPermissionTo($permission): bool { diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 8895e97..502d982 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -142,7 +142,7 @@ protected function ensureModelSharesGuard(Model $roleOrPermission) */ protected function getGuardNames(): Collection { - return Guard::getNames($this); + return (new Guard())->getNames($this); } /** @@ -151,7 +151,7 @@ protected function getGuardNames(): Collection */ protected function getDefaultGuardName(): string { - return Guard::getDefaultName($this); + return (new Guard())->getDefaultName($this); } /** diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 1da905a..e303415 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -9,6 +9,7 @@ use Jenssegers\Mongodb\Relations\BelongsToMany; use Maklad\Permission\Contracts\PermissionInterface as Permission; use Maklad\Permission\Contracts\RoleInterface as Role; +use ReflectionException; /** * Trait HasRoles @@ -159,26 +160,23 @@ public function hasRole($roles): bool if (\is_string($roles) && false !== \strpos($roles, '|')) { $roles = \explode('|', $roles); } - + $has_role = false; if (\is_string($roles)) { - return $this->roles->contains('name', $roles); - } - - if ($roles instanceof Role) { - return $this->roles->contains('id', $roles->id); - } - - if (\is_array($roles)) { + $has_role = $this->roles->contains('name', $roles); + }elseif ($roles instanceof Role) { + $has_role = $this->roles->contains('id', $roles->id); + }elseif (\is_array($roles)) { foreach ($roles as $role) { if ($this->hasRole($role)) { - return true; + $has_role = true; + break; } } - - return false; + }else { + $has_role = ! $roles->intersect($this->roles)->isEmpty(); } - return ! $roles->intersect($this->roles)->isEmpty(); + return $has_role; } /** @@ -207,18 +205,17 @@ public function hasAllRoles($roles): bool } if (\is_string($roles)) { - return $this->roles->contains('name', $roles); - } - - if ($roles instanceof Role) { - return $this->roles->contains('id', $roles->id); + $has_roles = $this->roles->contains('name', $roles); + }elseif ($roles instanceof Role) { + $has_roles = $this->roles->contains('id', $roles->id); + }else { + $roles = \collect()->make($roles)->map(function ($role) { + return $role instanceof Role ? $role->name : $role; + }); + $has_roles = $roles->intersect($this->roles->pluck('name')) == $roles; } - $roles = \collect()->make($roles)->map(function ($role) { - return $role instanceof Role ? $role->name : $role; - }); - - return $roles->intersect($this->roles->pluck('name')) == $roles; + return $has_roles; } /** @@ -228,6 +225,7 @@ public function hasAllRoles($roles): bool * @param string|null $guardName * * @return bool + * @throws ReflectionException */ public function hasPermissionTo($permission, $guardName = null): bool { @@ -281,6 +279,7 @@ protected function hasPermissionViaRole(Permission $permission): bool * @param string|Permission $permission * * @return bool + * @throws ReflectionException */ public function hasDirectPermission($permission): bool { @@ -327,6 +326,7 @@ public function getAllPermissions(): Collection * @param String|Role $role role name * * @return Role + * @throws ReflectionException */ protected function getStoredRole($role): Role { From ec07e57277f918789a13ca20cf8ca7ea973e7b9e Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sat, 17 Mar 2018 23:47:14 +0300 Subject: [PATCH 4/5] update styles --- src/Guard.php | 1 - src/Traits/HasRoles.php | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Guard.php b/src/Guard.php index 6c15c89..78fb7c6 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -1,5 +1,4 @@ roles->contains('name', $roles); - }elseif ($roles instanceof Role) { + } elseif ($roles instanceof Role) { $has_role = $this->roles->contains('id', $roles->id); - }elseif (\is_array($roles)) { + } elseif (\is_array($roles)) { foreach ($roles as $role) { if ($this->hasRole($role)) { $has_role = true; break; } } - }else { + } else { $has_role = ! $roles->intersect($this->roles)->isEmpty(); } @@ -206,9 +206,9 @@ public function hasAllRoles($roles): bool if (\is_string($roles)) { $has_roles = $this->roles->contains('name', $roles); - }elseif ($roles instanceof Role) { + } elseif ($roles instanceof Role) { $has_roles = $this->roles->contains('id', $roles->id); - }else { + } else { $roles = \collect()->make($roles)->map(function ($role) { return $role instanceof Role ? $role->name : $role; }); From bc76fe882b6386a9d78f1d6210c46ae3adfd7c21 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sat, 17 Mar 2018 23:50:14 +0300 Subject: [PATCH 5/5] update variable naming --- src/Traits/HasRoles.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 93cc089..7443e04 100644 --- a/src/Traits/HasRoles.php +++ b/src/Traits/HasRoles.php @@ -160,23 +160,23 @@ public function hasRole($roles): bool if (\is_string($roles) && false !== \strpos($roles, '|')) { $roles = \explode('|', $roles); } - $has_role = false; + $hasRole = false; if (\is_string($roles)) { - $has_role = $this->roles->contains('name', $roles); + $hasRole = $this->roles->contains('name', $roles); } elseif ($roles instanceof Role) { - $has_role = $this->roles->contains('id', $roles->id); + $hasRole = $this->roles->contains('id', $roles->id); } elseif (\is_array($roles)) { foreach ($roles as $role) { if ($this->hasRole($role)) { - $has_role = true; + $hasRole = true; break; } } } else { - $has_role = ! $roles->intersect($this->roles)->isEmpty(); + $hasRole = ! $roles->intersect($this->roles)->isEmpty(); } - return $has_role; + return $hasRole; } /** @@ -205,17 +205,17 @@ public function hasAllRoles($roles): bool } if (\is_string($roles)) { - $has_roles = $this->roles->contains('name', $roles); + $hasRoles = $this->roles->contains('name', $roles); } elseif ($roles instanceof Role) { - $has_roles = $this->roles->contains('id', $roles->id); + $hasRoles = $this->roles->contains('id', $roles->id); } else { $roles = \collect()->make($roles)->map(function ($role) { return $role instanceof Role ? $role->name : $role; }); - $has_roles = $roles->intersect($this->roles->pluck('name')) == $roles; + $hasRoles = $roles->intersect($this->roles->pluck('name')) == $roles; } - return $has_roles; + return $hasRoles; } /**