Merge pull request #1194 from acelaya-forks/feature/optinally-forward-query

Feature/optinally forward query
This commit is contained in:
Alejandro Celaya 2021-10-02 10:56:48 +02:00 committed by GitHub
commit a5874a3f80
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 258 additions and 118 deletions

View file

@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
The config generated with the installing tool still has precedence over the env vars, so it cannot be combined. Either you use the tool, or use env vars.
* [#1149](https://github.com/shlinkio/shlink/issues/1149) Allowed to set custom defaults for the QR codes.
* [#1112](https://github.com/shlinkio/shlink/issues/1112) Added new option to define if the query string should be forwarded on a per-short URL basis.
The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded.
### Changed
* [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`.

View file

@ -0,0 +1,26 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Types\Types;
use Doctrine\Migrations\AbstractMigration;
final class Version20211002072605 extends AbstractMigration
{
public function up(Schema $schema): void
{
$shortUrls = $schema->getTable('short_urls');
$this->skipIf($shortUrls->hasColumn('forward_query'));
$shortUrls->addColumn('forward_query', Types::BOOLEAN, ['default' => true]);
}
public function down(Schema $schema): void
{
$shortUrls = $schema->getTable('short_urls');
$this->skipIf(! $shortUrls->hasColumn('forward_query'));
$shortUrls->dropColumn('forward_query');
}
}

View file

@ -1,5 +1,18 @@
{
"type": "object",
"required": [
"shortCode",
"shortUrl",
"longUrl",
"dateCreated",
"visitsCount",
"tags",
"meta",
"domain",
"title",
"crawlable",
"forwardQuery"
],
"properties": {
"shortCode": {
"type": "string",
@ -45,6 +58,10 @@
"crawlable": {
"type": "boolean",
"description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt."
},
"forwardQuery": {
"type": "boolean",
"description": "Tells if this URL will forward the query params to the long URL when visited, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)."
}
}
}

View file

@ -0,0 +1,48 @@
{
"type": "object",
"properties": {
"longUrl": {
"description": "The long URL this short URL will redirect to",
"type": "string"
},
"validSince": {
"description": "The date (in ISO-8601 format) from which this short code will be valid",
"type": "string",
"nullable": true
},
"validUntil": {
"description": "The date (in ISO-8601 format) until which this short code will be valid",
"type": "string",
"nullable": true
},
"maxVisits": {
"description": "The maximum number of allowed visits for this short code",
"type": "number",
"nullable": true
},
"validateUrl": {
"description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config",
"type": "boolean"
},
"tags": {
"type": "array",
"items": {
"type": "string"
},
"description": "The list of tags to set to the short URL."
},
"title": {
"type": "string",
"description": "A descriptive title of the short URL.",
"nullable": true
},
"crawlable": {
"type": "boolean",
"description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt."
},
"forwardQuery": {
"type": "boolean",
"description": "Tells if the query params should be forwarded from the short URL to the long one, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)."
}
}
}

View file

@ -225,63 +225,37 @@
"content": {
"application/json": {
"schema": {
"type": "object",
"required": [
"longUrl"
],
"properties": {
"longUrl": {
"description": "The URL to parse",
"type": "string"
"allOf": [
{
"$ref": "../definitions/ShortUrlEdition.json"
},
"tags": {
"description": "The URL to parse",
"type": "array",
"items": {
"type": "string"
{
"type": "object",
"required": ["longUrl"],
"properties": {
"customSlug": {
"description": "A unique custom slug to be used instead of the generated short code",
"type": "string"
},
"findIfExists": {
"description": "Will force existing matching URL to be returned if found, instead of creating a new one",
"type": "boolean"
},
"domain": {
"description": "The domain to which the short URL will be attached",
"type": "string"
},
"shortCodeLength": {
"description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided",
"type": "number"
},
"validateUrl": {
"description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config",
"type": "boolean"
}
}
},
"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"
},
"customSlug": {
"description": "A unique custom slug to be used instead of the generated short code",
"type": "string"
},
"maxVisits": {
"description": "The maximum number of allowed visits for this short code",
"type": "number"
},
"findIfExists": {
"description": "Will force existing matching URL to be returned if found, instead of creating a new one",
"type": "boolean"
},
"domain": {
"description": "The domain to which the short URL will be attached",
"type": "string"
},
"shortCodeLength": {
"description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided",
"type": "number"
},
"validateUrl": {
"description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config",
"type": "boolean"
},
"title": {
"type": "string",
"description": "A descriptive title of the short URL."
},
"crawlable": {
"type": "boolean",
"description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt."
}
}
]
}
}
}

View file

@ -112,48 +112,7 @@
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"longUrl": {
"description": "The long URL this short URL will redirect to",
"type": "string"
},
"validSince": {
"description": "The date (in ISO-8601 format) from which this short code will be valid",
"type": "string",
"nullable": true
},
"validUntil": {
"description": "The date (in ISO-8601 format) until which this short code will be valid",
"type": "string",
"nullable": true
},
"maxVisits": {
"description": "The maximum number of allowed visits for this short code",
"type": "number",
"nullable": true
},
"validateUrl": {
"description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config",
"type": "boolean"
},
"tags": {
"type": "array",
"items": {
"type": "string"
},
"description": "The list of tags to set to the short URL."
},
"title": {
"type": "string",
"description": "A descriptive title of the short URL.",
"nullable": true
},
"crawlable": {
"type": "boolean",
"description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt."
}
}
"$ref": "../definitions/ShortUrlEdition.json"
}
}
}

View file

@ -104,7 +104,19 @@ class GenerateShortUrlCommand extends BaseCommand
'no-validate-url',
null,
InputOption::VALUE_NONE,
'Forces the long URL to not be validated, regardless what is globally configured.',
'[DEPRECATED] Forces the long URL to not be validated, regardless what is globally configured.',
)
->addOption(
'crawlable',
'r',
InputOption::VALUE_NONE,
'Tells if this URL will be included as "Allow" in Shlink\'s robots.txt.',
)
->addOption(
'no-forward-query',
'w',
InputOption::VALUE_NONE,
'Disables the forwarding of the query string to the long URL, when the new short URL is visited.',
);
}
@ -156,6 +168,8 @@ class GenerateShortUrlCommand extends BaseCommand
ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength,
ShortUrlInputFilter::VALIDATE_URL => $doValidateUrl,
ShortUrlInputFilter::TAGS => $tags,
ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'),
ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'),
]));
$io->writeln([

View file

@ -100,4 +100,9 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
->columnName('crawlable')
->option('default', false)
->build();
$builder->createField('forwardQuery', Types::BOOLEAN)
->columnName('forward_query')
->option('default', true)
->build();
};

View file

@ -43,6 +43,7 @@ class ShortUrl extends AbstractEntity
private ?string $title = null;
private bool $titleWasAutoResolved = false;
private bool $crawlable = false;
private bool $forwardQuery = true;
private function __construct()
{
@ -80,6 +81,7 @@ class ShortUrl extends AbstractEntity
$instance->title = $meta->getTitle();
$instance->titleWasAutoResolved = $meta->titleWasAutoResolved();
$instance->crawlable = $meta->isCrawlable();
$instance->forwardQuery = $meta->forwardQuery();
return $instance;
}
@ -207,6 +209,11 @@ class ShortUrl extends AbstractEntity
return $this->crawlable;
}
public function forwardQuery(): bool
{
return $this->forwardQuery;
}
public function update(
ShortUrlEdit $shortUrlEdit,
?ShortUrlRelationResolverInterface $relationResolver = null,
@ -238,6 +245,9 @@ class ShortUrl extends AbstractEntity
$this->title = $shortUrlEdit->title();
$this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved();
}
if ($shortUrlEdit->forwardQueryWasProvided()) {
$this->forwardQuery = $shortUrlEdit->forwardQuery();
}
}
/**

View file

@ -32,6 +32,8 @@ final class ShortUrlEdit implements TitleResolutionModelInterface
private ?bool $validateUrl = null;
private bool $crawlablePropWasProvided = false;
private bool $crawlable = false;
private bool $forwardQueryPropWasProvided = false;
private bool $forwardQuery = true;
private function __construct()
{
@ -64,6 +66,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface
$this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data);
$this->titlePropWasProvided = array_key_exists(ShortUrlInputFilter::TITLE, $data);
$this->crawlablePropWasProvided = array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data);
$this->forwardQueryPropWasProvided = array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data);
$this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL);
$this->validSince = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE));
@ -73,6 +76,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface
$this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS);
$this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE);
$this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE);
$this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true;
}
public function longUrl(): ?string
@ -176,4 +180,14 @@ final class ShortUrlEdit implements TitleResolutionModelInterface
{
return $this->crawlablePropWasProvided;
}
public function forwardQuery(): bool
{
return $this->forwardQuery;
}
public function forwardQueryWasProvided(): bool
{
return $this->forwardQueryPropWasProvided;
}
}

View file

@ -32,6 +32,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface
private ?string $title = null;
private bool $titleWasAutoResolved = false;
private bool $crawlable = false;
private bool $forwardQuery = true;
private function __construct()
{
@ -82,6 +83,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface
$this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS);
$this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE);
$this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE);
$this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true;
}
public function getLongUrl(): string
@ -195,4 +197,9 @@ final class ShortUrlMeta implements TitleResolutionModelInterface
{
return $this->crawlable;
}
public function forwardQuery(): bool
{
return $this->forwardQuery;
}
}

View file

@ -21,9 +21,10 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface
public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string
{
$uri = Uri::createFromString($shortUrl->getLongUrl());
$shouldForwardQuery = $shortUrl->forwardQuery();
return $uri
->withQuery($this->resolveQuery($uri, $currentQuery))
->withQuery($shouldForwardQuery ? $this->resolveQuery($uri, $currentQuery) : $uri->getQuery())
->withPath($this->resolvePath($uri, $extraPath))
->__toString();
}

View file

@ -33,6 +33,7 @@ class ShortUrlDataTransformer implements DataTransformerInterface
'domain' => $shortUrl->getDomain(),
'title' => $shortUrl->title(),
'crawlable' => $shortUrl->crawlable(),
'forwardQuery' => $shortUrl->forwardQuery(),
];
}

View file

@ -32,7 +32,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
*/
public function validateUrl(string $url, ?bool $doValidate): void
{
// If the URL validation is not enabled or it was explicitly set to not validate, skip check
// If the URL validation is not enabled, or it was explicitly set to not validate, skip check
$doValidate = $doValidate ?? $this->options->isUrlValidationEnabled();
if (! $doValidate) {
return;

View file

@ -33,6 +33,7 @@ class ShortUrlInputFilter extends InputFilter
public const TAGS = 'tags';
public const TITLE = 'title';
public const CRAWLABLE = 'crawlable';
public const FORWARD_QUERY = 'forwardQuery';
private function __construct(array $data, bool $requireLongUrl)
{
@ -89,9 +90,10 @@ class ShortUrlInputFilter extends InputFilter
$this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false));
// This cannot be defined as a boolean input because it can actually have 3 values, true, false and null.
// Defining it as boolean will make null fall back to false, which is not the desired behavior.
// These cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null.
// Defining them as boolean will make null fall back to false, which is not the desired behavior.
$this->add($this->createInput(self::VALIDATE_URL, false));
$this->add($this->createInput(self::FORWARD_QUERY, false));
$domain = $this->createInput(self::DOMAIN, false);
$domain->getValidatorChain()->attach(new Validation\HostAndPortValidator());

View file

@ -60,6 +60,7 @@ class MercureUpdatesGeneratorTest extends TestCase
'domain' => null,
'title' => $title,
'crawlable' => false,
'forwardQuery' => true,
],
'visit' => [
'referer' => '',

View file

@ -6,27 +6,34 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder;
class ShortUrlRedirectionBuilderTest extends TestCase
{
private ShortUrlRedirectionBuilder $redirectionBuilder;
private TrackingOptions $trackingOptions;
protected function setUp(): void
{
$this->trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']);
$this->redirectionBuilder = new ShortUrlRedirectionBuilder($this->trackingOptions);
$trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']);
$this->redirectionBuilder = new ShortUrlRedirectionBuilder($trackingOptions);
}
/**
* @test
* @dataProvider provideData
*/
public function buildShortUrlRedirectBuildsExpectedUrl(string $expectedUrl, array $query, ?string $extraPath): void
{
$shortUrl = ShortUrl::withLongUrl('https://domain.com/foo/bar?some=thing');
public function buildShortUrlRedirectBuildsExpectedUrl(
string $expectedUrl,
array $query,
?string $extraPath,
?bool $forwardQuery,
): void {
$shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
'longUrl' => 'https://domain.com/foo/bar?some=thing',
'forwardQuery' => $forwardQuery,
]));
$result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath);
self::assertEquals($expectedUrl, $result);
@ -34,18 +41,59 @@ class ShortUrlRedirectionBuilderTest extends TestCase
public function provideData(): iterable
{
yield ['https://domain.com/foo/bar?some=thing', [], null];
yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null];
yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null];
yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null];
yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null];
yield ['https://domain.com/foo/bar?some=overwritten&foo=bar', ['foo' => 'bar', 'some' => 'overwritten'], null];
yield ['https://domain.com/foo/bar?some=overwritten', ['foobar' => 'notrack', 'some' => 'overwritten'], null];
yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz'];
yield ['https://domain.com/foo/bar?some=thing', [], null, true];
yield ['https://domain.com/foo/bar?some=thing', [], null, null];
yield ['https://domain.com/foo/bar?some=thing', [], null, false];
yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null, true];
yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, true];
yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, null];
yield ['https://domain.com/foo/bar?some=thing', ['foo' => 'bar'], null, false];
yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null, true];
yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, true];
yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, null];
yield ['https://domain.com/foo/bar?some=thing', [456 => 'foo'], null, false];
yield [
'https://domain.com/foo/bar?some=overwritten&foo=bar',
['foo' => 'bar', 'some' => 'overwritten'],
null,
true,
];
yield [
'https://domain.com/foo/bar?some=overwritten',
['foobar' => 'notrack', 'some' => 'overwritten'],
null,
true,
];
yield [
'https://domain.com/foo/bar?some=overwritten',
['foobar' => 'notrack', 'some' => 'overwritten'],
null,
null,
];
yield [
'https://domain.com/foo/bar?some=thing',
['foobar' => 'notrack', 'some' => 'overwritten'],
null,
false,
];
yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz', true];
yield [
'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
['hello' => 'world'],
'/something/else-baz',
true,
];
yield [
'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
['hello' => 'world'],
'/something/else-baz',
null,
];
yield [
'https://domain.com/foo/bar/something/else-baz?some=thing',
['hello' => 'world'],
'/something/else-baz',
false,
];
}
}

View file

@ -27,6 +27,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => null,
'title' => 'My cool title',
'crawlable' => true,
'forwardQuery' => true,
];
private const SHORT_URL_DOCS = [
'shortCode' => 'ghi789',
@ -43,6 +44,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => null,
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
];
private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [
'shortCode' => 'custom-with-domain',
@ -59,6 +61,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => 'some-domain.com',
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
];
private const SHORT_URL_META = [
'shortCode' => 'def456',
@ -77,6 +80,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => null,
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
];
private const SHORT_URL_CUSTOM_SLUG = [
'shortCode' => 'custom',
@ -93,6 +97,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => null,
'title' => null,
'crawlable' => false,
'forwardQuery' => false,
];
private const SHORT_URL_CUSTOM_DOMAIN = [
'shortCode' => 'ghi789',
@ -111,6 +116,7 @@ class ListShortUrlsTest extends ApiTestCase
'domain' => 'example.com',
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
];
/**

View file

@ -51,9 +51,13 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf
]), $relationResolver), '2019-01-01 00:00:10');
$manager->persist($defShortUrl);
$customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData(
['customSlug' => 'custom', 'maxVisits' => 2, 'apiKey' => $authorApiKey, 'longUrl' => 'https://shlink.io'],
)), '2019-01-01 00:00:20');
$customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
'customSlug' => 'custom',
'maxVisits' => 2,
'apiKey' => $authorApiKey,
'longUrl' => 'https://shlink.io',
'forwardQuery' => false,
])), '2019-01-01 00:00:20');
$manager->persist($customShortUrl);
$ghiShortUrl = $this->setShortUrlDate(