From 00308ad4ab9302293afd0a65f815feeaca06b626 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 8 Dec 2020 23:46:38 +0000 Subject: [PATCH] Cleaned up some user/image areas of the app Further cleanup of docblocks and standardisation of repos. --- app/Auth/Role.php | 5 +- app/Auth/UserRepo.php | 87 ++++++---------- app/Console/Commands/CreateAdmin.php | 2 - app/Entities/Models/Entity.php | 8 +- app/Http/Controllers/UserController.php | 2 +- app/Ownable.php | 8 -- app/Uploads/ImageService.php | 85 +--------------- app/Uploads/UserAvatars.php | 100 +++++++++++++++++++ resources/views/books/sort-box.blade.php | 4 +- resources/views/partials/book-tree.blade.php | 2 +- 10 files changed, 146 insertions(+), 157 deletions(-) create mode 100644 app/Uploads/UserAvatars.php diff --git a/app/Auth/Role.php b/app/Auth/Role.php index 255158afb..629cd6a95 100644 --- a/app/Auth/Role.php +++ b/app/Auth/Role.php @@ -5,6 +5,7 @@ use BookStack\Auth\Permissions\RolePermission; use BookStack\Interfaces\Loggable; use BookStack\Model; use Illuminate\Database\Eloquent\Collection; +use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; /** @@ -23,7 +24,7 @@ class Role extends Model implements Loggable /** * The roles that belong to the role. */ - public function users() + public function users(): BelongsToMany { return $this->belongsToMany(User::class)->orderBy('name', 'asc'); } @@ -39,7 +40,7 @@ class Role extends Model implements Loggable /** * The RolePermissions that belong to the role. */ - public function permissions() + public function permissions(): BelongsToMany { return $this->belongsToMany(RolePermission::class, 'permission_role', 'role_id', 'permission_id'); } diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 884f53463..6b7de3259 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -8,25 +8,24 @@ use BookStack\Entities\Models\Page; use BookStack\Exceptions\NotFoundException; use BookStack\Exceptions\UserUpdateException; use BookStack\Uploads\Image; +use BookStack\Uploads\UserAvatars; use Exception; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Pagination\LengthAwarePaginator; use Images; use Log; class UserRepo { - - protected $user; - protected $role; + protected $userAvatar; /** * UserRepo constructor. */ - public function __construct(User $user, Role $role) + public function __construct(UserAvatars $userAvatar) { - $this->user = $user; - $this->role = $role; + $this->userAvatar = $userAvatar; } /** @@ -34,25 +33,23 @@ class UserRepo */ public function getByEmail(string $email): ?User { - return $this->user->where('email', '=', $email)->first(); + return User::query()->where('email', '=', $email)->first(); } /** - * @param int $id - * @return User + * Get a user by their ID. */ - public function getById($id) + public function getById(int $id): User { - return $this->user->newQuery()->findOrFail($id); + return User::query()->findOrFail($id); } /** * Get all the users with their permissions. - * @return Builder|static */ - public function getAllUsers() + public function getAllUsers(): Collection { - return $this->user->with('roles', 'avatar')->orderBy('name', 'asc')->get(); + return User::query()->with('roles', 'avatar')->orderBy('name', 'asc')->get(); } /** @@ -68,7 +65,7 @@ class UserRepo ->take(1); } - $query = $this->user->with(['roles', 'avatar', 'latestActivity']) + $query = User::query()->with(['roles', 'avatar', 'latestActivity']) ->orderBy($sort, $sortData['order']); if ($sortData['search']) { @@ -96,14 +93,12 @@ class UserRepo /** * Assign a user to a system-level role. - * @param User $user - * @param $systemRoleName * @throws NotFoundException */ - public function attachSystemRole(User $user, $systemRoleName) + public function attachSystemRole(User $user, string $systemRoleName) { - $role = $this->role->newQuery()->where('system_name', '=', $systemRoleName)->first(); - if ($role === null) { + $role = Role::getSystemRole($systemRoleName); + if (is_null($role)) { throw new NotFoundException("Role '{$systemRoleName}' not found"); } $user->attachRole($role); @@ -111,26 +106,23 @@ class UserRepo /** * Checks if the give user is the only admin. - * @param User $user - * @return bool */ - public function isOnlyAdmin(User $user) + public function isOnlyAdmin(User $user): bool { if (!$user->hasSystemRole('admin')) { return false; } - $adminRole = $this->role->getSystemRole('admin'); - if ($adminRole->users->count() > 1) { + $adminRole = Role::getSystemRole('admin'); + if ($adminRole->users()->count() > 1) { return false; } + return true; } /** * Set the assigned user roles via an array of role IDs. - * @param User $user - * @param array $roles * @throws UserUpdateException */ public function setUserRoles(User $user, array $roles) @@ -145,14 +137,11 @@ class UserRepo /** * Check if the given user is the last admin and their new roles no longer * contains the admin role. - * @param User $user - * @param array $newRoles - * @return bool */ protected function demotingLastAdmin(User $user, array $newRoles) : bool { if ($this->isOnlyAdmin($user)) { - $adminRole = $this->role->getSystemRole('admin'); + $adminRole = Role::getSystemRole('admin'); if (!in_array(strval($adminRole->id), $newRoles)) { return true; } @@ -166,18 +155,18 @@ class UserRepo */ public function create(array $data, bool $emailConfirmed = false): User { - return $this->user->forceCreate([ + $details = [ 'name' => $data['name'], 'email' => $data['email'], 'password' => bcrypt($data['password']), 'email_confirmed' => $emailConfirmed, 'external_auth_id' => $data['external_auth_id'] ?? '', - ]); + ]; + return User::query()->forceCreate($details); } /** * Remove the given user from storage, Delete all related content. - * @param User $user * @throws Exception */ public function destroy(User $user) @@ -187,7 +176,10 @@ class UserRepo $user->delete(); // Delete user profile images - $profileImages = Image::where('type', '=', 'user')->where('uploaded_to', '=', $user->id)->get(); + $profileImages = Image::query()->where('type', '=', 'user') + ->where('uploaded_to', '=', $user->id) + ->get(); + foreach ($profileImages as $image) { Images::destroy($image); } @@ -195,12 +187,8 @@ class UserRepo /** * Get the latest activity for a user. - * @param User $user - * @param int $count - * @param int $page - * @return array */ - public function getActivity(User $user, $count = 20, $page = 0) + public function getActivity(User $user, int $count = 20, int $page = 0): array { return Activity::userActivity($user, $count, $page); } @@ -241,33 +229,22 @@ class UserRepo /** * Get the roles in the system that are assignable to a user. - * @return mixed */ - public function getAllRoles() + public function getAllRoles(): Collection { - return $this->role->newQuery()->orderBy('display_name', 'asc')->get(); + return Role::query()->orderBy('display_name', 'asc')->get(); } /** * Get an avatar image for a user and set it as their avatar. * Returns early if avatars disabled or not set in config. - * @param User $user - * @return bool */ - public function downloadAndAssignUserAvatar(User $user) + public function downloadAndAssignUserAvatar(User $user): void { - if (!Images::avatarFetchEnabled()) { - return false; - } - try { - $avatar = Images::saveUserAvatar($user); - $user->avatar()->associate($avatar); - $user->save(); - return true; + $this->userAvatar->fetchAndAssignToUser($user); } catch (Exception $e) { Log::error('Failed to save user avatar image'); - return false; } } } diff --git a/app/Console/Commands/CreateAdmin.php b/app/Console/Commands/CreateAdmin.php index e67da8717..3d1a3dca0 100644 --- a/app/Console/Commands/CreateAdmin.php +++ b/app/Console/Commands/CreateAdmin.php @@ -28,8 +28,6 @@ class CreateAdmin extends Command /** * Create a new command instance. - * - * @param UserRepo $userRepo */ public function __construct(UserRepo $userRepo) { diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index f764ad16e..e681a4e22 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -205,12 +205,12 @@ abstract class Entity extends Ownable } /** - * Get entity type. - * @return mixed + * Get the entity type as a simple lowercase word. */ - public static function getType() + public static function getType(): string { - return strtolower(static::getClassName()); + $className = array_slice(explode('\\', static::class), -1, 1)[0]; + return strtolower($className); } /** diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 1713565fc..8d688ed84 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -188,7 +188,7 @@ class UserController extends Controller $user->image_id = $image->id; } - // Delete the profile image if set to + // Delete the profile image if reset option is in request if ($request->has('profile_image_reset')) { $this->imageRepo->destroyImage($user->avatar); } diff --git a/app/Ownable.php b/app/Ownable.php index bf24fad5d..b118bc742 100644 --- a/app/Ownable.php +++ b/app/Ownable.php @@ -26,12 +26,4 @@ abstract class Ownable extends Model return $this->belongsTo(User::class, 'updated_by'); } - /** - * Gets the class name. - * @return string - */ - public static function getClassName() - { - return strtolower(array_slice(explode('\\', static::class), -1, 1)[0]); - } } diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 1e5ad8aa1..5c16827cb 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -1,7 +1,5 @@ image = $image; $this->imageTool = $imageTool; $this->fileSystem = $fileSystem; $this->cache = $cache; - $this->http = $http; } /** @@ -77,14 +72,9 @@ class ImageService /** * Save a new image from a uri-encoded base64 string of data. - * @param string $base64Uri - * @param string $name - * @param string $type - * @param int $uploadedTo - * @return Image * @throws ImageUploadException */ - public function saveNewFromBase64Uri(string $base64Uri, string $name, string $type, $uploadedTo = 0) + public function saveNewFromBase64Uri(string $base64Uri, string $name, string $type, int $uploadedTo = 0): Image { $splitData = explode(';base64,', $base64Uri); if (count($splitData) < 2) { @@ -94,30 +84,11 @@ class ImageService return $this->saveNew($name, $data, $type, $uploadedTo); } - /** - * Gets an image from url and saves it to the database. - * @param $url - * @param string $type - * @param bool|string $imageName - * @return mixed - * @throws Exception - */ - private function saveNewFromUrl($url, $type, $imageName = false) - { - $imageName = $imageName ? $imageName : basename($url); - try { - $imageData = $this->http->fetch($url); - } catch (HttpFetchException $exception) { - throw new Exception(trans('errors.cannot_get_image_from_url', ['url' => $url])); - } - return $this->saveNew($imageName, $imageData, $type); - } - /** * Save a new image into storage. * @throws ImageUploadException */ - private function saveNew(string $imageName, string $imageData, string $type, int $uploadedTo = 0): Image + public function saveNew(string $imageName, string $imageData, string $type, int $uploadedTo = 0): Image { $storage = $this->getStorage($type); $secureUploads = setting('app-secure-images'); @@ -327,56 +298,6 @@ class ImageService return (count($files) === 0 && count($folders) === 0); } - /** - * Save an avatar image from an external service. - * @throws Exception - */ - public function saveUserAvatar(User $user, int $size = 500): Image - { - $avatarUrl = $this->getAvatarUrl(); - $email = strtolower(trim($user->email)); - - $replacements = [ - '${hash}' => md5($email), - '${size}' => $size, - '${email}' => urlencode($email), - ]; - - $userAvatarUrl = strtr($avatarUrl, $replacements); - $imageName = str_replace(' ', '-', $user->name . '-avatar.png'); - $image = $this->saveNewFromUrl($userAvatarUrl, 'user', $imageName); - $image->created_by = $user->id; - $image->updated_by = $user->id; - $image->uploaded_to = $user->id; - $image->save(); - - return $image; - } - - /** - * Check if fetching external avatars is enabled. - */ - public function avatarFetchEnabled(): bool - { - $fetchUrl = $this->getAvatarUrl(); - return is_string($fetchUrl) && strpos($fetchUrl, 'http') === 0; - } - - /** - * Get the URL to fetch avatars from. - * @return string|mixed - */ - protected function getAvatarUrl() - { - $url = trim(config('services.avatar_url')); - - if (empty($url) && !config('services.disable_services')) { - $url = 'https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon'; - } - - return $url; - } - /** * Delete gallery and drawings that are not within HTML content of pages or page revisions. * Checks based off of only the image name. diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php new file mode 100644 index 000000000..92b06bc8a --- /dev/null +++ b/app/Uploads/UserAvatars.php @@ -0,0 +1,100 @@ +imageService = $imageService; + $this->http = $http; + } + + /** + * Fetch and assign an avatar image to the given user. + */ + public function fetchAndAssignToUser(User $user): void + { + if (!$this->avatarFetchEnabled()) { + return; + } + + try { + $avatar = $this->saveAvatarImage($user); + $user->avatar()->associate($avatar); + $user->save(); + } catch (Exception $e) { + Log::error('Failed to save user avatar image'); + } + } + + /** + * Save an avatar image from an external service. + * @throws Exception + */ + protected function saveAvatarImage(User $user, int $size = 500): Image + { + $avatarUrl = $this->getAvatarUrl(); + $email = strtolower(trim($user->email)); + + $replacements = [ + '${hash}' => md5($email), + '${size}' => $size, + '${email}' => urlencode($email), + ]; + + $userAvatarUrl = strtr($avatarUrl, $replacements); + $imageName = str_replace(' ', '-', $user->id . '-avatar.png'); + $imageData = $this->getAvatarImageData($userAvatarUrl); + + $image = $this->imageService->saveNew($imageName, $imageData, 'user', $user->id); + $image->created_by = $user->id; + $image->updated_by = $user->id; + $image->save(); + + return $image; + } + + /** + * Gets an image from url and returns it as a string of image data. + * @throws Exception + */ + protected function getAvatarImageData(string $url): string + { + try { + $imageData = $this->http->fetch($url); + } catch (HttpFetchException $exception) { + throw new Exception(trans('errors.cannot_get_image_from_url', ['url' => $url])); + } + return $imageData; + } + + /** + * Check if fetching external avatars is enabled. + */ + protected function avatarFetchEnabled(): bool + { + $fetchUrl = $this->getAvatarUrl(); + return is_string($fetchUrl) && strpos($fetchUrl, 'http') === 0; + } + + /** + * Get the URL to fetch avatars from. + */ + protected function getAvatarUrl(): string + { + $url = trim(config('services.avatar_url')); + + if (empty($url) && !config('services.disable_services')) { + $url = 'https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon'; + } + + return $url; + } + +} \ No newline at end of file diff --git a/resources/views/books/sort-box.blade.php b/resources/views/books/sort-box.blade.php index 98f0af87e..5d14e3598 100644 --- a/resources/views/books/sort-box.blade.php +++ b/resources/views/books/sort-box.blade.php @@ -13,8 +13,8 @@