[feature/avatars] Add service containers for avatars

PHPBB3-10018

phpBB/common.php

@@ -128,7 +128,7 @@ $phpbb_subscriber_loader = $phpbb_container->get('event.subscriber_loader');
 $template = $phpbb_container->get('template');
 $phpbb_style = $phpbb_container->get('style');
 
-$phpbb_avatar_manager = new phpbb_avatar_manager($phpbb_root_path, $phpEx, $config, $request, $phpbb_extension_manager, $cache->get_driver());
+$phpbb_avatar_manager = $phpbb_container->get('avatar.manager');
 
 // Add own hook handler
 require($phpbb_root_path . 'includes/hooks/index.' . $phpEx);

phpBB/config/avatars.yml

@@ -0,0 +1,44 @@
+services:
+    avatar.driver.core.gravatar:
+        class: phpbb_avatar_driver_core_gravatar
+        arguments:
+            - @config
+            - @request
+            - %core.root_path%
+            - .%core.php_ext%
+            - @cache.driver
+        tags:
+            - { name: avatar.driver }
+
+    avatar.driver.core.local:
+        class: phpbb_avatar_driver_core_local
+        arguments:
+            - @config
+            - @request
+            - %core.root_path%
+            - .%core.php_ext%
+            - @cache.driver
+        tags:
+            - { name: avatar.driver }
+
+    avatar.driver.core.remote:
+        class: phpbb_avatar_driver_core_remote
+        arguments:
+            - @config
+            - @request
+            - %core.root_path%
+            - .%core.php_ext%
+            - @cache.driver
+        tags:
+            - { name: avatar.driver }
+
+    avatar.driver.core.upload:
+        class: phpbb_avatar_driver_core_upload
+        arguments:
+            - @config
+            - @request
+            - %core.root_path%
+            - .%core.php_ext%
+            - @cache.driver
+        tags:
+            - { name: avatar.driver }

phpBB/config/services.yml

@@ -1,11 +1,30 @@
 imports:
     - { resource: tables.yml }
     - { resource: cron_tasks.yml }
+    - { resource: avatars.yml }
 
 services:
     auth:
         class: phpbb_auth
 
+    avatar.driver_collection:
+        class: phpbb_di_service_collection
+        arguments:
+            - @service_container
+        tags:
+            - { name: service_collection, tag: avatar.driver }
+
+    avatar.manager:
+        class: phpbb_avatar_manager
+        arguments:
+            - %core.root_path%
+            - .%core.php_ext%
+            - @config
+            - @request
+            - @cache.driver
+            - @avatar.driver_collection
+            - @service_container
+
     cache:
         class: phpbb_cache_service
         arguments:

phpBB/includes/avatar/driver/core/gravatar.php

@@ -70,9 +70,9 @@ class phpbb_avatar_driver_core_gravatar extends phpbb_avatar_driver
 	public function prepare_form($template, $row, &$error)
 	{
 		$template->assign_vars(array(
-			'AV_GRAVATAR_WIDTH' => (($row['avatar_type'] == __CLASS__ || $row['avatar_type'] == 'gravatar') && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('av_local_width', 0),
+			'AV_GRAVATAR_WIDTH' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('av_local_width', 0),
-			'AV_GRAVATAR_HEIGHT' => (($row['avatar_type'] == __CLASS__ || $row['avatar_type'] == 'gravatar') && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('av_local_width', 0),
+			'AV_GRAVATAR_HEIGHT' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('av_local_width', 0),
-			'AV_GRAVATAR_EMAIL' => (($row['avatar_type'] == __CLASS__ || $row['avatar_type'] == 'gravatar') && $row['avatar']) ? $row['avatar'] : '',
+			'AV_GRAVATAR_EMAIL' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar']) ? $row['avatar'] : '',
 		));
 
 		return true;
@@ -86,16 +86,8 @@ class phpbb_avatar_driver_core_gravatar extends phpbb_avatar_driver
 		$email = $this->request->variable('av_gravatar_email', '');
 		$width = $this->request->variable('av_gravatar_width', 0);
 		$height = $this->request->variable('av_gravatar_height', 0);
-		var_dump($width, $height);
 
-		/*
+		require_once($this->phpbb_root_path . 'includes/functions_user' . $this->phpEx);
-		if (!preg_match('#^(http|https|ftp)://#i', $email))
-		{
-			$url = 'http://' . $url;
-		}*/
-		// @todo: check if we need to check emails
-
-		require_once($this->phpbb_root_path . 'includes/functions_user.' . $this->phpEx);
 
 		$error = array_merge($error, validate_data(array(
 			'email' => $email,

phpBB/includes/avatar/driver/core/remote.php

@@ -50,9 +50,9 @@ class phpbb_avatar_driver_core_remote extends phpbb_avatar_driver
 	public function prepare_form($template, $row, &$error)
 	{
 		$template->assign_vars(array(
-			'AV_REMOTE_WIDTH' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('av_local_width', 0),
+			'AV_REMOTE_WIDTH' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('av_local_width', 0),
-			'AV_REMOTE_HEIGHT' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('av_local_width', 0),
+			'AV_REMOTE_HEIGHT' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('av_local_width', 0),
-			'AV_REMOTE_URL' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar']) ? $row['avatar'] : '',
+			'AV_REMOTE_URL' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar']) ? $row['avatar'] : '',
 		));
 
 		return true;
@@ -72,7 +72,7 @@ class phpbb_avatar_driver_core_remote extends phpbb_avatar_driver
 			$url = 'http://' . $url;
 		}
 
-		require_once($this->phpbb_root_path . 'includes/functions_user.' . $this->phpEx);
+		require_once($this->phpbb_root_path . 'includes/functions_user' . $this->phpEx);
 
 		$error = array_merge($error, validate_data(array(
 			'url' => $url,
@@ -118,7 +118,7 @@ class phpbb_avatar_driver_core_remote extends phpbb_avatar_driver
 			return false;
 		}
 
-		include_once($this->phpbb_root_path . 'includes/functions_upload.' . $this->phpEx);
+		include_once($this->phpbb_root_path . 'includes/functions_upload' . $this->phpEx);
 		$types = fileupload::image_types();
 		$extension = strtolower(filespec::get_extension($url));
 

phpBB/includes/avatar/driver/core/upload.php

@@ -72,7 +72,7 @@ class phpbb_avatar_driver_core_upload extends phpbb_avatar_driver
 			return false;
 		}
 
-		include_once($this->phpbb_root_path . 'includes/functions_upload.' . $this->phpEx);
+		include_once($this->phpbb_root_path . 'includes/functions_upload' . $this->phpEx);
 
 		$upload = new fileupload('AVATAR_', array('jpg', 'jpeg', 'gif', 'png'), $this->config['avatar_filesize'], $this->config['avatar_min_width'], $this->config['avatar_min_height'], $this->config['avatar_max_width'], $this->config['avatar_max_height'], (isset($this->config['mime_triggers']) ? explode('|', $this->config['mime_triggers']) : false));
 

phpBB/includes/avatar/driver/driver.php

@@ -21,6 +21,28 @@ if (!defined('IN_PHPBB'))
 */
 abstract class phpbb_avatar_driver implements phpbb_avatar_driver_interface
 {
+	private $name;
+
+	/**
+	* Returns the name of the driver.
+	*
+	* @return string		Name of wrapped driver.
+	*/
+	public function get_name()
+	{
+		return $this->name;
+	}
+
+	/**
+	* Sets the name of the driver.
+	*
+	* @param string	$name The driver name
+	*/
+	public function set_name($name)
+	{
+		$this->name = $name;
+	}
+
 	/**
 	* Current board configuration
 	* @type phpbb_config

phpBB/includes/avatar/driver/interface.php

@@ -21,6 +21,13 @@ if (!defined('IN_PHPBB'))
 */
 interface phpbb_avatar_driver_interface
 {
+	/**
+	* Returns the name of the driver.
+	*
+	* @return string		Name of wrapped driver.
+	*/
+	public function get_name();
+
 	/**
 	* Get the avatar url and dimensions
 	*

phpBB/includes/avatar/manager.php

@@ -24,21 +24,23 @@ class phpbb_avatar_manager
 	private $phpEx;
 	private $config;
 	private $request;
-	private $extension_manager;
 	private $cache;
 	private static $valid_drivers = false;
+	private $tasks;
+	private $container;
 
 	/**
 	* @TODO
 	**/
-	public function __construct($phpbb_root_path, $phpEx, phpbb_config $config, phpbb_request $request, phpbb_extension_manager $extension_manager, phpbb_cache_driver_interface $cache = null)
+	public function __construct($phpbb_root_path, $phpEx, phpbb_config $config, phpbb_request $request, phpbb_cache_driver_interface $cache, $tasks, $container)
 	{
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->phpEx = $phpEx;
 		$this->config = $config;
 		$this->request = $request;
-		$this->extension_manager = $extension_manager;
 		$this->cache = $cache;
+		$this->tasks = $tasks;
+		$this->container = $container;
 	}
 
 	/**
@@ -55,13 +57,13 @@ class phpbb_avatar_manager
 		switch ($avatar_type)
 		{
 			case AVATAR_GALLERY:
-				$avatar_type = 'phpbb_avatar_driver_local';
+				$avatar_type = 'avatar.driver.core.local';
 				break;
 			case AVATAR_UPLOAD:
-				$avatar_type = 'phpbb_avatar_driver_upload';
+				$avatar_type = 'avatar.driver.core.upload';
 				break;
 			case AVATAR_REMOTE:
-				$avatar_type = 'phpbb_avatar_driver_remote';
+				$avatar_type = 'avatar.driver.core.remote';
 				break;
 		}
 
@@ -70,19 +72,14 @@ class phpbb_avatar_manager
 			return null;
 		}
 
-		$r = new ReflectionClass($avatar_type);
+		$driver = $this->container->get($avatar_type);
-
+		if ($driver !== false)
-		if ($r->isSubClassOf('phpbb_avatar_driver'))
 		{
-			$driver = new $avatar_type($this->config, $this->request, $this->phpbb_root_path, $this->phpEx, $this->cache);
+			return $driver;
-		}
-		else if ($r->implementsInterface('phpbb_avatar_driver'))
-		{
-			$driver = new $avatar_type();
 		}
 		else
 		{
-			$message = "Invalid avatar driver class name '%s' provided. It must implement phpbb_avatar_driver_interface.";
+			$message = "Invalid avatar driver class name '%s' provided.";
 			trigger_error(sprintf($message, $avatar_type));
 		}
 
@@ -94,25 +91,12 @@ class phpbb_avatar_manager
 	**/
 	private function load_valid_drivers()
 	{
-		if ($this->cache)
+		if (!empty($this->tasks))
-		{
-			self::$valid_drivers = $this->cache->get('avatar_drivers');
-		}
-
-		if (empty($this->valid_drivers))
 		{
 			self::$valid_drivers = array();
-
+			foreach ($this->tasks as $driver)
-			$finder = $this->extension_manager->get_finder();
-
-			self::$valid_drivers = $finder
-				->extension_directory('/avatar/driver/')
-				->core_path('includes/avatar/driver/core/')
-				->get_classes();
-
-			if ($this->cache)
 			{
-				$this->cache->put('avatar_drivers', self::$valid_drivers);
+				self::$valid_drivers[] = $driver->get_name();
 			}
 		}
 	}

phpBB/includes/ucp/ucp_profile.php

@@ -563,8 +563,9 @@ class ucp_profile
 					{
 						if (check_form_key('ucp_avatar'))
 						{
-							$driver = request_var('avatar_driver', '');
+							$driver = str_replace('_', '.', request_var('avatar_driver', ''));
-							if (in_array($driver, $avatar_drivers) && $config["allow_avatar_$driver"])
+							$config_name = preg_replace('#^avatar.driver.core.#', '', $driver);
+							if (in_array($driver, $avatar_drivers) && $config["allow_avatar_$config_name"])
 							{
 								$avatar = $phpbb_avatar_manager->get_driver($driver);
 								$result = $avatar->process_form($template, $avatar_data, $error);
@@ -643,7 +644,7 @@ class ucp_profile
 									'L_TITLE' => $user->lang('AVATAR_DRIVER_' . $driver_u . '_TITLE'), // @TODO add lang values
 									'L_EXPLAIN' => $user->lang('AVATAR_DRIVER_' . $driver_u . '_EXPLAIN'),
 
-									'DRIVER' => $driver,
+									'DRIVER' => str_replace('.', '_', $driver),
 									'SELECTED' => ($driver == $focused_driver),
 									'OUTPUT' => $template->assign_display('avatar'),
 								));