Skip to content

Commit

Permalink
Merge pull request #2238 from acelaya-forks/feature/rules-flag
Browse files Browse the repository at this point in the history
Feature/rules flag
  • Loading branch information
acelaya authored Oct 27, 2024
2 parents 0d627ce + 01936d7 commit 468662b
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 24 deletions.
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).

## [Unreleased]
### Added
* [#2207](https://github.com/shlinkio/shlink/issues/2207) Add `hasRedirectRules` flag to short URL API model. This flag tells if a specific short URL has any redirect rules attached to it.

### Changed
* *Nothing*

### Deprecated
* *Nothing*

### Removed
* *Nothing*

### Fixed
* *Nothing*


## [4.2.4] - 2024-10-27
### Added
* *Nothing*
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"pagerfanta/core": "^3.8",
"ramsey/uuid": "^4.7",
"shlinkio/doctrine-specification": "^2.1.1",
"shlinkio/shlink-common": "^6.4",
"shlinkio/shlink-common": "dev-main#5fbe58c as 6.5",
"shlinkio/shlink-config": "^3.3",
"shlinkio/shlink-event-dispatcher": "^4.1",
"shlinkio/shlink-importer": "^5.3.2",
Expand Down
12 changes: 11 additions & 1 deletion docs/async-api/async-api.json
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@
"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)."
},
"hasRedirectRules": {
"type": "boolean",
"description": "Whether this short URL has redirect rules attached to it or not. Use [this endpoint](https://api-spec.shlink.io/#/Redirect%20rules/listShortUrlRedirectRules) to get the actual list of rules."
}
},
"example": {
Expand All @@ -164,7 +172,9 @@
},
"domain": "example.com",
"title": "The title",
"crawlable": false
"crawlable": false,
"forwardQuery": false,
"hasRedirectRules": true
}
},
"ShortUrlMeta": {
Expand Down
7 changes: 6 additions & 1 deletion docs/swagger/definitions/ShortUrl.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"domain",
"title",
"crawlable",
"forwardQuery"
"forwardQuery",
"hasRedirectRules"
],
"properties": {
"shortCode": {
Expand Down Expand Up @@ -59,6 +60,10 @@
"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)."
},
"hasRedirectRules": {
"type": "boolean",
"description": "Whether this short URL has redirect rules attached to it or not. Use [this endpoint](https://api-spec.shlink.io/#/Redirect%20rules/listShortUrlRedirectRules) to get the actual list of rules."
}
}
}
16 changes: 12 additions & 4 deletions docs/swagger/paths/v1_short-urls.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@
},
"domain": null,
"title": "Welcome to Steam",
"crawlable": false
"crawlable": false,
"forwardQuery": true,
"hasRedirectRules": true
},
{
"shortCode": "12Kb3",
Expand All @@ -202,7 +204,9 @@
},
"domain": null,
"title": null,
"crawlable": false
"crawlable": false,
"forwardQuery": true,
"hasRedirectRules": false
},
{
"shortCode": "123bA",
Expand All @@ -222,7 +226,9 @@
},
"domain": "example.com",
"title": null,
"crawlable": false
"crawlable": false,
"forwardQuery": false,
"hasRedirectRules": true
}
],
"pagination": {
Expand Down Expand Up @@ -337,7 +343,9 @@
},
"domain": null,
"title": null,
"crawlable": false
"crawlable": false,
"forwardQuery": true,
"hasRedirectRules": false
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion docs/swagger/paths/v1_short-urls_shorten.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@
},
"domain": null,
"title": null,
"crawlable": false
"crawlable": false,
"forwardQuery": true,
"hasRedirectRules": false
}
},
"text/plain": {
Expand Down
8 changes: 6 additions & 2 deletions docs/swagger/paths/v1_short-urls_{shortCode}.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@
},
"domain": null,
"title": null,
"crawlable": false
"crawlable": false,
"forwardQuery": true,
"hasRedirectRules": true
}
}
}
Expand Down Expand Up @@ -163,7 +165,9 @@
},
"domain": null,
"title": "Shlink - The URL shortener",
"crawlable": false
"crawlable": false,
"forwardQuery": false,
"hasRedirectRules": true
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,9 @@
->columnName('forward_query')
->option('default', true)
->build();

$builder->createOneToMany('redirectRules', RedirectRule\Entity\ShortUrlRedirectRule::class)
->mappedBy('shortUrl')
->fetchExtraLazy()
->build();
};
14 changes: 12 additions & 2 deletions module/Core/src/ShortUrl/Entity/ShortUrl.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;
Expand Down Expand Up @@ -39,6 +40,7 @@ class ShortUrl extends AbstractEntity
* @param Collection<int, Tag> $tags
* @param Collection<int, Visit> & Selectable<int, Visit> $visits
* @param Collection<int, ShortUrlVisitsCount> & Selectable<int, ShortUrlVisitsCount> $visitsCounts
* @param Collection<int, ShortUrlRedirectRule> $redirectRules
*/
private function __construct(
private string $longUrl,
Expand All @@ -60,6 +62,7 @@ private function __construct(
private bool $forwardQuery = true,
private ?string $importSource = null,
private ?string $importOriginalShortCode = null,
private Collection $redirectRules = new ArrayCollection(),
) {
}

Expand Down Expand Up @@ -261,7 +264,13 @@ public function isEnabled(): bool
return true;
}

public function toArray(?VisitsSummary $precalculatedSummary = null): array
/**
* @param null|(callable(): ?string) $getAuthority -
* This is a callback so that we trust its return value if provided, even if it is null.
* Providing the raw authority as `string|null` would result in a fallback to `$this->domain` when the authority
* was null.
*/
public function toArray(?VisitsSummary $precalculatedSummary = null, callable|null $getAuthority = null): array
{
return [
'shortCode' => $this->shortCode,
Expand All @@ -273,7 +282,7 @@ public function toArray(?VisitsSummary $precalculatedSummary = null): array
'validUntil' => $this->validUntil?->toAtomString(),
'maxVisits' => $this->maxVisits,
],
'domain' => $this->domain,
'domain' => $getAuthority !== null ? $getAuthority() : $this->domain?->authority,
'title' => $this->title,
'crawlable' => $this->crawlable,
'forwardQuery' => $this->forwardQuery,
Expand All @@ -283,6 +292,7 @@ public function toArray(?VisitsSummary $precalculatedSummary = null): array
Criteria::create()->where(Criteria::expr()->eq('potentialBot', false)),
)),
),
'hasRedirectRules' => count($this->redirectRules) > 0,
];
}
}
23 changes: 15 additions & 8 deletions module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,26 @@

final readonly class ShortUrlWithVisitsSummary
{
private function __construct(public ShortUrl $shortUrl, private ?VisitsSummary $visitsSummary = null)
{
private function __construct(
public ShortUrl $shortUrl,
private VisitsSummary|null $visitsSummary = null,
private string|null $authority = null,
) {
}

/**
* @param array{shortUrl: ShortUrl, visits: string|int, nonBotVisits: string|int} $data
* @param array{shortUrl: ShortUrl, visits: string|int, nonBotVisits: string|int, authority: string|null} $data
*/
public static function fromArray(array $data): self
{
return new self($data['shortUrl'], VisitsSummary::fromTotalAndNonBots(
(int) $data['visits'],
(int) $data['nonBotVisits'],
));
return new self(
shortUrl: $data['shortUrl'],
visitsSummary: VisitsSummary::fromTotalAndNonBots(
total: (int) $data['visits'],
nonBots: (int) $data['nonBotVisits'],
),
authority: $data['authority'] ?? null,
);
}

public static function fromShortUrl(ShortUrl $shortUrl): self
Expand All @@ -31,6 +38,6 @@ public static function fromShortUrl(ShortUrl $shortUrl): self

public function toArray(): array
{
return $this->shortUrl->toArray($this->visitsSummary);
return $this->shortUrl->toArray($this->visitsSummary, fn() => $this->authority);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function findList(ShortUrlsListFiltering $filtering): array

$qb = $this->createListQueryBuilder($filtering);
$qb->select(
'DISTINCT s AS shortUrl',
'DISTINCT s AS shortUrl, d.authority',
'(' . $buildVisitsSubQuery('v', excludingBots: false) . ') AS ' . OrderableField::VISITS->value,
'(' . $buildVisitsSubQuery('v2', excludingBots: true) . ') AS ' . OrderableField::NON_BOT_VISITS->value,
// This is added only to have a consistent order by title between database engines
Expand All @@ -56,7 +56,7 @@ public function findList(ShortUrlsListFiltering $filtering): array

$this->processOrderByForList($qb, $filtering);

/** @var array{shortUrl: ShortUrl, visits: string, nonBotVisits: string}[] $result */
/** @var array{shortUrl: ShortUrl, visits: string, nonBotVisits: string, authority: string|null}[] $result */
$result = $qb->getQuery()->getResult();
return map($result, static fn (array $s) => ShortUrlWithVisitsSummary::fromArray($s));
}
Expand Down Expand Up @@ -89,6 +89,7 @@ private function createListQueryBuilder(ShortUrlsCountFiltering $filtering): Que
{
$qb = $this->getEntityManager()->createQueryBuilder();
$qb->from(ShortUrl::class, 's')
->leftJoin('s.domain', 'd')
->where('1=1');

$dateRange = $filtering->dateRange;
Expand Down Expand Up @@ -129,8 +130,7 @@ private function createListQueryBuilder(ShortUrlsCountFiltering $filtering): Que
$conditions[] = $qb->expr()->like('t.name', ':searchPattern');
}

$qb->leftJoin('s.domain', 'd')
->andWhere($qb->expr()->orX(...$conditions))
$qb->andWhere($qb->expr()->orX(...$conditions))
->setParameter('searchPattern', '%' . $searchTerm . '%');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public function visitIsProperlySerializedIntoUpdate(string $method, string $expe
'crawlable' => false,
'forwardQuery' => true,
'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0),
'hasRedirectRules' => false,
],
'visit' => [
'referer' => '',
Expand Down Expand Up @@ -145,6 +146,7 @@ public function shortUrlIsProperlySerializedIntoUpdate(): void
'crawlable' => false,
'forwardQuery' => true,
'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0),
'hasRedirectRules' => false,
]], $update->payload);
}
}
6 changes: 6 additions & 0 deletions module/Rest/test-api/Action/ListShortUrlsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => 'My cool title',
'crawlable' => true,
'forwardQuery' => true,
'hasRedirectRules' => false,
];
private const SHORT_URL_DOCS = [
'shortCode' => 'ghi789',
Expand All @@ -55,6 +56,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
'hasRedirectRules' => false,
];
private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [
'shortCode' => 'custom-with-domain',
Expand All @@ -76,6 +78,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
'hasRedirectRules' => false,
];
private const SHORT_URL_META = [
'shortCode' => 'def456',
Expand All @@ -99,6 +102,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
'hasRedirectRules' => true,
];
private const SHORT_URL_CUSTOM_SLUG = [
'shortCode' => 'custom',
Expand All @@ -120,6 +124,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => null,
'crawlable' => true,
'forwardQuery' => false,
'hasRedirectRules' => false,
];
private const SHORT_URL_CUSTOM_DOMAIN = [
'shortCode' => 'ghi789',
Expand All @@ -143,6 +148,7 @@ class ListShortUrlsTest extends ApiTestCase
'title' => null,
'crawlable' => false,
'forwardQuery' => true,
'hasRedirectRules' => false,
];

#[Test, DataProvider('provideFilteredLists')]
Expand Down

0 comments on commit 468662b

Please sign in to comment.