Merge pull request #407 from acelaya/feature/edit-patch

Feature/edit patch
This commit is contained in:
Alejandro Celaya 2019-05-05 10:22:52 +02:00 committed by GitHub
commit 8d412e7d4c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 174 additions and 35 deletions

View file

@ -22,7 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
### Deprecated
* *Nothing*
* [#406](https://github.com/shlinkio/shlink/issues/406) Deprecated `PUT /short-urls/{shortCode}` REST endpoint in favor of `PATCH /short-urls/{shortCode}`.
#### Removed

View file

@ -81,7 +81,7 @@
}
},
"put": {
"patch": {
"operationId": "editShortUrl",
"tags": [
"Short URLs"
@ -169,6 +169,95 @@
}
},
"put": {
"deprecated": true,
"operationId": "editShortUrlPut",
"tags": [
"Short URLs"
],
"summary": "[DEPRECATED] Edit short URL",
"description": "**[DEPRECATED]** Use [editShortUrl](#/Short_URLs/getShortUrl) instead",
"parameters": [
{
"name": "shortCode",
"in": "path",
"description": "The short code to edit.",
"required": true,
"schema": {
"type": "string"
}
}
],
"requestBody": {
"description": "Request body.",
"required": true,
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"validSince": {
"description": "The date (in ISO-8601 format) from which this short code will be valid",
"type": "string"
},
"validUntil": {
"description": "The date (in ISO-8601 format) until which this short code will be valid",
"type": "string"
},
"maxVisits": {
"description": "The maximum number of allowed visits for this short code",
"type": "number"
}
}
}
}
}
},
"security": [
{
"ApiKey": []
},
{
"Bearer": []
}
],
"responses": {
"204": {
"description": "The short code has been properly updated."
},
"400": {
"description": "Provided meta arguments are invalid.",
"content": {
"application/json": {
"schema": {
"$ref": "../definitions/Error.json"
}
}
}
},
"404": {
"description": "No short URL was found for provided short code.",
"content": {
"application/json": {
"schema": {
"$ref": "../definitions/Error.json"
}
}
}
},
"500": {
"description": "Unexpected error.",
"content": {
"application/json": {
"schema": {
"$ref": "../definitions/Error.json"
}
}
}
}
}
},
"delete": {
"operationId": "deleteShortUrl",
"tags": [

View file

@ -35,7 +35,7 @@
"name": "X-Api-Key"
},
"Bearer": {
"description": "**[Deprecated]** The JWT identifying a previously authenticated API key",
"description": "**[DEPRECATED]** The JWT identifying a previously authenticated API key",
"type": "http",
"scheme": "bearer",
"bearerFormat": "JWT"
@ -66,7 +66,7 @@
},
{
"name": "Authentication",
"description": "**[Deprecated]** Authentication-related endpoints"
"description": "**[DEPRECATED]** Authentication-related endpoints"
}
],

View file

@ -19,7 +19,7 @@ use function sprintf;
class EditShortUrlAction extends AbstractRestAction
{
protected const ROUTE_PATH = '/short-urls/{shortCode}';
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT];
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH, self::METHOD_PUT];
/** @var ShortUrlServiceInterface */
private $shortUrlService;

View file

@ -4,29 +4,19 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Middleware;
use Fig\Http\Message\RequestMethodInterface;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Authentication;
use Zend\Expressive\Router\RouteResult;
use function implode;
class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface
{
/**
* Process an incoming server request and return a response, optionally delegating
* to the next middleware component to create the response.
*
* @param Request $request
* @param RequestHandlerInterface $handler
*
* @return Response
* @throws \InvalidArgumentException
*/
public function process(Request $request, RequestHandlerInterface $handler): Response
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
/** @var Response $response */
$response = $handler->handle($request);
if (! $request->hasHeader('Origin')) {
return $response;
@ -42,13 +32,28 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
return $response;
}
// Add OPTIONS-specific headers
foreach ([
'Access-Control-Allow-Methods' => 'GET,POST,PUT,DELETE,OPTIONS', // TODO Should be dynamic
// 'Access-Control-Allow-Methods' => $response->getHeaderLine('Allow'),
return $this->addOptionsHeaders($request, $response);
}
private function addOptionsHeaders(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
{
/** @var RouteResult|null $matchedRoute */
$matchedRoute = $request->getAttribute(RouteResult::class);
$matchedMethods = $matchedRoute !== null ? $matchedRoute->getAllowedMethods() : [
self::METHOD_GET,
self::METHOD_POST,
self::METHOD_PUT,
self::METHOD_PATCH,
self::METHOD_DELETE,
self::METHOD_OPTIONS,
];
$corsHeaders = [
'Access-Control-Allow-Methods' => implode(',', $matchedMethods),
'Access-Control-Max-Age' => '1000',
'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'),
] as $key => $value) {
];
foreach ($corsHeaders as $key => $value) {
$response = $response->withHeader($key, $value);
}

View file

@ -10,63 +10,108 @@ use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware;
use Zend\Diactoros\Response;
use Zend\Diactoros\ServerRequest;
use Zend\Expressive\Router\Route;
use Zend\Expressive\Router\RouteResult;
use function Zend\Stratigility\middleware;
class CrossDomainMiddlewareTest extends TestCase
{
/** @var CrossDomainMiddleware */
private $middleware;
/** @var ObjectProphecy */
private $delegate;
private $handler;
public function setUp(): void
{
$this->middleware = new CrossDomainMiddleware();
$this->delegate = $this->prophesize(RequestHandlerInterface::class);
$this->handler = $this->prophesize(RequestHandlerInterface::class);
}
/** @test */
public function nonCrossDomainRequestsAreNotAffected()
public function nonCrossDomainRequestsAreNotAffected(): void
{
$originalResponse = new Response();
$this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$this->handler->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$response = $this->middleware->process(new ServerRequest(), $this->delegate->reveal());
$response = $this->middleware->process(new ServerRequest(), $this->handler->reveal());
$this->assertSame($originalResponse, $response);
$headers = $response->getHeaders();
$this->assertArrayNotHasKey('Access-Control-Allow-Origin', $headers);
$this->assertArrayNotHasKey('Access-Control-Expose-Headers', $headers);
$this->assertArrayNotHasKey('Access-Control-Allow-Methods', $headers);
$this->assertArrayNotHasKey('Access-Control-Max-Age', $headers);
$this->assertArrayNotHasKey('Access-Control-Allow-Headers', $headers);
}
/** @test */
public function anyRequestIncludesTheAllowAccessHeader()
public function anyRequestIncludesTheAllowAccessHeader(): void
{
$originalResponse = new Response();
$this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$this->handler->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$response = $this->middleware->process(
(new ServerRequest())->withHeader('Origin', 'local'),
$this->delegate->reveal()
$this->handler->reveal()
);
$this->assertNotSame($originalResponse, $response);
$headers = $response->getHeaders();
$this->assertArrayHasKey('Access-Control-Allow-Origin', $headers);
$this->assertArrayHasKey('Access-Control-Expose-Headers', $headers);
$this->assertArrayNotHasKey('Access-Control-Allow-Methods', $headers);
$this->assertArrayNotHasKey('Access-Control-Max-Age', $headers);
$this->assertArrayNotHasKey('Access-Control-Allow-Headers', $headers);
}
/** @test */
public function optionsRequestIncludesMoreHeaders()
public function optionsRequestIncludesMoreHeaders(): void
{
$originalResponse = new Response();
$request = (new ServerRequest())->withMethod('OPTIONS')->withHeader('Origin', 'local');
$this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$this->handler->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$response = $this->middleware->process($request, $this->delegate->reveal());
$response = $this->middleware->process($request, $this->handler->reveal());
$this->assertNotSame($originalResponse, $response);
$headers = $response->getHeaders();
$this->assertArrayHasKey('Access-Control-Allow-Origin', $headers);
$this->assertArrayHasKey('Access-Control-Expose-Headers', $headers);
$this->assertArrayHasKey('Access-Control-Allow-Methods', $headers);
$this->assertArrayHasKey('Access-Control-Max-Age', $headers);
$this->assertArrayHasKey('Access-Control-Allow-Headers', $headers);
}
/**
* @test
* @dataProvider provideRouteResults
*/
public function optionsRequestParsesRouteMatchToDetermineAllowedMethods(
?RouteResult $result,
string $expectedAllowedMethods
): void {
$originalResponse = new Response();
$request = (new ServerRequest())->withAttribute(RouteResult::class, $result)
->withMethod('OPTIONS')
->withHeader('Origin', 'local');
$this->handler->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
$response = $this->middleware->process($request, $this->handler->reveal());
$this->assertEquals($response->getHeaderLine('Access-Control-Allow-Methods'), $expectedAllowedMethods);
}
public function provideRouteResults(): iterable
{
yield 'with no route result' => [null, 'GET,POST,PUT,PATCH,DELETE,OPTIONS'];
yield 'with failed route result' => [RouteResult::fromRouteFailure(['POST', 'GET']), 'POST,GET'];
yield 'with success route result' => [
RouteResult::fromRoute(
new Route('/', middleware(function () {
}), ['DELETE', 'PATCH', 'PUT'])
),
'DELETE,PATCH,PUT',
];
}
}