From 4f2146dd9cb1f06f7989b3db999987138926908d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 14 Sep 2018 19:38:52 +0200 Subject: [PATCH 1/9] Replaced commands namespace shortcode by short-code, using the old one as an alias --- .../Shortcode/GeneratePreviewCommand.php | 23 ++--- .../Shortcode/GenerateShortcodeCommand.php | 63 +++++++------- .../Command/Shortcode/GetVisitsCommand.php | 15 ++-- .../Shortcode/ListShortcodesCommand.php | 87 ++++++++++--------- .../Command/Shortcode/ResolveUrlCommand.php | 25 +++--- 5 files changed, 114 insertions(+), 99 deletions(-) diff --git a/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php index 6fa0a4872..e777b3312 100644 --- a/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php +++ b/module/CLI/src/Command/Shortcode/GeneratePreviewCommand.php @@ -14,7 +14,8 @@ class GeneratePreviewCommand extends Command { - const NAME = 'shortcode:process-previews'; + public const NAME = 'short-code:process-previews'; + private const ALIASES = ['shortcode:process-previews']; /** * @var PreviewGeneratorInterface @@ -40,17 +41,19 @@ public function __construct( parent::__construct(null); } - public function configure() + protected function configure(): void { - $this->setName(self::NAME) - ->setDescription( - $this->translator->translate( - 'Processes and generates the previews for every URL, improving performance for later web requests.' - ) - ); + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) + ->setDescription( + $this->translator->translate( + 'Processes and generates the previews for every URL, improving performance for later web requests.' + ) + ); } - public function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $page = 1; do { @@ -65,7 +68,7 @@ public function execute(InputInterface $input, OutputInterface $output) (new SymfonyStyle($input, $output))->success($this->translator->translate('Finished processing all URLs')); } - protected function processUrl($url, OutputInterface $output) + private function processUrl($url, OutputInterface $output): void { try { $output->write(\sprintf($this->translator->translate('Processing URL %s...'), $url)); diff --git a/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php b/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php index 08af7a86a..bc0dc6854 100644 --- a/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php +++ b/module/CLI/src/Command/Shortcode/GenerateShortcodeCommand.php @@ -20,7 +20,8 @@ class GenerateShortcodeCommand extends Command { use ShortUrlBuilderTrait; - public const NAME = 'shortcode:generate'; + public const NAME = 'short-code:generate'; + private const ALIASES = ['shortcode:generate']; /** * @var UrlShortenerInterface @@ -46,36 +47,38 @@ public function __construct( parent::__construct(null); } - public function configure() + protected function configure(): void { - $this->setName(self::NAME) - ->setDescription( - $this->translator->translate('Generates a short code for provided URL and returns the short URL') - ) - ->addArgument('longUrl', InputArgument::REQUIRED, $this->translator->translate('The long URL to parse')) - ->addOption( - 'tags', - 't', - InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, - $this->translator->translate('Tags to apply to the new short URL') - ) - ->addOption('validSince', 's', InputOption::VALUE_REQUIRED, $this->translator->translate( - 'The date from which this short URL will be valid. ' - . 'If someone tries to access it before this date, it will not be found.' - )) - ->addOption('validUntil', 'u', InputOption::VALUE_REQUIRED, $this->translator->translate( - 'The date until which this short URL will be valid. ' - . 'If someone tries to access it after this date, it will not be found.' - )) - ->addOption('customSlug', 'c', InputOption::VALUE_REQUIRED, $this->translator->translate( - 'If provided, this slug will be used instead of generating a short code' - )) - ->addOption('maxVisits', 'm', InputOption::VALUE_REQUIRED, $this->translator->translate( - 'This will limit the number of visits for this short URL.' - )); + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) + ->setDescription( + $this->translator->translate('Generates a short code for provided URL and returns the short URL') + ) + ->addArgument('longUrl', InputArgument::REQUIRED, $this->translator->translate('The long URL to parse')) + ->addOption( + 'tags', + 't', + InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, + $this->translator->translate('Tags to apply to the new short URL') + ) + ->addOption('validSince', 's', InputOption::VALUE_REQUIRED, $this->translator->translate( + 'The date from which this short URL will be valid. ' + . 'If someone tries to access it before this date, it will not be found.' + )) + ->addOption('validUntil', 'u', InputOption::VALUE_REQUIRED, $this->translator->translate( + 'The date until which this short URL will be valid. ' + . 'If someone tries to access it after this date, it will not be found.' + )) + ->addOption('customSlug', 'c', InputOption::VALUE_REQUIRED, $this->translator->translate( + 'If provided, this slug will be used instead of generating a short code' + )) + ->addOption('maxVisits', 'm', InputOption::VALUE_REQUIRED, $this->translator->translate( + 'This will limit the number of visits for this short URL.' + )); } - public function interact(InputInterface $input, OutputInterface $output) + protected function interact(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); $longUrl = $input->getArgument('longUrl'); @@ -91,7 +94,7 @@ public function interact(InputInterface $input, OutputInterface $output) } } - public function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); $longUrl = $input->getArgument('longUrl'); @@ -140,7 +143,7 @@ public function execute(InputInterface $input, OutputInterface $output) } } - private function getOptionalDate(InputInterface $input, string $fieldName) + private function getOptionalDate(InputInterface $input, string $fieldName): ?\DateTime { $since = $input->getOption($fieldName); return $since !== null ? new \DateTime($since) : null; diff --git a/module/CLI/src/Command/Shortcode/GetVisitsCommand.php b/module/CLI/src/Command/Shortcode/GetVisitsCommand.php index 27cf198d3..b27b6962d 100644 --- a/module/CLI/src/Command/Shortcode/GetVisitsCommand.php +++ b/module/CLI/src/Command/Shortcode/GetVisitsCommand.php @@ -15,7 +15,8 @@ class GetVisitsCommand extends Command { - const NAME = 'shortcode:visits'; + public const NAME = 'short-code:visits'; + private const ALIASES = ['shortcode:visits']; /** * @var VisitsTrackerInterface @@ -33,9 +34,11 @@ public function __construct(VisitsTrackerInterface $visitsTracker, TranslatorInt parent::__construct(); } - public function configure() + protected function configure(): void { - $this->setName(self::NAME) + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) ->setDescription( $this->translator->translate('Returns the detailed visits information for provided short code') ) @@ -58,7 +61,7 @@ public function configure() ); } - public function interact(InputInterface $input, OutputInterface $output) + protected function interact(InputInterface $input, OutputInterface $output): void { $shortCode = $input->getArgument('shortCode'); if (! empty($shortCode)) { @@ -74,7 +77,7 @@ public function interact(InputInterface $input, OutputInterface $output) } } - public function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); $shortCode = $input->getArgument('shortCode'); @@ -101,7 +104,7 @@ public function execute(InputInterface $input, OutputInterface $output) ], $rows); } - protected function getDateOption(InputInterface $input, $key) + private function getDateOption(InputInterface $input, $key) { $value = $input->getOption($key); if (! empty($value)) { diff --git a/module/CLI/src/Command/Shortcode/ListShortcodesCommand.php b/module/CLI/src/Command/Shortcode/ListShortcodesCommand.php index 85caf840c..fd9b0c8d2 100644 --- a/module/CLI/src/Command/Shortcode/ListShortcodesCommand.php +++ b/module/CLI/src/Command/Shortcode/ListShortcodesCommand.php @@ -18,7 +18,8 @@ class ListShortcodesCommand extends Command { use PaginatorUtilsTrait; - const NAME = 'shortcode:list'; + public const NAME = 'short-code:list'; + private const ALIASES = ['shortcode:list']; /** * @var ShortUrlServiceInterface @@ -46,49 +47,51 @@ public function __construct( protected function configure(): void { - $this->setName(self::NAME) - ->setDescription($this->translator->translate('List all short URLs')) - ->addOption( - 'page', - 'p', - InputOption::VALUE_OPTIONAL, - sprintf( - $this->translator->translate('The first page to list (%s items per page)'), - PaginableRepositoryAdapter::ITEMS_PER_PAGE - ), - 1 - ) - ->addOption( - 'searchTerm', - 's', - InputOption::VALUE_OPTIONAL, - $this->translator->translate( - 'A query used to filter results by searching for it on the longUrl and shortCode fields' - ) - ) - ->addOption( - 'tags', - 't', - InputOption::VALUE_OPTIONAL, - $this->translator->translate('A comma-separated list of tags to filter results') - ) - ->addOption( - 'orderBy', - 'o', - InputOption::VALUE_OPTIONAL, - $this->translator->translate( - 'The field from which we want to order by. Pass ASC or DESC separated by a comma' - ) - ) - ->addOption( - 'showTags', - null, - InputOption::VALUE_NONE, - $this->translator->translate('Whether to display the tags or not') - ); + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) + ->setDescription($this->translator->translate('List all short URLs')) + ->addOption( + 'page', + 'p', + InputOption::VALUE_OPTIONAL, + sprintf( + $this->translator->translate('The first page to list (%s items per page)'), + PaginableRepositoryAdapter::ITEMS_PER_PAGE + ), + 1 + ) + ->addOption( + 'searchTerm', + 's', + InputOption::VALUE_OPTIONAL, + $this->translator->translate( + 'A query used to filter results by searching for it on the longUrl and shortCode fields' + ) + ) + ->addOption( + 'tags', + 't', + InputOption::VALUE_OPTIONAL, + $this->translator->translate('A comma-separated list of tags to filter results') + ) + ->addOption( + 'orderBy', + 'o', + InputOption::VALUE_OPTIONAL, + $this->translator->translate( + 'The field from which we want to order by. Pass ASC or DESC separated by a comma' + ) + ) + ->addOption( + 'showTags', + null, + InputOption::VALUE_NONE, + $this->translator->translate('Whether to display the tags or not') + ); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); $page = (int) $input->getOption('page'); diff --git a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php index 2bd87a89f..b4a8394f6 100644 --- a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php +++ b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php @@ -15,7 +15,8 @@ class ResolveUrlCommand extends Command { - const NAME = 'shortcode:parse'; + public const NAME = 'short-code:parse'; + private const ALIASES = ['shortcode:parse']; /** * @var UrlShortenerInterface @@ -33,18 +34,20 @@ public function __construct(UrlShortenerInterface $urlShortener, TranslatorInter parent::__construct(null); } - public function configure() + protected function configure(): void { - $this->setName(self::NAME) - ->setDescription($this->translator->translate('Returns the long URL behind a short code')) - ->addArgument( - 'shortCode', - InputArgument::REQUIRED, - $this->translator->translate('The short code to parse') - ); + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) + ->setDescription($this->translator->translate('Returns the long URL behind a short code')) + ->addArgument( + 'shortCode', + InputArgument::REQUIRED, + $this->translator->translate('The short code to parse') + ); } - public function interact(InputInterface $input, OutputInterface $output) + protected function interact(InputInterface $input, OutputInterface $output): void { $shortCode = $input->getArgument('shortCode'); if (! empty($shortCode)) { @@ -60,7 +63,7 @@ public function interact(InputInterface $input, OutputInterface $output) } } - public function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); $shortCode = $input->getArgument('shortCode'); From 07165f344fbf2efe240c0a6f560b4c35ec6acfbe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 10:03:42 +0200 Subject: [PATCH 2/9] Normalized entities adding missing type hints and removing superfluous comments --- module/Common/src/Entity/AbstractEntity.php | 13 +- module/Core/src/Entity/ShortUrl.php | 96 +++--------- module/Core/src/Entity/Tag.php | 22 +-- module/Core/src/Entity/Visit.php | 3 +- module/Core/src/Entity/VisitLocation.php | 137 +++++------------- module/Core/test/Service/UrlShortenerTest.php | 2 +- module/Rest/src/Entity/ApiKey.php | 48 +----- .../test/Action/AuthenticateActionTest.php | 2 +- .../test/Authentication/JWTServiceTest.php | 2 +- 9 files changed, 78 insertions(+), 247 deletions(-) diff --git a/module/Common/src/Entity/AbstractEntity.php b/module/Common/src/Entity/AbstractEntity.php index 8b71b4ec9..b798a087b 100644 --- a/module/Common/src/Entity/AbstractEntity.php +++ b/module/Common/src/Entity/AbstractEntity.php @@ -8,26 +8,19 @@ abstract class AbstractEntity { /** - * @var int + * @var string * @ORM\Id * @ORM\GeneratedValue(strategy="IDENTITY") * @ORM\Column(name="id", type="bigint", options={"unsigned"=true}) */ protected $id; - /** - * @return int - */ - public function getId() + public function getId(): string { return $this->id; } - /** - * @param int $id - * @return $this - */ - public function setId($id) + public function setId(string $id): self { $this->id = $id; return $this; diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index def81f016..48f597f35 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -7,13 +7,14 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; /** * Class ShortUrl * @author * @link * - * @ORM\Entity(repositoryClass="Shlinkio\Shlink\Core\Repository\ShortUrlRepository") + * @ORM\Entity(repositoryClass=ShortUrlRepository::class) * @ORM\Table(name="short_urls") */ class ShortUrl extends AbstractEntity @@ -22,7 +23,7 @@ class ShortUrl extends AbstractEntity * @var string * @ORM\Column(name="original_url", type="string", nullable=false, length=1024) */ - protected $originalUrl; + private $originalUrl; /** * @var string * @ORM\Column( @@ -33,17 +34,17 @@ class ShortUrl extends AbstractEntity * unique=true * ) */ - protected $shortCode; + private $shortCode; /** * @var \DateTime * @ORM\Column(name="date_created", type="datetime") */ - protected $dateCreated; + private $dateCreated; /** * @var Collection|Visit[] * @ORM\OneToMany(targetEntity=Visit::class, mappedBy="shortUrl", fetch="EXTRA_LAZY") */ - protected $visits; + private $visits; /** * @var Collection|Tag[] * @ORM\ManyToMany(targetEntity=Tag::class, cascade={"persist"}) @@ -53,46 +54,36 @@ class ShortUrl extends AbstractEntity * @ORM\JoinColumn(name="tag_id", referencedColumnName="id") * }) */ - protected $tags; + private $tags; /** * @var \DateTime * @ORM\Column(name="valid_since", type="datetime", nullable=true) */ - protected $validSince; + private $validSince; /** * @var \DateTime * @ORM\Column(name="valid_until", type="datetime", nullable=true) */ - protected $validUntil; + private $validUntil; /** * @var integer * @ORM\Column(name="max_visits", type="integer", nullable=true) */ - protected $maxVisits; + private $maxVisits; - /** - * ShortUrl constructor. - */ public function __construct() { + $this->shortCode = ''; $this->dateCreated = new \DateTime(); $this->visits = new ArrayCollection(); - $this->shortCode = ''; $this->tags = new ArrayCollection(); } - /** - * @return string - */ public function getLongUrl(): string { return $this->originalUrl; } - /** - * @param string $longUrl - * @return $this - */ public function setLongUrl(string $longUrl): self { $this->originalUrl = $longUrl; @@ -100,7 +91,6 @@ public function setLongUrl(string $longUrl): self } /** - * @return string * @deprecated Use getLongUrl() instead */ public function getOriginalUrl(): string @@ -109,8 +99,6 @@ public function getOriginalUrl(): string } /** - * @param string $originalUrl - * @return $this * @deprecated Use setLongUrl() instead */ public function setOriginalUrl(string $originalUrl): self @@ -118,37 +106,23 @@ public function setOriginalUrl(string $originalUrl): self return $this->setLongUrl($originalUrl); } - /** - * @return string - */ public function getShortCode(): string { return $this->shortCode; } - /** - * @param string $shortCode - * @return $this - */ - public function setShortCode(string $shortCode) + public function setShortCode(string $shortCode): self { $this->shortCode = $shortCode; return $this; } - /** - * @return \DateTime - */ public function getDateCreated(): \DateTime { return $this->dateCreated; } - /** - * @param \DateTime $dateCreated - * @return $this - */ - public function setDateCreated(\DateTime $dateCreated) + public function setDateCreated(\DateTime $dateCreated): self { $this->dateCreated = $dateCreated; return $this; @@ -164,55 +138,36 @@ public function getTags(): Collection /** * @param Collection|Tag[] $tags - * @return $this */ - public function setTags(Collection $tags) + public function setTags(Collection $tags): self { $this->tags = $tags; return $this; } - /** - * @param Tag $tag - * @return $this - */ - public function addTag(Tag $tag) + public function addTag(Tag $tag): self { $this->tags->add($tag); return $this; } - /** - * @return \DateTime|null - */ - public function getValidSince() + public function getValidSince(): ?\DateTime { return $this->validSince; } - /** - * @param \DateTime|null $validSince - * @return $this|self - */ - public function setValidSince($validSince): self + public function setValidSince(?\DateTime $validSince): self { $this->validSince = $validSince; return $this; } - /** - * @return \DateTime|null - */ - public function getValidUntil() + public function getValidUntil(): ?\DateTime { return $this->validUntil; } - /** - * @param \DateTime|null $validUntil - * @return $this|self - */ - public function setValidUntil($validUntil): self + public function setValidUntil(?\DateTime $validUntil): self { $this->validUntil = $validUntil; return $this; @@ -224,7 +179,7 @@ public function getVisitsCount(): int } /** - * @param Collection $visits + * @param Collection|Visit[] $visits * @return ShortUrl * @internal */ @@ -234,19 +189,12 @@ public function setVisits(Collection $visits): self return $this; } - /** - * @return int|null - */ - public function getMaxVisits() + public function getMaxVisits(): ?int { return $this->maxVisits; } - /** - * @param int|null $maxVisits - * @return $this|self - */ - public function setMaxVisits($maxVisits): self + public function setMaxVisits(?int $maxVisits): self { $this->maxVisits = $maxVisits; return $this; diff --git a/module/Core/src/Entity/Tag.php b/module/Core/src/Entity/Tag.php index cd8952684..aad6b7135 100644 --- a/module/Core/src/Entity/Tag.php +++ b/module/Core/src/Entity/Tag.php @@ -21,39 +21,25 @@ class Tag extends AbstractEntity implements \JsonSerializable * @var string * @ORM\Column(unique=true) */ - protected $name; + private $name; public function __construct($name = null) { $this->name = $name; } - /** - * @return string - */ - public function getName() + public function getName(): string { return $this->name; } - /** - * @param string $name - * @return $this - */ - public function setName($name) + public function setName(string $name) { $this->name = $name; return $this; } - /** - * Specify data which should be serialized to JSON - * @link http://php.net/manual/en/jsonserializable.jsonserialize.php - * @return mixed data which can be serialized by json_encode, - * which is a value of any type other than a resource. - * @since 5.4.0 - */ - public function jsonSerialize() + public function jsonSerialize(): string { return $this->name; } diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 9390005ca..f23657d48 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -7,13 +7,14 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\Util\IpAddress; +use Shlinkio\Shlink\Core\Repository\VisitRepository; /** * Class Visit * @author * @link * - * @ORM\Entity(repositoryClass="Shlinkio\Shlink\Core\Repository\VisitRepository") + * @ORM\Entity(repositoryClass=VisitRepository::class) * @ORM\Table(name="visits") */ class Visit extends AbstractEntity implements \JsonSerializable diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php index ca30596ac..2027f7e32 100644 --- a/module/Core/src/Entity/VisitLocation.php +++ b/module/Core/src/Entity/VisitLocation.php @@ -21,159 +21,110 @@ class VisitLocation extends AbstractEntity implements ArraySerializableInterface * @var string * @ORM\Column(nullable=true) */ - protected $countryCode; + private $countryCode; /** * @var string * @ORM\Column(nullable=true) */ - protected $countryName; + private $countryName; /** * @var string * @ORM\Column(nullable=true) */ - protected $regionName; + private $regionName; /** * @var string * @ORM\Column(nullable=true) */ - protected $cityName; + private $cityName; /** * @var string * @ORM\Column(nullable=true) */ - protected $latitude; + private $latitude; /** * @var string * @ORM\Column(nullable=true) */ - protected $longitude; + private $longitude; /** * @var string * @ORM\Column(nullable=true) */ - protected $timezone; + private $timezone; - /** - * @return string - */ - public function getCountryCode() + public function getCountryCode(): string { - return $this->countryCode; + return $this->countryCode ?? ''; } - /** - * @param string $countryCode - * @return $this - */ - public function setCountryCode($countryCode) + public function setCountryCode(string $countryCode) { $this->countryCode = $countryCode; return $this; } - /** - * @return string - */ - public function getCountryName() + public function getCountryName(): string { - return $this->countryName; + return $this->countryName ?? ''; } - /** - * @param string $countryName - * @return $this - */ - public function setCountryName($countryName) + public function setCountryName(string $countryName): self { $this->countryName = $countryName; return $this; } - /** - * @return string - */ - public function getRegionName() + public function getRegionName(): string { - return $this->regionName; + return $this->regionName ?? ''; } - /** - * @param string $regionName - * @return $this - */ - public function setRegionName($regionName) + public function setRegionName(string $regionName): self { $this->regionName = $regionName; return $this; } - /** - * @return string - */ - public function getCityName() + public function getCityName(): string { - return $this->cityName; + return $this->cityName ?? ''; } - /** - * @param string $cityName - * @return $this - */ - public function setCityName($cityName) + public function setCityName(string $cityName): self { $this->cityName = $cityName; return $this; } - /** - * @return string - */ - public function getLatitude() + public function getLatitude(): string { - return $this->latitude; + return $this->latitude ?? ''; } - /** - * @param string $latitude - * @return $this - */ - public function setLatitude($latitude) + public function setLatitude(string $latitude): self { $this->latitude = $latitude; return $this; } - /** - * @return string - */ - public function getLongitude() + public function getLongitude(): string { - return $this->longitude; + return $this->longitude ?? ''; } - /** - * @param string $longitude - * @return $this - */ - public function setLongitude($longitude) + public function setLongitude(string $longitude): self { $this->longitude = $longitude; return $this; } - /** - * @return string - */ - public function getTimezone() + public function getTimezone(): string { - return $this->timezone; + return $this->timezone ?? ''; } - /** - * @param string $timezone - * @return $this - */ - public function setTimezone($timezone) + public function setTimezone(string $timezone): self { $this->timezone = $timezone; return $this; @@ -181,41 +132,36 @@ public function setTimezone($timezone) /** * Exchange internal values from provided array - * - * @param array $array - * @return void */ - public function exchangeArray(array $array) + public function exchangeArray(array $array): void { - if (array_key_exists('country_code', $array)) { + if (\array_key_exists('country_code', $array)) { $this->setCountryCode($array['country_code']); } - if (array_key_exists('country_name', $array)) { + if (\array_key_exists('country_name', $array)) { $this->setCountryName($array['country_name']); } - if (array_key_exists('region_name', $array)) { + if (\array_key_exists('region_name', $array)) { $this->setRegionName($array['region_name']); } - if (array_key_exists('city', $array)) { + if (\array_key_exists('city', $array)) { $this->setCityName($array['city']); } - if (array_key_exists('latitude', $array)) { + if (\array_key_exists('latitude', $array)) { $this->setLatitude($array['latitude']); } - if (array_key_exists('longitude', $array)) { + if (\array_key_exists('longitude', $array)) { $this->setLongitude($array['longitude']); } - if (array_key_exists('time_zone', $array)) { + if (\array_key_exists('time_zone', $array)) { $this->setTimezone($array['time_zone']); } } /** * Return an array representation of the object - * - * @return array */ - public function getArrayCopy() + public function getArrayCopy(): array { return [ 'countryCode' => $this->countryCode, @@ -228,14 +174,7 @@ public function getArrayCopy() ]; } - /** - * Specify data which should be serialized to JSON - * @link http://php.net/manual/en/jsonserializable.jsonserialize.php - * @return mixed data which can be serialized by json_encode, - * which is a value of any type other than a resource. - * @since 5.4.0 - */ - public function jsonSerialize() + public function jsonSerialize(): array { return $this->getArrayCopy(); } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 234240b7c..44a759b75 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -54,7 +54,7 @@ public function setUp() $this->em->persist(Argument::any())->will(function ($arguments) { /** @var ShortUrl $shortUrl */ $shortUrl = $arguments[0]; - $shortUrl->setId(10); + $shortUrl->setId('10'); }); $repo = $this->prophesize(ObjectRepository::class); $repo->findOneBy(Argument::any())->willReturn(null); diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index fe475c3aa..2246f9098 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -23,17 +23,17 @@ class ApiKey extends AbstractEntity * @var string * @ORM\Column(name="`key`", nullable=false, unique=true) */ - protected $key; + private $key; /** * @var \DateTime|null * @ORM\Column(name="expiration_date", nullable=true, type="datetime") */ - protected $expirationDate; + private $expirationDate; /** * @var bool * @ORM\Column(type="boolean") */ - protected $enabled; + private $enabled; public function __construct() { @@ -41,45 +41,28 @@ public function __construct() $this->key = $this->generateV4Uuid(); } - /** - * @return string - */ public function getKey(): string { return $this->key; } - /** - * @param string $key - * @return $this - */ - public function setKey($key): self + public function setKey(string $key): self { $this->key = $key; return $this; } - /** - * @return \DateTime|null - */ public function getExpirationDate(): ?\DateTime { return $this->expirationDate; } - /** - * @param \DateTime $expirationDate - * @return $this - */ - public function setExpirationDate($expirationDate): self + public function setExpirationDate(\DateTime $expirationDate): self { $this->expirationDate = $expirationDate; return $this; } - /** - * @return bool - */ public function isExpired(): bool { if ($this->expirationDate === null) { @@ -89,29 +72,17 @@ public function isExpired(): bool return $this->expirationDate < new \DateTime(); } - /** - * @return boolean - */ public function isEnabled(): bool { return $this->enabled; } - /** - * @param boolean $enabled - * @return $this - */ - public function setEnabled($enabled): self + public function setEnabled(bool $enabled): self { $this->enabled = $enabled; return $this; } - /** - * Disables this API key - * - * @return $this - */ public function disable(): self { return $this->setEnabled(false); @@ -119,19 +90,12 @@ public function disable(): self /** * Tells if this api key is enabled and not expired - * - * @return bool */ public function isValid(): bool { return $this->isEnabled() && ! $this->isExpired(); } - /** - * The string representation of an API key is the key itself - * - * @return string - */ public function __toString(): string { return $this->getKey(); diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 4825a1006..131efb6d1 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -55,7 +55,7 @@ public function notProvidingAuthDataReturnsError() */ public function properApiKeyReturnsTokenInResponse() { - $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setId(5)) + $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setId('5')) ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ diff --git a/module/Rest/test/Authentication/JWTServiceTest.php b/module/Rest/test/Authentication/JWTServiceTest.php index a80680f8b..e592e8657 100644 --- a/module/Rest/test/Authentication/JWTServiceTest.php +++ b/module/Rest/test/Authentication/JWTServiceTest.php @@ -30,7 +30,7 @@ public function setUp() */ public function tokenIsProperlyCreated() { - $id = 34; + $id = '34'; $token = $this->service->create((new ApiKey())->setId($id)); $payload = (array) JWT::decode($token, 'foo', [JWTService::DEFAULT_ENCRYPTION_ALG]); $this->assertGreaterThanOrEqual($payload['iat'], time()); From 394d9ff4d2695bff17462fecf5ff1eb3a444e850 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 11:01:23 +0200 Subject: [PATCH 3/9] Defined config and implementation to delete short URLs --- config/autoload/app_options.global.php | 1 + config/autoload/delete_short_urls.global.php | 13 +++++++ module/Core/config/dependencies.config.php | 1 + module/Core/src/Options/AppOptionsFactory.php | 2 +- .../src/Options/DeleteShortUrlsOptions.php | 34 +++++++++++++++++++ .../Options/DeleteShortUrlsOptionsFactory.php | 31 +++++++++++++++++ module/Core/src/Service/ShortUrlService.php | 23 +++++++------ .../src/Service/ShortUrlServiceInterface.php | 18 +++++----- .../Action/ShortCode/ListShortCodesAction.php | 2 +- 9 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 config/autoload/delete_short_urls.global.php create mode 100644 module/Core/src/Options/DeleteShortUrlsOptions.php create mode 100644 module/Core/src/Options/DeleteShortUrlsOptionsFactory.php diff --git a/config/autoload/app_options.global.php b/config/autoload/app_options.global.php index 12e22c0f0..e280b4cf0 100644 --- a/config/autoload/app_options.global.php +++ b/config/autoload/app_options.global.php @@ -9,6 +9,7 @@ 'name' => 'Shlink', 'version' => '%SHLINK_VERSION%', 'secret_key' => env('SECRET_KEY'), + 'disable_track_param' => null, ], ]; diff --git a/config/autoload/delete_short_urls.global.php b/config/autoload/delete_short_urls.global.php new file mode 100644 index 000000000..e3d4ca64e --- /dev/null +++ b/config/autoload/delete_short_urls.global.php @@ -0,0 +1,13 @@ + [ + 'visits_threshold' => 15, + 'check_visits_threshold' => true, + ], + +]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8a696525d..8eb6e9b65 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -17,6 +17,7 @@ 'dependencies' => [ 'factories' => [ Options\AppOptions::class => Options\AppOptionsFactory::class, + Options\DeleteShortUrlsOptions::class => Options\DeleteShortUrlsOptionsFactory::class, NotFoundHandler::class => ConfigAbstractFactory::class, // Services diff --git a/module/Core/src/Options/AppOptionsFactory.php b/module/Core/src/Options/AppOptionsFactory.php index d61d517e6..0df25a2af 100644 --- a/module/Core/src/Options/AppOptionsFactory.php +++ b/module/Core/src/Options/AppOptionsFactory.php @@ -26,6 +26,6 @@ class AppOptionsFactory implements FactoryInterface public function __invoke(ContainerInterface $container, $requestedName, array $options = null) { $config = $container->has('config') ? $container->get('config') : []; - return new AppOptions(isset($config['app_options']) ? $config['app_options'] : []); + return new AppOptions($config['app_options'] ?? []); } } diff --git a/module/Core/src/Options/DeleteShortUrlsOptions.php b/module/Core/src/Options/DeleteShortUrlsOptions.php new file mode 100644 index 000000000..65dffdc4f --- /dev/null +++ b/module/Core/src/Options/DeleteShortUrlsOptions.php @@ -0,0 +1,34 @@ +visitsThreshold; + } + + protected function setVisitsThreshold(int $visitsThreshold): self + { + $this->visitsThreshold = $visitsThreshold; + return $this; + } + + public function doCheckVisitsThreshold(): bool + { + return $this->checkVisitsThreshold; + } + + protected function setCheckVisitsThreshold(bool $checkVisitsThreshold): self + { + $this->checkVisitsThreshold = $checkVisitsThreshold; + return $this; + } +} diff --git a/module/Core/src/Options/DeleteShortUrlsOptionsFactory.php b/module/Core/src/Options/DeleteShortUrlsOptionsFactory.php new file mode 100644 index 000000000..fab7cfee4 --- /dev/null +++ b/module/Core/src/Options/DeleteShortUrlsOptionsFactory.php @@ -0,0 +1,31 @@ +has('config') ? $container->get('config') : []; + return new DeleteShortUrlsOptions($config['delete_short_urls'] ?? []); + } +} diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 3a89b8281..b96adff03 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -27,13 +27,11 @@ public function __construct(ORM\EntityManagerInterface $em) } /** - * @param int $page - * @param string $searchQuery - * @param array $tags - * @param null $orderBy + * @param string[] $tags + * @param array|string|null $orderBy * @return ShortUrl[]|Paginator */ - public function listShortUrls($page = 1, $searchQuery = null, array $tags = [], $orderBy = null) + public function listShortUrls(int $page = 1, string $searchQuery = null, array $tags = [], $orderBy = null) { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); @@ -45,9 +43,7 @@ public function listShortUrls($page = 1, $searchQuery = null, array $tags = [], } /** - * @param string $shortCode * @param string[] $tags - * @return ShortUrl * @throws InvalidShortCodeException */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl @@ -60,9 +56,6 @@ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUr } /** - * @param string $shortCode - * @param ShortUrlMeta $shortCodeMeta - * @return ShortUrl * @throws InvalidShortCodeException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl @@ -81,9 +74,19 @@ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $short /** @var ORM\EntityManager $em */ $em = $this->em; $em->flush($shortUrl); + return $shortUrl; } + /** + * @throws InvalidShortCodeException + */ + public function deleteByShortCode(string $shortCode): void + { + $this->em->remove($this->findByShortCode($shortCode)); + $this->em->flush(); + } + /** * @param string $shortCode * @return ShortUrl diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index 2350105b6..d616648c1 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -11,27 +11,25 @@ interface ShortUrlServiceInterface { /** - * @param int $page - * @param string $searchQuery - * @param array $tags - * @param null $orderBy + * @param string[] $tags + * @param array|string|null $orderBy * @return ShortUrl[]|Paginator */ - public function listShortUrls($page = 1, $searchQuery = null, array $tags = [], $orderBy = null); + public function listShortUrls(int $page = 1, string $searchQuery = null, array $tags = [], $orderBy = null); /** - * @param string $shortCode * @param string[] $tags - * @return ShortUrl * @throws InvalidShortCodeException */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl; /** - * @param string $shortCode - * @param ShortUrlMeta $shortCodeMeta - * @return ShortUrl * @throws InvalidShortCodeException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl; + + /** + * @throws InvalidShortCodeException + */ + public function deleteByShortCode(string $shortCode): void; } diff --git a/module/Rest/src/Action/ShortCode/ListShortCodesAction.php b/module/Rest/src/Action/ShortCode/ListShortCodesAction.php index e69840a5d..f95be7899 100644 --- a/module/Rest/src/Action/ShortCode/ListShortCodesAction.php +++ b/module/Rest/src/Action/ShortCode/ListShortCodesAction.php @@ -75,7 +75,7 @@ public function handle(Request $request): Response private function queryToListParams(array $query): array { return [ - $query['page'] ?? 1, + (int) ($query['page'] ?? 1), $query['searchTerm'] ?? null, $query['tags'] ?? [], $query['orderBy'] ?? null, From 159529937df94e33e5adea6daf05e088423f3a4a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 11:54:58 +0200 Subject: [PATCH 4/9] Created specific service to delete short URLs --- module/Core/src/Entity/ShortUrl.php | 2 +- .../src/Exception/DeleteShortUrlException.php | 34 +++++++ .../Exception/InvalidShortCodeException.php | 6 +- .../ShortUrl/DeleteShortUrlService.php | 56 +++++++++++ .../DeleteShortUrlServiceInterface.php | 15 +++ .../Service/ShortUrl/FindShortCodeTrait.php | 29 ++++++ module/Core/src/Service/ShortUrlService.php | 33 +------ .../src/Service/ShortUrlServiceInterface.php | 5 - .../Exception/DeleteShortUrlExceptionTest.php | 61 ++++++++++++ .../ShortUrl/DeleteShortUrlServiceTest.php | 97 +++++++++++++++++++ 10 files changed, 300 insertions(+), 38 deletions(-) create mode 100644 module/Core/src/Exception/DeleteShortUrlException.php create mode 100644 module/Core/src/Service/ShortUrl/DeleteShortUrlService.php create mode 100644 module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php create mode 100644 module/Core/src/Service/ShortUrl/FindShortCodeTrait.php create mode 100644 module/Core/test/Exception/DeleteShortUrlExceptionTest.php create mode 100644 module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 48f597f35..fcaaa5dc8 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -175,7 +175,7 @@ public function setValidUntil(?\DateTime $validUntil): self public function getVisitsCount(): int { - return count($this->visits); + return \count($this->visits); } /** diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php new file mode 100644 index 000000000..8e2914d7a --- /dev/null +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -0,0 +1,34 @@ +visitsThreshold = $visitsThreshold; + parent::__construct($message, $code, $previous); + } + + public static function fromVisitsThreshold(int $threshold, string $shortCode): self + { + return new self($threshold, \sprintf( + 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', + $shortCode, + $threshold + )); + } + + public function getVisitsThreshold(): int + { + return $this->visitsThreshold; + } +} diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php index d2201f1de..c8123a74b 100644 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ b/module/Core/src/Exception/InvalidShortCodeException.php @@ -7,9 +7,9 @@ class InvalidShortCodeException extends RuntimeException { public static function fromCharset($shortCode, $charSet, \Exception $previous = null) { - $code = isset($previous) ? $previous->getCode() : -1; + $code = $previous !== null ? $previous->getCode() : -1; return new static( - sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), + \sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), $code, $previous ); @@ -17,6 +17,6 @@ public static function fromCharset($shortCode, $charSet, \Exception $previous = public static function fromNotFoundShortCode($shortCode) { - return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); + return new static(\sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); } } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php new file mode 100644 index 000000000..4f7607ebd --- /dev/null +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -0,0 +1,56 @@ +em = $em; + $this->deleteShortUrlsOptions = $deleteShortUrlsOptions; + } + + /** + * @throws Exception\InvalidShortCodeException + * @throws Exception\DeleteShortUrlException + */ + public function deleteByShortCode(string $shortCode): void + { + $shortUrl = $this->findByShortCode($this->em, $shortCode); + if ($this->isThresholdReached($shortUrl)) { + throw Exception\DeleteShortUrlException::fromVisitsThreshold( + $this->deleteShortUrlsOptions->getVisitsThreshold(), + $shortUrl->getShortCode() + ); + } + + $this->em->remove($shortUrl); + $this->em->flush(); + } + + private function isThresholdReached(ShortUrl $shortUrl): bool + { + if (! $this->deleteShortUrlsOptions->doCheckVisitsThreshold()) { + return false; + } + + return $shortUrl->getVisitsCount() >= $this->deleteShortUrlsOptions->getVisitsThreshold(); + } +} diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php new file mode 100644 index 000000000..4afbb15af --- /dev/null +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php @@ -0,0 +1,15 @@ +getRepository(ShortUrl::class)->findOneBy([ + 'shortCode' => $shortCode, + ]); + if ($shortUrl === null) { + throw InvalidShortCodeException::fromNotFoundShortCode($shortCode); + } + + return $shortUrl; + } +} diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index b96adff03..91e64bf1c 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -9,11 +9,13 @@ use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Service\ShortUrl\FindShortCodeTrait; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Zend\Paginator\Paginator; class ShortUrlService implements ShortUrlServiceInterface { + use FindShortCodeTrait; use TagManagerTrait; /** @@ -48,7 +50,7 @@ public function listShortUrls(int $page = 1, string $searchQuery = null, array $ */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl { - $shortUrl = $this->findByShortCode($shortCode); + $shortUrl = $this->findByShortCode($this->em, $shortCode); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); $this->em->flush(); @@ -60,7 +62,7 @@ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUr */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl { - $shortUrl = $this->findByShortCode($shortCode); + $shortUrl = $this->findByShortCode($this->em, $shortCode); if ($shortCodeMeta->hasValidSince()) { $shortUrl->setValidSince($shortCodeMeta->getValidSince()); } @@ -77,31 +79,4 @@ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $short return $shortUrl; } - - /** - * @throws InvalidShortCodeException - */ - public function deleteByShortCode(string $shortCode): void - { - $this->em->remove($this->findByShortCode($shortCode)); - $this->em->flush(); - } - - /** - * @param string $shortCode - * @return ShortUrl - * @throws InvalidShortCodeException - */ - private function findByShortCode(string $shortCode): ShortUrl - { - /** @var ShortUrl|null $shortUrl */ - $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ - 'shortCode' => $shortCode, - ]); - if ($shortUrl === null) { - throw InvalidShortCodeException::fromNotFoundShortCode($shortCode); - } - - return $shortUrl; - } } diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index d616648c1..c6c61126c 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -27,9 +27,4 @@ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUr * @throws InvalidShortCodeException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortCodeMeta): ShortUrl; - - /** - * @throws InvalidShortCodeException - */ - public function deleteByShortCode(string $shortCode): void; } diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php new file mode 100644 index 000000000..6412233ee --- /dev/null +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -0,0 +1,61 @@ +assertEquals($expectedMessage, $e->getMessage()); + } + + public function provideMessages(): array + { + return [ + [ + 50, + 'abc123', + 'Impossible to delete short URL with short code "abc123" since it has more than "50" visits.', + ], + [ + 33, + 'def456', + 'Impossible to delete short URL with short code "def456" since it has more than "33" visits.', + ], + [ + 5713, + 'foobar', + 'Impossible to delete short URL with short code "foobar" since it has more than "5713" visits.', + ], + ]; + } + + /** + * @test + * @dataProvider provideThresholds + */ + public function visitsThresholdIsProperlyReturned(int $threshold) + { + $e = new DeleteShortUrlException($threshold); + $this->assertEquals($threshold, $e->getVisitsThreshold()); + } + + public function provideThresholds(): array + { + return \array_map(function (int $number) { + return [$number]; + }, \range(5, 50, 5)); + } +} diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php new file mode 100644 index 000000000..727ad1878 --- /dev/null +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -0,0 +1,97 @@ +setShortCode('abc123') + ->setVisits(new ArrayCollection(\array_map(function () { + return new Visit(); + }, \range(0, 10)))); + + $this->em = $this->prophesize(EntityManagerInterface::class); + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $repo->findOneBy(Argument::type('array'))->willReturn($shortUrl); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + } + + /** + * @test + */ + public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached() + { + $service = $this->createService(); + + $this->expectException(DeleteShortUrlException::class); + $this->expectExceptionMessage( + 'Impossible to delete short URL with short code "abc123" since it has more than "5" visits.' + ); + + $service->deleteByShortCode('abc123'); + } + + /** + * @test + */ + public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButCheckIsDisabled() + { + $service = $this->createService(false); + + $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); + $flush = $this->em->flush()->willReturn(null); + + $service->deleteByShortCode('abc123'); + + $remove->shouldHaveBeenCalledTimes(1); + $flush->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function deleteByShortCodeDeletesUrlWhenThresholdIsNotReached() + { + $service = $this->createService(true, 100); + + $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); + $flush = $this->em->flush()->willReturn(null); + + $service->deleteByShortCode('abc123'); + + $remove->shouldHaveBeenCalledTimes(1); + $flush->shouldHaveBeenCalledTimes(1); + } + + private function createService(bool $checkVisitsThreshold = true, int $visitsThreshold = 5): DeleteShortUrlService + { + return new DeleteShortUrlService($this->em->reveal(), new DeleteShortUrlsOptions([ + 'visitsThreshold' => $visitsThreshold, + 'checkVisitsThreshold' => $checkVisitsThreshold, + ])); + } +} From 5714a8f884fed1f04d1989ffda5fe9ce68c78510 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 12:56:17 +0200 Subject: [PATCH 5/9] Created action to delete short URLs --- module/Core/config/dependencies.config.php | 2 + module/Core/src/Action/QrCodeAction.php | 12 ++-- module/Rest/config/dependencies.config.php | 8 ++- module/Rest/config/routes.config.php | 1 + .../ShortCode/DeleteShortCodeAction.php | 71 ++++++++++++++++++ module/Rest/src/Util/RestUtils.php | 3 + .../ShortCode/DeleteShortCodeActionTest.php | 72 +++++++++++++++++++ 7 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 module/Rest/src/Action/ShortCode/DeleteShortCodeAction.php create mode 100644 module/Rest/test/Action/ShortCode/DeleteShortCodeActionTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8eb6e9b65..31fd12970 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -26,6 +26,7 @@ Service\ShortUrlService::class => ConfigAbstractFactory::class, Service\VisitService::class => ConfigAbstractFactory::class, Service\Tag\TagService::class => ConfigAbstractFactory::class, + Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, // Middleware Action\RedirectAction::class => ConfigAbstractFactory::class, @@ -50,6 +51,7 @@ Service\ShortUrlService::class => ['em'], Service\VisitService::class => ['em'], Service\Tag\TagService::class => ['em'], + Service\ShortUrl\DeleteShortUrlService::class => ['em', Options\DeleteShortUrlsOptions::class], // Middleware Action\RedirectAction::class => [ diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 81b1d3261..c410802f3 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -22,6 +22,10 @@ class QrCodeAction implements MiddlewareInterface { use ErrorResponseBuilderTrait; + private const DEFAULT_SIZE = 300; + private const MIN_SIZE = 50; + private const MAX_SIZE = 1000; + /** * @var RouterInterface */ @@ -82,11 +86,11 @@ public function process(Request $request, RequestHandlerInterface $handler): Res */ private function getSizeParam(Request $request): int { - $size = (int) $request->getAttribute('size', 300); - if ($size < 50) { - return 50; + $size = (int) $request->getAttribute('size', self::DEFAULT_SIZE); + if ($size < self::MIN_SIZE) { + return self::MIN_SIZE; } - return $size > 1000 ? 1000 : $size; + return $size > self::MAX_SIZE ? self::MAX_SIZE : $size; } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 92c991e1b..ae500a225 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -23,6 +23,7 @@ Action\ShortCode\CreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\EditShortCodeAction::class => ConfigAbstractFactory::class, + Action\ShortCode\DeleteShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\Visit\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ShortCode\ListShortCodesAction::class => ConfigAbstractFactory::class, @@ -58,7 +59,12 @@ 'config.url_shortener.domain', 'Logger_Shlink', ], - Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], + Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\ShortCode\DeleteShortCodeAction::class => [ + Service\ShortUrl\DeleteShortUrlService::class, + 'translator', + 'Logger_Shlink', + ], Action\ShortCode\ResolveUrlAction::class => [ Service\UrlShortener::class, 'translator', diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 0f453e266..7058ad598 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -18,6 +18,7 @@ Middleware\ShortCode\CreateShortCodeContentNegotiationMiddleware::class, ]), Action\ShortCode\EditShortCodeAction::getRouteDef(), + Action\ShortCode\DeleteShortCodeAction::getRouteDef(), Action\ShortCode\ResolveUrlAction::getRouteDef(), Action\ShortCode\ListShortCodesAction::getRouteDef(), Action\ShortCode\EditShortCodeTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/ShortCode/DeleteShortCodeAction.php b/module/Rest/src/Action/ShortCode/DeleteShortCodeAction.php new file mode 100644 index 000000000..8fa9ca5be --- /dev/null +++ b/module/Rest/src/Action/ShortCode/DeleteShortCodeAction.php @@ -0,0 +1,71 @@ +deleteShortUrlService = $deleteShortUrlService; + $this->translator = $translator; + } + + /** + * Handle the request and return a response. + */ + public function handle(ServerRequestInterface $request): ResponseInterface + { + $shortCode = $request->getAttribute('shortCode', ''); + + try { + $this->deleteShortUrlService->deleteByShortCode($shortCode); + return new EmptyResponse(); + } catch (Exception\InvalidShortCodeException $e) { + $this->logger->warning( + \sprintf('Provided short code %s does not belong to any URL.', $shortCode) . PHP_EOL . $e + ); + return new JsonResponse([ + 'error' => RestUtils::getRestErrorCodeFromException($e), + 'message' => \sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), + ], self::STATUS_NOT_FOUND); + } catch (Exception\DeleteShortUrlException $e) { + $this->logger->warning('Provided data is invalid.' . PHP_EOL . $e); + $messagePlaceholder = $this->translator->translate( + 'It is not possible to delete URL with short code "%s" because it has reached more than "%s" visits.' + ); + + return new JsonResponse([ + 'error' => RestUtils::getRestErrorCodeFromException($e), + 'message' => \sprintf($messagePlaceholder, $shortCode, $e->getVisitsThreshold()), + ], self::STATUS_BAD_REQUEST); + } + } +} diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 666469edf..c783f6cb2 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -10,6 +10,7 @@ class RestUtils { public const INVALID_SHORTCODE_ERROR = 'INVALID_SHORTCODE'; + public const INVALID_SHORTCODE_DELETION_ERROR = 'INVALID_SHORTCODE_DELETION'; public const INVALID_URL_ERROR = 'INVALID_URL'; public const INVALID_ARGUMENT_ERROR = 'INVALID_ARGUMENT'; public const INVALID_SLUG_ERROR = 'INVALID_SLUG'; @@ -35,6 +36,8 @@ public static function getRestErrorCodeFromException(\Throwable $e) return self::INVALID_ARGUMENT_ERROR; case $e instanceof Rest\AuthenticationException: return self::INVALID_CREDENTIALS_ERROR; + case $e instanceof Core\DeleteShortUrlException: + return self::INVALID_SHORTCODE_DELETION_ERROR; default: return self::UNKNOWN_ERROR; } diff --git a/module/Rest/test/Action/ShortCode/DeleteShortCodeActionTest.php b/module/Rest/test/Action/ShortCode/DeleteShortCodeActionTest.php new file mode 100644 index 000000000..3c5a5c871 --- /dev/null +++ b/module/Rest/test/Action/ShortCode/DeleteShortCodeActionTest.php @@ -0,0 +1,72 @@ +service = $this->prophesize(DeleteShortUrlServiceInterface::class); + $this->action = new DeleteShortCodeAction($this->service->reveal(), Translator::factory([])); + } + + /** + * @test + */ + public function emptyResponseIsReturnedIfProperlyDeleted() + { + $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->will(function () { + }); + + $resp = $this->action->handle(ServerRequestFactory::fromGlobals()); + + $this->assertEquals(204, $resp->getStatusCode()); + $deleteByShortCode->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + * @dataProvider provideExceptions + */ + public function returnsErrorResponseInCaseOfException(\Throwable $e, string $error, int $statusCode) + { + $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->willThrow($e); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle(ServerRequestFactory::fromGlobals()); + $payload = $resp->getPayload(); + + $this->assertEquals($statusCode, $resp->getStatusCode()); + $this->assertEquals($error, $payload['error']); + $deleteByShortCode->shouldHaveBeenCalledTimes(1); + } + + public function provideExceptions(): array + { + return [ + [new Exception\InvalidShortCodeException(), RestUtils::INVALID_SHORTCODE_ERROR, 404], + [new Exception\DeleteShortUrlException(5), RestUtils::INVALID_SHORTCODE_DELETION_ERROR, 400], + ]; + } +} From 929d7670cbd77aefded0605fe46c1f879baf5f22 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 13:07:52 +0200 Subject: [PATCH 6/9] Documented delete short URLs endpoint in swagger --- .../paths/v1_short-codes_{shortCode}.json | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/docs/swagger/paths/v1_short-codes_{shortCode}.json b/docs/swagger/paths/v1_short-codes_{shortCode}.json index dba85af41..e17881af8 100644 --- a/docs/swagger/paths/v1_short-codes_{shortCode}.json +++ b/docs/swagger/paths/v1_short-codes_{shortCode}.json @@ -159,5 +159,70 @@ } } } + }, + + "delete": { + "tags": [ + "ShortCodes" + ], + "summary": "Delete short code", + "description": "Deletes the short URL for provided short code.", + "parameters": [ + { + "name": "shortCode", + "in": "path", + "description": "The short code to edit.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "security": [ + { + "Bearer": [] + } + ], + "responses": { + "204": { + "description": "The short code has been properly deleted." + }, + "400": { + "description": "The visits threshold in shlink does not allow this short URL to be deleted.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + }, + "examples": { + "application/json": { + "error": "INVALID_SHORTCODE_DELETION", + "message": "It is not possible to delete URL with short code \"abc123\" because it has reached more than \"15\" visits." + } + } + }, + "404": { + "description": "No short URL was found for provided short code.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + }, + "500": { + "description": "Unexpected error.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + } + } } } From 9d10c8627a3cf2938d42ac132c77dff9c53635c2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 13:20:13 +0200 Subject: [PATCH 7/9] Created migration fixing cascade delete on visits table --- data/migrations/Version20180915110857.php | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 data/migrations/Version20180915110857.php diff --git a/data/migrations/Version20180915110857.php b/data/migrations/Version20180915110857.php new file mode 100644 index 000000000..983ecd6d3 --- /dev/null +++ b/data/migrations/Version20180915110857.php @@ -0,0 +1,50 @@ + 'SET NULL', + 'short_urls' => 'CASCADE', + ]; + + /** + * @param Schema $schema + * @throws SchemaException + */ + public function up(Schema $schema): void + { + $visits = $schema->getTable('visits'); + $foreignKeys = $visits->getForeignKeys(); + + // Remove all existing foreign keys and add them again with CASCADE delete + foreach ($foreignKeys as $foreignKey) { + $visits->removeForeignKey($foreignKey->getName()); + $foreignTable = $foreignKey->getForeignTableName(); + + $visits->addForeignKeyConstraint( + $foreignTable, + $foreignKey->getLocalColumns(), + $foreignKey->getForeignColumns(), + [ + 'onDelete' => self::ON_DELETE_MAP[$foreignTable], + 'onUpdate' => 'RESTRICT', + ] + ); + } + } + + public function down(Schema $schema): void + { + // Nothing to run + } +} From 9651b3d6921aec60a7c68b1dddc9141e164e5483 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 17:57:12 +0200 Subject: [PATCH 8/9] Created command to delete short URLs --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 13 ++ .../Shortcode/DeleteShortCodeCommand.php | 101 +++++++++++++++ .../Shortcode/DeleteShortCodeCommandTest.php | 120 ++++++++++++++++++ .../ShortUrl/DeleteShortUrlService.php | 4 +- .../DeleteShortUrlServiceInterface.php | 2 +- .../ShortUrl/DeleteShortUrlServiceTest.php | 16 +++ 7 files changed, 254 insertions(+), 3 deletions(-) create mode 100644 module/CLI/src/Command/Shortcode/DeleteShortCodeCommand.php create mode 100644 module/CLI/test/Command/Shortcode/DeleteShortCodeCommandTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index c1f9043b3..eb27be7f3 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -14,6 +14,7 @@ Command\Shortcode\ListShortcodesCommand::NAME => Command\Shortcode\ListShortcodesCommand::class, Command\Shortcode\GetVisitsCommand::NAME => Command\Shortcode\GetVisitsCommand::class, Command\Shortcode\GeneratePreviewCommand::NAME => Command\Shortcode\GeneratePreviewCommand::class, + Command\Shortcode\DeleteShortCodeCommand::NAME => Command\Shortcode\DeleteShortCodeCommand::class, Command\Visit\ProcessVisitsCommand::NAME => Command\Visit\ProcessVisitsCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 236b8d87d..065c720ce 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -22,12 +22,17 @@ Command\Shortcode\ListShortcodesCommand::class => ConfigAbstractFactory::class, Command\Shortcode\GetVisitsCommand::class => ConfigAbstractFactory::class, Command\Shortcode\GeneratePreviewCommand::class => ConfigAbstractFactory::class, + Command\Shortcode\DeleteShortCodeCommand::class => ConfigAbstractFactory::class, + Command\Visit\ProcessVisitsCommand::class => ConfigAbstractFactory::class, + Command\Config\GenerateCharsetCommand::class => ConfigAbstractFactory::class, Command\Config\GenerateSecretCommand::class => ConfigAbstractFactory::class, + Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class, Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class, + Command\Tag\ListTagsCommand::class => ConfigAbstractFactory::class, Command\Tag\CreateTagCommand::class => ConfigAbstractFactory::class, Command\Tag\RenameTagCommand::class => ConfigAbstractFactory::class, @@ -53,16 +58,24 @@ PreviewGenerator::class, 'translator', ], + Command\Shortcode\DeleteShortCodeCommand::class => [ + Service\ShortUrl\DeleteShortUrlService::class, + 'translator', + ], + Command\Visit\ProcessVisitsCommand::class => [ Service\VisitService::class, IpApiLocationResolver::class, 'translator', ], + Command\Config\GenerateCharsetCommand::class => ['translator'], Command\Config\GenerateSecretCommand::class => ['translator'], + Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, 'translator'], Command\Api\DisableKeyCommand::class => [ApiKeyService::class, 'translator'], Command\Api\ListKeysCommand::class => [ApiKeyService::class, 'translator'], + Command\Tag\ListTagsCommand::class => [Service\Tag\TagService::class, Translator::class], Command\Tag\CreateTagCommand::class => [Service\Tag\TagService::class, Translator::class], Command\Tag\RenameTagCommand::class => [Service\Tag\TagService::class, Translator::class], diff --git a/module/CLI/src/Command/Shortcode/DeleteShortCodeCommand.php b/module/CLI/src/Command/Shortcode/DeleteShortCodeCommand.php new file mode 100644 index 000000000..f02750a1b --- /dev/null +++ b/module/CLI/src/Command/Shortcode/DeleteShortCodeCommand.php @@ -0,0 +1,101 @@ +deleteShortUrlService = $deleteShortUrlService; + $this->translator = $translator; + parent::__construct(); + } + + protected function configure(): void + { + $this + ->setName(self::NAME) + ->setAliases(self::ALIASES) + ->setDescription( + $this->translator->translate('Deletes a short URL') + ) + ->addArgument( + 'shortCode', + InputArgument::REQUIRED, + $this->translator->translate('The short code to be deleted') + ) + ->addOption( + 'ignore-threshold', + 'i', + InputOption::VALUE_NONE, + $this->translator->translate( + 'Ignores the safety visits threshold check, which could make short URLs with many visits to be ' + . 'accidentally deleted' + ) + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): void + { + $io = new SymfonyStyle($input, $output); + $shortCode = $input->getArgument('shortCode'); + $ignoreThreshold = $input->getOption('ignore-threshold'); + + try { + $this->runDelete($io, $shortCode, $ignoreThreshold); + } catch (Exception\InvalidShortCodeException $e) { + $io->error( + \sprintf($this->translator->translate('Provided short code "%s" could not be found.'), $shortCode) + ); + } catch (Exception\DeleteShortUrlException $e) { + $this->retry($io, $shortCode, $e); + } + } + + private function retry(SymfonyStyle $io, string $shortCode, Exception\DeleteShortUrlException $e): void + { + $warningMsg = \sprintf($this->translator->translate( + 'It was not possible to delete the short URL with short code "%s" because it has more than %s visits.' + ), $shortCode, $e->getVisitsThreshold()); + $io->writeln('' . $warningMsg . ''); + $forceDelete = $io->confirm($this->translator->translate('Do you want to delete it anyway?'), false); + + if ($forceDelete) { + $this->runDelete($io, $shortCode, true); + } else { + $io->warning($this->translator->translate('Short URL was not deleted.')); + } + } + + private function runDelete(SymfonyStyle $io, string $shortCode, bool $ignoreThreshold): void + { + $this->deleteShortUrlService->deleteByShortCode($shortCode, $ignoreThreshold); + $io->success(\sprintf( + $this->translator->translate('Short URL with short code "%s" successfully deleted.'), + $shortCode + )); + } +} diff --git a/module/CLI/test/Command/Shortcode/DeleteShortCodeCommandTest.php b/module/CLI/test/Command/Shortcode/DeleteShortCodeCommandTest.php new file mode 100644 index 000000000..e74b16b2a --- /dev/null +++ b/module/CLI/test/Command/Shortcode/DeleteShortCodeCommandTest.php @@ -0,0 +1,120 @@ +service = $this->prophesize(DeleteShortUrlServiceInterface::class); + + $command = new DeleteShortCodeCommand($this->service->reveal(), Translator::factory([])); + $app = new Application(); + $app->add($command); + + $this->commandTester = new CommandTester($command); + } + + /** + * @test + */ + public function successMessageIsPrintedIfUrlIsProperlyDeleted() + { + $shortCode = 'abc123'; + $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->will(function () { + }); + + $this->commandTester->execute(['shortCode' => $shortCode]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains(\sprintf('Short URL with short code "%s" successfully deleted.', $shortCode), $output); + $deleteByShortCode->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function invalidShortCodePrintsMessage() + { + $shortCode = 'abc123'; + $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( + Exception\InvalidShortCodeException::class + ); + + $this->commandTester->execute(['shortCode' => $shortCode]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains(\sprintf('Provided short code "%s" could not be found.', $shortCode), $output); + $deleteByShortCode->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted() + { + $shortCode = 'abc123'; + $deleteByShortCode = $this->service->deleteByShortCode($shortCode, Argument::type('bool'))->will( + function (array $args) { + $ignoreThreshold = \array_pop($args); + + if (!$ignoreThreshold) { + throw new Exception\DeleteShortUrlException(10); + } + } + ); + $this->commandTester->setInputs(['yes']); + + $this->commandTester->execute(['shortCode' => $shortCode]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains(\sprintf( + 'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.', + $shortCode + ), $output); + $this->assertContains(\sprintf('Short URL with short code "%s" successfully deleted.', $shortCode), $output); + $deleteByShortCode->shouldHaveBeenCalledTimes(2); + } + + /** + * @test + */ + public function deleteIsNotRetriedWhenThresholdIsReachedAndQuestionIsDeclined() + { + $shortCode = 'abc123'; + $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( + new Exception\DeleteShortUrlException(10) + ); + $this->commandTester->setInputs(['no']); + + $this->commandTester->execute(['shortCode' => $shortCode]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains(\sprintf( + 'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.', + $shortCode + ), $output); + $this->assertContains('Short URL was not deleted.', $output); + $deleteByShortCode->shouldHaveBeenCalledTimes(1); + } +} diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index 4f7607ebd..287a5a59b 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -31,10 +31,10 @@ public function __construct(EntityManagerInterface $em, DeleteShortUrlsOptions $ * @throws Exception\InvalidShortCodeException * @throws Exception\DeleteShortUrlException */ - public function deleteByShortCode(string $shortCode): void + public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void { $shortUrl = $this->findByShortCode($this->em, $shortCode); - if ($this->isThresholdReached($shortUrl)) { + if (! $ignoreThreshold && $this->isThresholdReached($shortUrl)) { throw Exception\DeleteShortUrlException::fromVisitsThreshold( $this->deleteShortUrlsOptions->getVisitsThreshold(), $shortUrl->getShortCode() diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php index 4afbb15af..de119bbb2 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php @@ -11,5 +11,5 @@ interface DeleteShortUrlServiceInterface * @throws Exception\InvalidShortCodeException * @throws Exception\DeleteShortUrlException */ - public function deleteByShortCode(string $shortCode): void; + public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void; } diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index 727ad1878..d79ce44d5 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -55,6 +55,22 @@ public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached() $service->deleteByShortCode('abc123'); } + /** + * @test + */ + public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButExplicitlyIgnored() + { + $service = $this->createService(); + + $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); + $flush = $this->em->flush()->willReturn(null); + + $service->deleteByShortCode('abc123', true); + + $remove->shouldHaveBeenCalledTimes(1); + $flush->shouldHaveBeenCalledTimes(1); + } + /** * @test */ From 9d8fb055b1b0b59e251d7a4713e4353ecaacd90d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Sep 2018 18:03:54 +0200 Subject: [PATCH 9/9] Updated translations --- module/CLI/lang/es.mo | Bin 9046 -> 9985 bytes module/CLI/lang/es.po | 62 +++++++++++++++++++++++++++++++++-------- module/Rest/lang/es.mo | Bin 2978 -> 3192 bytes module/Rest/lang/es.po | 12 ++++++-- 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index fb888fbf936bd243d618df6a9be5ce8132f5bd52..8492b93940c0f9efcf1c8681a5cd50155cb3c40a 100644 GIT binary patch delta 2885 zcmZ|Qe`u9e9LMpqA8DJLKW5f!{y5Fd*5=(@sk5>*m#ZyXrdc^5cy{lT_sM(jWA}M( zYed~lq963bx(Y@_L>LuF)UCiG1TRHF68=+!Wg!(t7DXu$1ie3Z&)kYS>^!gM=Q-bV zzUSP1*0*CYf2^T;kD+X#&ZGWbX-pnJIg113qp8N6iyz~8_zia8A9xG4oNdfnd;oj# zLu|&{nz8nl;sUM*P~UslgFBG-^QOefB5q8rHD)%pVgq&~eV7e67l*JBL#)OpunwO= zefI+Dy&|52htcA3)L5#f88ZWGunFhlJPma@CyHzX>cczn5zM0^Kbu|#a0x2%EUv=0 zun&Jmb-cWuXLuvhr#XZPJd9P?Ieje96?g&H19*<6I>iaIW_BQxVII>1+>OLwUPX=Y zZFKNsjA1Lop=~pSdOwLP@jh(Bx3L|M;~+N79Gg)G6|jf-ZccV`axs>WKXa0U0{IKi z$EmZ7xeS|8BfAwfg~K=p??3DjC@BEKWp*}(j33SWzE{E9bn z?J?X-@BpfVZ&9iEGjcA&)ne^JrEU}I{WNyrUR0n*knET*QGxt`Em%jR-&Yv3Vh;1K zshCeI%E?O9D&CAk_&lz|KTv_LWd&*^kK;l-h)eNnoR9T1rUA6#M!Xjn%VTQfM#u|d+$hIZ42b%kR! z9rhaEj`}$};$%V_`ZnRF+|adNXr0XHh%?$lW5G zHWp?#_Esrb{f(;^9y-`ldN*U8ptRrKmGfNX#7%i=FXLoWPIvL4#+NGd{YsX@eVZ!n zPkUsOX$ah{xvZCP61pho&I#J=_FTeqGYQ|)P%afZS>{|H4*S`x!|jR8=rf`S$ZW=@ zb8$^$)Qnjhgryg4(i!$rUg+GOaLk}jhictF6P`^uYJetLeNMtDHxw@w6W&&z^DJwx zoT}}9_O2Y+AuiXZOD_iMc|1){5G!;w-BWnEskeB#XwOVe}tQ+g1e8FlV zBpY}WF(jPW_$&X2f&u?WAkn;*Bh6;rc<7{Aqi)RfP7d1i7LP7?u&UV8Jg4Gsv8~+{ delta 2008 zcmY+^TWl0n9LMqhZc8uQy7Y=twOz`J^+MZSYA=X(ODRe%Td87@igC+s+BIuQF9cu6 zN{C>LA%O|TNPuc$d@#|_ZDON|J~Tv{NHl7UQKY^|qA5P`WJ!#Ef3u^Jll{+UE<1Dn z=bZn{k@)x9vsbGMPaEwi;x1y&Z#IDgtJu&gip|zwCEkur*orZH1}Cr&FJUhROUxQ^ z2=%;!SdXVr_g}ysTtxn#wX#yP^_&>QYMjC(_-(F`&8CI8>^A6TZK*%F&-`nP3v5@@%XHNyzO7tV_#zkzxQl_N>qo@J&VpdZWXQvilL{0H4-tnuRXHaW;0i*aG zswRqg+99k*&Cr|P@y|HO@$O2q`|&;0^S?&T^fk{_Rm{KEB+P>Ak6W-4M^PEhpr+~r zR08vOAO3{be5^O~u8}S?O`7^5dQlNJ;a<$(0Dgf=u!gLoIE)Q=Vh!`Zk)01YaW`H; zWquR&M)Z^220VxA=pw2Zm#_~1MBP`-NfloN$;~=Y=lea!yz9rj>u;czNbgP2n_BfA%Tr`Xa{gs%HbkR?)qH&*BUg;Z0=O%|`)* zuo`#i{cmF@!ii(3k=!(Wf8!oJ z8e-IV1$SbSf@#)%m{{)HMyn6H@+ zHxU}hWAQ9>22qEUgVJwi~2`N~F3Kg;T_qn7S*LPZ-R zR12zXR^gVW;_fA$B$^4;TEF{LcG`$-gxZcg_E_#>@d>{(92js$0#(lCKy9v|Y^T3; zDwG;ejAT+tClcJ_oC>ZlADm2&rpFVR(C(pu++6TuU-?T1rcuobYG(_bmb&MivvsYx>viw?EBdCU7v2etB}USj^mHPb za5D8TJAc%7l^pQ8$Tc=p_$#-iCsX_Prx$YLp~)0|C7j5*PjbVJCkvdC4J-cu*8kMm diff --git a/module/CLI/lang/es.po b/module/CLI/lang/es.po index 1555bf1e3..db1408e87 100644 --- a/module/CLI/lang/es.po +++ b/module/CLI/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2018-08-04 16:35+0200\n" -"PO-Revision-Date: 2018-08-04 16:37+0200\n" +"POT-Creation-Date: 2018-09-15 17:57+0200\n" +"PO-Revision-Date: 2018-09-15 18:02+0200\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -80,6 +80,41 @@ msgstr "" msgid "Secret key: \"%s\"" msgstr "Clave secreta: \"%s\"" +msgid "Deletes a short URL" +msgstr "Elimina una URL" + +msgid "The short code to be deleted" +msgstr "El código corto a eliminar" + +msgid "" +"Ignores the safety visits threshold check, which could make short URLs with " +"many visits to be accidentally deleted" +msgstr "" +"Ignora el límite de seguridad de visitas, pudiendo resultar en el borrado " +"accidental de URLs con muchas visitas" + +#, php-format +msgid "Provided short code \"%s\" could not be found." +msgstr "El código corto proporcionado \"%s\" no ha podido ser encontrado." + +#, php-format +msgid "" +"It was not possible to delete the short URL with short code \"%s\" because " +"it has more than %s visits." +msgstr "" +"No se pudo eliminar la URL acortada con código corto \"%s\" porque tiene más " +"de %s visitas." + +msgid "Do you want to delete it anyway?" +msgstr "¿Aún así quieres eliminarla?" + +msgid "Short URL was not deleted." +msgstr "La URL corta no ha sido eliminada." + +#, php-format +msgid "Short URL with short code \"%s\" successfully deleted." +msgstr "La URL acortada con el código corto \"%s\" eliminada correctamente." + msgid "" "Processes and generates the previews for every URL, improving performance " "for later web requests." @@ -183,12 +218,12 @@ msgstr "Origen" msgid "Date" msgstr "Fecha" -msgid "Remote Address" -msgstr "Dirección remota" - msgid "User agent" msgstr "Agente de usuario" +msgid "Country" +msgstr "País" + msgid "List all short URLs" msgstr "Listar todas las URLs cortas" @@ -218,8 +253,11 @@ msgstr "Si se desea mostrar las etiquetas o no" msgid "Short code" msgstr "Código corto" -msgid "Original URL" -msgstr "URL original" +msgid "Short URL" +msgstr "URL corta" + +msgid "Long URL" +msgstr "URL larga" msgid "Date created" msgstr "Fecha de creación" @@ -253,10 +291,6 @@ msgstr "URL larga:" msgid "Provided short code \"%s\" has an invalid format." msgstr "El código corto proporcionado \"%s\" tiene un formato inválido." -#, php-format -msgid "Provided short code \"%s\" could not be found." -msgstr "El código corto proporcionado \"%s\" no ha podido ser encontrado." - msgid "Creates one or more tags." msgstr "Crea una o más etiquetas." @@ -327,6 +361,12 @@ msgstr "Limite del localizador de IPs alcanzado. Esperando %s segundos..." msgid "Finished processing all IPs" msgstr "Finalizado el procesado de todas las IPs" +#~ msgid "Remote Address" +#~ msgstr "Dirección remota" + +#~ msgid "Original URL" +#~ msgstr "URL original" + #~ msgid "You have reached last page" #~ msgstr "Has alcanzado la última página" diff --git a/module/Rest/lang/es.mo b/module/Rest/lang/es.mo index 95fbf8b6f194f05a258e938c22a1d8e1a3ee9dc2..5c1a1db85e453930f41c9653edbbab26355f550d 100644 GIT binary patch delta 675 zcmYk(y=xRf7{~GF%*AMYiAGQlG(H3ZMo2Vh@Qf&)#x7Mr5EOB;JGd)*J8@@EC54BD z*rf91571Urp>h zN-xwHs4sXBzv40ci4kt#NxZpDWEEHO1U3dl6z6doEzaT#oX2n2z>#|Ry(^PkOz_}3 zUcg6Kz3>Sy;}5)njlupz9sEQ18yw^Lvmucw{D203V-s6DL_XsRp2y}+kvS~!EWXE< zbVb&=r~)R3MULYWY~eesCjN_8aco!rgkoIe{xRnG9navE-6E_@AkO3N{m}`g_>0uo+=J!YPK@^Zae?;_cLH?n+&D-lt2M)zWEf zGh14{wz!}V(lSw>xT4g^#a72B{J1XJ$aK8bwA93SEv$(W8*AGYe4d!R|JMD~r=_3n zt=FCo44<2tnbn!*+^N&OhlBrWx2&%cL!iylcAA?)GZP{sm#fV*TI9Y<`zl0xpX eEADkHnQF4gxHJ;>oKJRny<$dpc?+~FAB;|xCH zB7Wd0W@FMkw$R5XT);03(2q-1+{X#LL^J<@vrPa%nkG|G`(L>j8=ONbkR!H@N?&hmeg_8^hY{Z;F3HBw edDqzt4`a=^7vu}8{?c-JB?vzg-%eOf7TrGuv@2); diff --git a/module/Rest/lang/es.po b/module/Rest/lang/es.po index 162fb0077..61f492070 100644 --- a/module/Rest/lang/es.po +++ b/module/Rest/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2018-05-06 12:34+0200\n" -"PO-Revision-Date: 2018-05-06 12:35+0200\n" +"POT-Creation-Date: 2018-09-15 18:02+0200\n" +"PO-Revision-Date: 2018-09-15 18:03+0200\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -43,6 +43,14 @@ msgstr "No se ha proporcionado una URL" msgid "No URL found for short code \"%s\"" msgstr "No se ha encontrado ninguna URL para el código corto \"%s\"" +#, php-format +msgid "" +"It is not possible to delete URL with short code \"%s\" because it has " +"reached more than \"%s\" visits." +msgstr "" +"No es posible eliminar la URL con el código corto \"%s\" porque ha alcanzado " +"más de \"%s\" visitas." + msgid "Provided data is invalid." msgstr "Los datos proporcionados son inválidos."