Merge remote-tracking branch 'remotes/Marc/ticket/11465' into develop

# By Marc Alexander (8) and Joas Schilling (3)
# Via Marc Alexander
* remotes/Marc/ticket/11465:
  [ticket/11465] Increase count of disabled extensions to 5 in functional test
  [ticket/11465] Add disabled ext to allow proper testing of get_module_infos()
  [ticket/11465] Add tests for optional arguments of get_module_infos()
  [ticket/11465] Add comments explaining the tests
  [ticket/11465] Check if class exists before including info file
  [ticket/11465] Correctly set the root path for the test
  [ticket/11465] The info file does not have _info suffix
  [ticket/11465] Add phpBB module to test
  [ticket/11465] Move require_once() in unit test to the top of the file
  [ticket/11465] Add unit tests for acp_modules::get_module_infos()
  [ticket/11465] Use extension finder when adding extensions' acp modules

phpBB/includes/acp/acp_modules.php

@@ -544,81 +544,60 @@ class acp_modules
 	*/
 	function get_module_infos($module = '', $module_class = false, $use_all_available = false)
 	{
-		global $phpbb_root_path, $phpEx;
+		global $phpbb_extension_manager, $phpbb_root_path, $phpEx;
 
 		$module_class = ($module_class === false) ? $this->module_class : $module_class;
 
 		$directory = $phpbb_root_path . 'includes/' . $module_class . '/info/';
 		$fileinfo = array();
 
-		if (!$module)
+		$finder = $phpbb_extension_manager->get_finder();
+
+		$modules = $finder
+			->extension_suffix('_module')
+			->extension_directory("/$module_class")
+			->core_path("includes/$module_class/info/")
+			->core_prefix($module_class . '_')
+			->get_classes(true, $use_all_available);
+
+		foreach ($modules as $cur_module)
 		{
-			global $phpbb_extension_manager;
+			// Skip entries we do not need if we know the module we are
-
+			// looking for
-			$finder = $phpbb_extension_manager->get_finder();
+			if ($module && strpos($cur_module, $module) === false)
-
-			$modules = $finder
-				->extension_suffix('_module')
-				->extension_directory("/$module_class")
-				->core_path("includes/$module_class/info/")
-				->core_prefix($module_class . '_')
-				->get_classes(true, $use_all_available);
-
-			foreach ($modules as $module)
 			{
-				$info_class = preg_replace('/_module$/', '_info', $module);
+				continue;
-
-				// If the class does not exist it might be following the old
-				// format. phpbb_acp_info_acp_foo needs to be turned into
-				// acp_foo_info and the respective file has to be included
-				// manually because it does not support auto loading
-				if (!class_exists($info_class))
-				{
-					$info_class = str_replace("phpbb_{$module_class}_info_", '', $module) . '_info';
-					if (file_exists($directory . $info_class . '.' . $phpEx))
-					{
-						include($directory . $info_class . '.' . $phpEx);
-					}
-				}
-
-				if (class_exists($info_class))
-				{
-					$info = new $info_class();
-					$module_info = $info->module();
-
-					$main_class = (isset($module_info['filename'])) ? $module_info['filename'] : $module;
-
-					$fileinfo[$main_class] = $module_info;
-				}
 			}
 
-			ksort($fileinfo);
+			$info_class = preg_replace('/_module$/', '_info', $cur_module);
-		}
-		else
-		{
-			$info_class = preg_replace('/_module$/', '_info', $module);
 
+			// If the class does not exist it might be following the old
+			// format. phpbb_acp_info_acp_foo needs to be turned into
+			// acp_foo_info and the respective file has to be included
+			// manually because it does not support auto loading
 			if (!class_exists($info_class))
 			{
-				$info_class = $module . '_info';
+				$info_class_file = str_replace("phpbb_{$module_class}_info_", '', $cur_module);
-				if (!class_exists($info_class) && file_exists($directory . $module . '.' . $phpEx))
+				$info_class = $info_class_file . '_info';
+				if (!class_exists($info_class) && file_exists($directory . $info_class_file . '.' . $phpEx))
 				{
-					include($directory . $module . '.' . $phpEx);
+					include($directory . $info_class_file . '.' . $phpEx);
 				}
 			}
 
-			// Get module title tag
 			if (class_exists($info_class))
 			{
 				$info = new $info_class();
 				$module_info = $info->module();
 
-				$main_class = (isset($module_info['filename'])) ? $module_info['filename'] : $module;
+				$main_class = (isset($module_info['filename'])) ? $module_info['filename'] : $cur_module;
 
 				$fileinfo[$main_class] = $module_info;
 			}
 		}
 
+		ksort($fileinfo);
+
 		return $fileinfo;
 	}
 

tests/extension/ext/barfoo/acp/a_info.php

@@ -0,0 +1,16 @@
+<?php
+
+class phpbb_ext_barfoo_acp_a_info
+{
+	public function module()
+	{
+		return array(
+			'filename'	=> 'phpbb_ext_barfoo_acp_a_module',
+			'title'		=> 'Barfoo',
+			'version'	=> '3.1.0-dev',
+			'modes'		=> array(
+				'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+			),
+		);
+	}
+}

tests/extension/ext/barfoo/acp/a_module.php

@@ -0,0 +1,5 @@
+<?php
+
+class phpbb_ext_barfoo_acp_a_module
+{
+}

tests/extension/ext/barfoo/ext.php

@@ -0,0 +1,5 @@
+<?php
+
+class phpbb_ext_barfoo_ext extends phpbb_extension_base
+{
+}

tests/extension/ext/foo/acp/a_info.php

@@ -0,0 +1,16 @@
+<?php
+
+class phpbb_ext_foo_acp_a_info
+{
+	public function module()
+	{
+		return array(
+			'filename'	=> 'phpbb_ext_foo_acp_a_module',
+			'title'		=> 'Foobar',
+			'version'	=> '3.1.0-dev',
+			'modes'		=> array(
+				'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+			),
+		);
+	}
+}

tests/extension/ext/foo/acp/a_module.php

@@ -0,0 +1,5 @@
+<?php
+
+class phpbb_ext_foo_acp_a_module
+{
+}

tests/extension/ext/foo/acp/fail_info.php

@@ -0,0 +1,19 @@
+<?php
+/*
+* Due to the mismatch between the class name and the file name, this module
+* file shouldn't be found by the extension finder
+*/
+class phpbb_ext_foo_acp_foo_info
+{
+	public function module()
+	{
+		return array(
+			'filename'	=> 'phpbb_ext_foo_acp_fail_module',
+			'title'		=> 'Foobar',
+			'version'	=> '3.1.0-dev',
+			'modes'		=> array(
+				'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+			),
+		);
+	}
+}

tests/extension/ext/foo/acp/fail_module.php

@@ -0,0 +1,8 @@
+<?php
+/*
+* Due to the mismatch between the class name and the file name of the module
+* info file, this module's info file shouldn't be found
+*/
+class phpbb_ext_foo_acp_fail_module
+{
+}

tests/extension/ext/foo/mcp/a_info.php

@@ -0,0 +1,16 @@
+<?php
+
+class phpbb_ext_foo_mcp_a_info
+{
+	public function module()
+	{
+		return array(
+			'filename'	=> 'phpbb_ext_foo_mcp_a_module',
+			'title'		=> 'Foobar',
+			'version'	=> '3.1.0-dev',
+			'modes'		=> array(
+				'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('MCP_MAIN')),
+			),
+		);
+	}
+}

tests/extension/ext/foo/mcp/a_module.php

@@ -0,0 +1,5 @@
+<?php
+
+class phpbb_ext_foo_mcp_a_module
+{
+}

tests/extension/includes/acp/acp_foobar.php

@@ -0,0 +1,28 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+/**
+* @package acp
+*/
+class acp_foobar
+{
+	var $u_action;
+
+	function main($id, $mode)
+	{
+	}
+}

tests/extension/includes/acp/info/acp_foobar.php

@@ -0,0 +1,26 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+/**
+* @package module_install
+*/
+class acp_foobar_info
+{
+	function module()
+	{
+		return array(
+			'filename'	=> 'acp_foobar',
+			'title'		=> 'ACP Foobar',
+			'version'	=> '3.1.0-dev',
+			'modes'		=> array(
+				'test'		=> array('title' => 'Test', 'auth' => '', 'cat' => array('ACP_GENERAL')),
+			),
+		);
+	}
+}

tests/extension/manager_test.php

@@ -30,7 +30,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 
 	public function test_available()
 	{
-		$this->assertEquals(array('bar', 'foo', 'vendor/moo'), array_keys($this->extension_manager->all_available()));
+		$this->assertEquals(array('bar', 'barfoo', 'foo', 'vendor/moo'), array_keys($this->extension_manager->all_available()));
 	}
 
 	public function test_enabled()

tests/extension/modules_test.php

@@ -0,0 +1,192 @@
+<?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__) . '/ext/foo/acp/a_info.php';
+require_once dirname(__FILE__) . '/ext/foo/mcp/a_info.php';
+require_once dirname(__FILE__) . '/ext/foo/acp/fail_info.php';
+require_once dirname(__FILE__) . '/ext/barfoo/acp/a_info.php';
+require_once dirname(__FILE__) . '/../../phpBB/includes/acp/acp_modules.php';
+
+class phpbb_extension_modules_test extends phpbb_test_case
+{
+	protected $extension_manager;
+	protected $finder;
+
+	public function setUp()
+	{
+		global $phpbb_extension_manager;
+
+		$this->extension_manager = new phpbb_mock_extension_manager(
+			dirname(__FILE__) . '/',
+			array(
+				'foo' => array(
+					'ext_name' => 'foo',
+					'ext_active' => '1',
+					'ext_path' => 'ext/foo/',
+				),
+				'bar' => array(
+					'ext_name' => 'bar',
+					'ext_active' => '1',
+					'ext_path' => 'ext/bar/',
+				),
+			));
+		$phpbb_extension_manager = $this->extension_manager;
+
+		$this->acp_modules = new acp_modules();
+	}
+
+	public function test_get_module_infos()
+	{
+		global $phpbb_root_path;
+
+		// Correctly set the root path for this test to this directory, so the classes can be found
+		$phpbb_root_path = dirname(__FILE__) . '/';
+
+		// Find acp module info files
+		$this->acp_modules->module_class = 'acp';
+		$acp_modules = $this->acp_modules->get_module_infos();
+		$this->assertEquals(array(
+				'phpbb_ext_foo_acp_a_module' => array(
+					'filename'	=> 'phpbb_ext_foo_acp_a_module',
+					'title'		=> 'Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+					),
+				),
+				'acp_foobar' => array(
+					'filename'	=> 'acp_foobar',
+					'title'		=> 'ACP Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'test'		=> array('title' => 'Test', 'auth' => '', 'cat' => array('ACP_GENERAL')),
+					),
+				),
+			), $acp_modules);
+
+		// Find mcp module info files
+		$this->acp_modules->module_class = 'mcp';
+		$acp_modules = $this->acp_modules->get_module_infos();
+		$this->assertEquals(array(
+				'phpbb_ext_foo_mcp_a_module' => array(
+					'filename'	=> 'phpbb_ext_foo_mcp_a_module',
+					'title'		=> 'Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('MCP_MAIN')),
+					),
+				),
+			), $acp_modules);
+
+		// Find a specific module info file (mcp_a_module)
+		$this->acp_modules->module_class = 'mcp';
+		$acp_modules = $this->acp_modules->get_module_infos('mcp_a_module');
+		$this->assertEquals(array(
+				'phpbb_ext_foo_mcp_a_module' => array(
+					'filename'	=> 'phpbb_ext_foo_mcp_a_module',
+					'title'		=> 'Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('MCP_MAIN')),
+					),
+				),
+			), $acp_modules);
+
+		// Find a specific module info file (mcp_a_module) with passing the module_class
+		$this->acp_modules->module_class = '';
+		$acp_modules = $this->acp_modules->get_module_infos('mcp_a_module', 'mcp');
+		$this->assertEquals(array(
+				'phpbb_ext_foo_mcp_a_module' => array(
+					'filename'	=> 'phpbb_ext_foo_mcp_a_module',
+					'title'		=> 'Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('MCP_MAIN')),
+					),
+				),
+			), $acp_modules);
+
+		// The mcp module info file we're looking for shouldn't exist
+		$this->acp_modules->module_class = 'mcp';
+		$acp_modules = $this->acp_modules->get_module_infos('mcp_a_fail');
+		$this->assertEquals(array(), $acp_modules);
+
+		// As there are no ucp modules we shouldn't find any
+		$this->acp_modules->module_class = 'ucp';
+		$acp_modules = $this->acp_modules->get_module_infos();
+		$this->assertEquals(array(), $acp_modules);
+
+		// Get module info of specified extension module
+		$this->acp_modules->module_class = 'acp';
+		$acp_modules = $this->acp_modules->get_module_infos('phpbb_ext_foo_acp_a_module');
+		$this->assertEquals(array(
+				'phpbb_ext_foo_acp_a_module' => array (
+					'filename' => 'phpbb_ext_foo_acp_a_module',
+					'title' => 'Foobar',
+					'version' => '3.1.0-dev',
+					'modes' => array (
+						'config'		=> array ('title' => 'Config', 'auth' => '', 'cat' => array ('ACP_MODS')),
+					),
+				),
+			), $acp_modules);
+
+		// No specific module and module class set to an incorrect name
+		$acp_modules = $this->acp_modules->get_module_infos('', 'wcp', true);
+		$this->assertEquals(array(), $acp_modules);
+
+		// No specific module, no module_class set in the function parameter, and an incorrect module class
+		$this->acp_modules->module_class = 'wcp';
+		$acp_modules = $this->acp_modules->get_module_infos();
+		$this->assertEquals(array(), $acp_modules);
+
+		// No specific module, module class set to false (will default to the above acp)
+		// Setting $use_all_available will cause get_module_infos() to also load not enabled extensions (barfoo)
+		$this->acp_modules->module_class = 'acp';
+		$acp_modules = $this->acp_modules->get_module_infos('', false, true);
+		$this->assertEquals(array(
+				'phpbb_ext_foo_acp_a_module' => array(
+					'filename'	=> 'phpbb_ext_foo_acp_a_module',
+					'title'		=> 'Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+					),
+				),
+				'acp_foobar' => array(
+					'filename'	=> 'acp_foobar',
+					'title'		=> 'ACP Foobar',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'test'		=> array('title' => 'Test', 'auth' => '', 'cat' => array('ACP_GENERAL')),
+					),
+				),
+				'phpbb_ext_barfoo_acp_a_module' => array(
+					'filename'	=> 'phpbb_ext_barfoo_acp_a_module',
+					'title'		=> 'Barfoo',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+					),
+				)
+			), $acp_modules);
+
+		// Specific module set to disabled extension
+		$acp_modules = $this->acp_modules->get_module_infos('phpbb_ext_barfoo_acp_a_module', 'acp', true);
+		$this->assertEquals(array(
+				'phpbb_ext_barfoo_acp_a_module' => array(
+					'filename'	=> 'phpbb_ext_barfoo_acp_a_module',
+					'title'		=> 'Barfoo',
+					'version'	=> '3.1.0-dev',
+					'modes'		=> array(
+						'config'		=> array('title' => 'Config',	'auth' => '', 'cat' => array('ACP_MODS')),
+					),
+				)
+			), $acp_modules);
+	}
+}

tests/functional/extension_acp_test.php

@@ -112,7 +112,7 @@ class phpbb_functional_extension_acp_test extends phpbb_functional_test_case
         $crawler = $this->request('GET', 'adm/index.php?i=acp_extensions&mode=main&sid=' . $this->sid);
 
         $this->assertCount(1, $crawler->filter('.ext_enabled'));
-        $this->assertCount(4, $crawler->filter('.ext_disabled'));
+        $this->assertCount(5, $crawler->filter('.ext_disabled'));
 
         $this->assertContains('phpBB Foo Extension', $crawler->filter('.ext_enabled')->eq(0)->text());
         $this->assertContainsLang('PURGE', $crawler->filter('.ext_enabled')->eq(0)->text());