Improved ValidationException to avoid polluting the message with invalid data but keeping it on the string representation

This commit is contained in:
Alejandro Celaya 2019-11-21 20:05:06 +01:00
parent ad592a563c
commit 6ddb60d047
6 changed files with 55 additions and 53 deletions

View file

@ -34,24 +34,34 @@ class ValidationException extends RuntimeException
return static::fromArray($inputFilter->getMessages(), $prev);
}
private static function fromArray(array $invalidData, ?Throwable $prev = null): self
public static function fromArray(array $invalidData, ?Throwable $prev = null): self
{
return new self(
sprintf(
'Provided data is not valid. These are the messages:%s%s%s',
PHP_EOL,
self::formMessagesToString($invalidData),
PHP_EOL
),
$invalidData,
-1,
$prev
return new self('Provided data is not valid', $invalidData, -1, $prev);
}
public function getInvalidElements(): array
{
return $this->invalidElements;
}
public function __toString(): string
{
return sprintf(
'%s %s in %s:%s%s%sStack trace:%s%s',
__CLASS__,
$this->getMessage(),
$this->getFile(),
$this->getLine(),
$this->invalidElementsToString(),
PHP_EOL,
PHP_EOL,
$this->getTraceAsString()
);
}
private static function formMessagesToString(array $messages = []): string
private function invalidElementsToString(): string
{
return reduce_left($messages, function ($messageSet, $name, $_, string $acc) {
return reduce_left($this->invalidElements, function ($messageSet, string $name, $_, string $acc) {
return $acc . sprintf(
"\n '%s' => %s",
$name,
@ -59,9 +69,4 @@ class ValidationException extends RuntimeException
);
}, '');
}
public function getInvalidElements(): array
{
return $this->invalidElements;
}
}

View file

@ -55,12 +55,9 @@ class ValidationExceptionTest extends TestCase
'something' => ['baz', 'foo'],
];
$barValue = print_r(['baz', 'foo'], true);
$expectedMessage = <<<EOT
Provided data is not valid. These are the messages:
$expectedStringRepresentation = <<<EOT
'foo' => bar
'something' => {$barValue}
EOT;
$inputFilter = $this->prophesize(InputFilterInterface::class);
@ -69,9 +66,10 @@ EOT;
$e = ValidationException::fromInputFilter($inputFilter->reveal());
$this->assertEquals($invalidData, $e->getInvalidElements());
$this->assertEquals($expectedMessage, $e->getMessage());
$this->assertEquals('Provided data is not valid', $e->getMessage());
$this->assertEquals(-1, $e->getCode());
$this->assertEquals($prev, $e->getPrevious());
$this->assertStringContainsString($expectedStringRepresentation, (string) $e);
$getMessages->shouldHaveBeenCalledOnce();
}

View file

@ -7,9 +7,9 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer;
@ -44,7 +44,7 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
{
try {
$shortUrlData = $this->buildShortUrlData($request);
} catch (InvalidArgumentException $e) {
} catch (ValidationException $e) {
$this->logger->warning('Provided data is invalid. {e}', ['e' => $e]);
return new JsonResponse([
'error' => RestUtils::INVALID_ARGUMENT_ERROR,
@ -79,7 +79,7 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
/**
* @param Request $request
* @return CreateShortUrlData
* @throws InvalidArgumentException
* @throws ValidationException
*/
abstract protected function buildShortUrlData(Request $request): CreateShortUrlData;
}

View file

@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Cake\Chronos\Chronos;
use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
@ -20,30 +19,27 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction
/**
* @param Request $request
* @return CreateShortUrlData
* @throws InvalidArgumentException
* @throws \InvalidArgumentException
* @throws ValidationException
*/
protected function buildShortUrlData(Request $request): CreateShortUrlData
{
$postData = (array) $request->getParsedBody();
if (! isset($postData['longUrl'])) {
throw new InvalidArgumentException('A URL was not provided');
throw ValidationException::fromArray([
'longUrl' => 'A URL was not provided',
]);
}
try {
$meta = ShortUrlMeta::createFromParams(
$this->getOptionalDate($postData, 'validSince'),
$this->getOptionalDate($postData, 'validUntil'),
$postData['customSlug'] ?? null,
$postData['maxVisits'] ?? null,
$postData['findIfExists'] ?? null,
$postData['domain'] ?? null
);
$meta = ShortUrlMeta::createFromParams(
$this->getOptionalDate($postData, 'validSince'),
$this->getOptionalDate($postData, 'validUntil'),
$postData['customSlug'] ?? null,
$postData['maxVisits'] ?? null,
$postData['findIfExists'] ?? null,
$postData['domain'] ?? null
);
return new CreateShortUrlData(new Uri($postData['longUrl']), (array) ($postData['tags'] ?? []), $meta);
} catch (ValidationException $e) {
throw new InvalidArgumentException('Provided meta data is not valid', -1, $e);
}
return new CreateShortUrlData(new Uri($postData['longUrl']), (array) ($postData['tags'] ?? []), $meta);
}
private function getOptionalDate(array $postData, string $fieldName): ?Chronos

View file

@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
@ -33,19 +33,22 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
/**
* @param Request $request
* @return CreateShortUrlData
* @throws \InvalidArgumentException
* @throws InvalidArgumentException
* @throws ValidationException
*/
protected function buildShortUrlData(Request $request): CreateShortUrlData
{
$query = $request->getQueryParams();
if (! $this->apiKeyService->check($query['apiKey'] ?? '')) {
throw new InvalidArgumentException('No API key was provided or it is not valid');
throw ValidationException::fromArray([
'apiKey' => 'No API key was provided or it is not valid',
]);
}
if (! isset($query['longUrl'])) {
throw new InvalidArgumentException('A URL was not provided');
throw ValidationException::fromArray([
'longUrl' => 'A URL was not provided',
]);
}
return new CreateShortUrlData(new Uri($query['longUrl']));

View file

@ -42,7 +42,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
}
/** @test */
public function errorResponseIsReturnedIfInvalidApiKeyIsProvided()
public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void
{
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(false);
@ -53,12 +53,12 @@ class SingleStepCreateShortUrlActionTest extends TestCase
$this->assertEquals(400, $resp->getStatusCode());
$this->assertEquals('INVALID_ARGUMENT', $payload['error']);
$this->assertEquals('No API key was provided or it is not valid', $payload['message']);
$this->assertEquals('Provided data is not valid', $payload['message']);
$findApiKey->shouldHaveBeenCalled();
}
/** @test */
public function errorResponseIsReturnedIfNoUrlIsProvided()
public function errorResponseIsReturnedIfNoUrlIsProvided(): void
{
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(true);
@ -69,12 +69,12 @@ class SingleStepCreateShortUrlActionTest extends TestCase
$this->assertEquals(400, $resp->getStatusCode());
$this->assertEquals('INVALID_ARGUMENT', $payload['error']);
$this->assertEquals('A URL was not provided', $payload['message']);
$this->assertEquals('Provided data is not valid', $payload['message']);
$findApiKey->shouldHaveBeenCalled();
}
/** @test */
public function properDataIsPassedWhenGeneratingShortCode()
public function properDataIsPassedWhenGeneratingShortCode(): void
{
$request = (new ServerRequest())->withQueryParams([
'apiKey' => 'abc123',