Simplify NotFoundRedirectConfigInterface with property hooks and asymetric visibility

This commit is contained in:
Alejandro Celaya 2025-11-08 22:47:24 +01:00
parent ad15ae1922
commit c42fb67efc
15 changed files with 54 additions and 152 deletions

View File

@ -88,15 +88,15 @@ class DomainRedirectsCommand extends Command
$this->domainService->configureNotFoundRedirects($domainAuthority, NotFoundRedirects::withRedirects( $this->domainService->configureNotFoundRedirects($domainAuthority, NotFoundRedirects::withRedirects(
$ask( $ask(
'URL to redirect to when a user hits this domain\'s base URL', 'URL to redirect to when a user hits this domain\'s base URL',
$domain?->baseUrlRedirect(), $domain?->baseUrlRedirect,
), ),
$ask( $ask(
'URL to redirect to when a user hits a not found URL other than an invalid short URL', 'URL to redirect to when a user hits a not found URL other than an invalid short URL',
$domain?->regular404Redirect(), $domain?->regular404Redirect,
), ),
$ask( $ask(
'URL to redirect to when a user hits an invalid short URL', 'URL to redirect to when a user hits an invalid short URL',
$domain?->invalidShortUrlRedirect(), $domain?->invalidShortUrlRedirect,
), ),
)); ));

View File

@ -59,9 +59,9 @@ class ListDomainsCommand extends Command
private function notFoundRedirectsToString(NotFoundRedirectConfigInterface $config): string private function notFoundRedirectsToString(NotFoundRedirectConfigInterface $config): string
{ {
$baseUrl = $config->baseUrlRedirect() ?? 'N/A'; $baseUrl = $config->baseUrlRedirect ?? 'N/A';
$regular404 = $config->regular404Redirect() ?? 'N/A'; $regular404 = $config->regular404Redirect ?? 'N/A';
$invalidShortUrl = $config->invalidShortUrlRedirect() ?? 'N/A'; $invalidShortUrl = $config->invalidShortUrlRedirect ?? 'N/A';
return <<<EOL return <<<EOL
* Base URL: {$baseUrl} * Base URL: {$baseUrl}

View File

@ -41,8 +41,8 @@ class ListDomainsCommandTest extends TestCase
$this->domainService->expects($this->once())->method('listDomains')->with()->willReturn([ $this->domainService->expects($this->once())->method('listDomains')->with()->willReturn([
DomainItem::forDefaultDomain('foo.com', new NotFoundRedirectOptions( DomainItem::forDefaultDomain('foo.com', new NotFoundRedirectOptions(
invalidShortUrl: 'https://foo.com/default/invalid', invalidShortUrlRedirect: 'https://foo.com/default/invalid',
baseUrl: 'https://foo.com/default/base', baseUrlRedirect: 'https://foo.com/default/base',
)), )),
DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')), DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')),
DomainItem::forNonDefaultDomain($bazDomain), DomainItem::forNonDefaultDomain($bazDomain),

View File

@ -6,33 +6,7 @@ namespace Shlinkio\Shlink\Core\Config;
final class EmptyNotFoundRedirectConfig implements NotFoundRedirectConfigInterface final class EmptyNotFoundRedirectConfig implements NotFoundRedirectConfigInterface
{ {
public function invalidShortUrlRedirect(): string|null private(set) string|null $invalidShortUrlRedirect = null;
{ private(set) string|null $regular404Redirect = null;
return null; private(set) string|null $baseUrlRedirect = null;
}
public function hasInvalidShortUrlRedirect(): bool
{
return false;
}
public function regular404Redirect(): string|null
{
return null;
}
public function hasRegular404Redirect(): bool
{
return false;
}
public function baseUrlRedirect(): string|null
{
return null;
}
public function hasBaseUrlRedirect(): bool
{
return false;
}
} }

View File

@ -6,15 +6,7 @@ namespace Shlinkio\Shlink\Core\Config;
interface NotFoundRedirectConfigInterface interface NotFoundRedirectConfigInterface
{ {
public function invalidShortUrlRedirect(): string|null; public string|null $invalidShortUrlRedirect { get; }
public string|null $regular404Redirect { get; }
public function hasInvalidShortUrlRedirect(): bool; public string|null $baseUrlRedirect { get; }
public function regular404Redirect(): string|null;
public function hasRegular404Redirect(): bool;
public function baseUrlRedirect(): string|null;
public function hasBaseUrlRedirect(): bool;
} }

View File

@ -32,10 +32,9 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface
UriInterface $currentUri, UriInterface $currentUri,
): ResponseInterface|null { ): ResponseInterface|null {
$urlToRedirectTo = match (true) { $urlToRedirectTo = match (true) {
$notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => $config->baseUrlRedirect(), $notFoundType->isBaseUrl() => $config->baseUrlRedirect,
$notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => $config->regular404Redirect(), $notFoundType->isRegularNotFound() => $config->regular404Redirect,
$notFoundType->isInvalidShortUrl() && $config->hasInvalidShortUrlRedirect() => $notFoundType->isInvalidShortUrl() => $config->invalidShortUrlRedirect,
$config->invalidShortUrlRedirect(),
default => null, default => null,
}; };

View File

@ -6,12 +6,12 @@ namespace Shlinkio\Shlink\Core\Config;
use JsonSerializable; use JsonSerializable;
final class NotFoundRedirects implements JsonSerializable final readonly class NotFoundRedirects implements JsonSerializable
{ {
private function __construct( private function __construct(
public readonly string|null $baseUrlRedirect, public string|null $baseUrlRedirect,
public readonly string|null $regular404Redirect, public string|null $regular404Redirect,
public readonly string|null $invalidShortUrlRedirect, public string|null $invalidShortUrlRedirect,
) { ) {
} }
@ -30,7 +30,7 @@ final class NotFoundRedirects implements JsonSerializable
public static function fromConfig(NotFoundRedirectConfigInterface $config): self public static function fromConfig(NotFoundRedirectConfigInterface $config): self
{ {
return new self($config->baseUrlRedirect(), $config->regular404Redirect(), $config->invalidShortUrlRedirect()); return new self($config->baseUrlRedirect, $config->regular404Redirect, $config->invalidShortUrlRedirect);
} }
public function jsonSerialize(): array public function jsonSerialize(): array

View File

@ -10,48 +10,18 @@ use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface;
final readonly class NotFoundRedirectOptions implements NotFoundRedirectConfigInterface final readonly class NotFoundRedirectOptions implements NotFoundRedirectConfigInterface
{ {
public function __construct( public function __construct(
public string|null $invalidShortUrl = null, public string|null $invalidShortUrlRedirect = null,
public string|null $regular404 = null, public string|null $regular404Redirect = null,
public string|null $baseUrl = null, public string|null $baseUrlRedirect = null,
) { ) {
} }
public static function fromEnv(): self public static function fromEnv(): self
{ {
return new self( return new self(
invalidShortUrl: EnvVars::DEFAULT_INVALID_SHORT_URL_REDIRECT->loadFromEnv(), invalidShortUrlRedirect: EnvVars::DEFAULT_INVALID_SHORT_URL_REDIRECT->loadFromEnv(),
regular404: EnvVars::DEFAULT_REGULAR_404_REDIRECT->loadFromEnv(), regular404Redirect: EnvVars::DEFAULT_REGULAR_404_REDIRECT->loadFromEnv(),
baseUrl: EnvVars::DEFAULT_BASE_URL_REDIRECT->loadFromEnv(), baseUrlRedirect: EnvVars::DEFAULT_BASE_URL_REDIRECT->loadFromEnv(),
); );
} }
public function invalidShortUrlRedirect(): string|null
{
return $this->invalidShortUrl;
}
public function hasInvalidShortUrlRedirect(): bool
{
return $this->invalidShortUrl !== null;
}
public function regular404Redirect(): string|null
{
return $this->regular404;
}
public function hasRegular404Redirect(): bool
{
return $this->regular404 !== null;
}
public function baseUrlRedirect(): string|null
{
return $this->baseUrl;
}
public function hasBaseUrlRedirect(): bool
{
return $this->baseUrl !== null;
}
} }

View File

@ -15,9 +15,9 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec
private function __construct( private function __construct(
public readonly string $authority, public readonly string $authority,
private string|null $baseUrlRedirect = null, private(set) string|null $baseUrlRedirect = null,
private string|null $regular404Redirect = null, private(set) string|null $regular404Redirect = null,
private string|null $invalidShortUrlRedirect = null, private(set) string|null $invalidShortUrlRedirect = null,
) { ) {
} }
@ -31,36 +31,6 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec
return $this->authority; return $this->authority;
} }
public function invalidShortUrlRedirect(): string|null
{
return $this->invalidShortUrlRedirect;
}
public function hasInvalidShortUrlRedirect(): bool
{
return $this->invalidShortUrlRedirect !== null;
}
public function regular404Redirect(): string|null
{
return $this->regular404Redirect;
}
public function hasRegular404Redirect(): bool
{
return $this->regular404Redirect !== null;
}
public function baseUrlRedirect(): string|null
{
return $this->baseUrlRedirect;
}
public function hasBaseUrlRedirect(): bool
{
return $this->baseUrlRedirect !== null;
}
public function configureNotFoundRedirects(NotFoundRedirects $redirects): void public function configureNotFoundRedirects(NotFoundRedirects $redirects): void
{ {
$this->baseUrlRedirect = $redirects->baseUrlRedirect; $this->baseUrlRedirect = $redirects->baseUrlRedirect;

View File

@ -20,11 +20,8 @@ class EmptyNotFoundRedirectConfigTest extends TestCase
#[Test] #[Test]
public function allMethodsReturnHardcodedValues(): void public function allMethodsReturnHardcodedValues(): void
{ {
self::assertNull($this->redirectsConfig->invalidShortUrlRedirect()); self::assertNull($this->redirectsConfig->invalidShortUrlRedirect);
self::assertFalse($this->redirectsConfig->hasInvalidShortUrlRedirect()); self::assertNull($this->redirectsConfig->regular404Redirect);
self::assertNull($this->redirectsConfig->regular404Redirect()); self::assertNull($this->redirectsConfig->baseUrlRedirect);
self::assertFalse($this->redirectsConfig->hasRegular404Redirect());
self::assertNull($this->redirectsConfig->baseUrlRedirect());
self::assertFalse($this->redirectsConfig->hasBaseUrlRedirect());
} }
} }

View File

@ -57,57 +57,57 @@ class NotFoundRedirectResolverTest extends TestCase
yield 'base URL with trailing slash' => [ yield 'base URL with trailing slash' => [
$uri = new Uri('/'), $uri = new Uri('/'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'https://example.com/baseUrl'), new NotFoundRedirectOptions(baseUrlRedirect: 'https://example.com/baseUrl'),
'https://example.com/baseUrl', 'https://example.com/baseUrl',
]; ];
yield 'base URL without trailing slash' => [ yield 'base URL without trailing slash' => [
$uri = new Uri(''), $uri = new Uri(''),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'https://example.com/baseUrl'), new NotFoundRedirectOptions(baseUrlRedirect: 'https://example.com/baseUrl'),
'https://example.com/baseUrl', 'https://example.com/baseUrl',
]; ];
yield 'base URL with domain placeholder' => [ yield 'base URL with domain placeholder' => [
$uri = new Uri('https://s.test'), $uri = new Uri('https://s.test'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'https://redirect-here.com/{DOMAIN}'), new NotFoundRedirectOptions(baseUrlRedirect: 'https://redirect-here.com/{DOMAIN}'),
'https://redirect-here.com/s.test', 'https://redirect-here.com/s.test',
]; ];
yield 'base URL with domain placeholder in query' => [ yield 'base URL with domain placeholder in query' => [
$uri = new Uri('https://s.test'), $uri = new Uri('https://s.test'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'https://redirect-here.com/?domain={DOMAIN}'), new NotFoundRedirectOptions(baseUrlRedirect: 'https://redirect-here.com/?domain={DOMAIN}'),
'https://redirect-here.com/?domain=s.test', 'https://redirect-here.com/?domain=s.test',
]; ];
yield 'regular 404' => [ yield 'regular 404' => [
$uri = new Uri('/foo/bar'), $uri = new Uri('/foo/bar'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(regular404: 'https://example.com/regular404'), new NotFoundRedirectOptions(regular404Redirect: 'https://example.com/regular404'),
'https://example.com/regular404', 'https://example.com/regular404',
]; ];
yield 'regular 404 with path placeholder in query' => [ yield 'regular 404 with path placeholder in query' => [
$uri = new Uri('/foo/bar'), $uri = new Uri('/foo/bar'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(regular404: 'https://redirect-here.com/?path={ORIGINAL_PATH}'), new NotFoundRedirectOptions(regular404Redirect: 'https://redirect-here.com/?path={ORIGINAL_PATH}'),
'https://redirect-here.com/?path=%2Ffoo%2Fbar', 'https://redirect-here.com/?path=%2Ffoo%2Fbar',
]; ];
yield 'regular 404 with multiple placeholders' => [ yield 'regular 404 with multiple placeholders' => [
$uri = new Uri('https://s.test/foo/bar'), $uri = new Uri('https://s.test/foo/bar'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions( new NotFoundRedirectOptions(
regular404: 'https://redirect-here.com/{ORIGINAL_PATH}/{DOMAIN}/?d={DOMAIN}&p={ORIGINAL_PATH}', regular404Redirect: 'https://redirect-here.com/{ORIGINAL_PATH}/{DOMAIN}/?d={DOMAIN}&p={ORIGINAL_PATH}',
), ),
'https://redirect-here.com/foo/bar/s.test/?d=s.test&p=%2Ffoo%2Fbar', 'https://redirect-here.com/foo/bar/s.test/?d=s.test&p=%2Ffoo%2Fbar',
]; ];
yield 'invalid short URL' => [ yield 'invalid short URL' => [
new Uri('/foo'), new Uri('/foo'),
self::notFoundType(self::requestForRoute(RedirectAction::class)), self::notFoundType(self::requestForRoute(RedirectAction::class)),
new NotFoundRedirectOptions(invalidShortUrl: 'https://example.com/invalidShortUrl'), new NotFoundRedirectOptions(invalidShortUrlRedirect: 'https://example.com/invalidShortUrl'),
'https://example.com/invalidShortUrl', 'https://example.com/invalidShortUrl',
]; ];
yield 'invalid short URL with path placeholder' => [ yield 'invalid short URL with path placeholder' => [
new Uri('/foo'), new Uri('/foo'),
self::notFoundType(self::requestForRoute(RedirectAction::class)), self::notFoundType(self::requestForRoute(RedirectAction::class)),
new NotFoundRedirectOptions(invalidShortUrl: 'https://redirect-here.com/{ORIGINAL_PATH}'), new NotFoundRedirectOptions(invalidShortUrlRedirect: 'https://redirect-here.com/{ORIGINAL_PATH}'),
'https://redirect-here.com/foo', 'https://redirect-here.com/foo',
]; ];
} }

View File

@ -181,9 +181,9 @@ class DomainServiceTest extends TestCase
if ($foundDomain !== null) { if ($foundDomain !== null) {
self::assertSame($result, $foundDomain); self::assertSame($result, $foundDomain);
} }
self::assertEquals('foo.com', $result->baseUrlRedirect()); self::assertEquals('foo.com', $result->baseUrlRedirect);
self::assertEquals('bar.com', $result->regular404Redirect()); self::assertEquals('bar.com', $result->regular404Redirect);
self::assertEquals('baz.com', $result->invalidShortUrlRedirect()); self::assertEquals('baz.com', $result->invalidShortUrlRedirect);
} }
public static function provideFoundDomains(): iterable public static function provideFoundDomains(): iterable

View File

@ -69,11 +69,11 @@ class DomainRedirectsRequest
public function toNotFoundRedirects(NotFoundRedirectConfigInterface|null $defaults = null): NotFoundRedirects public function toNotFoundRedirects(NotFoundRedirectConfigInterface|null $defaults = null): NotFoundRedirects
{ {
return NotFoundRedirects::withRedirects( return NotFoundRedirects::withRedirects(
$this->baseUrlRedirectWasProvided ? $this->baseUrlRedirect : $defaults?->baseUrlRedirect(), $this->baseUrlRedirectWasProvided ? $this->baseUrlRedirect : $defaults?->baseUrlRedirect,
$this->regular404RedirectWasProvided ? $this->regular404Redirect : $defaults?->regular404Redirect(), $this->regular404RedirectWasProvided ? $this->regular404Redirect : $defaults?->regular404Redirect,
$this->invalidShortUrlRedirectWasProvided $this->invalidShortUrlRedirectWasProvided
? $this->invalidShortUrlRedirect ? $this->invalidShortUrlRedirect
: $defaults?->invalidShortUrlRedirect(), : $defaults?->invalidShortUrlRedirect,
); );
} }
} }

View File

@ -68,13 +68,13 @@ class DomainRedirectsActionTest extends TestCase
NotFoundRedirects::withRedirects( NotFoundRedirects::withRedirects(
array_key_exists(DomainRedirectsInputFilter::BASE_URL_REDIRECT, $redirects) array_key_exists(DomainRedirectsInputFilter::BASE_URL_REDIRECT, $redirects)
? $redirects[DomainRedirectsInputFilter::BASE_URL_REDIRECT] ? $redirects[DomainRedirectsInputFilter::BASE_URL_REDIRECT]
: $domain->baseUrlRedirect(), : $domain->baseUrlRedirect,
array_key_exists(DomainRedirectsInputFilter::REGULAR_404_REDIRECT, $redirects) array_key_exists(DomainRedirectsInputFilter::REGULAR_404_REDIRECT, $redirects)
? $redirects[DomainRedirectsInputFilter::REGULAR_404_REDIRECT] ? $redirects[DomainRedirectsInputFilter::REGULAR_404_REDIRECT]
: $domain->regular404Redirect(), : $domain->regular404Redirect,
array_key_exists(DomainRedirectsInputFilter::INVALID_SHORT_URL_REDIRECT, $redirects) array_key_exists(DomainRedirectsInputFilter::INVALID_SHORT_URL_REDIRECT, $redirects)
? $redirects[DomainRedirectsInputFilter::INVALID_SHORT_URL_REDIRECT] ? $redirects[DomainRedirectsInputFilter::INVALID_SHORT_URL_REDIRECT]
: $domain->invalidShortUrlRedirect(), : $domain->invalidShortUrlRedirect,
), ),
$apiKey, $apiKey,
); );

View File

@ -51,7 +51,7 @@ class DomainRedirectsRequestTest extends TestCase
yield 'some values' => [['domain' => 'foo', 'regular404Redirect' => 'bar'], null, 'foo', null, 'bar', null]; yield 'some values' => [['domain' => 'foo', 'regular404Redirect' => 'bar'], null, 'foo', null, 'bar', null];
yield 'fallbacks' => [ yield 'fallbacks' => [
['domain' => 'domain', 'baseUrlRedirect' => 'bar'], ['domain' => 'domain', 'baseUrlRedirect' => 'bar'],
new NotFoundRedirectOptions(invalidShortUrl: 'fallback2', regular404: 'fallback'), new NotFoundRedirectOptions(invalidShortUrlRedirect: 'fallback2', regular404Redirect: 'fallback'),
'domain', 'domain',
'bar', 'bar',
'fallback', 'fallback',
@ -59,7 +59,7 @@ class DomainRedirectsRequestTest extends TestCase
]; ];
yield 'fallback ignored' => [ yield 'fallback ignored' => [
['domain' => 'domain', 'regular404Redirect' => 'bar', 'invalidShortUrlRedirect' => null], ['domain' => 'domain', 'regular404Redirect' => 'bar', 'invalidShortUrlRedirect' => null],
new NotFoundRedirectOptions(invalidShortUrl: 'fallback2', regular404: 'fallback'), new NotFoundRedirectOptions(invalidShortUrlRedirect: 'fallback2', regular404Redirect: 'fallback'),
'domain', 'domain',
null, null,
'bar', 'bar',