Reduce duplicated logic when checking if an API key is admin

This commit is contained in:
Alejandro Celaya 2023-03-04 10:22:46 +01:00
parent 83c53c8b2e
commit e51384fcc0
6 changed files with 17 additions and 13 deletions

View File

@ -9,6 +9,7 @@ use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface;
use Shlinkio\Shlink\CLI\Util\ExitCodes;
use Shlinkio\Shlink\CLI\Util\ShlinkTable;
use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
@ -99,7 +100,7 @@ class GenerateKeyCommand extends Command
$io = new SymfonyStyle($input, $output);
$io->success(sprintf('Generated API key: "%s"', $apiKey->toString()));
if (! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
ShlinkTable::default($io)->render(
['Role name', 'Role metadata'],
$apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, 0)]),

View File

@ -59,7 +59,7 @@ class ListKeysCommand extends Command
$rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
}
$rowData[] = $expiration?->toAtomString() ?? '-';
$rowData[] = $apiKey->isAdmin() ? 'Admin' : implode("\n", $apiKey->mapRoles(
$rowData[] = ApiKey::isAdmin($apiKey) ? 'Admin' : implode("\n", $apiKey->mapRoles(
fn (Role $role, array $meta) =>
empty($meta)
? $role->toFriendlyName()

View File

@ -17,8 +17,8 @@ use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use function Functional\map;
use function Functional\each;
use function Functional\map;
use const PHP_INT_MAX;
@ -50,13 +50,13 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
$tagsSubQb = $conn->createQueryBuilder();
// For admins and when no API key is present, we'll return tags which are not linked to any short URL
$joiningMethod = $apiKey === null || $apiKey->isAdmin() ? 'leftJoin' : 'join';
$joiningMethod = ApiKey::isAdmin($apiKey) ? 'leftJoin' : 'join';
$tagsSubQb
->select('t.id', 't.name', 'COUNT(DISTINCT s.id) AS short_urls_count')
->from('tags', 't')
->groupBy('t.id', 't.name')
->{$joiningMethod}('t', 'short_urls_in_tags', 'st', $tagsSubQb->expr()->eq('st.tag_id', 't.id'))
->{$joiningMethod}('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id'))
->groupBy('t.id', 't.name');
->{$joiningMethod}('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id'));
$searchTerm = $filtering?->searchTerm;
if ($searchTerm !== null) {
@ -115,7 +115,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
->setFirstResult($filtering?->offset ?? 0);
$orderByTag = $orderField == null || $orderField === OrderableField::TAG->value;
$orderByTag = $orderField === null || $orderField === OrderableField::TAG->value;
if ($orderByTag) {
$mainQb->orderBy('t.name', $orderDir ?? 'ASC');
} else {

View File

@ -59,7 +59,7 @@ class TagService implements TagServiceInterface
*/
public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void
{
if ($apiKey !== null && ! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
throw ForbiddenTagOperationException::forDeletion();
}
@ -75,7 +75,7 @@ class TagService implements TagServiceInterface
*/
public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag
{
if ($apiKey !== null && ! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
throw ForbiddenTagOperationException::forRenaming();
}

View File

@ -11,14 +11,14 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
class WithApiKeySpecsEnsuringJoin extends BaseSpecification
{
public function __construct(private ?ApiKey $apiKey, private string $fieldToJoin = 'shortUrls')
public function __construct(private readonly ?ApiKey $apiKey, private readonly string $fieldToJoin = 'shortUrls')
{
parent::__construct();
}
protected function getSpec(): Specification
{
return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX(
return $this->apiKey === null || ApiKey::isAdmin($this->apiKey) ? Spec::andX() : Spec::andX(
Spec::join($this->fieldToJoin, 's'),
$this->apiKey->spec($this->fieldToJoin),
);

View File

@ -114,9 +114,12 @@ class ApiKey extends AbstractEntity
return Spec::andX(...$specs);
}
public function isAdmin(): bool
/**
* @return ($apiKey is null ? true : boolean)
*/
public static function isAdmin(?ApiKey $apiKey): bool
{
return $this->roles->isEmpty();
return $apiKey === null || $apiKey->roles->isEmpty();
}
public function hasRole(Role $role): bool