From 348ac78f5af1e14f2470453d4217cb3c7cec69fc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 9 Dec 2021 12:11:09 +0100 Subject: [PATCH] Enhanced ListDomainsAction so that it returns default redirects in the response --- docs/swagger/paths/v2_domains.json | 10 ++++++- .../src/Options/NotFoundRedirectOptions.php | 12 ++++++++- module/Rest/config/dependencies.config.php | 6 ++--- .../src/Action/Domain/ListDomainsAction.php | 4 ++- .../test-api/Action/DomainRedirectsTest.php | 26 ++++++------------- .../Rest/test-api/Action/ListDomainsTest.php | 5 ++++ .../Action/Domain/ListDomainsActionTest.php | 5 +++- 7 files changed, 43 insertions(+), 25 deletions(-) diff --git a/docs/swagger/paths/v2_domains.json b/docs/swagger/paths/v2_domains.json index ef63ee4e..40448016 100644 --- a/docs/swagger/paths/v2_domains.json +++ b/docs/swagger/paths/v2_domains.json @@ -46,6 +46,9 @@ } } } + }, + "defaultRedirects": { + "$ref": "../definitions/NotFoundRedirects.json" } } } @@ -84,7 +87,12 @@ "invalidShortUrlRedirect": "https://example.com/invalid-url" } } - ] + ], + "defaultRedirects": { + "baseUrlRedirect": "https://somewhere.com", + "regular404Redirect": null, + "invalidShortUrlRedirect": null + } } } } diff --git a/module/Core/src/Options/NotFoundRedirectOptions.php b/module/Core/src/Options/NotFoundRedirectOptions.php index 2f2d813b..27f410a8 100644 --- a/module/Core/src/Options/NotFoundRedirectOptions.php +++ b/module/Core/src/Options/NotFoundRedirectOptions.php @@ -4,10 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; +use JsonSerializable; use Laminas\Stdlib\AbstractOptions; use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; -class NotFoundRedirectOptions extends AbstractOptions implements NotFoundRedirectConfigInterface +class NotFoundRedirectOptions extends AbstractOptions implements NotFoundRedirectConfigInterface, JsonSerializable { private ?string $invalidShortUrl = null; private ?string $regular404 = null; @@ -60,4 +61,13 @@ class NotFoundRedirectOptions extends AbstractOptions implements NotFoundRedirec $this->baseUrl = $baseUrl; return $this; } + + public function jsonSerialize(): array + { + return [ + 'baseUrlRedirect' => $this->baseUrl, + 'regular404Redirect' => $this->regular404, + 'invalidShortUrlRedirect' => $this->invalidShortUrl, + ]; + } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 5e0267d6..98b385b0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -9,7 +9,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; use Shlinkio\Shlink\Common\Mercure\LcobucciJwtProvider; use Shlinkio\Shlink\Core\Domain\DomainService; -use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Core\Tag\TagService; @@ -55,7 +55,7 @@ return [ ConfigAbstractFactory::class => [ ApiKeyService::class => ['em'], - Action\HealthAction::class => ['em', AppOptions::class], + Action\HealthAction::class => ['em', Options\AppOptions::class], Action\MercureInfoAction::class => [LcobucciJwtProvider::class, 'config.mercure'], Action\ShortUrl\CreateShortUrlAction::class => [Service\UrlShortener::class, ShortUrlDataTransformer::class], Action\ShortUrl\SingleStepCreateShortUrlAction::class => [ @@ -81,7 +81,7 @@ return [ Action\Tag\DeleteTagsAction::class => [TagService::class], Action\Tag\CreateTagsAction::class => [TagService::class], Action\Tag\UpdateTagAction::class => [TagService::class], - Action\Domain\ListDomainsAction::class => [DomainService::class], + Action\Domain\ListDomainsAction::class => [DomainService::class, Options\NotFoundRedirectOptions::class], Action\Domain\DomainRedirectsAction::class => [DomainService::class], Middleware\CrossDomainMiddleware::class => ['config.cors'], diff --git a/module/Rest/src/Action/Domain/ListDomainsAction.php b/module/Rest/src/Action/Domain/ListDomainsAction.php index c8f9a475..11b9e151 100644 --- a/module/Rest/src/Action/Domain/ListDomainsAction.php +++ b/module/Rest/src/Action/Domain/ListDomainsAction.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -16,7 +17,7 @@ class ListDomainsAction extends AbstractRestAction protected const ROUTE_PATH = '/domains'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private DomainServiceInterface $domainService) + public function __construct(private DomainServiceInterface $domainService, private NotFoundRedirectOptions $options) { } @@ -28,6 +29,7 @@ class ListDomainsAction extends AbstractRestAction return new JsonResponse([ 'domains' => [ 'data' => $domainItems, + 'defaultRedirects' => $this->options, ], ]); } diff --git a/module/Rest/test-api/Action/DomainRedirectsTest.php b/module/Rest/test-api/Action/DomainRedirectsTest.php index 987c09d6..fdeec3b3 100644 --- a/module/Rest/test-api/Action/DomainRedirectsTest.php +++ b/module/Rest/test-api/Action/DomainRedirectsTest.php @@ -9,24 +9,6 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class DomainRedirectsTest extends ApiTestCase { - /** @test */ - public function anErrorIsReturnedWhenTryingToEditDefaultDomain(): void - { - $resp = $this->callApiWithKey(self::METHOD_PATCH, '/domains/redirects', [ - RequestOptions::JSON => ['domain' => 'doma.in'], - ]); - $payload = $this->getJsonResponsePayload($resp); - - self::assertEquals(self::STATUS_FORBIDDEN, $resp->getStatusCode()); - self::assertEquals(self::STATUS_FORBIDDEN, $payload['status']); - self::assertEquals('INVALID_DOMAIN', $payload['type']); - self::assertEquals( - 'You cannot configure default domain\'s redirects this way. Use the configuration or env vars.', - $payload['detail'], - ); - self::assertEquals('Invalid domain', $payload['title']); - } - /** * @test * @dataProvider provideInvalidDomains @@ -78,6 +60,14 @@ class DomainRedirectsTest extends ApiTestCase 'regular404Redirect' => 'foo.com', 'invalidShortUrlRedirect' => null, ]]; + yield 'default domain' => [[ + 'domain' => 'doma.in', + 'regular404Redirect' => 'foo-for-default.com', + ], [ + 'baseUrlRedirect' => null, + 'regular404Redirect' => 'foo-for-default.com', + 'invalidShortUrlRedirect' => null, + ]]; yield 'existing domain with redirects' => [[ 'domain' => 'detached-with-redirects.com', 'baseUrlRedirect' => null, diff --git a/module/Rest/test-api/Action/ListDomainsTest.php b/module/Rest/test-api/Action/ListDomainsTest.php index 5f33c20b..54039c41 100644 --- a/module/Rest/test-api/Action/ListDomainsTest.php +++ b/module/Rest/test-api/Action/ListDomainsTest.php @@ -21,6 +21,11 @@ class ListDomainsTest extends ApiTestCase self::assertEquals([ 'domains' => [ 'data' => $expectedDomains, + 'defaultRedirects' => [ + 'baseUrlRedirect' => null, + 'regular404Redirect' => null, + 'invalidShortUrlRedirect' => null, + ], ], ], $respPayload); } diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index 45575cc6..9bbe9723 100644 --- a/module/Rest/test/Action/Domain/ListDomainsActionTest.php +++ b/module/Rest/test/Action/Domain/ListDomainsActionTest.php @@ -22,11 +22,13 @@ class ListDomainsActionTest extends TestCase private ListDomainsAction $action; private ObjectProphecy $domainService; + private NotFoundRedirectOptions $options; public function setUp(): void { $this->domainService = $this->prophesize(DomainServiceInterface::class); - $this->action = new ListDomainsAction($this->domainService->reveal()); + $this->options = new NotFoundRedirectOptions(); + $this->action = new ListDomainsAction($this->domainService->reveal(), $this->options); } /** @test */ @@ -46,6 +48,7 @@ class ListDomainsActionTest extends TestCase self::assertEquals([ 'domains' => [ 'data' => $domains, + 'defaultRedirects' => $this->options, ], ], $payload); $listDomains->shouldHaveBeenCalledOnce();