minor #2352 [Icons] Fetch icons in batch in Import command (smnandre)

smnandre · smnandre · commit 3a1ae4e11897 · 2024-11-10T01:32:41.000+01:00
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Icons] Fetch icons in batch in Import command | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | Fix #... | License | MIT First usage of #2351 with the import command --- **Update:** - add icons counts (imported / total) - handle same-prefix icons per batch - improve visual feedback <img width="878" alt="Capture d’écran 2024-11-09 à 19 26 16" src="https://github.com/user-attachments/assets/fc6c377e-ba40-43f3-b62a-0070ba14125b"> --- Next ones: * lock * warm-up Commits ------- 88d24e0 [Icons] Fetch icons in batch in Import command
diff --git a/src/Icons/src/Command/ImportIconCommand.php b/src/Icons/src/Command/ImportIconCommand.php
@@ -18,7 +18,6 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
-use Symfony\UX\Icons\Exception\IconNotFoundException;
 use Symfony\UX\Icons\Iconify;
 use Symfony\UX\Icons\Registry\LocalSvgIconRegistry;
 
@@ -41,11 +40,7 @@ public function __construct(private Iconify $iconify, private LocalSvgIconRegist
     protected function configure(): void
     {
         $this
-            ->addArgument(
-                'names',
-                InputArgument::IS_ARRAY | InputArgument::REQUIRED,
-                'Icon name from ux.symfony.com/icons (e.g. "mdi:home")',
-            )
+            ->addArgument('names', InputArgument::IS_ARRAY | InputArgument::REQUIRED, 'Icon name from ux.symfony.com/icons (e.g. "mdi:home")')
         ;
     }
 
@@ -54,7 +49,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $io = new SymfonyStyle($input, $output);
         $names = $input->getArgument('names');
         $result = Command::SUCCESS;
+        $importedIcons = 0;
 
+        $prefixIcons = [];
         foreach ($names as $name) {
             if (!preg_match('#^([\w-]+):([\w-]+)$#', $name, $matches)) {
                 $io->error(\sprintf('Invalid icon name "%s".', $name));
@@ -63,35 +60,63 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 continue;
             }
 
-            [$fullName, $prefix, $name] = $matches;
-
-            $io->comment(\sprintf('Importing %s...', $fullName));
+            [, $prefix, $name] = $matches;
+            $prefixIcons[$prefix] ??= [];
+            $prefixIcons[$prefix][$name] = $name;
+        }
 
-            try {
-                $iconSvg = $this->iconify->fetchIcon($prefix, $name)->toHtml();
-            } catch (IconNotFoundException $e) {
-                $io->error($e->getMessage());
+        foreach ($prefixIcons as $prefix => $icons) {
+            if (!$this->iconify->hasIconSet($prefix)) {
+                $io->error(\sprintf('Icon set "%s" not found.', $prefix));
                 $result = Command::FAILURE;
 
                 continue;
             }
 
-            $cursor = new Cursor($output);
-            $cursor->moveUp(2);
-
-            $this->registry->add(\sprintf('%s/%s', $prefix, $name), $iconSvg);
-
-            $license = $this->iconify->metadataFor($prefix)['license'];
-
-            $io->text(\sprintf(
-                " <fg=bright-green;options=bold>✓</> Imported <fg=bright-white;bg=black>%s:</><fg=bright-magenta;bg=black;options>%s</> (License: <href=%s>%s</>). Render with: <comment>{{ ux_icon('%s') }}</comment>",
-                $prefix,
-                $name,
-                $license['url'] ?? '#',
-                $license['title'],
-                $fullName,
-            ));
+            $metadata = $this->iconify->metadataFor($prefix);
             $io->newLine();
+            $io->writeln(\sprintf(' Icon set: %s (License: %s)', $metadata['name'], $metadata['license']['title']));
+
+            foreach ($this->iconify->chunk($prefix, array_keys($icons)) as $iconNames) {
+                $cursor = new Cursor($output);
+                foreach ($iconNames as $name) {
+                    $io->writeln(\sprintf('  Importing %s:%s ...', $prefix, $name));
+                }
+                $cursor->moveUp(\count($iconNames));
+
+                try {
+                    $batchResults = $this->iconify->fetchIcons($prefix, $iconNames);
+                } catch (\InvalidArgumentException $e) {
+                    // At this point no exception should be thrown
+                    $io->error($e->getMessage());
+
+                    return Command::FAILURE;
+                }
+
+                foreach ($iconNames as $name) {
+                    $cursor->clearLineAfter();
+
+                    // If the icon is not found, the value will be null
+                    if (null === $icon = $batchResults[$name] ?? null) {
+                        $io->writeln(\sprintf(' <fg=red;options=bold>✗</> Not Found <fg=bright-white;bg=black>%s:</><fg=bright-red;bg=black>%s</>', $prefix, $name));
+
+                        continue;
+                    }
+
+                    ++$importedIcons;
+                    $this->registry->add(\sprintf('%s/%s', $prefix, $name), (string) $icon);
+                    $io->writeln(\sprintf(' <fg=bright-green;options=bold>✓</> Imported <fg=bright-white;bg=black>%s:</><fg=bright-magenta;bg=black;options>%s</>', $prefix, $name));
+                }
+            }
+        }
+
+        if ($importedIcons === $totalIcons = \count($names)) {
+            $io->success(\sprintf('Imported %d/%d icons.', $importedIcons, $totalIcons));
+        } elseif ($importedIcons > 0) {
+            $io->warning(\sprintf('Imported %d/%d icons.', $importedIcons, $totalIcons));
+        } else {
+            $io->error(\sprintf('Imported %d/%d icons.', $importedIcons, $totalIcons));
+            $result = Command::FAILURE;
         }
 
         return $result;
diff --git a/src/Icons/src/Iconify.php b/src/Icons/src/Iconify.php
@@ -27,19 +27,27 @@ final class Iconify
 {
     public const API_ENDPOINT = 'https://api.iconify.design';
 
+    // URL must be 500 chars max (iconify limit)
+    // -39 chars: https://api.iconify.design/XXX.json?icons=
+    // -safe margin
+    private const MAX_ICONS_QUERY_LENGTH = 400;
+
     private HttpClientInterface $http;
     private \ArrayObject $sets;
+    private int $maxIconsQueryLength;
 
     public function __construct(
         private CacheInterface $cache,
         string $endpoint = self::API_ENDPOINT,
         ?HttpClientInterface $http = null,
+        ?int $maxIconsQueryLength = null,
     ) {
         if (!class_exists(HttpClient::class)) {
             throw new \LogicException('You must install "symfony/http-client" to use Iconify. Try running "composer require symfony/http-client".');
         }
 
         $this->http = ScopingHttpClient::forBaseUri($http ?? HttpClient::create(), $endpoint);
+        $this->maxIconsQueryLength = min(self::MAX_ICONS_QUERY_LENGTH, $maxIconsQueryLength ?? self::MAX_ICONS_QUERY_LENGTH);
     }
 
     public function metadataFor(string $prefix): array
@@ -95,13 +103,10 @@ public function fetchIcons(string $prefix, array $names): array
         sort($names);
         $queryString = implode(',', $names);
         if (!preg_match('#^[a-z0-9-,]+$#', $queryString)) {
-            throw new \InvalidArgumentException('Invalid icon names.');
+            throw new \InvalidArgumentException('Invalid icon names.'.$queryString);
         }
 
-        // URL must be 500 chars max (iconify limit)
-        // -39 chars: https://api.iconify.design/XXX.json?icons=
-        // -safe margin
-        if (450 < \strlen($prefix.$queryString)) {
+        if (self::MAX_ICONS_QUERY_LENGTH < \strlen($prefix.$queryString)) {
             throw new \InvalidArgumentException('The query string is too long.');
         }
 
@@ -155,6 +160,40 @@ public function searchIcons(string $prefix, string $query)
         return new \ArrayObject($response->toArray());
     }
 
+    /**
+     * @return iterable<string[]>
+     */
+    public function chunk(string $prefix, array $names): iterable
+    {
+        if (100 < ($prefixLength = \strlen($prefix))) {
+            throw new \InvalidArgumentException(\sprintf('The icon prefix "%s" is too long.', $prefix));
+        }
+
+        $maxLength = $this->maxIconsQueryLength - $prefixLength;
+
+        $curBatch = [];
+        $curLength = 0;
+        foreach ($names as $name) {
+            if (100 < ($nameLength = \strlen($name))) {
+                throw new \InvalidArgumentException(\sprintf('The icon name "%s" is too long.', $name));
+            }
+            if ($curLength && ($maxLength < ($curLength + $nameLength + 1))) {
+                yield $curBatch;
+
+                $curBatch = [];
+                $curLength = 0;
+            }
+            $curLength += $nameLength + 1;
+            $curBatch[] = $name;
+        }
+
+        if ($curLength) {
+            yield $curBatch;
+        }
+
+        yield from [];
+    }
+
     private function sets(): \ArrayObject
     {
         return $this->sets ??= $this->cache->get('ux-iconify-sets', function () {
diff --git a/src/Icons/tests/Integration/Command/ImportIconCommandTest.php b/src/Icons/tests/Integration/Command/ImportIconCommandTest.php
@@ -23,7 +23,7 @@ final class ImportIconCommandTest extends KernelTestCase
     use InteractsWithConsole;
 
     private const ICON_DIR = __DIR__.'/../../Fixtures/icons';
-    private const ICONS = ['uiw/dashboard.svg'];
+    private const ICONS = ['uiw/dashboard.svg', 'lucide/circle.svg'];
 
     /**
      * @before
@@ -45,8 +45,8 @@ public function testCanImportIcon(): void
 
         $this->executeConsoleCommand('ux:icons:import uiw:dashboard')
             ->assertSuccessful()
+            ->assertOutputContains('Icon set: uiw icons (License: MIT)')
             ->assertOutputContains('Importing uiw:dashboard')
-            ->assertOutputContains("Imported uiw:dashboard (License: MIT). Render with: {{ ux_icon('uiw:dashboard') }}")
         ;
 
         $this->assertFileExists($expectedFile);
@@ -60,13 +60,34 @@ public function testImportInvalidIconName(): void
         ;
     }
 
-    public function testImportNonExistentIcon(): void
+    public function testImportNonExistentIconSet(): void
     {
         $this->executeConsoleCommand('ux:icons:import something:invalid')
             ->assertStatusCode(1)
-            ->assertOutputContains('[ERROR] The icon "something:invalid" does not exist on iconify.design.')
+            ->assertOutputContains('[ERROR] Icon set "something" not found.')
+        ;
+    }
+
+    public function testImportNonExistentIcon(): void
+    {
+        $this->executeConsoleCommand('ux:icons:import lucide:not-existing-icon')
+            ->assertStatusCode(1)
+            ->assertOutputContains('Not Found lucide:not-existing-icon')
+            ->assertOutputContains('[ERROR] Imported 0/1 icons.')
+        ;
+
+        $this->assertFileDoesNotExist(self::ICON_DIR.'/not-existing-icon.svg');
+    }
+
+    public function testImportNonExistentIconWithExistentOne(): void
+    {
+        $this->executeConsoleCommand('ux:icons:import lucide:circle lucide:not-existing-icon')
+            ->assertStatusCode(0)
+            ->assertOutputContains('Imported lucide:circle')
+            ->assertOutputContains('Not Found lucide:not-existing-icon')
+            ->assertOutputContains('[WARNING] Imported 1/2 icons.')
         ;
 
-        $this->assertFileDoesNotExist(self::ICON_DIR.'/invalid.svg');
+        $this->assertFileDoesNotExist(self::ICON_DIR.'/not-existing-icon.svg');
     }
 }
diff --git a/src/Icons/tests/Unit/IconifyTest.php b/src/Icons/tests/Unit/IconifyTest.php
@@ -238,6 +238,85 @@ public function testGetMetadata(): void
         $this->assertSame('Font Awesome Solid', $metadata['name']);
     }
 
+    /**
+     * @dataProvider provideChunkCases
+     */
+    public function testChunk(int $maxQueryLength, string $prefix, array $names, array $chunks): void
+    {
+        $iconify = new Iconify(
+            new NullAdapter(),
+            'https://example.com',
+            new MockHttpClient([]),
+            $maxQueryLength,
+        );
+
+        $this->assertSame($chunks, iterator_to_array($iconify->chunk($prefix, $names)));
+    }
+
+    public static function provideChunkCases(): iterable
+    {
+        yield 'no icon should make no chunk' => [
+            10,
+            'ppppp',
+            [],
+            [],
+        ];
+
+        yield 'one icon should make one chunk' => [
+            10,
+            'ppppp',
+            ['aaaa1'],
+            [['aaaa1']],
+        ];
+
+        yield 'two icons that should make two chunck' => [
+            10,
+            'ppppp',
+            ['aa1', 'aa2'],
+            [['aa1'], ['aa2']],
+        ];
+
+        yield 'three icons that should make two chunck' => [
+            15,
+            'ppppp',
+            ['aaa1', 'aaa2', 'aaa3'],
+            [['aaa1', 'aaa2'], ['aaa3']],
+        ];
+
+        yield 'four icons that should make two chunck' => [
+            15,
+            'ppppp',
+            ['aaaaaaaa1', 'a2', 'a3', 'a4'],
+            [['aaaaaaaa1'], ['a2', 'a3', 'a4']],
+        ];
+    }
+
+    public function testChunkThrowWithIconPrefixTooLong(): void
+    {
+        $iconify = new Iconify(new NullAdapter(), 'https://example.com', new MockHttpClient([]));
+
+        $prefix = str_pad('p', 101, 'p');
+        $name = 'icon';
+
+        $this->expectExceptionMessage(\sprintf('The icon prefix "%s" is too long.', $prefix));
+
+        // We need to iterate over the iterator to trigger the exception
+        $result = iterator_to_array($iconify->chunk($prefix, [$name]));
+    }
+
+    public function testChunkThrowWithIconNameTooLong(): void
+    {
+        $iconify = new Iconify(new NullAdapter(), 'https://example.com', new MockHttpClient([]));
+
+        $prefix = 'prefix';
+        $name = str_pad('n', 101, 'n');
+
+        $this->expectExceptionMessage(\sprintf('The icon name "%s" is too long.', $name));
+
+        // We need to iterate over the iterator to trigger the exception
+        $result = iterator_to_array($iconify->chunk($prefix, [$name]));
+    }
+
     private function createHttpClient(mixed $data, int $code = 200): MockHttpClient
     {
         $mockResponse = new JsonMockResponse($data, ['http_code' => $code]);
