Ticket/11450 phpbb_extension_metadata_manager has wrong docs and too many arguments

phpBB/includes/acp/acp_extensions.php

@@ -54,7 +54,7 @@ function main()
 		// If they've specified an extension, let's load the metadata manager and validate it.
 		if ($ext_name)
 		{
-			$md_manager = new phpbb_extension_metadata_manager($ext_name, $db, $phpbb_extension_manager, $phpbb_root_path, $phpEx, $template, $config);
+			$md_manager = new phpbb_extension_metadata_manager($ext_name, $config, $phpbb_extension_manager, $template, $phpbb_root_path);
 
 			try
 			{

phpBB/includes/extension/manager.php

@@ -155,7 +155,7 @@ public function get_extension($name)
 	*/
 	public function create_extension_metadata_manager($name, phpbb_template $template)
 	{
-		return new phpbb_extension_metadata_manager($name, $this->db, $this, $this->phpbb_root_path, $this->php_ext, $template, $this->config);
+		return new phpbb_extension_metadata_manager($name, $this->config, $this, $template, $this->phpbb_root_path);
 	}
 
 	/**

phpBB/includes/extension/metadata_manager.php

@@ -22,31 +22,64 @@
 */
 class phpbb_extension_metadata_manager
 {
-	protected $phpEx;
+	/**
+	* phpBB Config instance
+	* @var phpbb_config
+	*/
+	protected $config;
+
+	/**
+	* phpBB Extension Manager
+	* @var phpbb_extension_manager
+	*/
 	protected $extension_manager;
-	protected $db;
-	protected $phpbb_root_path;
+
+	/**
+	* phpBB Template instance
+	* @var phpbb_template
+	*/
 	protected $template;
+
+	/**
+	* phpBB root path
+	* @var string
+	*/
+	protected $phpbb_root_path;
+
+	/**
+	* Name (including vendor) of the extension
+	* @var string
+	*/
 	protected $ext_name;
+
+	/**
+	* Metadata from the composer.json file
+	* @var array
+	*/
 	protected $metadata;
+
+	/**
+	* Link (including root path) to the metadata file
+	* @var string
+	*/
 	protected $metadata_file;
 
 	/**
 	* Creates the metadata manager
 	*
-	* @param phpbb_db_driver $db A database connection
-	* @param string $extension_manager An instance of the phpbb extension manager
-	* @param string $phpbb_root_path Path to the phpbb includes directory.
-	* @param string $phpEx php file extension
+	* @param string				$ext_name			Name (including vendor) of the extension
+	* @param phpbb_config		$config				phpBB Config instance
+	* @param phpbb_extension_manager	$extension_manager An instance of the phpBBb extension manager
+	* @param phpbb_template		$template			phpBB Template instance
+	* @param string				$phpbb_root_path	Path to the phpbb includes directory.
 	*/
-	public function __construct($ext_name, phpbb_db_driver $db, phpbb_extension_manager $extension_manager, $phpbb_root_path, $phpEx = 'php', phpbb_template $template, phpbb_config $config)
+	public function __construct($ext_name, phpbb_config $config, phpbb_extension_manager $extension_manager, phpbb_template $template, $phpbb_root_path)
 	{
-		$this->phpbb_root_path = $phpbb_root_path;
-		$this->db = $db;
 		$this->config = $config;
-		$this->phpEx = $phpEx;
-		$this->template = $template;
 		$this->extension_manager = $extension_manager;
+		$this->template = $template;
+		$this->phpbb_root_path = $phpbb_root_path;
+
 		$this->ext_name = $ext_name;
 		$this->metadata = array();
 		$this->metadata_file = '';

tests/extension/metadata_manager_test.php

@@ -7,7 +7,9 @@
 *
 */
 
-class metadata_manager_test extends phpbb_database_test_case
+require_once dirname(__FILE__) . '/../../phpBB/includes/db/db_tools.php';
+
+class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 {
 	protected $class_loader;
 	protected $extension_manager;
@@ -416,31 +418,16 @@ public function test_validator_requirements()
 	* Get an instance of the metadata manager
 	*
 	* @param string $ext_name
-	* @return phpbb_extension_metadata_manager_test
+	* @return phpbb_mock_metadata_manager
 	*/
 	private function get_metadata_manager($ext_name)
 	{
-		return new phpbb_extension_metadata_manager_test(
+		return new phpbb_mock_metadata_manager(
 			$ext_name,
-			$this->db,
+			$this->config,
 			$this->extension_manager,
-			$this->phpbb_root_path,
-			$this->phpEx,
 			$this->template,
-			$this->config
+			$this->phpbb_root_path
 		);
 	}
 }
-
-class phpbb_extension_metadata_manager_test extends phpbb_extension_metadata_manager
-{
-	public function set_metadata($metadata)
-	{
-		$this->metadata = $metadata;
-	}
-
-	public function merge_metadata($metadata)
-	{
-		$this->metadata = array_merge($this->metadata, $metadata);
-	}
-}

tests/functional/fixtures/ext/foo/bar/composer.json

@@ -0,0 +1,23 @@
+{
+	"name": "foo/bar",
+	"type": "phpbb3-extension",
+	"description": "Testing extensions",
+	"homepage": "",
+	"version": "1.0.0",
+	"time": "2013-03-21 01:01:01",
+	"licence": "GPL-2.0",
+	"authors": [{
+		"name": "Joas Schilling",
+		"username": "nickvergessen",
+		"email": "nickvergessen@phpbb.com",
+		"homepage": "http://www.phpbb.com",
+		"role": "Developer"
+	}],
+	"require": {
+		"php": ">=5.3",
+		"phpbb": ">=3.1.0-dev"
+	},
+	"extra": {
+		"display-name": "phpBB 3.1 Extension Testing"
+	}
+}

tests/functional/metadata_manager_test.php

@@ -0,0 +1,111 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/db/db_tools.php';
+
+/**
+* @group functional
+*/
+class phpbb_functional_metadata_manager_test extends phpbb_functional_test_case
+{
+	protected $phpbb_extension_manager;
+
+	static private $helpers;
+
+	static protected $fixtures = array(
+		'foo/bar/',
+	);
+
+	/**
+	* This should only be called once before the tests are run.
+	* This is used to copy the fixtures to the phpBB install
+	*/
+	static public function setUpBeforeClass()
+	{
+		global $phpbb_root_path;
+		parent::setUpBeforeClass();
+
+		self::$helpers = new phpbb_test_case_helpers(self);
+
+		if (!file_exists($phpbb_root_path . 'ext/foo/bar/'))
+		{
+			self::$helpers->makedirs($phpbb_root_path . 'ext/foo/bar/');
+		}
+
+		foreach (self::$fixtures as $fixture)
+		{
+			self::$helpers->copy_dir(dirname(__FILE__) . '/fixtures/ext/' . $fixture, $phpbb_root_path . 'ext/' . $fixture);
+		}
+	}
+
+	/**
+	* This should only be called once after the tests are run.
+	* This is used to remove the fixtures from the phpBB install
+	*/
+	static public function tearDownAfterClass()
+	{
+		global $phpbb_root_path;
+
+		foreach (self::$fixtures as $fixture)
+		{
+			self::$helpers->empty_dir($phpbb_root_path . 'ext/' . $fixture);
+		}
+		self::$helpers->empty_dir($phpbb_root_path . 'ext/foo/');
+	}
+
+	public function setUp()
+	{
+		parent::setUp();
+
+		$this->phpbb_extension_manager = $this->get_extension_manager();
+
+		$this->purge_cache();
+		$this->phpbb_extension_manager->enable('foo/bar');
+
+		$this->login();
+		$this->admin_login();
+		$this->add_lang('acp/extensions');
+	}
+
+	public function test_extensions_list()
+	{
+		$crawler = $this->request('GET', 'adm/index.php?i=acp_extensions&mode=main&sid=' . $this->sid);
+		$this->assert_response_success();
+
+		$this->assertContains($this->lang('EXTENSIONS_EXPLAIN'), $crawler->filter('#main')->text());
+		$this->assertContains('phpBB 3.1 Extension Testing', $crawler->filter('#main')->text());
+		$this->assertContains('Details', $crawler->filter('#main')->text());
+	}
+
+	public function test_extensions_details()
+	{
+		$crawler = $this->request('GET', 'adm/index.php?i=acp_extensions&mode=main&action=details&ext_name=foo%2Fbar&sid=' . $this->sid);
+		$this->assert_response_success();
+
+		// Test whether the details are displayed
+		$this->assertContains($this->lang('CLEAN_NAME'), $crawler->filter('#main')->text());
+		$this->assertContains('foo/bar', $crawler->filter('#meta_name')->text());
+
+		$this->assertContains($this->lang('PHP_VERSION'), $crawler->filter('#main')->text());
+		$this->assertContains('>=5.3', $crawler->filter('#require_php')->text());
+		// Details should be html escaped
+		// However, text() only returns the displayed text, so HTML Special Chars are decoded.
+		// So we test this directly on the content of the response.
+		$this->assertContains('<p id="require_php">&gt;=5.3</p>', $this->client->getResponse()->getContent());
+	}
+
+	public function test_extensions_details_notexists()
+	{
+		$crawler = $this->request('GET', 'adm/index.php?i=acp_extensions&mode=main&action=details&ext_name=not%2Fexists&sid=' . $this->sid);
+		$this->assert_response_success();
+
+		// Error message because the files do not exist
+		$this->assertContains('The required file does not exist:', $crawler->filter('#main')->text());
+	}
+}

tests/mock/metadata_manager.php

@@ -0,0 +1,21 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+class phpbb_mock_metadata_manager extends phpbb_extension_metadata_manager
+{
+	public function set_metadata($metadata)
+	{
+		$this->metadata = $metadata;
+	}
+
+	public function merge_metadata($metadata)
+	{
+		$this->metadata = array_merge($this->metadata, $metadata);
+	}
+}