- Fixed some existing issues in new aligned process.
- Manually tested each external call scenario.
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;
-use Psr\Http\Client\ClientExceptionInterface;
class DispatchWebhookJob implements ShouldQueue
{
$lastError = "Response status from endpoint was {$statusCode}";
Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with status {$statusCode}");
}
- } catch (ClientExceptionInterface $error) {
+ } catch (\Exception $error) {
$lastError = $error->getMessage();
Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");
}
{
return $this->requestAt($this->requestCount() - 1);
}
+
+ public function all(): array
+ {
+ return $this->container;
+ }
}
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Middleware;
use GuzzleHttp\Psr7\Request as GuzzleRequest;
+use GuzzleHttp\Psr7\Response;
use Psr\Http\Client\ClientInterface;
class HttpRequestService
/**
* Build a new http client for sending requests on.
*/
- public function buildClient(int $timeout, array $options): ClientInterface
+ public function buildClient(int $timeout, array $options = []): ClientInterface
{
$defaultOptions = [
'timeout' => $timeout,
* Returns history which can then be queried.
* @link https://docs.guzzlephp.org/en/stable/testing.html#history-middleware
*/
- public function mockClient(array $responses = []): HttpClientHistory
+ public function mockClient(array $responses = [], bool $pad = true): HttpClientHistory
{
+ // By default, we pad out the responses with 10 successful values so that requests will be
+ // properly recorded for inspection. Otherwise, we can't later check if we're received
+ // too many requests.
+ if ($pad) {
+ $response = new Response(200, [], 'success');
+ $responses = array_merge($responses, array_fill(0, 10, $response));
+ }
+
$container = [];
$history = Middleware::history($container);
$mock = new MockHandler($responses);
+++ /dev/null
-<?php
-
-namespace BookStack\Uploads;
-
-use BookStack\Exceptions\HttpFetchException;
-
-class HttpFetcher
-{
- /**
- * Fetch content from an external URI.
- *
- * @param string $uri
- *
- * @throws HttpFetchException
- *
- * @return bool|string
- */
- public function fetch(string $uri)
- {
- $ch = curl_init();
- curl_setopt_array($ch, [
- CURLOPT_URL => $uri,
- CURLOPT_RETURNTRANSFER => 1,
- CURLOPT_CONNECTTIMEOUT => 5,
- ]);
-
- $data = curl_exec($ch);
- $err = curl_error($ch);
- curl_close($ch);
-
- if ($err) {
- $errno = curl_errno($ch);
- throw new HttpFetchException($err, $errno);
- }
-
- return $data;
- }
-}
namespace BookStack\Uploads;
use BookStack\Exceptions\HttpFetchException;
+use BookStack\Http\HttpRequestService;
use BookStack\Users\Models\User;
use Exception;
+use GuzzleHttp\Psr7\Request;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Str;
+use Psr\Http\Client\ClientExceptionInterface;
class UserAvatars
{
- protected $imageService;
- protected $http;
-
- public function __construct(ImageService $imageService, HttpFetcher $http)
- {
- $this->imageService = $imageService;
- $this->http = $http;
+ public function __construct(
+ protected ImageService $imageService,
+ protected HttpRequestService $http
+ ) {
}
/**
protected function getAvatarImageData(string $url): string
{
try {
- $imageData = $this->http->fetch($url);
- } catch (HttpFetchException $exception) {
+ $client = $this->http->buildClient(5);
+ $response = $client->sendRequest(new Request('GET', $url));
+ $imageData = (string) $response->getBody();
+ } catch (ClientExceptionInterface $exception) {
throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]), $exception->getCode(), $exception);
}
{
$fetchUrl = $this->getAvatarUrl();
- return is_string($fetchUrl) && strpos($fetchUrl, 'http') === 0;
+ return str_starts_with($fetchUrl, 'http');
}
/**
use BookStack\Http\HttpClientHistory;
use BookStack\Http\HttpRequestService;
use BookStack\Settings\SettingService;
-use BookStack\Uploads\HttpFetcher;
use BookStack\Users\Models\User;
use Illuminate\Contracts\Console\Kernel;
use Illuminate\Foundation\Testing\DatabaseTransactions;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
use Illuminate\Testing\Assert as PHPUnit;
-use Mockery;
use Monolog\Handler\TestHandler;
use Monolog\Logger;
use Ssddanbrown\AssertHtml\TestsHtml;
}
}
- /**
- * Mock the HttpFetcher service and return the given data on fetch.
- */
- protected function mockHttpFetch($returnData, int $times = 1)
- {
- // TODO - Remove
- $mockHttp = Mockery::mock(HttpFetcher::class);
- $this->app[HttpFetcher::class] = $mockHttp;
- $mockHttp->shouldReceive('fetch')
- ->times($times)
- ->andReturn($returnData);
- }
-
/**
* Mock the http client used in BookStack http calls.
*/
namespace Tests\Uploads;
use BookStack\Exceptions\HttpFetchException;
-use BookStack\Uploads\HttpFetcher;
use BookStack\Uploads\UserAvatars;
use BookStack\Users\Models\User;
+use GuzzleHttp\Exception\ConnectException;
+use GuzzleHttp\Psr7\Request;
+use GuzzleHttp\Psr7\Response;
use Tests\TestCase;
class AvatarTest extends TestCase
return User::query()->where('email', '=', $user->email)->first();
}
- protected function assertImageFetchFrom(string $url)
- {
- $http = $this->mock(HttpFetcher::class);
-
- $http->shouldReceive('fetch')
- ->once()->with($url)
- ->andReturn($this->files->pngImageData());
- }
-
- protected function deleteUserImage(User $user)
+ protected function deleteUserImage(User $user): void
{
$this->files->deleteAtRelativePath($user->avatar->path);
}
public function test_gravatar_fetched_on_user_create()
{
- config()->set([
- 'services.disable_services' => false,
- ]);
+ $requests = $this->mockHttpClient([new Response(200, ['Content-Type' => 'image/png'], $this->files->pngImageData())]);
+ config()->set(['services.disable_services' => false]);
$user = User::factory()->make();
- $this->assertImageFetchFrom('https://www.gravatar.com/avatar/' . md5(strtolower($user->email)) . '?s=500&d=identicon');
$user = $this->createUserRequest($user);
$this->assertDatabaseHas('images', [
'created_by' => $user->id,
]);
$this->deleteUserImage($user);
+
+ $expectedUri = 'https://www.gravatar.com/avatar/' . md5(strtolower($user->email)) . '?s=500&d=identicon';
+ $this->assertEquals($expectedUri, $requests->latestRequest()->getUri());
}
public function test_custom_url_used_if_set()
$user = User::factory()->make();
$url = 'https://example.com/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500';
- $this->assertImageFetchFrom($url);
+ $requests = $this->mockHttpClient([new Response(200, ['Content-Type' => 'image/png'], $this->files->pngImageData())]);
$user = $this->createUserRequest($user);
+ $this->assertEquals($url, $requests->latestRequest()->getUri());
$this->deleteUserImage($user);
}
public function test_avatar_not_fetched_if_no_custom_url_and_services_disabled()
{
- config()->set([
- 'services.disable_services' => true,
- ]);
-
+ config()->set(['services.disable_services' => true]);
$user = User::factory()->make();
-
- $http = $this->mock(HttpFetcher::class);
- $http->shouldNotReceive('fetch');
+ $requests = $this->mockHttpClient([new Response()]);
$this->createUserRequest($user);
+
+ $this->assertEquals(0, $requests->requestCount());
}
public function test_avatar_not_fetched_if_avatar_url_option_set_to_false()
]);
$user = User::factory()->make();
-
- $http = $this->mock(HttpFetcher::class);
- $http->shouldNotReceive('fetch');
+ $requests = $this->mockHttpClient([new Response()]);
$this->createUserRequest($user);
+
+ $this->assertEquals(0, $requests->requestCount());
}
public function test_no_failure_but_error_logged_on_failed_avatar_fetch()
{
- config()->set([
- 'services.disable_services' => false,
- ]);
+ config()->set(['services.disable_services' => false]);
- $http = $this->mock(HttpFetcher::class);
- $http->shouldReceive('fetch')->andThrow(new HttpFetchException());
+ $this->mockHttpClient([new ConnectException('Failed to connect', new Request('GET', ''))]);
$logger = $this->withTestLogger();
$user = User::factory()->make();
$avatar = app()->make(UserAvatars::class);
- $url = 'http_malformed_url/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500';
$logger = $this->withTestLogger();
+ $this->mockHttpClient([new ConnectException('Could not resolve host http_malformed_url', new Request('GET', ''))]);
$avatar->fetchAndAssignToUser($user);
+ $url = 'http_malformed_url/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500';
$this->assertTrue($logger->hasError('Failed to save user avatar image'));
$exception = $logger->getRecords()[0]['context']['exception'];
- $this->assertEquals(new HttpFetchException(
- 'Cannot get image from ' . $url,
- 6,
- (new HttpFetchException('Could not resolve host: http_malformed_url', 6))
- ), $exception);
+ $this->assertInstanceOf(HttpFetchException::class, $exception);
+ $this->assertEquals('Cannot get image from ' . $url, $exception->getMessage());
+ $this->assertEquals('Could not resolve host http_malformed_url', $exception->getPrevious()->getMessage());
}
}
projects / bookstack / commitdiff