Merge pull request #345 from acelaya/bugfix/charset-installation

Bugfix/charset installation

CHANGELOG.md

@@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Changed
 
+* [#342](https://github.com/shlinkio/shlink/issues/342) The installer no longer asks for a charset to be provided, and instead, it shuffles the base62 charset.
 * [#320](https://github.com/shlinkio/shlink/issues/320) Replaced query builder by plain DQL for all queries which do not need to be dynamically generated.
 * [#330](https://github.com/shlinkio/shlink/issues/330) No longer allow failures on PHP 7.3 envs during project CI build.
 * [#335](https://github.com/shlinkio/shlink/issues/335) Renamed functional test suite to database test suite, since that better describes what it actually does.

config/install-container.php

@@ -1,7 +1,8 @@
 <?php
 declare(strict_types=1);
 
-use Shlinkio\Shlink\Installer\Config\Plugin\DatabaseConfigCustomizer;
+use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
+use Shlinkio\Shlink\Installer\Config\Plugin;
 use Shlinkio\Shlink\Installer\Factory\InstallApplicationFactory;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Filesystem\Filesystem;
@@ -13,17 +14,35 @@ chdir(dirname(__DIR__));
 
 require __DIR__ . '/../vendor/autoload.php';
 
-$container = new ServiceManager([
+$config = [
-    'factories' => [
+    'dependencies' => [
-        Application::class => InstallApplicationFactory::class,
+        'factories' => [
-        Filesystem::class => InvokableFactory::class,
+            Application::class => InstallApplicationFactory::class,
-    ],
+            Filesystem::class => InvokableFactory::class,
-    'services' => [
+        ],
-        'config' => [
+        'services' => [
-            ConfigAbstractFactory::class => [
+            'random-chars-generator' => function () {
-                DatabaseConfigCustomizer::class => [Filesystem::class],
+                return str_shuffle(UrlShortenerOptions::DEFAULT_CHARS);
-            ],
+            },
         ],
     ],
-]);
+
+    'config_customizer_plugins' => [
+        'factories' => [
+            Plugin\DatabaseConfigCustomizer::class => ConfigAbstractFactory::class,
+            Plugin\UrlShortenerConfigCustomizer::class => ConfigAbstractFactory::class,
+            Plugin\LanguageConfigCustomizer::class => InvokableFactory::class,
+            Plugin\ApplicationConfigCustomizer::class => InvokableFactory::class,
+        ],
+    ],
+
+    ConfigAbstractFactory::class => [
+        Plugin\DatabaseConfigCustomizer::class => [Filesystem::class],
+        Plugin\UrlShortenerConfigCustomizer::class => ['random-chars-generator'],
+    ],
+];
+
+$container = new ServiceManager($config['dependencies']);
+$container->setService('config', $config);
+
 return $container;

module/Core/src/Options/UrlShortenerOptions.php

@@ -7,7 +7,7 @@ use Zend\Stdlib\AbstractOptions;
 
 class UrlShortenerOptions extends AbstractOptions
 {
-    public const DEFAULT_CHARS = '123456789bcdfghjkmnpqrstvwxyzBCDFGHJKLMNPQRSTVWXYZ';
+    public const DEFAULT_CHARS = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
 
     // phpcs:disable
     protected $__strictMode__ = false;

module/Core/test/Service/UrlShortenerTest.php

@@ -71,13 +71,13 @@ class UrlShortenerTest extends TestCase
      */
     public function urlIsProperlyShortened(): void
     {
-        // 10 -> 12C1c
+        // 10 -> 0Q1Y
         $shortUrl = $this->urlShortener->urlToShortCode(
             new Uri('http://foobar.com/12345/hello?foo=bar'),
             [],
             ShortUrlMeta::createEmpty()
         );
-        $this->assertEquals('12C1c', $shortUrl->getShortCode());
+        $this->assertEquals('0Q1Y', $shortUrl->getShortCode());
     }
 
     /**
@@ -215,7 +215,6 @@ class UrlShortenerTest extends TestCase
      */
     public function shortCodeIsProperlyParsed(): void
     {
-        // 12C1c -> 10
         $shortCode = '12C1c';
         $shortUrl = new ShortUrl('expected_url');
         $shortUrl->setShortCode($shortCode);

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

@@ -3,13 +3,13 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Installer\Config\Plugin;
 
-use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Installer\Model\CustomizableAppConfig;
 use Shlinkio\Shlink\Installer\Util\AskUtilsTrait;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use function array_diff;
 use function array_keys;
-use function str_shuffle;
+use function count;
+use function Functional\contains;
 
 class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
 {
@@ -30,6 +30,14 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
         self::NOT_FOUND_REDIRECT_TO,
     ];
 
+    /** @var callable */
+    private $randomCharsGenerator;
+
+    public function __construct(callable $randomCharsGenerator)
+    {
+        $this->randomCharsGenerator = $randomCharsGenerator;
+    }
+
     public function process(SymfonyStyle $io, CustomizableAppConfig $appConfig): void
     {
         $urlShortener = $appConfig->getUrlShortener();
@@ -40,7 +48,11 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
             return;
         }
 
-        $io->title('URL SHORTENER');
+        // Print title if there are keys other than "chars"
+        $onlyKeyIsChars = count($keysToAskFor) === 1 && contains($keysToAskFor, self::CHARS);
+        if (! $onlyKeyIsChars) {
+            $io->title('URL SHORTENER');
+        }
         foreach ($keysToAskFor as $key) {
             // Skip not found redirect URL when the user decided not to redirect
             if ($key === self::NOT_FOUND_REDIRECT_TO && ! $urlShortener[self::ENABLE_NOT_FOUND_REDIRECTION]) {
@@ -64,9 +76,8 @@ class UrlShortenerConfigCustomizer implements ConfigCustomizerInterface
             case self::HOSTNAME:
                 return $this->askRequired($io, 'hostname', 'Hostname for generated URLs');
             case self::CHARS:
-                return $io->ask(
+                // This won't actually ask anything, just generate the chars. Asking for this was confusing for users
-                    'Character set for generated short codes (leave empty to autogenerate one)'
+                return ($this->randomCharsGenerator)();
-                ) ?: str_shuffle(UrlShortenerOptions::DEFAULT_CHARS);
             case self::VALIDATE_URL:
                 return $io->confirm('Do you want to validate long urls by 200 HTTP status code on response');
             case self::ENABLE_NOT_FOUND_REDIRECTION:

module/Installer/src/Factory/InstallApplicationFactory.php

@@ -7,16 +7,13 @@ use Interop\Container\ContainerInterface;
 use Interop\Container\Exception\ContainerException;
 use Shlinkio\Shlink\Installer\Command\InstallCommand;
 use Shlinkio\Shlink\Installer\Config\ConfigCustomizerManager;
-use Shlinkio\Shlink\Installer\Config\Plugin;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Exception\LogicException;
 use Symfony\Component\Filesystem\Filesystem;
 use Zend\Config\Writer\PhpArray;
-use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Zend\ServiceManager\Exception\ServiceNotCreatedException;
 use Zend\ServiceManager\Exception\ServiceNotFoundException;
 use Zend\ServiceManager\Factory\FactoryInterface;
-use Zend\ServiceManager\Factory\InvokableFactory;
 
 class InstallApplicationFactory implements FactoryInterface
 {
@@ -41,12 +38,7 @@ class InstallApplicationFactory implements FactoryInterface
         $command = new InstallCommand(
             new PhpArray(),
             $container->get(Filesystem::class),
-            new ConfigCustomizerManager($container, ['factories' => [
+            new ConfigCustomizerManager($container, $container->get('config')['config_customizer_plugins']),
-                Plugin\DatabaseConfigCustomizer::class => ConfigAbstractFactory::class,
-                Plugin\UrlShortenerConfigCustomizer::class => InvokableFactory::class,
-                Plugin\LanguageConfigCustomizer::class => InvokableFactory::class,
-                Plugin\ApplicationConfigCustomizer::class => InvokableFactory::class,
-            ]]),
             $isUpdate
         );
         $app->add($command);

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

@@ -21,7 +21,9 @@ class UrlShortenerConfigCustomizerTest extends TestCase
     {
         $this->io = $this->prophesize(SymfonyStyle::class);
         $this->io->title(Argument::any())->willReturn(null);
-        $this->plugin = new UrlShortenerConfigCustomizer();
+        $this->plugin = new UrlShortenerConfigCustomizer(function () {
+            return 'the_chars';
+        });
     }
 
     /**
@@ -40,12 +42,12 @@ class UrlShortenerConfigCustomizerTest extends TestCase
         $this->assertEquals([
             'SCHEMA' => 'chosen',
             'HOSTNAME' => 'asked',
-            'CHARS' => 'asked',
+            'CHARS' => 'the_chars',
             'VALIDATE_URL' => true,
             'ENABLE_NOT_FOUND_REDIRECTION' => true,
             'NOT_FOUND_REDIRECT_TO' => 'asked',
         ], $config->getUrlShortener());
-        $ask->shouldHaveBeenCalledTimes(3);
+        $ask->shouldHaveBeenCalledTimes(2);
         $choice->shouldHaveBeenCalledOnce();
         $confirm->shouldHaveBeenCalledTimes(2);
     }
@@ -61,7 +63,6 @@ class UrlShortenerConfigCustomizerTest extends TestCase
         $config = new CustomizableAppConfig();
         $config->setUrlShortener([
             'SCHEMA' => 'foo',
-            'HOSTNAME' => 'foo',
             'ENABLE_NOT_FOUND_REDIRECTION' => true,
             'NOT_FOUND_REDIRECT_TO' => 'foo',
         ]);
@@ -70,8 +71,8 @@ class UrlShortenerConfigCustomizerTest extends TestCase
 
         $this->assertEquals([
             'SCHEMA' => 'foo',
-            'HOSTNAME' => 'foo',
+            'HOSTNAME' => 'asked',
-            'CHARS' => 'asked',
+            'CHARS' => 'the_chars',
             'VALIDATE_URL' => false,
             'ENABLE_NOT_FOUND_REDIRECTION' => true,
             'NOT_FOUND_REDIRECT_TO' => 'foo',

module/Installer/test/Factory/InstallApplicationFactoryTest.php

@@ -14,7 +14,7 @@ class InstallApplicationFactoryTest extends TestCase
     /** @var InstallApplicationFactory */
     private $factory;
 
-    public function setUp()
+    public function setUp(): void
     {
         $this->factory = new InstallApplicationFactory();
     }
@@ -22,10 +22,11 @@ class InstallApplicationFactoryTest extends TestCase
     /**
      * @test
      */
-    public function serviceIsCreated()
+    public function serviceIsCreated(): void
     {
-        $instance = $this->factory->__invoke(new ServiceManager(['services' => [
+        $instance = ($this->factory)(new ServiceManager(['services' => [
             Filesystem::class => $this->prophesize(Filesystem::class)->reveal(),
+            'config' => ['config_customizer_plugins' => []],
         ]]), '');
 
         $this->assertInstanceOf(Application::class, $instance);