[ticket/16944] Generate iconify bundle as needed via icons & assets bag

PHPBB3-16944

phpBB/phpbb/assets/iconify_bundler.php

@@ -13,41 +13,32 @@
 
 namespace phpbb\assets;
 
-use phpbb\exception\runtime_exception;
 use Iconify\JSONTools\Collection;
-use Symfony\Component\Finder\Finder;
 
 class iconify_bundler
 {
-	protected const BUNDLE_PATH = 'assets/iconify/iconify-bundle.js';
-
-	protected $db;
-
-	protected $ext_manager;
-
 	/** @var \phpbb\log\log_interface */
 	protected $log;
 
-	/** @var \phpbb\filesystem\filesystem_interface */
-	protected $filesystem;
-
-	protected $root_path = '';
-
-	protected $bundle_path = '';
+	/** @var string[] Icons list */
 	protected $icons_list = [];
 
-	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\extension\manager $ext_manager, \phpbb\log\log_interface $log, string $root_path)
+	/**
+	 * Constructor for iconify bundler
+	 *
+	 * @param \phpbb\log\log_interface $log Logger
+	 */
+	public function __construct(\phpbb\log\log_interface $log)
 	{
-		$this->db = $db;
-		$this->ext_manager = $ext_manager;
-		$this->filesystem = new \phpbb\filesystem\filesystem();
 		$this->log = $log;
-		$this->root_path = $root_path;
-
-		$this->bundle_path = $root_path . self::BUNDLE_PATH;
 	}
 
-	protected function run()
+	/**
+	 * Run iconify bundler
+	 *
+	 * @return string Iconify bundle
+	 */
+	public function run()
 	{
 		// Sort icons first
 		sort($this->icons_list, SORT_NATURAL);
@@ -76,127 +67,12 @@ class iconify_bundler
 		return $output;
 	}
 
-	public function get_bundle(bool $force_rebuild = false): string
-	{
-		if (!$force_rebuild && $this->is_dumped())
-		{
-			return file_get_contents($this->bundle_path);
-		}
-
-		return $this->create_bundle();
-	}
-
-	public function create_bundle(): string
-	{
-		$iconify_bundle = $this->with_extensions()
-			->with_styles()
-			->run();
-
-		$this->filesystem->dump_file($this->bundle_path, $iconify_bundle);
-
-		return $iconify_bundle;
-	}
-
 	/**
-	 * @param array $paths Icon paths
+	 * Add icon to icons list
 	 *
+	 * @param string $icon_name
 	 * @return void
 	 */
-	public function find_icons(array $paths): void
-	{
-		if (!count($paths))
-		{
-			return;
-		}
-
-		$finder = new Finder();
-		$finder->files();
-
-		foreach ($paths as $cur_path)
-		{
-			$finder->in($cur_path);
-		}
-
-		$finder->name('*.html')
-			->name('*.twig')
-			->contains("Icon('iconify',");
-
-		foreach ($finder as $file)
-		{
-			$contents = $file->getContents();
-			$matches = [];
-			preg_match_all("/Icon\('iconify', *(?:'(?<text>[^']+(?<content_flow>' ~ S_CONTENT_FLOW_(?:BEGIN|END))?)|(?<json>{[^}]+}))/m", $contents, $matches, PREG_SET_ORDER);
-			foreach ($matches as $match_data)
-			{
-				if (!empty($match_data['content_flow']))
-				{
-					$base_icon_name = str_replace($match_data['content_flow'], '', $match_data['text']);
-					$this->add_icon($base_icon_name . 'left');
-					$this->add_icon($base_icon_name . 'right');
-				}
-				else if (!empty($match_data['json']))
-				{
-					preg_match_all("/\s'(?<text>[^']+)'/", $match_data['json'], $icons_array, PREG_SET_ORDER);
-					foreach ($icons_array as $icon)
-					{
-						$this->add_icon($icon['text']);
-					}
-				}
-				else if (!empty($match_data['text']))
-				{
-					$this->add_icon($match_data['text']);
-				}
-				else
-				{
-					throw new runtime_exception('Found unexpected icon name `%1$s` in `%2$s`', [$match_data[0], $file->getPath()]);
-				}
-			}
-		}
-	}
-
-	public function with_extensions(): iconify_bundler
-	{
-		$extensions = $this->ext_manager->all_enabled();
-
-		$search_paths = [];
-
-		foreach ($extensions as $path)
-		{
-			if (file_exists($path))
-			{
-				$search_paths[] = $path;
-			}
-		}
-
-		$this->find_icons($search_paths);
-
-		return $this;
-	}
-
-	public function with_styles(): iconify_bundler
-	{
-		$sql = 'SELECT *
-			FROM ' . STYLES_TABLE;
-		$result = $this->db->sql_query($sql);
-
-		$style_paths = [];
-
-		while ($row = $this->db->sql_fetchrow($result))
-		{
-			$style_paths[] = $this->root_path . 'styles/' . $row['style_path'];
-		}
-		$this->db->sql_freeresult($result);
-
-		$this->find_icons($style_paths);
-
-		return $this;
-	}
-
-	public function is_dumped(): bool
-	{
-		return $this->filesystem->exists($this->bundle_path) && $this->filesystem->is_readable($this->bundle_path);
-	}
-
 	protected function add_icon(string $icon_name): void
 	{
 		if (!in_array($icon_name, $this->icons_list))
@@ -205,6 +81,20 @@ class iconify_bundler
 		}
 	}
 
+	/**
+	 * Add multiple icons to icons list
+	 *
+	 * @param array $icons
+	 * @return void
+	 */
+	public function add_icons(array $icons): void
+	{
+		foreach ($icons as $icon)
+		{
+			$this->add_icon($icon);
+		}
+	}
+
 	/**
 	 * Organize icons list by prefix
 	 *
@@ -222,9 +112,9 @@ class iconify_bundler
 			$icon = $this->name_to_icon($icon_name);
 			if ($icon === null || $icon['provider'] !== '')
 			{
-				// Invalid name or icon name has provider.
-				// All icons in this example are from Iconify, so providers are not supported.
-				throw new \Error('Invalid icon name: ' . $icon_name);
+				// Invalid name or icon name does not have provider
+				$this->log->add('critical', ANONYMOUS, '', 'LOG_ICON_INVALID', false, [$icon_name]);
+				continue;
 			}
 
 			$prefix = $icon['prefix'];
@@ -308,7 +198,13 @@ class iconify_bundler
 		return null;
 	}
 
-	protected function load_icons_data($icons): string
+	/**
+	 * Load icons date for supplied icons array
+	 *
+	 * @param array $icons
+	 * @return string
+	 */
+	protected function load_icons_data(array $icons): string
 	{
 		// Load icons data
 		$output = '';
@@ -322,10 +218,11 @@ class iconify_bundler
 			}
 
 			// Make sure all icons exist
-			foreach ($iconsList as $name) {
+			foreach ($iconsList as $name)
+			{
 				if (!$collection->iconExists($name))
 				{
-					$this->log->add('critical', ANONYMOUS, '', 'LOG_ICON_INVALID', false, [$prefix, $name]);
+					$this->log->add('critical', ANONYMOUS, '', 'LOG_ICON_INVALID', false, [$prefix . ':' . $name]);
 				}
 			}
 

phpBB/phpbb/template/assets_bag.php

@@ -13,6 +13,8 @@
 
 namespace phpbb\template;
 
+use phpbb\assets\iconify_bundler;
+
 class assets_bag
 {
 	/** @var asset[] */
@@ -21,6 +23,17 @@ class assets_bag
 	/** @var asset[] */
 	protected $scripts = [];
 
+	/** @var string[] */
+	protected $iconify_icons = [];
+
+	/**
+	 * Constructor for assets bag
+	 *
+	 * @param iconify_bundler $iconify_bundler
+	 */
+	public function __construct(protected iconify_bundler $iconify_bundler)
+	{}
+
 	/**
 	 * Add a css asset to the bag
 	 *
@@ -41,6 +54,30 @@ class assets_bag
 		$this->scripts[] = $asset;
 	}
 
+	public function add_iconify_icon(string $icon): void
+	{
+		$this->iconify_icons[] = $icon;
+	}
+
+	/**
+	 * Inject iconify icons into template
+	 *
+	 * @param string $output Output before injection
+	 * @param string $variable_name Variable name for injection
+	 * @param bool $use_cdn Flag whether to use CDN or local data
+	 *
+	 * @return string Output after injection
+	 */
+	public function inject_iconify_icons(string $output, string $variable_name, bool $use_cdn): string
+	{
+		if (str_contains($output, $variable_name))
+		{
+			$output = str_replace($variable_name, $use_cdn ? '' : $this->get_iconify_content(), $output);
+		}
+
+		return $output;
+	}
+
 	/**
 	 * Returns all css assets
 	 *
@@ -92,4 +129,22 @@ class assets_bag
 
 		return $output;
 	}
+
+	/**
+	 * Gets the HTML code to include all iconify icons
+	 *
+	 * @return string HTML code for iconify bundle
+	 */
+	public function get_iconify_content(): string
+	{
+		$output = '';
+		if (count($this->iconify_icons))
+		{
+			$output .= '<script>';
+			$this->iconify_bundler->add_icons($this->iconify_icons);
+			$output .= $this->iconify_bundler->run();
+			$output .= '</script>';
+		}
+		return $output;
+	}
 }

phpBB/phpbb/template/twig/environment.php

@@ -13,26 +13,32 @@
 
 namespace phpbb\template\twig;
 
+use phpbb\config\config;
+use phpbb\event\dispatcher_interface;
+use phpbb\extension\manager;
+use phpbb\filesystem\filesystem;
+use phpbb\path_helper;
 use phpbb\template\assets_bag;
+use Twig\Loader\LoaderInterface;
 
 class environment extends \Twig\Environment
 {
-	/** @var \phpbb\config\config */
+	/** @var config */
 	protected $phpbb_config;
 
-	/** @var \phpbb\filesystem\filesystem */
+	/** @var filesystem */
 	protected $filesystem;
 
-	/** @var \phpbb\path_helper */
+	/** @var path_helper */
 	protected $phpbb_path_helper;
 
 	/** @var \Symfony\Component\DependencyInjection\ContainerInterface */
 	protected $container;
 
-	/** @var \phpbb\extension\manager */
+	/** @var manager */
 	protected $extension_manager;
 
-	/** @var \phpbb\event\dispatcher_interface */
+	/** @var dispatcher_interface */
 	protected $phpbb_dispatcher;
 
 	/** @var string */
@@ -50,16 +56,17 @@ class environment extends \Twig\Environment
 	/**
 	* Constructor
 	*
-	* @param \phpbb\config\config $phpbb_config The phpBB configuration
-	* @param \phpbb\filesystem\filesystem $filesystem
-	* @param \phpbb\path_helper $path_helper phpBB path helper
+	* @param assets_bag $assets_bag Assets bag
+	* @param config $phpbb_config The phpBB configuration
+	* @param filesystem $filesystem
+	* @param path_helper $path_helper phpBB path helper
 	* @param string $cache_path The path to the cache directory
-	* @param \phpbb\extension\manager|null $extension_manager phpBB extension manager
-	* @param \Twig\Loader\LoaderInterface|null $loader Twig loader interface
-	* @param \phpbb\event\dispatcher_interface|null	$phpbb_dispatcher	Event dispatcher object
+	* @param manager|null $extension_manager phpBB extension manager
+	* @param LoaderInterface|null $loader Twig loader interface
+	* @param dispatcher_interface|null	$phpbb_dispatcher	Event dispatcher object
 	* @param array $options Array of options to pass to Twig
 	*/
-	public function __construct(\phpbb\config\config $phpbb_config, \phpbb\filesystem\filesystem $filesystem, \phpbb\path_helper $path_helper, $cache_path, \phpbb\extension\manager $extension_manager = null, \Twig\Loader\LoaderInterface $loader = null, \phpbb\event\dispatcher_interface $phpbb_dispatcher = null, $options = array())
+	public function __construct(assets_bag $assets_bag, config $phpbb_config, filesystem $filesystem, path_helper $path_helper, $cache_path, manager $extension_manager = null, LoaderInterface $loader = null, dispatcher_interface $phpbb_dispatcher = null, $options = array())
 	{
 		$this->phpbb_config = $phpbb_config;
 
@@ -70,7 +77,7 @@ class environment extends \Twig\Environment
 
 		$this->phpbb_root_path = $this->phpbb_path_helper->get_phpbb_root_path();
 
-		$this->assets_bag = new assets_bag();
+		$this->assets_bag = $assets_bag;
 
 		$options = array_merge(array(
 			'cache'			=> (defined('IN_INSTALL')) ? false : $cache_path,
@@ -97,7 +104,7 @@ class environment extends \Twig\Environment
 	/**
 	* Get phpBB config
 	*
-	* @return \phpbb\config\config
+	* @return config
 	*/
 	public function get_phpbb_config()
 	{
@@ -117,7 +124,7 @@ class environment extends \Twig\Environment
 	/**
 	* Get the filesystem object
 	*
-	* @return \phpbb\filesystem\filesystem
+	* @return filesystem
 	*/
 	public function get_filesystem()
 	{
@@ -137,7 +144,7 @@ class environment extends \Twig\Environment
 	/**
 	* Get the phpbb path helper object
 	*
-	* @return \phpbb\path_helper
+	* @return path_helper
 	*/
 	public function get_path_helper()
 	{
@@ -204,6 +211,7 @@ class environment extends \Twig\Environment
 		{
 			$context['definition']->set('SCRIPTS', '__SCRIPTS_' . $placeholder_salt . '__');
 			$context['definition']->set('STYLESHEETS', '__STYLESHEETS_' . $placeholder_salt . '__');
+			$context['definition']->set('ICONIFY_ICONS', '__ICONIFY_ICONS_' . $placeholder_salt . '__');
 		}
 
 		/**
@@ -251,6 +259,7 @@ class environment extends \Twig\Environment
 	{
 		$output = str_replace('__STYLESHEETS_' . $placeholder_salt . '__', $this->assets_bag->get_stylesheets_content(), $output);
 		$output = str_replace('__SCRIPTS_' . $placeholder_salt . '__', $this->assets_bag->get_scripts_content(), $output);
+		$output = $this->assets_bag->inject_iconify_icons($output, '__ICONIFY_ICONS_' . $placeholder_salt . '__', $this->phpbb_config['allow_cdn']);
 
 		return $output;
 	}

phpBB/phpbb/template/twig/extension/icon.php

@@ -89,6 +89,8 @@ class icon extends AbstractExtension
 			case 'iconify':
 				$source = explode(':', $icon);
 				$source = $source[0];
+				$environment->get_assets_bag()->add_iconify_icon($icon);
+
 			break;
 
 			case 'png':