Merge pull request #227 from acelaya/feature/visits-threshold-config

Feature/visits threshold config

CHANGELOG.md

@@ -9,10 +9,20 @@
 
     It also allows automating the dist file generation in travis-ci builds.
 
+* [#207](https://github.com/shlinkio/shlink/issues/207) Added two new config options which are asked during installation process. The config options already existed in previous shlink version, but you had to manually set their values.
+
+    These are the new options:
+
+    * Visits threshold to allow short URLs to be deleted.
+    * Check the visits threshold when trying to delete a short URL via REST API.
+
 #### Changed
 
 * [#211](https://github.com/shlinkio/shlink/issues/211) Extracted installer to its own module, which will simplify moving it to a separated package in the future.
 * [#200](https://github.com/shlinkio/shlink/issues/200) and [#201](https://github.com/shlinkio/shlink/issues/201) Renamed REST Action classes and CLI Command classes to use the concept of `ShortUrl` instead of the concept of `ShortCode` when referring to the entity, and left the `short code` concept to the identifier which is used as a unique code for a specific `Short URL`.
+* [#181](https://github.com/shlinkio/shlink/issues/181) When importing the configuration from a previous shlink installation, it no longer asks to import every block. Instead, it is capable of detecting only new config options introduced in the new version, and ask only for those.
+
+    If no new options are found and you have selected to import config, no further questions will be asked and shlink will just import the old config.
 
 #### Deprecated
 

module/Installer/src/Config/Plugin/ApplicationConfigCustomizer.php

@@ -4,10 +4,13 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Installer\Config\Plugin;
 
 use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
+use Shlinkio\Shlink\Installer\Exception\InvalidConfigOptionException;
 use Shlinkio\Shlink\Installer\Model\CustomizableAppConfig;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use function array_diff;
 use function array_keys;
+use function is_numeric;
+use function sprintf;
 
 class ApplicationConfigCustomizer implements ConfigCustomizerInterface
 {
@@ -15,9 +18,13 @@ class ApplicationConfigCustomizer implements ConfigCustomizerInterface
 
     public const SECRET = 'SECRET';
     public const DISABLE_TRACK_PARAM = 'DISABLE_TRACK_PARAM';
+    public const CHECK_VISITS_THRESHOLD = 'CHECK_VISITS_THRESHOLD';
+    public const VISITS_THRESHOLD = 'VISITS_THRESHOLD';
     private const EXPECTED_KEYS = [
         self::SECRET,
         self::DISABLE_TRACK_PARAM,
+        self::CHECK_VISITS_THRESHOLD,
+        self::VISITS_THRESHOLD,
     ];
 
     public function process(SymfonyStyle $io, CustomizableAppConfig $appConfig): void
@@ -31,6 +38,11 @@ class ApplicationConfigCustomizer implements ConfigCustomizerInterface
 
         $io->title('APPLICATION');
         foreach ($keysToAskFor as $key) {
+            // Skip visits threshold when the user decided not to check visits on deletions
+            if ($key === self::VISITS_THRESHOLD && ! $app[self::CHECK_VISITS_THRESHOLD]) {
+                continue;
+            }
+
             $app[$key] = $this->ask($io, $key);
         }
         $appConfig->setApp($app);
@@ -49,8 +61,30 @@ class ApplicationConfigCustomizer implements ConfigCustomizerInterface
                     'Provide a parameter name that you will be able to use to disable tracking on specific request to '
                     . 'short URLs (leave empty and this feature won\'t be enabled)'
                 );
+            case self::CHECK_VISITS_THRESHOLD:
+                return $io->confirm(
+                    'Do you want to enable a safety check which will not allow short URLs to be deleted when they '
+                    . 'have more than a specific amount of visits?'
+                );
+            case self::VISITS_THRESHOLD:
+                return $io->ask(
+                    'What is the amount of visits from which the system will not allow short URLs to be deleted?',
+                    15,
+                    [$this, 'validateVisitsThreshold']
+                );
         }
 
         return '';
     }
+
+    public function validateVisitsThreshold($value): int
+    {
+        if (! is_numeric($value) || $value < 1) {
+            throw new InvalidConfigOptionException(
+                sprintf('Provided value "%s" is invalid. Expected a number greater than 1', $value)
+            );
+        }
+
+        return (int) $value;
+    }
 }

module/Installer/src/Exception/InvalidConfigOptionException.php

@@ -0,0 +1,10 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Installer\Exception;
+
+use RuntimeException;
+
+class InvalidConfigOptionException extends RuntimeException implements ExceptionInterface
+{
+}

module/Installer/src/Model/CustomizableAppConfig.php

@@ -121,6 +121,8 @@ final class CustomizableAppConfig implements ArraySerializableInterface
         $this->setApp($this->mapExistingPathsToKeys([
             ApplicationConfigCustomizer::SECRET => ['app_options', 'secret_key'],
             ApplicationConfigCustomizer::DISABLE_TRACK_PARAM => ['app_options', 'disable_track_param'],
+            ApplicationConfigCustomizer::CHECK_VISITS_THRESHOLD => ['delete_short_urls', 'check_visits_threshold'],
+            ApplicationConfigCustomizer::VISITS_THRESHOLD => ['delete_short_urls', 'visits_threshold'],
         ], $array));
 
         $this->setDatabase($this->mapExistingPathsToKeys([
@@ -165,6 +167,10 @@ final class CustomizableAppConfig implements ArraySerializableInterface
                 'secret_key' => $this->app[ApplicationConfigCustomizer::SECRET] ?? '',
                 'disable_track_param' => $this->app[ApplicationConfigCustomizer::DISABLE_TRACK_PARAM] ?? null,
             ],
+            'delete_short_urls' => [
+                'check_visits_threshold' => $this->app[ApplicationConfigCustomizer::CHECK_VISITS_THRESHOLD] ?? true,
+                'visits_threshold' => $this->app[ApplicationConfigCustomizer::VISITS_THRESHOLD] ?? 15,
+            ],
             'entity_manager' => [
                 'connection' => [
                     'driver' => $dbDriver,

module/Installer/test/Config/Plugin/ApplicationConfigCustomizerTest.php

@@ -7,8 +7,11 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Installer\Config\Plugin\ApplicationConfigCustomizer;
+use Shlinkio\Shlink\Installer\Exception\InvalidConfigOptionException;
 use Shlinkio\Shlink\Installer\Model\CustomizableAppConfig;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use function array_shift;
+use function strpos;
 
 class ApplicationConfigCustomizerTest extends TestCase
 {
@@ -34,17 +37,47 @@ class ApplicationConfigCustomizerTest extends TestCase
      */
     public function configIsRequestedToTheUser()
     {
-        $ask = $this->io->ask(Argument::cetera())->willReturn('the_secret');
+        $ask = $this->io->ask(Argument::cetera())->willReturn('asked');
+        $confirm = $this->io->confirm(Argument::cetera())->willReturn(false);
+
         $config = new CustomizableAppConfig();
 
         $this->plugin->process($this->io->reveal(), $config);
 
         $this->assertTrue($config->hasApp());
         $this->assertEquals([
-            'SECRET' => 'the_secret',
+            'SECRET' => 'asked',
-            'DISABLE_TRACK_PARAM' => 'the_secret',
+            'DISABLE_TRACK_PARAM' => 'asked',
+            'CHECK_VISITS_THRESHOLD' => false,
         ], $config->getApp());
         $ask->shouldHaveBeenCalledTimes(2);
+        $confirm->shouldHaveBeenCalledTimes(1);
+    }
+
+    /**
+     * @test
+     */
+    public function visitsThresholdIsRequestedIfCheckIsEnabled()
+    {
+        $ask = $this->io->ask(Argument::cetera())->will(function (array $args) {
+            $message = array_shift($args);
+            return strpos($message, 'What is the amount of visits') === 0 ? 20 : 'asked';
+        });
+        $confirm = $this->io->confirm(Argument::cetera())->willReturn(true);
+
+        $config = new CustomizableAppConfig();
+
+        $this->plugin->process($this->io->reveal(), $config);
+
+        $this->assertTrue($config->hasApp());
+        $this->assertEquals([
+            'SECRET' => 'asked',
+            'DISABLE_TRACK_PARAM' => 'asked',
+            'CHECK_VISITS_THRESHOLD' => true,
+            'VISITS_THRESHOLD' => 20,
+        ], $config->getApp());
+        $ask->shouldHaveBeenCalledTimes(3);
+        $confirm->shouldHaveBeenCalledTimes(1);
     }
 
     /**
@@ -56,6 +89,8 @@ class ApplicationConfigCustomizerTest extends TestCase
         $config = new CustomizableAppConfig();
         $config->setApp([
             'SECRET' => 'foo',
+            'CHECK_VISITS_THRESHOLD' => true,
+            'VISITS_THRESHOLD' => 20,
         ]);
 
         $this->plugin->process($this->io->reveal(), $config);
@@ -63,6 +98,8 @@ class ApplicationConfigCustomizerTest extends TestCase
         $this->assertEquals([
             'SECRET' => 'foo',
             'DISABLE_TRACK_PARAM' => 'disable_param',
+            'CHECK_VISITS_THRESHOLD' => true,
+            'VISITS_THRESHOLD' => 20,
         ], $config->getApp());
         $ask->shouldHaveBeenCalledTimes(1);
     }
@@ -78,6 +115,8 @@ class ApplicationConfigCustomizerTest extends TestCase
         $config->setApp([
             'SECRET' => 'foo',
             'DISABLE_TRACK_PARAM' => 'the_new_secret',
+            'CHECK_VISITS_THRESHOLD' => true,
+            'VISITS_THRESHOLD' => 20,
         ]);
 
         $this->plugin->process($this->io->reveal(), $config);
@@ -85,7 +124,52 @@ class ApplicationConfigCustomizerTest extends TestCase
         $this->assertEquals([
             'SECRET' => 'foo',
             'DISABLE_TRACK_PARAM' => 'the_new_secret',
+            'CHECK_VISITS_THRESHOLD' => true,
+            'VISITS_THRESHOLD' => 20,
         ], $config->getApp());
         $ask->shouldNotHaveBeenCalled();
     }
+
+    /**
+     * @test
+     * @dataProvider provideInvalidValues
+     * @param mixed $value
+     */
+    public function validateVisitsThresholdThrowsExceptionWhenProvidedValueIsInvalid($value)
+    {
+        $this->expectException(InvalidConfigOptionException::class);
+        $this->plugin->validateVisitsThreshold($value);
+    }
+
+    public function provideInvalidValues(): array
+    {
+        return [
+            'string' => ['foo'],
+            'empty string' => [''],
+            'negative number' => [-5],
+            'negative number as string' => ['-5'],
+            'zero' => [0],
+            'zero as string' => ['0'],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider provideValidValues
+     * @param mixed $value
+     */
+    public function validateVisitsThresholdCastsToIntWhenProvidedValueIsValid($value, int $expected)
+    {
+        $this->assertEquals($expected, $this->plugin->validateVisitsThreshold($value));
+    }
+
+    public function provideValidValues(): array
+    {
+        return [
+            'positive as string' => ['20', 20],
+            'positive as integer' => [5, 5],
+            'one as string' => ['1', 1],
+            'one as integer' => [1, 1],
+        ];
+    }
 }