Merge pull request phpbb#5163 from VSEphpbb/ticket/15595

[ticket/15595] Fix module exists tool when ignoring parent check

phpBB/phpbb/db/migration/tool/module.php

@@ -86,7 +86,8 @@ public function get_name()
 	* 		check for to see if it exists
 	* @param bool $lazy Checks lazily if the module exists. Returns true if it exists in at
 	*       least one given parent.
-	* @return bool true if module exists in *all* given parents, false if not
+	* @return bool true if module exists in *all* given parents, false if not in any given parent;
+	 *      true if ignoring parent check and module exists class wide, false if not found at all.
 	*/
 	public function exists($class, $parent, $module, $lazy = false)
 	{
@@ -110,6 +111,10 @@ public function exists($class, $parent, $module, $lazy = false)
 				$parent_sqls[] = 'AND parent_id = ' . (int) $parent_id;
 			}
 		}
+		else
+		{
+			$parent_sqls[] = '';
+		}
 
 		foreach ($parent_sqls as $parent_sql)
 		{
@@ -126,7 +131,7 @@ public function exists($class, $parent, $module, $lazy = false)
 			{
 				return false;
 			}
-			else if ($lazy && $module_id)
+			if ($lazy && $module_id)
 			{
 				return true;
 			}

tests/dbal/migrator_tool_module_test.php

@@ -52,11 +52,39 @@ public function exists_data_acp()
 			array(
 				'',
 				'ACP_CAT',
+				false,
 				true,
 			),
 			array(
 				0,
 				'ACP_CAT',
+				false,
+				true,
+			),
+			array(
+				false,
+				'ACP_CAT',
+				false,
+				true,
+			),
+
+			// Test the existing category lazily
+			array(
+				'',
+				'ACP_CAT',
+				true,
+				true,
+			),
+			array(
+				0,
+				'ACP_CAT',
+				true,
+				true,
+			),
+			array(
+				false,
+				'ACP_CAT',
+				true,
 				true,
 			),
 
@@ -65,38 +93,89 @@ public function exists_data_acp()
 				'',
 				'ACP_MODULE',
 				false,
+				false,
+			),
+			array(
+				false,
+				'ACP_MODULE',
+				false,
+				true,
+			),
+			array(
+				'ACP_CAT',
+				'ACP_MODULE',
+				false,
+				true,
+			),
+
+			// Test the existing module lazily
+			array(
+				'',
+				'ACP_MODULE',
+				true,
+				false,
 			),
 			array(
 				false,
 				'ACP_MODULE',
 				true,
+				true,
 			),
 			array(
 				'ACP_CAT',
 				'ACP_MODULE',
 				true,
+				true,
 			),
 
 			// Test for non-existant modules
 			array(
 				'',
 				'ACP_NON_EXISTANT_CAT',
 				false,
+				false,
+			),
+			array(
+				false,
+				'ACP_NON_EXISTANT_CAT',
+				false,
+				false,
 			),
 			array(
 				'ACP_CAT',
 				'ACP_NON_EXISTANT_MODULE',
 				false,
+				false,
+			),
+
+			// Test for non-existant modules lazily
+			array(
+				'',
+				'ACP_NON_EXISTANT_CAT',
+				true,
+				false,
+			),
+			array(
+				false,
+				'ACP_NON_EXISTANT_CAT',
+				true,
+				false,
+			),
+			array(
+				'ACP_CAT',
+				'ACP_NON_EXISTANT_MODULE',
+				true,
+				false,
 			),
 		);
 	}
 
 	/**
 	* @dataProvider exists_data_acp
 	*/
-	public function test_exists_acp($parent, $module, $expected)
+	public function test_exists_acp($parent, $module, $lazy, $expected)
 	{
-		$this->assertEquals($expected, $this->tool->exists('acp', $parent, $module));
+		$this->assertEquals($expected, $this->tool->exists('acp', $parent, $module, $lazy));
 	}
 
 	public function exists_data_ucp()
@@ -106,11 +185,39 @@ public function exists_data_ucp()
 			array(
 				'',
 				'UCP_MAIN_CAT',
+				false,
 				true,
 			),
 			array(
 				0,
 				'UCP_MAIN_CAT',
+				false,
+				true,
+			),
+			array(
+				false,
+				'UCP_MAIN_CAT',
+				false,
+				true,
+			),
+
+			// Test the existing category lazily
+			array(
+				'',
+				'UCP_MAIN_CAT',
+				true,
+				true,
+			),
+			array(
+				0,
+				'UCP_MAIN_CAT',
+				true,
+				true,
+			),
+			array(
+				false,
+				'UCP_MAIN_CAT',
+				true,
 				true,
 			),
 
@@ -119,20 +226,50 @@ public function exists_data_ucp()
 				'',
 				'UCP_SUBCATEGORY',
 				false,
+				false,
 			),
 			array(
 				false,
 				'UCP_SUBCATEGORY',
+				false,
 				true,
 			),
 			array(
 				'UCP_MAIN_CAT',
 				'UCP_SUBCATEGORY',
+				false,
 				true,
 			),
 			array(
 				'UCP_SUBCATEGORY',
 				'UCP_MODULE',
+				false,
+				true,
+			),
+
+			// Test the existing module lazily
+			array(
+				'',
+				'UCP_SUBCATEGORY',
+				true,
+				false,
+			),
+			array(
+				false,
+				'UCP_SUBCATEGORY',
+				true,
+				true,
+			),
+			array(
+				'UCP_MAIN_CAT',
+				'UCP_SUBCATEGORY',
+				true,
+				true,
+			),
+			array(
+				'UCP_SUBCATEGORY',
+				'UCP_MODULE',
+				true,
 				true,
 			),
 
@@ -141,10 +278,26 @@ public function exists_data_ucp()
 				'',
 				'UCP_NON_EXISTANT_CAT',
 				false,
+				false,
+			),
+			array(
+				'UCP_MAIN_CAT',
+				'UCP_NON_EXISTANT_MODULE',
+				false,
+				false,
+			),
+
+			// Test for non-existant modules lazily
+			array(
+				'',
+				'UCP_NON_EXISTANT_CAT',
+				true,
+				false,
 			),
 			array(
 				'UCP_MAIN_CAT',
 				'UCP_NON_EXISTANT_MODULE',
+				true,
 				false,
 			),
 		);
@@ -153,9 +306,9 @@ public function exists_data_ucp()
 	/**
 	* @dataProvider exists_data_ucp
 	*/
-	public function test_exists_ucp($parent, $module, $expected)
+	public function test_exists_ucp($parent, $module, $lazy, $expected)
 	{
-		$this->assertEquals($expected, $this->tool->exists('ucp', $parent, $module));
+		$this->assertEquals($expected, $this->tool->exists('ucp', $parent, $module, $lazy));
 	}
 
 	public function test_add()