From 68017e25534b13db5b784c634c0d6f2d3981affb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 23 Dec 2018 15:34:38 +0000 Subject: [PATCH] Added testing for avatar fetching systems & config Abstracts imageservice http interaction. Closes #1193 --- app/Exceptions/HttpFetchException.php | 5 ++ app/Providers/CustomFacadeProvider.php | 4 +- app/Uploads/HttpFetcher.php | 34 +++++++++++ app/Uploads/ImageService.php | 28 +++++---- composer.json | 1 + config/services.php | 4 +- phpunit.xml | 1 + tests/{ => Uploads}/AttachmentTest.php | 0 tests/Uploads/AvatarTest.php | 84 ++++++++++++++++++++++++++ tests/{ => Uploads}/ImageTest.php | 58 +----------------- tests/Uploads/UsesImages.php | 69 +++++++++++++++++++++ 11 files changed, 217 insertions(+), 71 deletions(-) create mode 100644 app/Exceptions/HttpFetchException.php create mode 100644 app/Uploads/HttpFetcher.php rename tests/{ => Uploads}/AttachmentTest.php (100%) create mode 100644 tests/Uploads/AvatarTest.php rename tests/{ => Uploads}/ImageTest.php (85%) create mode 100644 tests/Uploads/UsesImages.php diff --git a/app/Exceptions/HttpFetchException.php b/app/Exceptions/HttpFetchException.php new file mode 100644 index 000000000..48e30e1e6 --- /dev/null +++ b/app/Exceptions/HttpFetchException.php @@ -0,0 +1,5 @@ +app->make(Image::class), $this->app->make(ImageManager::class), $this->app->make(Factory::class), - $this->app->make(Repository::class) + $this->app->make(Repository::class), + $this->app->make(HttpFetcher::class) ); }); } diff --git a/app/Uploads/HttpFetcher.php b/app/Uploads/HttpFetcher.php new file mode 100644 index 000000000..3ebe17eee --- /dev/null +++ b/app/Uploads/HttpFetcher.php @@ -0,0 +1,34 @@ + $uri, + CURLOPT_RETURNTRANSFER => 1, + CURLOPT_CONNECTTIMEOUT => 5 + ]); + + $data = curl_exec($ch); + $err = curl_error($ch); + curl_close($ch); + + if ($err) { + throw new HttpFetchException($err); + } + + return $data; + } + +} \ No newline at end of file diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index d5f4068ef..1dd8b713d 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -1,6 +1,7 @@ image = $image; $this->imageTool = $imageTool; $this->cache = $cache; + $this->http = $http; parent::__construct($fileSystem); } @@ -95,8 +99,9 @@ class ImageService extends UploadService private function saveNewFromUrl($url, $type, $imageName = false) { $imageName = $imageName ? $imageName : basename($url); - $imageData = file_get_contents($url); - if ($imageData === false) { + 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); @@ -322,7 +327,13 @@ class ImageService extends UploadService */ protected function getAvatarUrl() { - return trim(config('services.avatar_url')); + $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; } /** @@ -392,14 +403,7 @@ class ImageService extends UploadService } } else { try { - $ch = curl_init(); - curl_setopt_array($ch, [CURLOPT_URL => $uri, CURLOPT_RETURNTRANSFER => 1, CURLOPT_CONNECTTIMEOUT => 5]); - $imageData = curl_exec($ch); - $err = curl_error($ch); - curl_close($ch); - if ($err) { - throw new \Exception("Image fetch failed, Received error: " . $err); - } + $imageData = $this->http->fetch($uri); } catch (\Exception $e) { } } diff --git a/composer.json b/composer.json index 2850eb235..48b977e23 100644 --- a/composer.json +++ b/composer.json @@ -12,6 +12,7 @@ "ext-xml": "*", "ext-mbstring": "*", "ext-gd": "*", + "ext-curl": "*", "laravel/framework": "~5.5.44", "fideloper/proxy": "~3.3", "intervention/image": "^2.4", diff --git a/config/services.php b/config/services.php index 7b9cf4e74..a07fab23f 100644 --- a/config/services.php +++ b/config/services.php @@ -21,9 +21,7 @@ return [ 'drawio' => env('DRAWIO', !env('DISABLE_EXTERNAL_SERVICES', false)), // URL for fetching avatars - 'avatar_url' => env('AVATAR_URL', - env('DISABLE_EXTERNAL_SERVICES', false) ? false : 'https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon' - ), + 'avatar_url' => env('AVATAR_URL', ''), // Callback URL for social authentication methods 'callback_url' => env('APP_URL', false), diff --git a/phpunit.xml b/phpunit.xml index 3d18d9bbf..804afcf5d 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -31,6 +31,7 @@ + diff --git a/tests/AttachmentTest.php b/tests/Uploads/AttachmentTest.php similarity index 100% rename from tests/AttachmentTest.php rename to tests/Uploads/AttachmentTest.php diff --git a/tests/Uploads/AvatarTest.php b/tests/Uploads/AvatarTest.php new file mode 100644 index 000000000..ecf7037a9 --- /dev/null +++ b/tests/Uploads/AvatarTest.php @@ -0,0 +1,84 @@ +asAdmin()->post('/settings/users/create', [ + 'name' => $user->name, + 'email' => $user->email, + 'password' => 'testing', + 'password-confirm' => 'testing', + ]); + return User::where('email', '=', $user->email)->first(); + } + + protected function assertImageFetchFrom(string $url) + { + $http = \Mockery::mock(HttpFetcher::class); + $this->app->instance(HttpFetcher::class, $http); + + $http->shouldReceive('fetch') + ->once()->with($url) + ->andReturn($this->getTestImageContent()); + } + + protected function deleteUserImage(User $user) + { + $this->deleteImage($user->avatar->path); + } + + public function test_gravatar_fetched_on_user_create() + { + config()->set([ + 'services.disable_services' => false, + ]); + $user = factory(User::class)->make(); + $this->assertImageFetchFrom('https://www.gravatar.com/avatar/'.md5(strtolower($user->email)).'?s=500&d=identicon'); + + $user = $this->createUserRequest($user); + $this->assertDatabaseHas('images', [ + 'type' => 'user', + 'created_by' => $user->id + ]); + $this->deleteUserImage($user); + } + + + public function test_custom_url_used_if_set() + { + config()->set([ + 'services.avatar_url' => 'https://example.com/${email}/${hash}/${size}', + ]); + + $user = factory(User::class)->make(); + $url = 'https://example.com/'. urlencode(strtolower($user->email)) .'/'. md5(strtolower($user->email)).'/500'; + $this->assertImageFetchFrom($url); + + $user = $this->createUserRequest($user); + $this->deleteUserImage($user); + } + + public function test_avatar_not_fetched_if_no_custom_url_and_services_disabled() + { + config()->set([ + 'services.disable_services' => true, + ]); + + $user = factory(User::class)->make(); + + $http = \Mockery::mock(HttpFetcher::class); + $this->app->instance(HttpFetcher::class, $http); + $http->shouldNotReceive('fetch'); + + $this->createUserRequest($user); + } + +} diff --git a/tests/ImageTest.php b/tests/Uploads/ImageTest.php similarity index 85% rename from tests/ImageTest.php rename to tests/Uploads/ImageTest.php index 38bac2cca..6dafa7f5a 100644 --- a/tests/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -1,67 +1,15 @@ -getTestImageFilePath(), $fileName, 'image/png', 5238); - } - - /** - * Get the path for a test image. - * @param $type - * @param $fileName - * @return string - */ - protected function getTestImagePath($type, $fileName) - { - return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName; - } - - /** - * Uploads an image with the given name. - * @param $name - * @param int $uploadedTo - * @return \Illuminate\Foundation\Testing\TestResponse - */ - protected function uploadImage($name, $uploadedTo = 0) - { - $file = $this->getTestImage($name); - return $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []); - } - - /** - * Delete an uploaded image. - * @param $relPath - */ - protected function deleteImage($relPath) - { - $path = public_path($relPath); - if (file_exists($path)) { - unlink($path); - } - } + use UsesImages; public function test_image_upload() { diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php new file mode 100644 index 000000000..16cb7c2b9 --- /dev/null +++ b/tests/Uploads/UsesImages.php @@ -0,0 +1,69 @@ +getTestImageFilePath(), $fileName, 'image/png', 5238); + } + + /** + * Get the raw file data for the test image. + * @return false|string + */ + protected function getTestImageContent() + { + return file_get_contents($this->getTestImageFilePath()); + } + + /** + * Get the path for a test image. + * @param $type + * @param $fileName + * @return string + */ + protected function getTestImagePath($type, $fileName) + { + return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName; + } + + /** + * Uploads an image with the given name. + * @param $name + * @param int $uploadedTo + * @return \Illuminate\Foundation\Testing\TestResponse + */ + protected function uploadImage($name, $uploadedTo = 0) + { + $file = $this->getTestImage($name); + return $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []); + } + + /** + * Delete an uploaded image. + * @param $relPath + */ + protected function deleteImage($relPath) + { + $path = public_path($relPath); + if (file_exists($path)) { + unlink($path); + } + } + +} \ No newline at end of file